Merge pull request #25 from janpfeifer/autocomplete

Auto-complete: handle case where cell is not yet parseable

Dockerfile

@@ -22,12 +22,14 @@ FROM ${BASE_IMAGE}:${BASE_TAG}
 
 # Update apt and install basic utils
 USER root
-RUN apt-get update --yes && apt-get install --yes --no-install-recommends wget
+RUN apt-get update --yes
+RUN apt-get install --yes --no-install-recommends wget
+RUN apt-get install --yes --no-install-recommends git
 
 #######################################################################################################
 # Go and GoNB Libraries
 #######################################################################################################
-ENV GO_VERSION=1.20.3
+ENV GO_VERSION=1.20.4
 ENV GOROOT=/usr/local/go
 ENV GOPATH=${HOME}/go
 ENV PATH=$PATH:$GOROOT/bin:$GOPATH/bin
@@ -37,11 +39,6 @@ WORKDIR /usr/local
 RUN wget --quiet --output-document=- "https://go.dev/dl/go${GO_VERSION}.linux-amd64.tar.gz" | tar -xz \
     && go version
 
-# Other tools that may be useful for Go users -- including gcc for CGO libraries support.
-RUN apt-get install -y \
-    git libtool pkg-config build-essential autoconf automake uuid-dev libzmq3-dev \
-    gcc g++
-
 # Install GoNB (https://github.com/janpfeifer/gonb) in the jovyan's user account (default user)
 USER $NB_USER
 WORKDIR ${HOME}

docs/CHANGELOG.md

@@ -1,5 +1,11 @@
 # GoNB Changelog
 
+## v0.6.3 - 2023/05/18
+
+* Handle auto-complete case where cell is not parseable: now `gopls` is also called, and memorized
+  definitions are instead saved on a second `other.go` file, for `gopls` to pick content from.
+  (Issues #21 and #23).
+
 ## v0.6.2 - 2023/05/17
 
 * Issue #23: Fixed support for generic types.

goexec/composer.go

@@ -379,7 +379,7 @@ func (s *State) createMainFileFromDecls(decls *Declarations, mainDecl *Function)
 		err = errors.Wrapf(err, "Failed to create %q", s.MainPath())
 		return
 	}
-	cursor, fileToCellIdAndLine, err = s.createMainContentsFromDecls(f, decls, mainDecl)
+	cursor, fileToCellIdAndLine, err = s.createGoContentsFromDecls(f, decls, mainDecl)
 	err2 := f.Close()
 	if err != nil {
 		err = errors.Wrapf(err, "creating main.go")
@@ -393,10 +393,35 @@ func (s *State) createMainFileFromDecls(decls *Declarations, mainDecl *Function)
 	return
 }
 
-// createMainContentsFromDecls writes to the given file all the declarations.
+// createAlternativeFileFromDecls creates `other.go` and writes all memorized definitions.
+func (s *State) createAlternativeFileFromDecls(decls *Declarations) (err error) {
+	var f *os.File
+	fPath := s.AlternativeDefinitionsPath()
+	f, err = os.Create(fPath)
+	if err != nil {
+		err = errors.Wrapf(err, "Failed to create %q", fPath)
+		return
+	}
+	_, _, err = s.createGoContentsFromDecls(f, decls, nil)
+	err2 := f.Close()
+	if err != nil {
+		err = errors.Wrapf(err, "creating %q", fPath)
+		return
+	}
+	err = err2
+	if err != nil {
+		err = errors.Wrapf(err, "closing %q", fPath)
+		return
+	}
+	return
+}
+
+// createGoContentsFromDecls writes to the given file all the declarations.
 //
