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

feat: autofix feature for common linting errors #187

Closed
wants to merge 4 commits into from

Conversation

Savio629
Copy link
Contributor

@Savio629 Savio629 commented Aug 15, 2023

Description
Autofix feature for common linting errors

autofix.mp4

Errors that the autofix fixes:

  • Update
    • Asyncapi-version (The latest version is not used.)
  • Add
    • Title ('title' property type must be string)
    • Version ('version' property type must be string)
    • Name ('name' property type must be string)
    • Url ('url' property type must be string)
    • Description ('description' property type must be string)
    • Under contact (added name, url, email)
  • Delete
    • Tags object ( Delete the same value object)
  • Append
    • Tags object (Update the same value object)



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)

Copy link
Collaborator

@ivangsa ivangsa left a comment

Choose a reason for hiding this comment

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

Needs work.

Copy link
Collaborator

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.

Copy link
Contributor Author

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'){
Copy link
Collaborator

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?

Copy link
Contributor Author

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 {
Copy link
Collaborator

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.

Copy link
Contributor Author

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';
Copy link
Collaborator

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.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@derberg
Copy link
Member

derberg commented Sep 4, 2023

@Savio629 do you plan to continue working on this one?

@Savio629
Copy link
Contributor Author

Savio629 commented Sep 4, 2023

Yeah I plan to continue working on it

@ivangsa ivangsa closed this Dec 13, 2023
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.

Add "Autofix" feature for common linting errors (w/ spectral)
3 participants