Fix #1072 Avoid completions in block comments

[thomasdarimont]

Previously completions were suppressed in single line comments only.
We now also avoid providing completions within block comments.

[ramya-rao-a]

There are a few cases where this will give a false positive
One example is

	hi := "/*"

	bye := "*/"

You won't get completions between the above 2 lines

OR in between the below 2 lines

hi := "/*" 
	
	// This is a comment */

[ramya-rao-a]

Another thing to think about is that we don't want completions inside multi line strings as well. Whatever approach we end up using for multi line comments can be used in multi line strings as well

[thomasdarimont]

Ouch, I indeed missed that, thanks for pointing this out.

Those cases are IMHO rather difficult to handle with plain string magic - perhaps a better
approach would be to use the go tooling for this.
Perhaps we could leverage the AST
derived via go Parser to find those single line / block comment as well as multiline string literals.

But I don't know how well the parser can deal with incomplete source code...
WDYT?

[ramya-rao-a]

It would definitely be nice to have the AST in hand during completions

Apart from stopping completions inside multi line comments and multi line strings, it would also help in #168

But I don't know how well the parser can deal with incomplete source code...

I dont know much here either.

We currently use https://github.com/ramya-rao-a/go-outline to get the file outline which uses the go parser. You can try and see if we can re-use that here

[thomasdarimont]

Just gave this a spin - it seems that I need to adjust go-outline to also return the comments.

Would it be possible to change the output of go-outline?
For instance currently go-outline returns an array of "Symbol"-Declarations in a file.
How about returning a "CompilationUnit" that contains those declarations and some additional comments?

With my branch I can get the comments with their respective positions.

E.g. for the go file hello.go:

package main

import (
	"fmt"
)

func hello() string {

	// single line comment

	// single line comment line1
	// single line comment line2

	/* block comment in single line */

	/*
		multi line block comment
	*/

	slString := "single line string"
	mlSlString := `multiline single line string`
	mlString := `
		multiline
		string
	`

	commentStart := "/*"
	// foo
	commentEnd := "*/"

	fmt.Println(slString, mlSlString, mlString, commentStart, commentEnd)

	return "hello"
}

I get the following json representation:

{
  "declarations": [
    {
      "label": "main",
      "type": "package",
      "start": 1,
      "end": 476,
      "children": [
        {
          "label": "\"fmt\"",
          "type": "import",
          "start": 25,
          "end": 30
        },
        {
          "label": "hello",
          "type": "function",
          "start": 34,
          "end": 476
        }
      ]
    }
  ],
  "comments": [
    {
      "start": 58,
      "end": 80,
      "text": "// single line comment"
    },
    {
      "start": 83,
      "end": 111,
      "text": "// single line comment line1"
    },
    {
      "start": 113,
      "end": 141,
      "text": "// single line comment line2"
    },
    {
      "start": 144,
      "end": 178,
      "text": "/* block comment in single line */"
    },
    {
      "start": 181,
      "end": 214,
      "text": "/*\n\t\tmulti line block comment\n\t*/"
    },
    {
      "start": 359,
      "end": 365,
      "text": "// foo"
    }
  ]
}

I could also try to add the comments to the respective declarations where they
belong to... but I thought a flat list might be easier to work with.

I wonder whether it would make sense to transform the full go AST to JSON instead of
adapting the "minimal" representation for every new feature.

wdyt?

[ramya-rao-a]

I need some time to digest this. Give me a few days?

[thomasdarimont]

Sure no problem! Thanks for having a look into it :)

[ramya-rao-a]

@thomasdarimont So I finally had time to look at your changes to go-outline.

Am not very comfortable changing the structure of the output, we will have to maintain 2 different ways of consuming the output, one for people still on the old version of go-outline, the other for the new.

Why not re-use the Declaration struct for comments. You can have type as comments, label as the comment text, Children would be empty array.

This way the final output structure would be the same as before.
An outer declaration for package whose children would declarations who in turn can be functions, types, variables, constants and comments

We can have a flag -include-comments. So that existing users of this tool will see no change in the usage even if they got the latest. We would return the comments, if this flag has been provided.

[tohjustin]

Hi @ramya-rao-a, I've created a separate PR here (ramya-rao-a/go-outline#7) to push this fix forward.

[ramya-rao-a]

Closing this PR as it hasnt seen any activity in the past year