-
-
Notifications
You must be signed in to change notification settings - Fork 36
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
feat: autofix feature for common linting errors #187
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs work.
src/quickfixes.yaml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This list of spectral error codes for asyncapi that can be autofixed is insufficient and needs to be expanded to include mode rules.
Exploring existing spectral rules and find posible and creative fixes is a fundamental part of this GSoC project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this pr I wanted to get a review so I could expand the list and incorporate additional error codes.
if (errorCode && this.quickFixes[errorCode]) { | ||
const options = this.quickFixes[errorCode]; | ||
for (const option of options) { | ||
if (errorCode === 'asyncapi-schema'){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is something magic about 'asyncapi-schema' to be harcoded here in the provideCodeActions
method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many errors have the same error code i.e. asyncapi-schema
So for that specific error code I'm checking if errorMessage contains errorCheck word (which is present in the yaml file) and showing autofix directly
The errors can't be distinguish by just error codes so had to add one more condition for just asyncapi-schema errors
src/autoFix.ts
Outdated
); | ||
} | ||
|
||
export class MyCodeActionProvider implements vscode.CodeActionProvider { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a very bad name for a class. Snippets should be reviewed not just copy+paste.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed the name of the class. I'll not make the same mistake again.
@@ -0,0 +1,98 @@ | |||
import * as vscode from 'vscode'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this file can work as a generic framework to add different autofixes to the quickfixes.yaml
config file.
It seems very add-hoc to some particular cases.
Manipulating users workspace yaml files is a sensitive procedure that should easily tested and debuged.
- There should be a generic set of functions to manipulate yaml files by
JsonPath
: add, update, delete, append... - Those function should be as much as posible agnostic of vscode APIs because vscode apis can not be easily unit-tested.
- Those function should be unit-tested. This was an important task: to investigate different unit testing libs for javascript (like mocha and jest), try them and then request for guidance to community members to decide about which one to use.
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
@Savio629 do you plan to continue working on this one? |
Yeah I plan to continue working on it |
Description
Autofix feature for common linting errors
autofix.mp4
Errors that the autofix fixes:
The autofix also doesn't load it the extension till the user hovers or click on the error.
output.compress-video-online.com.mp4
Resolves #160
Related issue(s)