Skip to content
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

Fix compiled representation of wildcard variable property references #12048

Merged
merged 1 commit into from Oct 4, 2023

Conversation

jeskew
Copy link
Contributor

@jeskew jeskew commented Oct 4, 2023

Resolves #12042

#11879 introduced a regression in references to variables imported with via wildcard syntax were represented in a template's compiled form. For example,

import * as mod from 'mod.bicep'
output s string = mod.s

would generate a value of "[variables('mod').s]" in the compiled template. When implementing variable imports for .bicepparam files, I took the shortcut of generating a object value for wildcard imports whose properties were all the variables exported from the imported model. There was a gap in test coverage for variable imports in .bicep files, so the regression was not picked up. This PR updates how wildcard imports are handled in .bicepparam files to remove the aforementioned shortcut.

The PR also adds an extra step in TemplateWriter: after .bicep files are converted to the compiler's intermediate representation, expressions referring to imported values are replaced with expressions that refer to "synthetic" variables and types.

Microsoft Reviewers: Open in CodeFlow

@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2023

Test this change out locally with the following install scripts (Action run 6409533970)

VSCode
  • Mac/Linux
    bash <(curl -Ls https://aka.ms/bicep/nightly-vsix.sh) --run-id 6409533970
  • Windows
    iex "& { $(irm https://aka.ms/bicep/nightly-vsix.ps1) } -RunId 6409533970"
Azure CLI
  • Mac/Linux
    bash <(curl -Ls https://aka.ms/bicep/nightly-cli.sh) --run-id 6409533970
  • Windows
    iex "& { $(irm https://aka.ms/bicep/nightly-cli.ps1) } -RunId 6409533970"

@jeskew jeskew added this to the v0.22 milestone Oct 4, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2023

Test Results

     132 files  ±0       132 suites  ±0   4h 2m 57s ⏱️ -1s
10 666 tests +1  10 666 ✔️ +1  0 💤 ±0  0 ±0 
51 531 runs  +4  51 531 ✔️ +4  0 💤 ±0  0 ±0 

Results for commit df9776b. ± Comparison against base commit c644867.

Copy link
Contributor

@shenglol shenglol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@jeskew jeskew merged commit eb7ab40 into main Oct 4, 2023
47 checks passed
@jeskew jeskew deleted the jeskew/fix-wildcard-variable-property-refs branch October 4, 2023 17:37
jeskew added a commit that referenced this pull request Oct 6, 2023
…ses (#12055)

Resolves #12052 

A recent PR (#12048) updated TemplateWriter to preprocess a template's
references to imported symbols. This new step uses an
`ExpressionRewriteVisitor`, which had not been kept up to date when new
properties were added various kinds of IR expressions.

The PR also adds a few more lines to the `Imports_LF` baseline scenario
to exercise additional behavior. The scenario had originally been
written to validate the mechanics of importing types and variables but
didn't cover the mechanics of *using* imported symbols in a template.
###### Microsoft Reviewers: [Open in
CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/Azure/bicep/pull/12055)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Importing variables not working as expected, intellisense also not working
2 participants