Skip to content

Commit

Permalink
Distinguish between type syntax vs expression syntax in the parser (A…
Browse files Browse the repository at this point in the history
…zure#13671)

Resolves Azure#13618

In order to make type determinations, the `DeclaredTypeManager` needs to
know whether it is within type syntax or expression syntax. This
distinction is meaningful for property access (e.g., `<array type>[*]`
is legal within type syntax but not within expression syntax, whereas
`<array value>[?0]` is legal within expression syntax but not within
type syntax), variable access (variables within type syntax can refer to
other types but not values, and variables within expression syntax can
refer to values but not types), nullability control syntax, and literal
value syntax.

Prior to Bicep 0.26, the type manager was relying on internal state that
assumed the AST would be visited in order. This resulted in unstable
type assignments if the type of type syntax was requested out of order.
In Bicep 0.26.54, this was replaced with a deterministic check that used
the model's syntax hierarchy to determine whether a given syntax node
was a descendant of the type declaration of an `output` or `parameter`
statement or the assigned value of a `type` statement (indicating type
syntax) or not (indicating expression syntax). However, querying the
syntax hierarchy can throw an exception if a node was not part of the
original model (e.g., if it was created by a `SyntaxRewriteVisitor`).

This PR moves the responsibility for making this distinction to the
parser. The parser is already tracking whether it is within type syntax
or expression syntax, as the grammar for each syntax class is distinct.
This PR introduces some new descendent classes of `TypeSyntax` that are
minimally changed copies of descendent classes of `ExpressionSyntax`:
`VariableAccessSyntax` is a descendent of `ExpressionSyntax`, and
`TypeVariableAccessSyntax` is a descendent of `TypeSyntax`, but the two
classes are otherwise identical. This means that the compiler can
determine whether an arbitrary syntax node was encountered in type
syntax or expression syntax based solely on the C# type of the syntax
node rather than having to rely on the syntax hierarchy or order of
operations.

About 50% of the line count for this PR comes from baseline changes.
These are all to the `main.syntax.bicep` artifacts and consist of the
class name for specific nodes being changed (e.g.,
`VariableAccessSyntax` -> `TypeVariableAccessSyntax`, `StringSyntax` ->
`StringTypeLiteralSyntax`, etc.).

###### Microsoft Reviewers: [Open in
CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/Azure/bicep/pull/13671)
  • Loading branch information
