Refactor SourceFileGrouping to remove provider logic

[anthony-c-martin]

I felt like the logic in the SourceFileGroupingBuilder was becoming overly complex, with the introduction of concerns about how to parse and interpret provider declarations. This refactor moves provider interpretation logic into DeclarationVisitor, and keeps the responsibility of the SourceFileGroupingBuilder focused on obtaining the full set of source files + files that need to be restored from the registry.

Microsoft Reviewers: Open in CodeFlow

[github-actions]

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

VSCode

• Mac/Linux

  bash <(curl -Ls https://aka.ms/bicep/nightly-vsix.sh) --run-id 8165730326

• Windows

  iex "& { $(irm https://aka.ms/bicep/nightly-vsix.ps1) } -RunId 8165730326"

Azure CLI

• Mac/Linux

  bash <(curl -Ls https://aka.ms/bicep/nightly-cli.sh) --run-id 8165730326

• Windows

  iex "& { $(irm https://aka.ms/bicep/nightly-cli.ps1) } -RunId 8165730326"

[StephenWeatherford]

Please make sure #13495 gets merged first, in case there are conflicts.

[github-actions]

Test Results

    66 files   -     33      66 suites   - 33   22m 49s ⏱️ - 23m 40s
10 677 tests  -     19  10 675 ✅  -     20  2 💤 +1  0 ❌ ±0 
25 244 runs   - 12 615  25 240 ✅  - 12 616  4 💤 +1  0 ❌ ±0 

Results for commit 137d365. ± Comparison against base commit 3a300e5.

♻️ This comment has been updated with latest results.

[anthony-c-martin]

Please make sure #13495 gets merged first, in case there are conflicts.

Will do! And I'm sure there will be :(

[asilverman]

It's possible to evaluate GetArtifactRestoreResult(file, restorable); once above the if-clause and use its result rather than calling the function in two seaparate places.

[anthony-c-martin]

Could you explain why this is beneficial?

[asilverman]

This is just my opinion, but I think there are 2 main benefits:

1. As a reader it makes it clear that the artifact restore result doesn't depend on the restorable type, and that the restore result is terminal
2. It reduces the chance that one will change but not the other in future refactorings

These may not be strong enough justification, can you share your thoughts on the drawbacks of my suggestion and the benefits of keeping it as is?

[asilverman]

LGTM, ship it! 🚀

[asilverman]

Hey @anthony-c-martin , can this wait for #13537?

[anthony-c-martin]

Hey @anthony-c-martin , can this wait for #13537?

Sure, happy to wait