[WIP] Fixed open declaration completions when there is no identifier#4106
[WIP] Fixed open declaration completions when there is no identifier#4106TIHan wants to merge 6 commits intodotnet:masterfrom
Conversation
| %type <Ast.SynExceptionDefnRepr> exconCore | ||
| %type <Ast.SynModuleDecl list> moduleDefnsOrExprPossiblyEmptyOrBlock | ||
| %type <Ast.LongIdentWithDots> openDecl | ||
| %type <Ast.LongIdentWithDots option> openDecl |
There was a problem hiding this comment.
Hmm. Doesn’t this change the F# grammar, allowing empty open statements? If so, is it ok?
There was a problem hiding this comment.
Later on when we check the option, if it is None, we throw an error, "Identifier expected", so it doesn't pass-through.
|
Having range for open token is nice, I've wanted it on symbols level to gray out the entire "open xxx" in unused open decl analyzer. |
|
Maybe |
|
Either way would work for my purpose. Would you add this right into this PR? |
|
I've added openTokenRange. The final range is the combined openTokenRange and longDotId.Range. I'll definitely need some feedback regarding this approach. I feel better adding the openTokenRange, but AFAIK there isn't anything that indicates we include other token ranges anywhere in the syntax tree. So this one is special, wondering if it's appropriate. |
|
This is great, thanks! About tokens in parse tree, they are not there for two reasons, I think: they are not needed for compilation, and they should not "pollute" the next compilation stage, as parse tree does not pollute TAST. But, we do need it for tooling, so I’m appreciate this PR. |
|
Why it's not merged yet? |
|
Because it's not finished yet. Looking at this again, I remember I might need @dsyme's feedback if this is how we should start including keyword ranges into the AST. |
|
@TIHan, is this still an active PR? |
|
@TIHan please reopen when you have time to complete this work. Thanks Kevin |
3 participants
Referring to this issue: #4078
We are getting all symbols when we do completions for open delcarations that do not have an identifier. The reason is because the parser is giving us nothing useful at this specific point.
This fix modifies the parser slightly to give us something to work with. It also gives us a different error message if there is no identifier, "Identifier expected". This is what C# does as well.
This needs a bit of discussion.
In
pars.fsy, I'm not entirely sure this is a good idea returning:[ SynModuleDecl.Open(Ast.LongIdentWithDots([], []), rhsm) ].rhsmis the range of the OPEN token. Normally the range that is passed toSynModuleDecl.Openis the entire range ofAst.LongIdentWithDotswhen there is an identifier.This could be a little confusing. Perhaps there is a better way to represent this kind of thing. I need the range of the OPEN token when there is no identifier in order have completions that result in compilable code.
Also, need to add a little more tests.