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

fix: DH-18542: Remove duplicate and invalid ruff quick fixes #2360

Merged
merged 4 commits into from
Feb 7, 2025
Merged
Changes from all 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
80 changes: 55 additions & 25 deletions packages/console/src/monaco/MonacoProviders.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,10 @@ class MonacoProviders extends PureComponent<
d.end_location.row,
d.end_location.column
);
return diagnosticRange.intersectRanges(range);
return (
d.code != null && // Syntax errors have no code and can't be fixed/disabled
diagnosticRange.intersectRanges(range)
);
});

const fixActions: monaco.languages.CodeAction[] = diagnostics
Expand Down Expand Up @@ -296,8 +299,24 @@ class MonacoProviders extends PureComponent<
};
});

const disableActions: monaco.languages.CodeAction[] = diagnostics
const seenCodes = new Set<string>();
const duplicateCodes = new Set<string>();
diagnostics.forEach(d => {
if (d.code == null) {
return;
}
if (seenCodes.has(d.code)) {
duplicateCodes.add(d.code);
}
seenCodes.add(d.code);
});

const disableLineActions: monaco.languages.CodeAction[] = diagnostics
.map(d => {
if (d.code == null) {
// The nulls are already filtered out, but TS doesn't know that
return [];
}
const line = model.getLineContent(d.location.row);
const lastToken = monaco.editor
.tokenize(line, model.getLanguageId())[0]
Expand Down Expand Up @@ -327,7 +346,11 @@ class MonacoProviders extends PureComponent<
}
return [
{
title: `Disable ${d.code} for this line`,
title: `Disable ${d.code} for ${
duplicateCodes.has(d.code)
? `line ${d.location.row}`
: 'this line'
}`,
kind: 'quickfix',
edit: {
edits: [
Expand All @@ -339,33 +362,40 @@ class MonacoProviders extends PureComponent<
],
},
},
];
})
.flat()
.filter(
// Remove actions with duplicate titles as you can't disable the same rule on a line twice
(action, i, arr) => arr.find(a => a.title === action.title) === action
);

const disableGlobalActions: monaco.languages.CodeAction[] = [
...seenCodes,
].map(code => ({
title: `Disable ${code} for this file`,
kind: 'quickfix',
edit: {
edits: [
{
title: `Disable ${d.code} for this file`,
kind: 'quickfix',
edit: {
edits: [
{
resource: model.uri,
versionId: model.getVersionId(),
textEdit: {
range: {
startLineNumber: 1,
startColumn: 1,
endLineNumber: 1,
endColumn: 1,
},
text: `# ruff: noqa: ${d.code}\n`,
},
},
],
resource: model.uri,
versionId: model.getVersionId(),
textEdit: {
range: {
startLineNumber: 1,
startColumn: 1,
endLineNumber: 1,
endColumn: 1,
},
text: `# ruff: noqa: ${code}\n`,
},
},
];
})
.flat();
],
},
}));

return {
actions: [...fixActions, ...disableActions],
actions: [...fixActions, ...disableLineActions, ...disableGlobalActions],
dispose: () => {
/* no-op */
},
Expand Down
Loading