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(eslint-plugin): [member-delimiter-style] correct invalid fix for multiline with params on the same line #3177

Merged
93 changes: 75 additions & 18 deletions packages/eslint-plugin/src/rules/member-delimiter-style.ts
@@ -1,4 +1,5 @@
import {
TSESLint,
TSESTree,
AST_NODE_TYPES,
} from '@typescript-eslint/experimental-utils';
Expand All @@ -11,6 +12,9 @@ type TypeOptions = {
delimiter?: Delimiter;
requireLast?: boolean;
};
type TypeOptionsWithType = TypeOptions & {
type: string;
};
// eslint-disable-next-line @typescript-eslint/consistent-type-definitions
type BaseOptions = {
multiline?: TypeOptions;
Expand All @@ -29,6 +33,20 @@ type MessageIds =
| 'unexpectedSemi'
| 'expectedComma'
| 'expectedSemi';
type LastTokenType = TSESTree.Token;

interface MakeFixFunctionParams {
optsNone: boolean;
optsSemi: boolean;
lastToken: LastTokenType;
missingDelimiter: boolean;
lastTokenLine: string;
isSingleLine: boolean;
}

type MakeFixFunctionReturnType =
| ((fixer: TSESLint.RuleFixer) => TSESLint.RuleFix)
| null;

const definition = {
type: 'object',
Expand All @@ -54,6 +72,47 @@ const definition = {
additionalProperties: false,
};

const isLastTokenEndOfLine = (token: string, line: string): boolean => {
const positionInLine = line.indexOf(token);

return positionInLine === line.length - 1;
};

const makeFixFunction = ({
optsNone,
optsSemi,
lastToken,
missingDelimiter,
lastTokenLine,
isSingleLine,
}: MakeFixFunctionParams): MakeFixFunctionReturnType => {
// if removing is the action but last token is not the end of the line
if (
optsNone &&
!isLastTokenEndOfLine(lastToken.value, lastTokenLine) &&
!isSingleLine
) {
return null;
}

return (fixer: TSESLint.RuleFixer): TSESLint.RuleFix => {
if (optsNone) {
// remove the unneeded token
return fixer.remove(lastToken);
}

const token = optsSemi ? ';' : ',';

if (missingDelimiter) {
// add the missing delimiter
return fixer.insertTextAfter(lastToken, token);
}

// correct the current delimiter
return fixer.replaceText(lastToken, token);
};
};

export default util.createRule<Options, MessageIds>({
name: 'member-delimiter-style',
meta: {
Expand Down Expand Up @@ -127,7 +186,7 @@ export default util.createRule<Options, MessageIds>({
*/
function checkLastToken(
member: TSESTree.TypeElement,
opts: TypeOptions,
opts: TypeOptionsWithType,
isLast: boolean,
): void {
/**
Expand All @@ -147,10 +206,14 @@ export default util.createRule<Options, MessageIds>({
const lastToken = sourceCode.getLastToken(member, {
includeComments: false,
});

if (!lastToken) {
return;
}

const sourceCodeLines = sourceCode.getLines();
const lastTokenLine = sourceCodeLines[lastToken?.loc.start.line - 1];

const optsSemi = getOption('semi');
const optsComma = getOption('comma');
const optsNone = getOption('none');
Expand Down Expand Up @@ -193,22 +256,14 @@ export default util.createRule<Options, MessageIds>({
},
},
messageId,
fix(fixer) {
if (optsNone) {
// remove the unneeded token
return fixer.remove(lastToken);
}

const token = optsSemi ? ';' : ',';

if (missingDelimiter) {
// add the missing delimiter
return fixer.insertTextAfter(lastToken, token);
}

// correct the current delimiter
return fixer.replaceText(lastToken, token);
},
fix: makeFixFunction({
optsNone,
optsSemi,
lastToken,
missingDelimiter,
lastTokenLine,
isSingleLine: opts.type === 'single-line',
}),
});
}
}
Expand Down Expand Up @@ -239,7 +294,9 @@ export default util.createRule<Options, MessageIds>({
node.type === AST_NODE_TYPES.TSInterfaceBody
? interfaceOptions
: typeLiteralOptions;
const opts = isSingleLine ? typeOpts.singleline : typeOpts.multiline;
const opts = isSingleLine
? { ...typeOpts.singleline, type: 'single-line' }
: { ...typeOpts.multiline, type: 'multi-line' };

members.forEach((member, index) => {
checkLastToken(member, opts ?? {}, index === members.length - 1);
Expand Down
18 changes: 18 additions & 0 deletions packages/eslint-plugin/tests/rules/member-delimiter-style.test.ts
Expand Up @@ -851,6 +851,24 @@ interface Foo {
},
{
code: `
type Test = {
a: {
one: 1
}; b: 2
};
`,
output: null,
options: [{ multiline: { delimiter: 'none' } }],
errors: [
{
messageId: 'unexpectedSemi',
line: 5,
column: 5,
},
],
},
{
code: `
interface Foo {
name: string
age: number
Expand Down