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

Code actions/fixes for common edits #1406

Closed
majastrz opened this issue Jan 28, 2021 · 9 comments
Closed

Code actions/fixes for common edits #1406

majastrz opened this issue Jan 28, 2021 · 9 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@majastrz
Copy link
Member

majastrz commented Jan 28, 2021

We should consider adding code actions or commands to make common edits in Bicep. For example, parameters could allow actions like the following:

  • Make it secure (strings and objects only - adds the @secure decorator above it)
  • Add allowed values (adds an empty @allowed snippet and moves the cursor to the first item in the array)
  • creating a loop/set of resources from an existing one
  • ...
@majastrz majastrz added the enhancement New feature or request label Jan 28, 2021
@ghost ghost added the Needs: Triage 🔍 label Jan 28, 2021
@alex-frankel alex-frankel added good first issue Good for newcomers and removed Needs: Triage 🔍 labels Feb 10, 2021
@alex-frankel alex-frankel added this to the Committed Backlog milestone Feb 10, 2021
@nilshedstrom
Copy link
Contributor

In this file I create a new rule that provides a fix to add the @secure decorator to parameters of type string and object.
It seems to work fine. The problem is that I get a blue squiggly for the row which seems to indicate that there is something wrong with it. I would like to provide a code action not indicate that there is an error on the line.
image

I know how to provide code actions in typescript.
Is there a way to provide code actions (without blue squigglies) in the language server or should I implement the change in typescript instead?

@StefanIvemo
@StephenWeatherford

@StephenWeatherford
Copy link
Contributor

@nilshedstrom Hey Nils. I'm afraid the linter was not designed for this - it returns diagnostics. What we need for code refactorings is to create a code actions provider (vscode.languages.registerCodeActionsProvider). It would need to be hooked up to synchronize with the language server compiler.

@majastrz @anthony-c-martin This was marked as a good first issue, but it seems to me it will need some preparatory work first. After that, adding new refactorings ought to be fairly straightforward (we were just talking today about refactorings).

@nilshedstrom We want to avoid doing work in typescript for Bicep because it would be restricted to vscode. If it's kept in the language server, it can be reused for other editors (that wasn't possible when the ARM Tools extension was started).

@nilshedstrom
Copy link
Contributor

@majastrz @anthony-c-martin This was marked as a good first issue, but it seems to me it will need some preparatory work first. After that, adding new refactorings ought to be fairly straightforward (we were just talking today about refactorings).

Ok. Tell me when I can start working on this issue.

@anthony-c-martin
Copy link
Member

anthony-c-martin commented Dec 9, 2021

@nilshedstrom - I've pushed a branch here: https://github.com/Azure/bicep/compare/ant/codefix to serve as an example. Feel free to take this code and fork it for a starting point!

Note that my implementation for SecureParameterCodeFixProvider.cs is not robust - really intended to just provide an example of looking for a particular piece of syntax (parameter declaration), and suggesting a rewrite for it.

Let me know if you have any questions :)

Here's a demo of this code in action:

Screen.Recording.2021-12-08.at.11.12.44.PM.mov

@nilshedstrom
Copy link
Contributor

nilshedstrom commented Dec 12, 2021

Thanks @anthony-c-martin!
I have created a more robust version based on your code.

I do not find a way to set the cursor when providing a quick fix (this was requested in this issue when adding the allowed decorator).
It would probably be useful to set the cursor when adding the decorator description, maxLength, maxValue, minLength and minValue too.

@anthony-c-martin
Copy link
Member

@nilshedstrom - looks like moving the cursor in a codeAction is unfortunately not supported; it has been requested in microsoft/language-server-protocol#724.

@anthony-c-martin
Copy link
Member

Closing this issue as we now have support for parameter decorators (thanks @nilshedstrom!). Going forwards, I suggest we create separate issues for each code fix idea.

@StephenWeatherford
Copy link
Contributor

Re-opening/reverting, the PR has failing suites.

davidcho23 pushed a commit that referenced this issue Feb 1, 2022
* Codefix example

* Made SecureParameterCodeFixProvider more robust

* Trying to fix the tests for CodeActions (after adding the new QuickFix'es)

* Added DescriptionParameterCodeFixProvider

* Refactored and added support for @Allowed, @minlength, @maxlength, @minValue, @MaxValue

* Made ParameterCodeFixProvider more robust

Co-authored-by: Anthony Martin <[email protected]>
Co-authored-by: Hedström, Nils <[email protected]>
@anthony-c-martin
Copy link
Member

Closing again - the PR was eventually re-merged 😄

@ghost ghost locked as resolved and limited conversation to collaborators May 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

5 participants