bicep icon indicating copy to clipboard operation
bicep copied to clipboard

Nameof function

Open miqm opened this issue 1 year ago • 1 comments

Fixes #13031

Contributing a Pull Request

If you haven't already, read the full contribution guide. The guide may have changed since the last time you read it, so please double-check. Once you are done and ready to submit your PR, run through the relevant checklist below.

Contributing a feature

  • [x] I have opened a new issue for the proposal, or commented on an existing one, and ensured that the Bicep maintainers are good with the design of the feature being implemented
  • [x] I have included "Fixes #{issue_number}" in the PR description, so GitHub can link to the issue and close it when the PR is merged
  • [x] I have appropriate test coverage of my new feature
Microsoft Reviewers: Open in CodeFlow

miqm avatar Jul 29 '24 23:07 miqm

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 9.37%. Comparing base (108d816) to head (496bae5). Report is 1386 commits behind head on main.

:exclamation: There is a different number of reports uploaded between BASE (108d816) and HEAD (496bae5). Click for more details.

HEAD has 4 uploads less than BASE
Flag BASE (108d816) HEAD (496bae5)
dotnet 4 0
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main   #14704       +/-   ##
===========================================
- Coverage   94.28%    9.37%   -84.91%     
===========================================
  Files        1113        7     -1106     
  Lines      100791      288   -100503     
  Branches     8734      123     -8611     
===========================================
- Hits        95028       27    -95001     
+ Misses       4595      260     -4335     
+ Partials     1168        1     -1167     
Flag Coverage Δ
dotnet ?
typescript 9.37% <ø> (+1.66%) :arrow_up:

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

see 1105 files with indirect coverage changes

codecov-commenter avatar Jul 29 '24 23:07 codecov-commenter

@miqm we had a discussion on this yesterday, and I've pushed a few changes to genericize the check under a function flag IsArgumentValueIndependent.

The final thing we need to close on is whether or not this function should behave like other functions w.r.t inferring dependencies.

  • On one hand, it conceptually doesn't need to if you think about it as "the value isn't accessed".
  • On the other hand, Bicep's inferring of dependencies is already hard for people to understand, and adding a function which doesn't behave like others may make it even harder to understand.

For example, the difference between the following is quite subtle:

resource b '...' existing = { name: 'b' }

resource a '...' = {
  name: 'a'
  properties: { foo: nameof(b) }
}

and:

resource b '...' existing = { name: 'b' }

resource a '...' = {
  name: 'a'
  properties: { foo: b.name }
}

anthony-c-martin avatar Sep 12 '24 11:09 anthony-c-martin

I explained this on the community call today. We got a pretty strong consensus from the community that nameof() should infer dependencies:

image

anthony-c-martin avatar Oct 03 '24 16:10 anthony-c-martin

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

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

github-actions[bot] avatar Oct 04 '24 03:10 github-actions[bot]