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 for various issues noticed with provider syntax #13542

Merged
merged 2 commits into from Mar 7, 2024
Merged

Conversation

anthony-c-martin
Copy link
Member

@anthony-c-martin anthony-c-martin commented Mar 7, 2024

  1. The BCP395 codefix incorrectly replaces provider 'kubernetes@1.0.0' with non-working syntax provider 'kubernetes', whereas it should be provider kubernetes.
  2. Completions recommend the 'legacy' syntax instead of the recommended one.
  3. Completions don't work in the following case: provider k|
  4. Hovers show import <provider> instead of provider <provider>
  5. ImportKubernetesManifest generates the 'legacy' syntax instead of the recommended one.
  6. Many tests are using the legacy syntax with a suppression for BCP395, rather than the new one.

Copy link
Contributor

github-actions bot commented Mar 7, 2024

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

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

Copy link
Contributor

github-actions bot commented Mar 7, 2024

Test Results

    66 files   -     33      66 suites   - 33   22m 27s ⏱️ - 18m 27s
10 677 tests  -     20  10 675 ✅  -     20  2 💤 ±0  0 ❌ ±0 
25 244 runs   - 12 618  25 240 ✅  - 12 616  4 💤  - 2  0 ❌ ±0 

Results for commit f8872c2. ± Comparison against base commit 9fd452e.

♻️ This comment has been updated with latest results.

@anthony-c-martin anthony-c-martin changed the title Correct code fix logic for legacy provider Fix for various issues noticed with provider syntax Mar 7, 2024
Copy link
Contributor

@jeskew jeskew left a comment

Choose a reason for hiding this comment

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

:shipit:

@anthony-c-martin anthony-c-martin merged commit 6b4769f into main Mar 7, 2024
44 checks passed
@anthony-c-martin anthony-c-martin deleted the ant/codefix branch March 7, 2024 17:31
Copy link
Contributor

@asilverman asilverman left a comment

Choose a reason for hiding this comment

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

Regarding the use of the identifier syntax, I thought that the idea was to hide it behind a feature flag (FF), but it seems like the way to do it is adopting the new syntax without subjecting it to the presence of the dynamic type loading FF

anthony-c-martin added a commit that referenced this pull request Apr 3, 2024
This PR builds on top of #13078, #13542 and #13537 to complete the
implementation of "implicit" providers - e.g. giving the user the
ability to control which providers are available and imported by default
fully through the bicepconfig file.

## Example
Given the following bicepconfig file:
```json
{
  "providers": {
    "az": "builtin:",
    "foo": "br:example.azurecr.io/providers/foo:1.2.3"
  },
  "implicitProviders": ["foo"],
}
```
And `.bicep` files will now have `foo` restored from the registry, and
imported by default. In this example, `az` is *not* imported by default,
and would require an explicit `provider az` in any file wishing to use
it.

## Changes
* Implicit providers are fully supported in the restoration flow, and
errors are surfaced.
* The `DefaultNamespaceProvider` has been replaced with a purely
config-driven equivalent (`NamespaceProvider`). This also introduces a
stricter separation between `NamespaceProvider` (returns a list of
available namespace types including diagnostics), and
`NamespaceResolver` (take the list of available namespaces, and can be
queried to provide information about what is/isn't in scope - e.g. for
binding).
* Improve some of the diagnostics in error cases, add tests.
* Existing functionality should be unaffected.

## Outstanding issues
* Implicit providers introduce places where diagnostics can be surfaced
without a syntax to attach to. Some of the errors will need to be
updated to clarify the source of the problem (bicep config).
###### Microsoft Reviewers: [Open in
CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/Azure/bicep/pull/13725)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants