Skip to content

Commit

Permalink
Add quick fix for S4782 ('no-redundant-optional') (#3060)
Browse files Browse the repository at this point in the history
  • Loading branch information
@yassin-kammoun-sonarsource
yassin-kammoun-sonarsource committed Mar 24, 2022
1 parent 9fe2ce1 commit 624fdf3
Show file tree
Hide file tree
Showing 3 changed files with 206 additions and 3 deletions.
1 change: 1 addition & 0 deletions eslint-bridge/src/quickfix.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ const quickFixRules = new Set([
'no-global-this',
'no-in-misuse',
'no-primitive-wrappers',
'no-redundant-optional',
'no-redundant-parentheses',
'no-undefined-argument',
'no-unthrown-error',
Expand Down
61 changes: 59 additions & 2 deletions eslint-bridge/src/rules/no-redundant-optional.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,14 @@
*/
// https://jira.sonarsource.com/browse/RSPEC-4782

import { Rule } from 'eslint';
import { Rule, AST } from 'eslint';
import * as estree from 'estree';
import { TSESTree } from '@typescript-eslint/experimental-utils';
import { isRequiredParserServices, toEncodedMessage } from '../utils';

export const rule: Rule.RuleModule = {
meta: {
hasSuggestions: true,
schema: [
{
// internal parameter for rules having secondary locations
Expand All @@ -52,6 +53,7 @@ export const rule: Rule.RuleModule = {

const typeNode = getUndefinedTypeAnnotation(tsNode.typeAnnotation);
if (typeNode) {
const suggest = getQuickFixSuggestions(context, optionalToken, typeNode);
const secondaryLocations = [typeNode];
const message = toEncodedMessage(
"Consider removing 'undefined' type or '?' specifier, one of them is redundant.",
Expand All @@ -60,6 +62,7 @@ export const rule: Rule.RuleModule = {
context.report({
message,
loc: optionalToken.loc,
suggest,
});
}
}
Expand All @@ -81,7 +84,61 @@ function getUndefinedTypeNode(typeNode: TSESTree.TypeNode): TSESTree.TypeNode |
if (typeNode.type === 'TSUndefinedKeyword') {
return typeNode;
} else if (typeNode.type === 'TSUnionType') {
return typeNode.types.find(innerTypeNode => getUndefinedTypeNode(innerTypeNode));
return typeNode.types.map(getUndefinedTypeNode).find(tpe => tpe !== undefined);
}
return undefined;
}

function getQuickFixSuggestions(
context: Rule.RuleContext,
optionalToken: AST.Token,
undefinedType: TSESTree.TypeNode,
): Rule.SuggestionReportDescriptor[] {
const suggestions: Rule.SuggestionReportDescriptor[] = [
{
desc: 'Remove "?" operator',
fix: fixer => fixer.remove(optionalToken),
},
];
if (undefinedType.parent?.type === 'TSUnionType') {
suggestions.push(getUndefinedRemovalSuggestion(context, undefinedType));
}
return suggestions;
}

function getUndefinedRemovalSuggestion(
context: Rule.RuleContext,
undefinedType: TSESTree.TypeNode,
): Rule.SuggestionReportDescriptor {
return {
desc: 'Remove "undefined" type annotation',
fix: fixer => {
const fixes: Rule.Fix[] = [];
const unionType = undefinedType.parent as TSESTree.TSUnionType;
if (unionType.types.length === 2) {
const unionTypeNode = unionType as any as estree.Node;
const otherType =
unionType.types[0] === undefinedType ? unionType.types[1] : unionType.types[0];
const otherTypeText = context.getSourceCode().getText(otherType as any as estree.Node);
fixes.push(fixer.replaceText(unionTypeNode, otherTypeText));

const tokenBefore = context.getSourceCode().getTokenBefore(unionTypeNode);
const tokenAfter = context.getSourceCode().getTokenAfter(unionTypeNode);
if (tokenBefore?.value === '(' && tokenAfter?.value === ')') {
fixes.push(fixer.remove(tokenBefore));
fixes.push(fixer.remove(tokenAfter));
}
} else {
const index = unionType.types.indexOf(undefinedType);
if (index === 0) {
fixes.push(fixer.removeRange([undefinedType.range[0], unionType.types[1].range[0]]));
} else {
fixes.push(
fixer.removeRange([unionType.types[index - 1].range[1], undefinedType.range[1]]),
);
}
}
return fixes;
},
};
}
147 changes: 146 additions & 1 deletion eslint-bridge/tests/rules/no-redundant-optional.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ ruleTester.run(
address: string | undefined;
pet?: string;
}
class Car {
propWithoutType?;
brand: string;
Expand Down Expand Up @@ -69,11 +68,157 @@ ruleTester.run(
}`,
errors: [
{
message: JSON.stringify({
message:
"Consider removing 'undefined' type or '?' specifier, one of them is redundant.",
secondaryLocations: [
{
column: 33,
line: 3,
endColumn: 42,
endLine: 3,
},
],
}),
line: 3,
endLine: 3,
},
],
},
{
code: `interface T { p?: undefined | number; }`,
errors: [
{
suggestions: [
{
desc: 'Remove "?" operator',
output: 'interface T { p: undefined | number; }',
},
{
desc: 'Remove "undefined" type annotation',
output: 'interface T { p?: number; }',
},
],
},
],
},
{
code: `interface T { p?: number | undefined; }`,
errors: [
{
suggestions: [
{
output: 'interface T { p: number | undefined; }',
},
{
output: 'interface T { p?: number; }',
},
],
},
],
},
{
code: `interface T { p?: (undefined | number); }`,
errors: [
{
suggestions: [
{
output: 'interface T { p: (undefined | number); }',
},
{
output: 'interface T { p?: number; }',
},
],
},
],
},
{
code: `interface T { p?: undefined | number | string; }`,
errors: [
{
suggestions: [
{
output: 'interface T { p: undefined | number | string; }',
},
{
output: 'interface T { p?: number | string; }',
},
],
},
],
},
{
code: `interface T { p?: number | undefined | string; }`,
errors: [
{
suggestions: [
{
output: 'interface T { p: number | undefined | string; }',
},
{
output: 'interface T { p?: number | string; }',
},
],
},
],
},
{
code: `interface T { p?: number | string | undefined; }`,
errors: [
{
suggestions: [
{
output: 'interface T { p: number | string | undefined; }',
},
{
output: 'interface T { p?: number | string; }',
},
],
},
],
},
{
code: `interface T { p?: number | (string | undefined); }`,
errors: [
{
suggestions: [
{
output: 'interface T { p: number | (string | undefined); }',
},
{
output: 'interface T { p?: number | string; }',
},
],
},
],
},
{
code: `interface T { p?: undefined; }`,
errors: [
{
suggestions: [
{
output: 'interface T { p: undefined; }',
},
],
},
],
},
{
code: `interface T { p?: (undefined) | number; }`,
errors: [
{
suggestions: [
{
output: 'interface T { p: (undefined) | number; }',
},
{
output: 'interface T { p?: number; }',
},
],
},
],
},
],
},
);

0 comments on commit 624fdf3

Please sign in to comment.