New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
x/tools/gopls: completion doesn't suggest items following invalid code #31687
Comments
Another instance:
The completion request at the cursor position show returns no results. @stamblerre - is it worth kicking off separate |
They're both instances of the same issue actually. Basically when something fails to parse at the top of the file, it creates problems at the end, so anything below where you are will not be handled correctly. |
Yes indeed; I suspect however these are separate cases in |
Is there anyway that gopls could fallback to parsing the file on disk in these cases to attempt to return useful autocomplete? Not sure if that's actually the best way to resolve this type of situation. |
@muirrn as the completion guru, do you think this can be closed now? Or are you using it as a placeholder for future improvements? |
I think we can keep it open to track/consolidate this class of problems. I'm not planning any work in this area but I think the general plan is to expand our AST fixes to include these and other common syntax errors. For example, workarounds for the above cases might look like:
|
Change https://golang.org/cl/216482 mentions this issue: |
Change https://golang.org/cl/216483 mentions this issue: |
In cases like: func _() { if fo<> } func foo() {} Completing at "<>" does not include "foo" because the missing "if" opening curly brace renders the rest of the file unparseable. "foo" doesn't exist in the AST, so as far as gopls is concerned it doesn't exist at all. To fix this, we detect when a block is missing opening "{" and we insert stub "{}" to make things parse better. This is a different kind of fix than our previous *ast.BadExpr handling because we must reparse the file after tweaking the source. After reparsing we maintain the original syntax error so the user sees consistent error messages. This also avoids having the "{}" spring into existence when the user formats the file. Fixes golang/go#31907. Updates golang/go#31687. Change-Id: I95ba60a11f7dd23dc484c063b4fd7ad77daa4e08 Reviewed-on: https://go-review.googlesource.com/c/tools/+/216482 Run-TryBot: Muir Manders <muir@mnd.rs> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In cases like: if i := foo<> we get an *ast.BadExpr because the parser is expecting the condition expression, but "i := foo" is not a valid expression. Now we move the statement into the "init" field and add a dummy "cond" expression. We also needed a slight tweak to our missing curly brace fix. Now we insert an extra semicolon in cases like: for i := 0; i < foo yielding for i := 0; i < foo;{} The parser doesn't like having only two clauses in the three clause "for" statement. Updates golang/go#31687. Change-Id: I12d51e0d8af03436741227753f8e71452a463b05 Reviewed-on: https://go-review.googlesource.com/c/tools/+/216483 Run-TryBot: Muir Manders <muir@mnd.rs> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Change https://golang.org/cl/216484 mentions this issue: |
Dangling selectors such as: func _() { x. } var x struct { i int } tend to wreak havoc on the AST. In the above example you didn't used to get completions because the declaration of "x" was missing from the AST. We now work around this issue by inserting a "_" into the source code before parsing to make the selector valid: func _() { x._ // <-- insert "_" here } var x struct { i int } This makes completion work as expected because the declaration of "x" is present in the AST. I had to change fixAST() to be called before fixSrc() because otherwise this new workaround in fixSrc() breaks the "accidental keyword" countermeasures in fixAST(). Fixes golang/go#31973. Updates golang/go#31687. Change-Id: Ia7ef6c045a9c71502d1b8b36f187ac9b8a85fe21 Reviewed-on: https://go-review.googlesource.com/c/tools/+/216484 Run-TryBot: Muir Manders <muir@mnd.rs> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rebecca Stambler <rstambler@golang.org>
@myitcv can you test out master and see if it resolves your above issues satisfactorily? |
Looks good to me! Thank you very much, @muirdm |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
This is largely a "port" of mdempsky/gocode#22 to the
gopls
issue tracker where the issue remains.If completion is attempted at the position indicated by
^
, completion successfully returns the single candidate:Now consider:
This time we don't get any candidates.
What did you expect to see?
The single candidate
DoIt()
returned in example 2.What did you see instead?
As above
cc @stamblerre @ianthehat
The text was updated successfully, but these errors were encountered: