Enable completions inside parenthesized expressions

[shenglol]

Added error recovery for parenthesized expressions. After the change () is parsed as a ParenthesizedExpression (with OpenParen=(, Expression=SkippedTriviaSyntax, and CloseParen=)) rather than a SkippedTriviaSyntax. The existing completion handling logic is good enough to trigger completions inside an incomplete ParenthesizedExpression.

Closes #1223.

[TSunny007]

Is there a plan for intellisense for outside parenthesis? What about intellisense without a closed parenthesis (Error hints that you should probably insert the missing ) first )? I seem to be unfamiliar with using () in bicep outside of functions and if condition

var foo = (deploymentScript['properties']).azCli<Version> // outside parenthesis
var foo = (deploymentScript['properties'].a<zCliVersion> // no closed parenthesis

Additionally, I think parenthesis should be blocked inside PropertyAccessSyntax (or even ObjectSyntax?) when not used in a function syntax,

var foo = (deploymentScript(['apiVersion'] //insert as many closing parenthesis as you want at the end, breaks rest of the file
var foo = (deploymentScript.(location)) //errors but its not quite informative (nothing to lose sleep over though, this is a silly thing to try)

[shenglol]

@TarunSunkaraneni

Is there a plan for intellisense for outside parenthesis

var foo = (deploymentScript['properties']).azCli<Version> // outside parenthesis

This is a good find. We should open an issue and track this as a bug.

What about intellisense without a closed parenthesis (Error hints that you should probably insert the missing ) first )

var foo = (deploymentScript['properties'].a<zCliVersion> // no closed parenthesis

With this PR you now get a "missing )" error.

Additionally, I think parenthesis should be blocked inside PropertyAccessSyntax (or even ObjectSyntax?) when not used in a function syntax

var foo = (deploymentScript(['apiVersion'] //insert as many closing parenthesis as you want at the end, breaks rest of the file
var foo = (deploymentScript.(location)) //errors but its not quite informative (nothing to lose sleep over though, this is a silly thing to try)

These two cases are not syntactically valid. The first line doesn't work because we don't support single-line array. For the second one, I think the parser already handles it correctly, and it feels hacky to me to treat it as a special case.

[majastrz]

I think in general we do have a problem with how we handle un-closed parentheses, braces and brackets. c# intellisense continues to function even if you don't close whatever symbol was opened, so I think we will need to do the same in the future regardless. (Requires a bit change to the parser recovery logic.)

For dot property access completions on parenthesized expressions, we are missing the logic to flow the "declared type" through those types of syntax nodes. It should light up once that's fixed.

[majastrz]

) [](start = 42, length = 1)

That's actually confusing. The original error message was correct.

[shenglol]

Will try to tweak the error recovery logic to restore the original error message.

[shenglol]

@majastrz I tried several ways and the one works best is to change the type of CloseParen from Token to SyntaxBase in order to call WithRecovery. With this change the original error message is restored, and one bonus side effect is that we now have completions inside an un-closed parenthesized expression (... 😄

[majastrz]

Makes sense. The SyntaxBase approach is what we've done in other places too.

---

In reply to: 552174602 [](ancestors = 552174602)

[codecov-io]

Codecov Report

Merging #1221 (a951ee5) into main (4f4639f) will increase coverage by 0.54%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1221      +/-   ##
==========================================
+ Coverage   94.48%   95.03%   +0.54%     
==========================================
  Files         338      334       -4     
  Lines       16987    16853     -134     
  Branches       14        0      -14     
==========================================
- Hits        16050    16016      -34     
+ Misses        937      837     -100     

Flag	Coverage Δ
dotnet	95.03% <100.00%> (+<0.01%)	⬆️
typescript	?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/Bicep.Core/Parsing/Parser.cs	95.22% <100.00%> (+0.01%)	⬆️
...Bicep.Core/Syntax/ParenthesizedExpressionSyntax.cs	100.00% <100.00%> (ø)
src/vscode-bicep/src/extension.ts
src/vscode-bicep/src/utils/telemetry.ts
src/vscode-bicep/src/language/client.ts
src/vscode-bicep/src/utils/logger.ts
src/Bicep.Core.UnitTests/Utils/CompilerFlags.cs	40.00% <0.00%> (+6.66%)	⬆️

[majastrz]

Awesome!