jeskew authored Mar 27, 2024
1 parent b060945 commit 6b0e259
Show file tree
Hide file tree
Showing 74 changed files with 1,560 additions and 922 deletions.
20 changes: 10 additions & 10 deletions src/Bicep.Core.Samples/Files/baselines/AKS_LF/main.syntax.bicep
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ param dnsPrefix string
//@[000:0005) | ├─Token(Identifier) |param|
//@[006:0015) | ├─IdentifierSyntax
//@[006:0015) | | └─Token(Identifier) |dnsPrefix|
//@[016:0022) | └─VariableAccessSyntax
//@[016:0022) | └─TypeVariableAccessSyntax
//@[016:0022) | └─IdentifierSyntax
//@[016:0022) | └─Token(Identifier) |string|
//@[022:0023) ├─Token(NewLine) |\n|
Expand All @@ -15,7 +15,7 @@ param linuxAdminUsername string
//@[000:0005) | ├─Token(Identifier) |param|
//@[006:0024) | ├─IdentifierSyntax
//@[006:0024) | | └─Token(Identifier) |linuxAdminUsername|
//@[025:0031) | └─VariableAccessSyntax
//@[025:0031) | └─TypeVariableAccessSyntax
//@[025:0031) | └─IdentifierSyntax
//@[025:0031) | └─Token(Identifier) |string|
//@[031:0032) ├─Token(NewLine) |\n|
Expand All @@ -24,7 +24,7 @@ param sshRSAPublicKey string
//@[000:0005) | ├─Token(Identifier) |param|
//@[006:0021) | ├─IdentifierSyntax
//@[006:0021) | | └─Token(Identifier) |sshRSAPublicKey|
//@[022:0028) | └─VariableAccessSyntax
//@[022:0028) | └─TypeVariableAccessSyntax
//@[022:0028) | └─IdentifierSyntax
//@[022:0028) | └─Token(Identifier) |string|
//@[028:0030) ├─Token(NewLine) |\n\n|
Expand All @@ -43,7 +43,7 @@ param servcePrincipalClientId string
//@[000:0005) | ├─Token(Identifier) |param|
//@[006:0029) | ├─IdentifierSyntax
//@[006:0029) | | └─Token(Identifier) |servcePrincipalClientId|
//@[030:0036) | └─VariableAccessSyntax
//@[030:0036) | └─TypeVariableAccessSyntax
//@[030:0036) | └─IdentifierSyntax
//@[030:0036) | └─Token(Identifier) |string|
//@[036:0038) ├─Token(NewLine) |\n\n|
Expand All @@ -62,7 +62,7 @@ param servicePrincipalClientSecret string
//@[000:0005) | ├─Token(Identifier) |param|
//@[006:0034) | ├─IdentifierSyntax
//@[006:0034) | | └─Token(Identifier) |servicePrincipalClientSecret|
//@[035:0041) | └─VariableAccessSyntax
//@[035:0041) | └─TypeVariableAccessSyntax
//@[035:0041) | └─IdentifierSyntax
//@[035:0041) | └─Token(Identifier) |string|
//@[041:0043) ├─Token(NewLine) |\n\n|
Expand All @@ -74,7 +74,7 @@ param clusterName string = 'aks101cluster'
//@[000:0005) | ├─Token(Identifier) |param|
//@[006:0017) | ├─IdentifierSyntax
//@[006:0017) | | └─Token(Identifier) |clusterName|
//@[018:0024) | ├─VariableAccessSyntax
//@[018:0024) | ├─TypeVariableAccessSyntax
//@[018:0024) | | └─IdentifierSyntax
//@[018:0024) | | └─Token(Identifier) |string|
//@[025:0042) | └─ParameterDefaultValueSyntax
Expand All @@ -87,7 +87,7 @@ param location string = resourceGroup().location
//@[000:0005) | ├─Token(Identifier) |param|
//@[006:0014) | ├─IdentifierSyntax
//@[006:0014) | | └─Token(Identifier) |location|
//@[015:0021) | ├─VariableAccessSyntax
//@[015:0021) | ├─TypeVariableAccessSyntax
//@[015:0021) | | └─IdentifierSyntax
//@[015:0021) | | └─Token(Identifier) |string|
//@[022:0048) | └─ParameterDefaultValueSyntax
Expand Down Expand Up @@ -132,7 +132,7 @@ param osDiskSizeGB int = 0
//@[000:0005) | ├─Token(Identifier) |param|
//@[006:0018) | ├─IdentifierSyntax
//@[006:0018) | | └─Token(Identifier) |osDiskSizeGB|
//@[019:0022) | ├─VariableAccessSyntax
//@[019:0022) | ├─TypeVariableAccessSyntax
//@[019:0022) | | └─IdentifierSyntax
//@[019:0022) | | └─Token(Identifier) |int|
//@[023:0026) | └─ParameterDefaultValueSyntax
Expand Down Expand Up @@ -170,7 +170,7 @@ param agentCount int = 3
//@[000:0005) | ├─Token(Identifier) |param|
//@[006:0016) | ├─IdentifierSyntax
//@[006:0016) | | └─Token(Identifier) |agentCount|
//@[017:0020) | ├─VariableAccessSyntax
//@[017:0020) | ├─TypeVariableAccessSyntax
//@[017:0020) | | └─IdentifierSyntax
//@[017:0020) | | └─Token(Identifier) |int|
//@[021:0024) | └─ParameterDefaultValueSyntax
Expand All @@ -184,7 +184,7 @@ param agentVMSize string = 'Standard_DS2_v2'
//@[000:0005) | ├─Token(Identifier) |param|
//@[006:0017) | ├─IdentifierSyntax
//@[006:0017) | | └─Token(Identifier) |agentVMSize|
//@[018:0024) | ├─VariableAccessSyntax
//@[018:0024) | ├─TypeVariableAccessSyntax
//@[018:0024) | | └─IdentifierSyntax
//@[018:0024) | | └─Token(Identifier) |string|
//@[025:0044) | └─ParameterDefaultValueSyntax
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ param deployTimeParam string = 'steve'
//@[00:0005) | ├─Token(Identifier) |param|
//@[06:0021) | ├─IdentifierSyntax
//@[06:0021) | | └─Token(Identifier) |deployTimeParam|
//@[22:0028) | ├─VariableAccessSyntax
//@[22:0028) | ├─TypeVariableAccessSyntax
//@[22:0028) | | └─IdentifierSyntax
//@[22:0028) | | └─Token(Identifier) |string|
//@[29:0038) | └─ParameterDefaultValueSyntax
Expand Down Expand Up @@ -144,7 +144,7 @@ output resourceAType string = resA.type
//@[00:0006) | ├─Token(Identifier) |output|
//@[07:0020) | ├─IdentifierSyntax
//@[07:0020) | | └─Token(Identifier) |resourceAType|
//@[21:0027) | ├─VariableAccessSyntax
//@[21:0027) | ├─TypeVariableAccessSyntax
//@[21:0027) | | └─IdentifierSyntax
//@[21:0027) | | └─Token(Identifier) |string|
//@[28:0029) | ├─Token(Assignment) |=|
Expand Down Expand Up @@ -212,7 +212,7 @@ output resourceBId string = resB.id
//@[00:0006) | ├─Token(Identifier) |output|
//@[07:0018) | ├─IdentifierSyntax
//@[07:0018) | | └─Token(Identifier) |resourceBId|
//@[19:0025) | ├─VariableAccessSyntax
//@[19:0025) | ├─TypeVariableAccessSyntax
//@[19:0025) | | └─IdentifierSyntax
//@[19:0025) | | └─Token(Identifier) |string|
//@[26:0027) | ├─Token(Assignment) |=|
Expand Down Expand Up @@ -443,7 +443,7 @@ output resourceCProperties object = resC.properties
//@[00:0006) | ├─Token(Identifier) |output|
//@[07:0026) | ├─IdentifierSyntax
//@[07:0026) | | └─Token(Identifier) |resourceCProperties|
//@[27:0033) | ├─VariableAccessSyntax
//@[27:0033) | ├─TypeVariableAccessSyntax
//@[27:0033) | | └─IdentifierSyntax
//@[27:0033) | | └─Token(Identifier) |object|
//@[34:0035) | ├─Token(Assignment) |=|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ param storageAccount1 string = 'testStorageAccount'
//@[00:005) | ├─Token(Identifier) |param|
//@[06:021) | ├─IdentifierSyntax
//@[06:021) | | └─Token(Identifier) |storageAccount1|
//@[22:028) | ├─VariableAccessSyntax
//@[22:028) | ├─TypeVariableAccessSyntax
//@[22:028) | | └─IdentifierSyntax
//@[22:028) | | └─Token(Identifier) |string|
//@[29:051) | └─ParameterDefaultValueSyntax
Expand All @@ -128,7 +128,7 @@ param storageAccount2 string = 'testStorageAccount'
//@[00:005) | ├─Token(Identifier) |param|
//@[06:021) | ├─IdentifierSyntax
//@[06:021) | | └─Token(Identifier) |storageAccount2|
//@[22:028) | ├─VariableAccessSyntax
//@[22:028) | ├─TypeVariableAccessSyntax
//@[22:028) | | └─IdentifierSyntax
//@[22:028) | | └─Token(Identifier) |string|
//@[29:051) | └─ParameterDefaultValueSyntax
Expand All @@ -143,7 +143,7 @@ param storageAccount3 string = 'testStorageAccount'
//@[00:005) | ├─Token(Identifier) |param|
//@[06:021) | ├─IdentifierSyntax
//@[06:021) | | └─Token(Identifier) |storageAccount3|
//@[22:028) | ├─VariableAccessSyntax
//@[22:028) | ├─TypeVariableAccessSyntax
//@[22:028) | | └─IdentifierSyntax
//@[22:028) | | └─Token(Identifier) |string|
//@[29:051) | └─ParameterDefaultValueSyntax
Expand All @@ -158,7 +158,7 @@ param storageAccount5 string = 'testStorageAccount'
//@[00:005) | ├─Token(Identifier) |param|
//@[06:021) | ├─IdentifierSyntax
//@[06:021) | | └─Token(Identifier) |storageAccount5|
//@[22:028) | ├─VariableAccessSyntax
//@[22:028) | ├─TypeVariableAccessSyntax
//@[22:028) | | └─IdentifierSyntax
//@[22:028) | | └─Token(Identifier) |string|
//@[29:051) | └─ParameterDefaultValueSyntax
Expand Down
Loading

0 comments on commit 6b0e259

Please sign in to comment.