From 3565da9fff068feffc00e099b7657bc22fdd0aff Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Tue, 29 Oct 2024 21:38:56 -0600 Subject: [PATCH 1/5] [consistent-type-definitions] remove closing parens around a type --- .../src/rules/consistent-type-definitions.ts | 33 ++++++++++++----- .../rules/consistent-type-definitions.test.ts | 36 +++++++++++++++++++ 2 files changed, 60 insertions(+), 9 deletions(-) diff --git a/packages/eslint-plugin/src/rules/consistent-type-definitions.ts b/packages/eslint-plugin/src/rules/consistent-type-definitions.ts index 9c57c4f40327..bed1c447a824 100644 --- a/packages/eslint-plugin/src/rules/consistent-type-definitions.ts +++ b/packages/eslint-plugin/src/rules/consistent-type-definitions.ts @@ -59,6 +59,9 @@ export default createRule({ const firstToken = context.sourceCode.getTokenBefore(node.id); if (firstToken) { + // replace 'type' with 'interface', and remove everything between + // the name and the start of the { a: string } part, including + // all opening parens. fixes.push(fixer.replaceText(firstToken, 'interface')); fixes.push( fixer.replaceTextRange( @@ -68,15 +71,27 @@ export default createRule({ ); } - const afterToken = context.sourceCode.getTokenAfter( - node.typeAnnotation, - ); - if ( - afterToken && - afterToken.type === AST_TOKEN_TYPES.Punctuator && - afterToken.value === ';' - ) { - fixes.push(fixer.remove(afterToken)); + // remove all closing parens, and the semicolon if present. + let rangeStart = node.typeAnnotation.range[1]; + while (true) { + const afterToken = + context.sourceCode.getTokenByRangeStart(rangeStart); + + if (afterToken != null) { + if (afterToken.value === ')') { + fixes.push(fixer.remove(afterToken)); + rangeStart = afterToken.range[1]; + continue; + } else if ( + afterToken.type === AST_TOKEN_TYPES.Punctuator && + afterToken.value === ';' + ) { + fixes.push(fixer.remove(afterToken)); + break; + } + } + + break; } return fixes; diff --git a/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts b/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts index 3735fca22ffa..4dd688fc0568 100644 --- a/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts +++ b/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts @@ -347,6 +347,42 @@ export declare interface Test { export declare type Test = { foo: string; bar: string; +} + `, + }, + { + code: noFormat` +type Foo = ({ + a: string; +}); + `, + errors: [ + { + line: 2, + messageId: 'interfaceOverType', + }, + ], + output: ` +interface Foo { + a: string; +} + `, + }, + { + code: noFormat` +type Foo = ((((((((({ + a: string; +}))))))))); + `, + errors: [ + { + line: 2, + messageId: 'interfaceOverType', + }, + ], + output: ` +interface Foo { + a: string; } `, }, From 18939a1a7c0c3db77fad6cb442b1c63af99b6388 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Tue, 29 Oct 2024 22:05:49 -0600 Subject: [PATCH 2/5] cov --- .../rules/consistent-type-definitions.test.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts b/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts index 4dd688fc0568..cf6d6c84fd4c 100644 --- a/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts +++ b/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts @@ -383,6 +383,25 @@ type Foo = ((((((((({ output: ` interface Foo { a: string; +} + `, + }, + { + // no closing semicolon + code: noFormat` +type Foo = { + a: string; +} + `, + errors: [ + { + line: 2, + messageId: 'interfaceOverType', + }, + ], + output: ` +interface Foo { + a: string; } `, }, From 0afb8fef9722f1bacf80bd45bccb56a45245df60 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Sun, 3 Nov 2024 11:22:06 -0700 Subject: [PATCH 3/5] better fixer logic --- .../src/rules/consistent-type-definitions.ts | 73 +++++++++---------- .../rules/consistent-type-definitions.test.ts | 12 +++ 2 files changed, 48 insertions(+), 37 deletions(-) diff --git a/packages/eslint-plugin/src/rules/consistent-type-definitions.ts b/packages/eslint-plugin/src/rules/consistent-type-definitions.ts index bed1c447a824..52203a5974bd 100644 --- a/packages/eslint-plugin/src/rules/consistent-type-definitions.ts +++ b/packages/eslint-plugin/src/rules/consistent-type-definitions.ts @@ -1,8 +1,9 @@ import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; import { AST_NODE_TYPES, AST_TOKEN_TYPES } from '@typescript-eslint/utils'; +import * as ts from 'typescript'; -import { createRule } from '../util'; +import { createRule, nullThrows, NullThrowsReasons } from '../util'; export default createRule({ name: 'consistent-type-definitions', @@ -54,47 +55,45 @@ export default createRule({ node: node.id, messageId: 'interfaceOverType', fix(fixer) { - const typeNode = node.typeParameters ?? node.id; - const fixes: TSESLint.RuleFix[] = []; + const typeToken = nullThrows( + context.sourceCode.getTokenBefore( + node.id, + token => token.value === 'type', + ), + NullThrowsReasons.MissingToken('type keyword', 'type alias'), + ); - const firstToken = context.sourceCode.getTokenBefore(node.id); - if (firstToken) { - // replace 'type' with 'interface', and remove everything between - // the name and the start of the { a: string } part, including - // all opening parens. - fixes.push(fixer.replaceText(firstToken, 'interface')); - fixes.push( - fixer.replaceTextRange( - [typeNode.range[1], node.typeAnnotation.range[0]], - ' ', - ), - ); - } + const equalsToken = nullThrows( + context.sourceCode.getTokenBefore( + node.typeAnnotation, + token => token.value === '=', + ), + NullThrowsReasons.MissingToken('=', 'type alias'), + ); - // remove all closing parens, and the semicolon if present. - let rangeStart = node.typeAnnotation.range[1]; - while (true) { - const afterToken = - context.sourceCode.getTokenByRangeStart(rangeStart); + const beforeEqualsToken = nullThrows( + context.sourceCode.getTokenBefore(equalsToken, { + includeComments: true, + }), + NullThrowsReasons.MissingToken('before =', 'type alias'), + ); - if (afterToken != null) { - if (afterToken.value === ')') { - fixes.push(fixer.remove(afterToken)); - rangeStart = afterToken.range[1]; - continue; - } else if ( - afterToken.type === AST_TOKEN_TYPES.Punctuator && - afterToken.value === ';' - ) { - fixes.push(fixer.remove(afterToken)); - break; - } - } + return [ + // replace 'type' with 'interface'. + fixer.replaceText(typeToken, 'interface'), - break; - } + // delete from the = to the { of the type, and put a space to be pretty. + fixer.replaceTextRange( + [beforeEqualsToken.range[1], node.typeAnnotation.range[0]], + ' ', + ), - return fixes; + // remove from the closing } through the end of the statement. + fixer.removeRange([ + node.typeAnnotation.range[1], + node.range[1], + ]), + ]; }, }); }, diff --git a/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts b/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts index cf6d6c84fd4c..77bd89186fac 100644 --- a/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts +++ b/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts @@ -96,6 +96,18 @@ export type W = { options: ['interface'], output: `interface T { x: number; }`, }, + { + code: noFormat`type T /* comment */={ x: number; };`, + errors: [ + { + column: 6, + line: 1, + messageId: 'interfaceOverType', + }, + ], + options: ['interface'], + output: `interface T /* comment */ { x: number; }`, + }, { code: ` export type W = { From bf16cf2774e1f3ffb1004fff4b37955dd78c98d4 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Sun, 3 Nov 2024 17:09:00 -0800 Subject: [PATCH 4/5] test fixer edge cases --- .../rules/consistent-type-definitions.test.ts | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts b/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts index 77bd89186fac..b14fb8976939 100644 --- a/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts +++ b/packages/eslint-plugin/tests/rules/consistent-type-definitions.test.ts @@ -417,5 +417,49 @@ interface Foo { } `, }, + { + // no closing semicolon; ensure we don't erase subsequent code. + code: noFormat` +type Foo = { + a: string; +} +type Bar = string; + `, + errors: [ + { + line: 2, + messageId: 'interfaceOverType', + }, + ], + output: ` +interface Foo { + a: string; +} +type Bar = string; + `, + }, + { + // no closing semicolon; ensure we don't erase subsequent code. + code: noFormat` +type Foo = ((({ + a: string; +}))) + +const bar = 1; + `, + errors: [ + { + line: 2, + messageId: 'interfaceOverType', + }, + ], + output: ` +interface Foo { + a: string; +} + +const bar = 1; + `, + }, ], }); From c5d5163c86740978f46b4c205f2ed2336126f17f Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Sun, 3 Nov 2024 17:25:57 -0800 Subject: [PATCH 5/5] unused imports --- .../eslint-plugin/src/rules/consistent-type-definitions.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/consistent-type-definitions.ts b/packages/eslint-plugin/src/rules/consistent-type-definitions.ts index 52203a5974bd..488842f12705 100644 --- a/packages/eslint-plugin/src/rules/consistent-type-definitions.ts +++ b/packages/eslint-plugin/src/rules/consistent-type-definitions.ts @@ -1,7 +1,6 @@ import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; -import { AST_NODE_TYPES, AST_TOKEN_TYPES } from '@typescript-eslint/utils'; -import * as ts from 'typescript'; +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import { createRule, nullThrows, NullThrowsReasons } from '../util';