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

Implement type completions & validation for resource list functions #5145

Merged
merged 9 commits into from
Nov 17, 2021

Conversation

anthony-c-martin
Copy link
Member

@anthony-c-martin anthony-c-martin commented Nov 10, 2021

Changes

Limitations

  • list function completions are only offered for API versions matching the API version used for the resource. Wildcard functions are still supported as a fallback, but will lack type information.
  • It's somewhat opaque to users what properties are available inside a named object parameter, especially in situations where they haven't been marked as required. This problem exists generally in Bicep, so I haven't tried to solve it with this PR.

Demo

Screen.Recording.2021-11-15.at.8.08.30.PM.mov

@anthony-c-martin anthony-c-martin changed the title [DRAFT - Ignore] Experiment: list function types Implement type completions & validation for resource list functions Nov 16, 2021
@majastrz
Copy link
Member

majastrz commented Nov 17, 2021

It's really neat that this brings in type checking for things like this:
image

Although, we should call out in our release notes that the additional validation done by this may produce errors that weren't logged previously. (In cases of swagger inaccuracies.)

return Enumerable.Empty<CompletionItem>();
}

var argIndex = context.FunctionArgument is null ? 0 : functionCall.Arguments.IndexOf(context.FunctionArgument);
Copy link
Member

Choose a reason for hiding this comment

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

IndexOf

Would it make sense to put this type computation logic in the DeclaredTypeManager and only retrieve here?

Copy link
Member

@majastrz majastrz left a comment

Choose a reason for hiding this comment

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

This is great! Added a minor question.

@anthony-c-martin anthony-c-martin mentioned this pull request Nov 17, 2021
26 tasks
@anthony-c-martin anthony-c-martin enabled auto-merge (squash) November 17, 2021 02:03
@anthony-c-martin anthony-c-martin merged commit fea4214 into main Nov 17, 2021
@anthony-c-martin anthony-c-martin deleted the ant/exp/list_funcs branch November 17, 2021 03:02
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.

2 participants