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
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 76 additions & 0 deletions src/autoFix.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import * as vscode from 'vscode';
import * as path from 'path';
import * as yaml from 'js-yaml';
import * as fs from 'fs';
import { createFix } from './createFix';

interface QuickFixOption {
message: string;
errorCheck: string;
quickfix: {
text: string;
type: string;
};
}

interface QuickFixData {
[errorCode: string]: QuickFixOption[];
}

export function activate(context: vscode.ExtensionContext) {
// Register code action provider
const codeActionProvider = new MyCodeActionProvider();
context.subscriptions.push(
vscode.languages.registerCodeActionsProvider({ scheme: 'file', language: 'yaml' }, codeActionProvider)
);
}

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.

private quickFixes: QuickFixData = {};

constructor() {
// Load quick fixes from external YAML file
const yamlFilePath = path.join(__dirname, '../src/quickfixes.yaml');
const yamlContent = fs.readFileSync(yamlFilePath, 'utf8');
this.quickFixes = yaml.load(yamlContent) as QuickFixData;
}

provideCodeActions(
document: vscode.TextDocument,
range: vscode.Range,
context: vscode.CodeActionContext
): vscode.CodeAction[] | undefined {
const codeActions: vscode.CodeAction[] = [];

// Check if the file is YAML or YML
const extension = path.extname(document.fileName);
if (extension !== '.yaml' && extension !== '.yml') {
return codeActions;
}

for (const diagnostic of context.diagnostics) {
const errorCode = diagnostic.code?.toString();
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

if(diagnostic.message.includes(option.errorCheck)) {
const fix = createFix(document, range, errorCode, option, this.quickFixes);
if (fix) {
codeActions.push(fix);
}
}
}else{
const fix = createFix(document, range, errorCode, option, this.quickFixes);
if (fix) {
codeActions.push(fix);
}
}
}
}
}

return codeActions;
}
}

98 changes: 98 additions & 0 deletions src/createFix.ts
Original file line number Diff line number Diff line change
@@ -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.


interface QuickFixOption {
message: string;
quickfix: {
text: string;
type: string;
};
}

interface CustomCodeAction extends vscode.CodeAction {
[key: string]: any;
}

export function createFix(
document: vscode.TextDocument,
range: vscode.Range,
errorCode: string,
option: QuickFixOption,
quickFixes: { [errorCode: string]: QuickFixOption[] }
): vscode.CodeAction | undefined {
const { message, quickfix } = option;

const fix: CustomCodeAction = new vscode.CodeAction(
quickfix.text,
vscode.CodeActionKind.QuickFix
);
fix.edit = new vscode.WorkspaceEdit();

if (quickfix.type === 'update') {
// Replace the version within the range
fix.edit.replace(document.uri, range, quickfix.text);
} else if (quickfix.type === 'delete') {
// Delete the text within the range
const lineToDelete = range.start.line;
if (lineToDelete >= 0) {
const line = document.lineAt(lineToDelete);
const deleteRange = new vscode.Range(line.range.start, line.range.end);
fix.edit.delete(document.uri, deleteRange);
}
}else if (quickfix.type === 'append') {
// Append '-1' to the duplicate name value
const line = document.lineAt(range.start.line);
const lineText = line.text;

// Find the position of the duplicate name value
const nameStartIndex = lineText.indexOf(': "') + 3;
const nameEndIndex = lineText.lastIndexOf('"');

if (nameStartIndex !== -1 && nameEndIndex !== -1) {
const existingNameValue = lineText.substring(nameStartIndex, nameEndIndex);
const newNameValue = `${existingNameValue}-1`;
//vscode.window.showInformationMessage(`Existing Name: ${existingNameValue}, New Name: ${newNameValue}`);
// Replace the existing name value with the appended one
const nameValueRange = new vscode.Range(
range.start.line,
nameStartIndex,
range.start.line,
nameEndIndex
);
fix.edit.replace(document.uri, nameValueRange, newNameValue);
}
} else if (quickfix.type === 'add') {
const line = document.lineAt(range.start.line);
const lineText = line.text;

// Calculate the indentation based on the line before the error
const previousLine = document.lineAt(range.start.line - 1);
const previousIndentation = previousLine.text.match(/^\s*/)?.[0] || '';

// Check if the current line has an error, and then add the indentation
if (errorCode === 'asyncapi-schema') {
const insertionPosition = line.range.end;

// Insert the quick fix text after the ':'
const insertionText = ` ${quickfix.text}`;
fix.edit.insert(document.uri, insertionPosition, insertionText);
} else {
// Insert the quick fix text with the calculated indentation
const shouldAddIndentation = lineText.trim().length > 0;
const indentation = shouldAddIndentation ? previousIndentation : '';

const insertionPosition = line.range.end;
const insertionText = `\n${indentation}${quickfix.text}`;
fix.edit.insert(document.uri, insertionPosition, insertionText);
}


}

// Set the error message as the code action's title
fix.title = message;

// Add type field to code action metadata
fix.type = quickfix.type;

return fix;
}
7 changes: 6 additions & 1 deletion src/extension.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as vscode from 'vscode';
import { isAsyncAPIFile, openAsyncAPI, openAsyncapiFiles, previewAsyncAPI } from './PreviewWebPanel';
import { asyncapiSmartPaste } from './SmartPasteCommand';

import { MyCodeActionProvider } from './autoFix';

export function activate(context: vscode.ExtensionContext) {
console.log('Congratulations, your extension "asyncapi-preview" is now active!');
Expand Down Expand Up @@ -37,6 +37,11 @@ export function activate(context: vscode.ExtensionContext) {
context.subscriptions.push(vscode.commands.registerCommand('asyncapi.preview', previewAsyncAPI(context)));

context.subscriptions.push(vscode.commands.registerCommand("asyncapi.paste", asyncapiSmartPaste));

const codeActionProvider = new MyCodeActionProvider();
context.subscriptions.push(
vscode.languages.registerCodeActionsProvider({ scheme: 'file', language: 'yaml' }, codeActionProvider)
);
}

export function deactivate() {}
62 changes: 62 additions & 0 deletions src/quickfixes.yaml
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.

Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
asyncapi-latest-version:
- message: "The latest version is not used. You should update to the '2.4.0' version."
errorCheck: "version"
quickfix:
text: "2.4.0"
type: "update"
- message: "The latest version is not used. You should update to the '2.5.0' version."
errorCheck: "version"
quickfix:
text: "2.5.0"
type: "update"

asyncapi-schema:
- message: "'title' property type must be string"
errorCheck: "title"
quickfix:
text: "Example API"
type: "add"
- message: "'version' property type must be string"
errorCheck: "version"
quickfix:
text: "1.0.0"
type: "add"
- message: "'name' property type must be string"
errorCheck: "name"
quickfix:
text: "Dummy Name"
type: "add"
- message: "'url' property type must be string"
errorCheck: "url"
quickfix:
text: "https://example.com"
type: "add"
- message: "'description' property type must be string"
errorCheck: "description"
quickfix:
text: "Dummy description"
type: "add"


asyncapi-info-contact-properties:
- message: "Add a 'name'"
quickfix:
text: "name: Dummy Name"
type: "add"
- message: "Add a 'email'"
quickfix:
text: "email: [email protected]"
type: "add"
- message: "Add a 'url'"
quickfix:
text: "url: https://example.com"
type: "add"

asyncapi-tags-uniqueness:
- message: "Delete the same value object"
quickfix:
type: "delete"
- message: "Update the same value object"
quickfix:
type: "append"
text: "-1"