-// It returns the cursor position in the file as well as a mapping from the file lines to to the original cell ids and lines.
-func (s *State) createMainContentsFromDecls(writer io.Writer, decls *Declarations, mainDecl *Function) (cursor Cursor, fileToCellIdAndLine []CellIdAndLine, err error) {
+// mainDecl is optional, and if not given no `main` function is created.
+//
+// It returns the cursor position in the file as well as a mapping from the file lines to the original cell ids and lines.
+func (s *State) createGoContentsFromDecls(writer io.Writer, decls *Declarations, mainDecl *Function) (cursor Cursor, fileToCellIdAndLine []CellIdAndLine, err error) {
 	cursor = NoCursor
 	w := NewWriterWithCursor(writer)
 	w.Writef("package main\n\n")

goexec/execcode.go

@@ -45,14 +45,22 @@ func (s *State) ExecuteCell(msg kernel.Message, cellId int, lines []string, skip
 	return s.Execute(msg, fileToCellIdAndLine)
 }
 
+// BinaryPath is the path to the generated binary file.
 func (s *State) BinaryPath() string {
 	return path.Join(s.TempDir, s.Package)
 }
 
+// MainPath is the path to the main.go file.
 func (s *State) MainPath() string {
 	return path.Join(s.TempDir, "main.go")
 }
 
+// AlternativeDefinitionsPath is the path to a temporary file that holds the memorize definitions,
+// when we are not able to include them in the `main.go`, because the current cell is not parseable.
+func (s *State) AlternativeDefinitionsPath() string {
+	return path.Join(s.TempDir, "other.go")
+}
+
 func (s *State) Execute(msg kernel.Message, fileToCellIdAndLine []CellIdAndLine) error {
 	return kernel.PipeExecToJupyter(msg, s.BinaryPath(), s.Args...).
 		WithStderr(newJupyterStackTraceMapperWriter(msg, "stderr", s.MainPath(), fileToCellIdAndLine)).

goexec/goexec.go

@@ -5,6 +5,7 @@
 package goexec
 
 import (
+	"fmt"
 	"github.com/janpfeifer/gonb/goexec/goplsclient"
 	"github.com/pkg/errors"
 	"log"
@@ -19,7 +20,7 @@ import (
 // defined Go code.
 //
 // That is, if the user runs a cell that defines, let's say `func f(x int) int { return x+1 }`,
-// the declaration of `f` will be stored in Decls field.
+// the definition of `f` will be stored in Decls field.
 type State struct {
 	// Temporary directory where Go program is build at each execution.
 	UniqueID, Package, TempDir string
@@ -192,23 +193,32 @@ const NoCursorLine = int(-1)
 
 var NoCursor = Cursor{Line: NoCursorLine, Col: 0}
 
-func (c *Cursor) HasCursor() bool {
+func (c Cursor) HasCursor() bool {
 	return c.Line != NoCursorLine
 }
 
 // CursorFrom returns a new Cursor adjusted
-func (c *Cursor) CursorFrom(line, col int) Cursor {
+func (c Cursor) CursorFrom(line, col int) Cursor {
 	if !c.HasCursor() {
-		return *c
+		return c
 	}
 	return Cursor{Line: c.Line + line, Col: c.Col + col}
 }
 
-// ClearCursor resets the cursor to an invalid state.
+// ClearCursor resets the cursor to an invalid state. This method is needed
+// for the structs that embed Cursor.
 func (c *Cursor) ClearCursor() {
 	c.Line = NoCursorLine
 }
 
+// String implements the fmt.Stringer interface.
+func (c Cursor) String() string {
+	if c.HasCursor() {
+		return fmt.Sprintf("[L:%d, Col:%d]", c.Line, c.Col)
+	}
+	return "[NoCursor]"
+}
+
 // CellLines identifies a cell (by its execution id) and the lines
 // corresponding to a declaration.
 type CellLines struct {
@@ -233,7 +243,7 @@ func (c CellLines) Append(fileToCellIdAndLine []CellIdAndLine) []CellIdAndLine {
 	return fileToCellIdAndLine
 }
 
-// Function definition.
+// Function definition, parsed from a notebook cell.
 type Function struct {
 	Cursor
 	CellLines
@@ -244,6 +254,7 @@ type Function struct {
 
 }
 
+// Variable definition, parsed from a notebook cell.
 type Variable struct {
 	Cursor
 	CellLines
@@ -253,6 +264,7 @@ type Variable struct {
 	TypeDefinition, ValueDefinition           string // Type definition may be empty.
 }
 
+// TypeDecl definition, parsed from a notebook cell.
 type TypeDecl struct {
 	Cursor
 	CellLines

goexec/goplsclient/goplsclient.go

@@ -105,19 +105,22 @@ func isGoInternalOrCache(filePath string) bool {
 	return false
 }
 
+// NotifyAboutStandardFiles will notify `gopls` of open or change of the files that may be used by GoNB.
+func (c *Client) NotifyAboutStandardFiles(ctx context.Context) (err error) {
+	// Send "go.mod" and "go.sum" in case it changes.
+	for _, fileName := range []string{"go.mod", "go.sum", "go.work", "other.go"} {
+		err = c.NotifyDidOpenOrChange(ctx, path.Join(c.dir, fileName))
+		if err != nil {
+			return
+		}
+	}
+	return
+}
+
 // Definition return the definition for the identifier at the given position, rendered
 // in Markdown. It returns empty if position has no identifier.
 func (c *Client) Definition(ctx context.Context, filePath string, line, col int) (markdown string, err error) {
 	glog.V(2).Infof("goplsclient.Definition(ctx, %s, %d, %d)", filePath, line, col)
-	// Send "go.mod" and "go.sum" in case it changes.
-	err = c.NotifyDidOpenOrChange(ctx, path.Join(c.dir, "go.mod"))
-	if err != nil {
-		return "", err
-	}
-	err = c.NotifyDidOpenOrChange(ctx, path.Join(c.dir, "go.sum"))
-	if err != nil {
-		return "", err
-	}
 	// Send filePath.
 	err = c.NotifyDidOpenOrChange(ctx, filePath)
 	if err != nil {

goexec/inspect.go

@@ -6,6 +6,7 @@ import (
 	"github.com/janpfeifer/gonb/gonbui/protocol"
 	"github.com/janpfeifer/gonb/kernel"
 	"github.com/pkg/errors"
+	"os"
 	"strings"
 	"unicode/utf16"
 )
@@ -79,22 +80,39 @@ func (s *State) AutoCompleteOptionsInCell(cellLines []string, skipLines map[int]
 		return
 	}
 
-	// Generate `main.go` with contents of current cell.
+	// Generate `main.go` (and maybe `other.go`) with contents of current cell.
 	cellId := -1 // AutoComplete doesn't actually execute it, so parsed contents of cell are not kept.
 	cursorInCell := Cursor{cursorLine, cursorCol}
 	updatedDecls, mainDecl, cursorInFile, fileToCellIdAndLine, err := s.parseLinesAndComposeMain(nil, cellId, cellLines, skipLines, cursorInCell)
 	if err != nil {
-		if errors.Is(err, ParseError) || errors.Is(err, CursorLost) {
-			err = nil
+		glog.V(1).Infof("Non-ParseError during parsing: %+v", err)
+		err = nil
+		// Render memorized definitions on a side file, so `gopls` can pick those definitions if needed for
+		// auto-complete.
+		err = s.createAlternativeFileFromDecls(s.Decls)
+		glog.V(2).Infof(". Alternative file %q with memorized definitions created", s.AlternativeDefinitionsPath())
+		if err != nil {
+			return
+		}
+		defer func() {
+			// Remove alternative file after
+			err2 := os.Remove(s.AlternativeDefinitionsPath())
+			if err2 != nil && !os.IsNotExist(err2) {
+				glog.Errorf("Failed to remove alternative definitions: %+v", err2)
+			}
+			glog.V(2).Infof(". Alternative file %q with memorized definitions removed", s.AlternativeDefinitionsPath())
+		}()
+	} else {
+		// If parsing succeeded, execute `goimports`: we just want to make sure that "go get" is executed for the
+		// needed packages.
+		cursorInFile, _, err = s.GoImports(nil, updatedDecls, mainDecl, fileToCellIdAndLine)
+		if err != nil {
+			err = errors.WithMessagef(err, "goimports failed")
+			return
 		}
-		return
 	}
-
-	// Exec `goimports`: we just want to make sure that "go get" is executed for the needed packages.
-	cursorInFile, _, err = s.GoImports(nil, updatedDecls, mainDecl, fileToCellIdAndLine)
-	if err != nil {
-		err = errors.WithMessagef(err, "goimports failed")
-		return
+	if glog.V(1) {
+		s.logCursor(cursorInFile)
 	}
 
 	// Query `gopls`.

goexec/parser.go

@@ -2,13 +2,13 @@ package goexec
 
 import (
 	"fmt"
+	"github.com/golang/glog"
 	. "github.com/janpfeifer/gonb/common"
 	"github.com/janpfeifer/gonb/kernel"
 	"github.com/pkg/errors"
 	"go/ast"
 	"go/parser"
 	"go/token"
-	"log"
 	"math/rand"
 	"os"
 	"path"
@@ -124,7 +124,7 @@ func (s *State) parseFromMainGo(msg kernel.Message, cellId int, cursor Cursor, f
 		fileToCellIdAndLine: fileToCellIdAndLine,
 	}
 	var packages map[string]*ast.Package
-	packages, err = parser.ParseDir(pi.fileSet, s.TempDir, nil, parser.SkipObjectResolution|parser.AllErrors)
+	packages, err = parser.ParseDir(pi.fileSet, s.TempDir, nil, parser.SkipObjectResolution) // |parser.AllErrors
 	if err != nil {
 		if msg != nil {
 			s.DisplayErrorWithContext(msg, fileToCellIdAndLine, err.Error())
@@ -382,20 +382,16 @@ func (s *State) parseLinesAndComposeMain(msg kernel.Message, cellId int, lines [
 	updatedDecls *Declarations, mainDecl *Function, cursorInFile Cursor, fileToCellIdAndLine []CellIdAndLine, err error) {
 	cursorInFile = NoCursor
 
-	var (
-		cursorInTmpFile Cursor
-		fileToCellLine  []int
-	)
-
-	cursorInTmpFile, fileToCellLine, err = s.createGoFileFromLines(s.MainPath(), lines, skipLines, cursorInCell)
+	var fileToCellLine []int
+	cursorInFile, fileToCellLine, err = s.createGoFileFromLines(s.MainPath(), lines, skipLines, cursorInCell)
 	if err != nil {
 		return
 	}
 	fileToCellIdAndLine = MakeFileToCellIdAndLine(cellId, fileToCellLine)
 
 	// Parse declarations in created `main.go` file.
 	var newDecls *Declarations
-	newDecls, err = s.parseFromMainGo(msg, cellId, cursorInTmpFile, fileToCellIdAndLine)
+	newDecls, err = s.parseFromMainGo(msg, cellId, cursorInFile, fileToCellIdAndLine)
 	if err != nil {
 		return
 	}
@@ -439,12 +435,16 @@ const cursorStr = "‸"
 // logCursor will log the line in `main.go` the cursor is pointing to, and puts a
 // "*" where the
 func (s *State) logCursor(cursor Cursor) {
+	if !cursor.HasCursor() {
+		glog.Infof("Cursor not defined.")
+		return
+	}
 	content, err := s.readMainGo()
 	if err != nil {
-		log.Printf("Failed to read main.go, for debugging.")
-	} else {
-		log.Printf("Cursor in main.go (%+v): %s", cursor, lineWithCursor(content, cursor))
+		glog.Errorf("Failed to read main.go, for debugging.")
+		return
 	}
+	glog.Infof("Cursor in main.go (%+v): %s", cursor, lineWithCursor(content, cursor))
 }
 
 func lineWithCursor(content string, cursor Cursor) string {

goexec/parser_test.go

@@ -369,7 +369,7 @@ func TestCursorPositioning(t *testing.T) {
 			t.Fatalf("Failed to parse imports from main.go: %+v", err)
 		}
 
-		cursorInFile, fileToCellIdAndLine, err := s.createMainContentsFromDecls(buf, s.Decls, nil)
+		cursorInFile, fileToCellIdAndLine, err := s.createGoContentsFromDecls(buf, s.Decls, nil)
 		_ = fileToCellIdAndLine
 		require.NoError(t, err)
 		content := buf.String()