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.

Sign up for GitHub

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

Jump to bottom

fix(eslint-plugin): [member-delimiter-style] correct invalid fix for multiline with params on the same line #3177

Merged
merged 9 commits into from Mar 21, 2021
Merged

fix(eslint-plugin): [member-delimiter-style] correct invalid fix for multiline with params on the same line #3177

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
74d7e53
fix(eslint-plugin): do not fix code when semi creates error in multiline
tanohzana Mar 12, 2021
14491bc
fix(eslint-plugin): change Config multilineDetection type
tanohzana Mar 12, 2021
a27e2e3
fix(eslint-plugin): fix RuleFix and RuleFixer imports
tanohzana Mar 16, 2021
e5353a9
fix(eslint-plugin): put back multilineDetection option in Config type
tanohzana Mar 16, 2021
75e83de
fix(eslint-plugin): put back multilineDetection into schema
tanohzana Mar 16, 2021
f636acf
fix(eslint-plugin): put back tests as they are on master
tanohzana Mar 16, 2021
5d2b9c2
test(eslint-plugin): test new fix
tanohzana Mar 16, 2021
8c0e78f
Merge branch 'master' into fix/member-delimiter-style
tanohzana Mar 16, 2021
4697939
Merge branch 'master' into fix/member-delimiter-style
tanohzana Mar 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
100 changes: 77 additions & 23 deletions packages/eslint-plugin/src/rules/member-delimiter-style.ts
View file
Open in desktop
Expand Up @@ -2,6 +2,10 @@ import {
TSESTree,
AST_NODE_TYPES,
} from '@typescript-eslint/experimental-utils';
import {
RuleFix,
RuleFixer,
} from '@typescript-eslint/experimental-utils/src/ts-eslint';
Copy link
Member

Choose a reason for hiding this comment

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

don't add deep imports on /src from plugins.
This is availaible on the TSESLint export of @typescript-eslint/experimental-utils

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed here: a27e2e3

import * as util from '../util';

type Delimiter = 'comma' | 'none' | 'semi';
Expand All @@ -11,6 +15,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 @@ -21,14 +28,26 @@ type Config = BaseOptions & {
typeLiteral?: BaseOptions;
interface?: BaseOptions;
};
multilineDetection?: 'brackets' | 'last-member';
Copy link
Member

Choose a reason for hiding this comment

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

why have you removed this option entirely?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed here: e5353a9

multilineDetection?: 'last-member';
};
type Options = [Config];
type MessageIds =
| 'unexpectedComma'
| 'unexpectedSemi'
| 'expectedComma'
| 'expectedSemi';
type LastTokenType = TSESTree.Token;

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

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

const definition = {
type: 'object',
Expand All @@ -54,6 +73,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: RuleFixer): 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 @@ -83,9 +143,6 @@ export default util.createRule<Options, MessageIds>({
},
additionalProperties: false,
},
multilineDetection: {
enum: ['brackets', 'last-member'],
},
Copy link
Member

Choose a reason for hiding this comment

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

why have you removed this from the schema entirely?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed here: 75e83de

}),
additionalProperties: false,
},
Expand All @@ -101,7 +158,6 @@ export default util.createRule<Options, MessageIds>({
delimiter: 'semi',
requireLast: false,
},
multilineDetection: 'brackets',
},
],
create(context, [options]) {
Expand All @@ -127,7 +183,7 @@ export default util.createRule<Options, MessageIds>({
*/
function checkLastToken(
member: TSESTree.TypeElement,
opts: TypeOptions,
opts: TypeOptionsWithType,
isLast: boolean,
): void {
/**
Expand All @@ -147,10 +203,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 +253,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 +291,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
183 changes: 18 additions & 165 deletions packages/eslint-plugin/tests/rules/member-delimiter-style.test.ts
View file
Open in desktop
Expand Up @@ -705,75 +705,6 @@ type Bar = {
},
],
},
{
code: `
type Foo = {a: {
b: true;
}};
`,
options: [
{
multilineDetection: 'last-member',
},
],
},
`
type Foo = {a: {
b: true;
};};
`,
{
code: `
type Foo = {a: {
b: true;
};};
`,
options: [
{
multilineDetection: 'brackets',
},
],
},
{
code: `
type Foo = {
a: {
b: true;
};
};
`,
options: [
{
multilineDetection: 'last-member',
},
],
},
{
code: `
type Foo = {a: {
b: true;
};};
`,
options: [
{
singleline: { delimiter: 'semi', requireLast: true },
multilineDetection: 'last-member',
},
],
},
{
code: `
type Foo = {

};
`,
options: [
{
singleline: { delimiter: 'semi', requireLast: true },
multilineDetection: 'last-member',
},
],
},

{
code: `
Expand Down Expand Up @@ -905,6 +836,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 Expand Up @@ -3434,101 +3383,5 @@ type Foo = {
},
],
},
{
code: `
type Foo = {a: {
b: true;
};};
`,
output: `
type Foo = {a: {
b: true;
}};
`,
options: [
{
multilineDetection: 'last-member',
},
],
errors: [
{
messageId: 'unexpectedSemi',
line: 4,
column: 3,
},
],
},
{
code: `
type Foo = {a: {
b: true;
}};
`,
output: `
type Foo = {a: {
b: true;
};};
`,
errors: [
{
messageId: 'expectedSemi',
line: 4,
column: 2,
},
],
},
{
code: `
type Foo = {
a: {
b: true;
}
};
`,
output: `
type Foo = {
a: {
b: true;
};
};
`,
options: [
{
multilineDetection: 'last-member',
},
],
errors: [
{
messageId: 'expectedSemi',
line: 5,
column: 4,
},
],
},
{
code: `
type Foo = {a: {
b: true;
}};
`,
output: `
type Foo = {a: {
b: true;
};};
`,
options: [
{
singleline: { delimiter: 'semi', requireLast: true },
multilineDetection: 'last-member',
},
],
errors: [
{
messageId: 'expectedSemi',
line: 4,
column: 2,
},
],
},
],
});