From af6784e3cf86d8abe5cae2f1aaa11ad98bc742bb Mon Sep 17 00:00:00 2001 From: Sean Wood Date: Thu, 30 Mar 2023 12:50:05 +0100 Subject: [PATCH 1/7] fix: correctly resolve file type --- .../src/rules/no-unnecessary-type-constraint.ts | 13 ++++++++++++- packages/type-utils/src/index.ts | 2 ++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts index a337200a9a7..8456f990913 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts @@ -1,3 +1,7 @@ +import { + getLanguageVariant, + getScriptKind, +} from '@typescript-eslint/type-utils'; import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import * as semver from 'semver'; @@ -60,7 +64,14 @@ export default util.createRule({ ]) : new Map([[AST_NODE_TYPES.TSUnknownKeyword, 'unknown']]); - const inJsx = context.getFilename().toLowerCase().endsWith('tsx'); + const inJsx = + getLanguageVariant( + getScriptKind( + context.getFilename(), + !!context.parserOptions.ecmaFeatures?.jsx, + ), + ) === ts.LanguageVariant.JSX; + const source = context.getSourceCode(); const checkNode = ( diff --git a/packages/type-utils/src/index.ts b/packages/type-utils/src/index.ts index dde032e1770..8ed0009720a 100644 --- a/packages/type-utils/src/index.ts +++ b/packages/type-utils/src/index.ts @@ -16,4 +16,6 @@ export { getDecorators, getModifiers, typescriptVersionIsAtLeast, + getScriptKind, + getLanguageVariant, } from '@typescript-eslint/typescript-estree'; From 815b248b8e6428ccf67d587628f3eb614a2b3853 Mon Sep 17 00:00:00 2001 From: Sean Wood Date: Thu, 30 Mar 2023 15:04:48 +0100 Subject: [PATCH 2/7] fix: detect new module syntax --- .../rules/no-unnecessary-type-constraint.ts | 19 +++++++------ .../no-unnecessary-type-constraint.test.ts | 4 +++ packages/type-utils/src/index.ts | 1 + .../src/create-program/getScriptKind.ts | 28 ++++++++++++++++++- 4 files changed, 42 insertions(+), 10 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts index 8456f990913..65ded9a8cfe 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts @@ -1,6 +1,7 @@ import { getLanguageVariant, getScriptKind, + isNewModuleExtension, } from '@typescript-eslint/type-utils'; import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; @@ -64,21 +65,21 @@ export default util.createRule({ ]) : new Map([[AST_NODE_TYPES.TSUnknownKeyword, 'unknown']]); - const inJsx = - getLanguageVariant( - getScriptKind( - context.getFilename(), - !!context.parserOptions.ecmaFeatures?.jsx, - ), - ) === ts.LanguageVariant.JSX; - + const filename = context.getFilename(); + const scriptKind = getScriptKind( + filename, + !!context.parserOptions.ecmaFeatures?.jsx, + ); + const inJsx = getLanguageVariant(scriptKind) === ts.LanguageVariant.JSX; + const newModuleExtension = isNewModuleExtension(filename); const source = context.getSourceCode(); const checkNode = ( node: TypeParameterWithConstraint, inArrowFunction: boolean, ): void => { - const constraint = unnecessaryConstraints.get(node.constraint.type); + const constraint = + !newModuleExtension && unnecessaryConstraints.get(node.constraint.type); function shouldAddTrailingComma(): boolean { if (!inArrowFunction || !inJsx) { return false; diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-type-constraint.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-type-constraint.test.ts index 72bde5788ca..1b991168873 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-type-constraint.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-type-constraint.test.ts @@ -25,6 +25,10 @@ function data() {} 'const data = () => {};', 'const data = () => {};', 'const data = () => {};', + { + code: 'const data = () => {};', + filename: 'file.mts', + }, ], invalid: [ { diff --git a/packages/type-utils/src/index.ts b/packages/type-utils/src/index.ts index 8ed0009720a..0dbe433edb2 100644 --- a/packages/type-utils/src/index.ts +++ b/packages/type-utils/src/index.ts @@ -18,4 +18,5 @@ export { typescriptVersionIsAtLeast, getScriptKind, getLanguageVariant, + isNewModuleExtension, } from '@typescript-eslint/typescript-estree'; diff --git a/packages/typescript-estree/src/create-program/getScriptKind.ts b/packages/typescript-estree/src/create-program/getScriptKind.ts index 80158f468f7..01f76615096 100644 --- a/packages/typescript-estree/src/create-program/getScriptKind.ts +++ b/packages/typescript-estree/src/create-program/getScriptKind.ts @@ -1,6 +1,32 @@ import path from 'path'; import * as ts from 'typescript'; +function isNewModuleExtension(filePath: string): boolean { + const extension = path.extname(filePath).toLowerCase(); + // note - we only respect the user's jsx setting for unknown extensions + // this is so that we always match TS's internal script kind logic, preventing + // weird errors due to a mismatch. + // https://github.com/microsoft/TypeScript/blob/da00ba67ed1182ad334f7c713b8254fba174aeba/src/compiler/utilities.ts#L6948-L6968 + switch (extension) { + case ts.Extension.Js: + case ts.Extension.Ts: + case ts.Extension.Jsx: + case ts.Extension.Tsx: + case ts.Extension.Json: + return false; + + case ts.Extension.Cjs: + case ts.Extension.Mjs: + case ts.Extension.Cts: + case ts.Extension.Mts: + return true; + + default: + // unknown extension, we assume this is not a new module extension + return false; + } +} + function getScriptKind(filePath: string, jsx: boolean): ts.ScriptKind { const extension = path.extname(filePath).toLowerCase(); // note - we only respect the user's jsx setting for unknown extensions @@ -47,4 +73,4 @@ function getLanguageVariant(scriptKind: ts.ScriptKind): ts.LanguageVariant { } } -export { getScriptKind, getLanguageVariant }; +export { getScriptKind, getLanguageVariant, isNewModuleExtension }; From 5a5bc878a65e62682cba2c61cf8ccafc34b35e58 Mon Sep 17 00:00:00 2001 From: Sean Wood Date: Thu, 30 Mar 2023 15:31:47 +0100 Subject: [PATCH 3/7] chore: refactor to `isNodeNextModuleExtension` --- .../src/rules/no-unnecessary-type-constraint.ts | 7 ++++--- packages/type-utils/src/index.ts | 2 +- .../typescript-estree/src/create-program/getScriptKind.ts | 6 +++--- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts index 65ded9a8cfe..d05b81d99b8 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts @@ -1,7 +1,7 @@ import { getLanguageVariant, getScriptKind, - isNewModuleExtension, + isNodeNextModuleExtension, } from '@typescript-eslint/type-utils'; import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; @@ -71,7 +71,7 @@ export default util.createRule({ !!context.parserOptions.ecmaFeatures?.jsx, ); const inJsx = getLanguageVariant(scriptKind) === ts.LanguageVariant.JSX; - const newModuleExtension = isNewModuleExtension(filename); + const nodeNextModuleExtension = isNodeNextModuleExtension(filename); const source = context.getSourceCode(); const checkNode = ( @@ -79,7 +79,8 @@ export default util.createRule({ inArrowFunction: boolean, ): void => { const constraint = - !newModuleExtension && unnecessaryConstraints.get(node.constraint.type); + !nodeNextModuleExtension && + unnecessaryConstraints.get(node.constraint.type); function shouldAddTrailingComma(): boolean { if (!inArrowFunction || !inJsx) { return false; diff --git a/packages/type-utils/src/index.ts b/packages/type-utils/src/index.ts index 0dbe433edb2..82b1abc9b81 100644 --- a/packages/type-utils/src/index.ts +++ b/packages/type-utils/src/index.ts @@ -18,5 +18,5 @@ export { typescriptVersionIsAtLeast, getScriptKind, getLanguageVariant, - isNewModuleExtension, + isNodeNextModuleExtension, } from '@typescript-eslint/typescript-estree'; diff --git a/packages/typescript-estree/src/create-program/getScriptKind.ts b/packages/typescript-estree/src/create-program/getScriptKind.ts index 01f76615096..a022d00055d 100644 --- a/packages/typescript-estree/src/create-program/getScriptKind.ts +++ b/packages/typescript-estree/src/create-program/getScriptKind.ts @@ -1,7 +1,7 @@ import path from 'path'; import * as ts from 'typescript'; -function isNewModuleExtension(filePath: string): boolean { +function isNodeNextModuleExtension(filePath: string): boolean { const extension = path.extname(filePath).toLowerCase(); // note - we only respect the user's jsx setting for unknown extensions // this is so that we always match TS's internal script kind logic, preventing @@ -22,7 +22,7 @@ function isNewModuleExtension(filePath: string): boolean { return true; default: - // unknown extension, we assume this is not a new module extension + // unknown extension, we assume this is not a node-next module extension return false; } } @@ -73,4 +73,4 @@ function getLanguageVariant(scriptKind: ts.ScriptKind): ts.LanguageVariant { } } -export { getScriptKind, getLanguageVariant, isNewModuleExtension }; +export { getScriptKind, getLanguageVariant, isNodeNextModuleExtension }; From 58e250593eda810b41aea5922a40e67276ead3b5 Mon Sep 17 00:00:00 2001 From: Sean Wood Date: Sun, 2 Apr 2023 21:17:24 +0100 Subject: [PATCH 4/7] revert changes This reverts commit 5a5bc878a65e62682cba2c61cf8ccafc34b35e58. Revert "fix: detect new module syntax" This reverts commit 815b248b8e6428ccf67d587628f3eb614a2b3853. Revert "fix: correctly resolve file type" This reverts commit af6784e3cf86d8abe5cae2f1aaa11ad98bc742bb. --- .../rules/no-unnecessary-type-constraint.ts | 17 ++--------- .../no-unnecessary-type-constraint.test.ts | 4 --- packages/type-utils/src/index.ts | 3 -- .../src/create-program/getScriptKind.ts | 28 +------------------ 4 files changed, 3 insertions(+), 49 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts index d05b81d99b8..a337200a9a7 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts @@ -1,8 +1,3 @@ -import { - getLanguageVariant, - getScriptKind, - isNodeNextModuleExtension, -} from '@typescript-eslint/type-utils'; import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import * as semver from 'semver'; @@ -65,22 +60,14 @@ export default util.createRule({ ]) : new Map([[AST_NODE_TYPES.TSUnknownKeyword, 'unknown']]); - const filename = context.getFilename(); - const scriptKind = getScriptKind( - filename, - !!context.parserOptions.ecmaFeatures?.jsx, - ); - const inJsx = getLanguageVariant(scriptKind) === ts.LanguageVariant.JSX; - const nodeNextModuleExtension = isNodeNextModuleExtension(filename); + const inJsx = context.getFilename().toLowerCase().endsWith('tsx'); const source = context.getSourceCode(); const checkNode = ( node: TypeParameterWithConstraint, inArrowFunction: boolean, ): void => { - const constraint = - !nodeNextModuleExtension && - unnecessaryConstraints.get(node.constraint.type); + const constraint = unnecessaryConstraints.get(node.constraint.type); function shouldAddTrailingComma(): boolean { if (!inArrowFunction || !inJsx) { return false; diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-type-constraint.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-type-constraint.test.ts index 1b991168873..72bde5788ca 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-type-constraint.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-type-constraint.test.ts @@ -25,10 +25,6 @@ function data() {} 'const data = () => {};', 'const data = () => {};', 'const data = () => {};', - { - code: 'const data = () => {};', - filename: 'file.mts', - }, ], invalid: [ { diff --git a/packages/type-utils/src/index.ts b/packages/type-utils/src/index.ts index 82b1abc9b81..dde032e1770 100644 --- a/packages/type-utils/src/index.ts +++ b/packages/type-utils/src/index.ts @@ -16,7 +16,4 @@ export { getDecorators, getModifiers, typescriptVersionIsAtLeast, - getScriptKind, - getLanguageVariant, - isNodeNextModuleExtension, } from '@typescript-eslint/typescript-estree'; diff --git a/packages/typescript-estree/src/create-program/getScriptKind.ts b/packages/typescript-estree/src/create-program/getScriptKind.ts index a022d00055d..80158f468f7 100644 --- a/packages/typescript-estree/src/create-program/getScriptKind.ts +++ b/packages/typescript-estree/src/create-program/getScriptKind.ts @@ -1,32 +1,6 @@ import path from 'path'; import * as ts from 'typescript'; -function isNodeNextModuleExtension(filePath: string): boolean { - const extension = path.extname(filePath).toLowerCase(); - // note - we only respect the user's jsx setting for unknown extensions - // this is so that we always match TS's internal script kind logic, preventing - // weird errors due to a mismatch. - // https://github.com/microsoft/TypeScript/blob/da00ba67ed1182ad334f7c713b8254fba174aeba/src/compiler/utilities.ts#L6948-L6968 - switch (extension) { - case ts.Extension.Js: - case ts.Extension.Ts: - case ts.Extension.Jsx: - case ts.Extension.Tsx: - case ts.Extension.Json: - return false; - - case ts.Extension.Cjs: - case ts.Extension.Mjs: - case ts.Extension.Cts: - case ts.Extension.Mts: - return true; - - default: - // unknown extension, we assume this is not a node-next module extension - return false; - } -} - function getScriptKind(filePath: string, jsx: boolean): ts.ScriptKind { const extension = path.extname(filePath).toLowerCase(); // note - we only respect the user's jsx setting for unknown extensions @@ -73,4 +47,4 @@ function getLanguageVariant(scriptKind: ts.ScriptKind): ts.LanguageVariant { } } -export { getScriptKind, getLanguageVariant, isNodeNextModuleExtension }; +export { getScriptKind, getLanguageVariant }; From 6bfccc6f6987b8552dd6c16a659cc09dae7dad92 Mon Sep 17 00:00:00 2001 From: Sean Wood Date: Mon, 3 Apr 2023 10:00:24 +0100 Subject: [PATCH 5/7] feat: widen comma fix filetypes --- .../rules/no-unnecessary-type-constraint.ts | 23 ++++++++++- .../no-unnecessary-type-constraint.test.ts | 40 +++++++++++++++++++ 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts index a337200a9a7..77ebe81b107 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts @@ -1,3 +1,5 @@ +import { extname } from 'node:path'; + import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import * as semver from 'semver'; @@ -60,7 +62,24 @@ export default util.createRule({ ]) : new Map([[AST_NODE_TYPES.TSUnknownKeyword, 'unknown']]); - const inJsx = context.getFilename().toLowerCase().endsWith('tsx'); + function checkRequiresGenericDeclarationDisambiguation( + filename: string, + ): boolean { + const pathExt = extname(filename).toLocaleLowerCase(); + switch (pathExt) { + case ts.Extension.Cts: + case ts.Extension.Mts: + case ts.Extension.Tsx: + return true; + + default: + return false; + } + } + + const requiresGenericDeclarationDisambiguation = + checkRequiresGenericDeclarationDisambiguation(context.getFilename()); + const source = context.getSourceCode(); const checkNode = ( @@ -69,7 +88,7 @@ export default util.createRule({ ): void => { const constraint = unnecessaryConstraints.get(node.constraint.type); function shouldAddTrailingComma(): boolean { - if (!inArrowFunction || !inJsx) { + if (!inArrowFunction || !requiresGenericDeclarationDisambiguation) { return false; } // Only () => {} would need trailing comma diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-type-constraint.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-type-constraint.test.ts index 72bde5788ca..ebe16e87e2b 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-type-constraint.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-type-constraint.test.ts @@ -123,6 +123,46 @@ function data() {} ], filename: 'react.tsx', }, + { + code: 'const data = () => {};', + errors: [ + { + data: { constraint: 'any', name: 'T' }, + messageId: 'unnecessaryConstraint', + endColumn: 28, + column: 15, + line: 1, + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + data: { constraint: 'any' }, + output: `const data = () => {};`, + }, + ], + }, + ], + filename: 'file.mts', + }, + { + code: 'const data = () => {};', + errors: [ + { + data: { constraint: 'any', name: 'T' }, + messageId: 'unnecessaryConstraint', + endColumn: 28, + column: 15, + line: 1, + suggestions: [ + { + messageId: 'removeUnnecessaryConstraint', + data: { constraint: 'any' }, + output: `const data = () => {};`, + }, + ], + }, + ], + filename: 'file.cts', + }, { code: noFormat`const data = () => {};`, errors: [ From ff2a40310de43f1a96dc195ebab38006e67c7c25 Mon Sep 17 00:00:00 2001 From: Sean Wood Date: Fri, 21 Apr 2023 16:40:55 +0100 Subject: [PATCH 6/7] chore: refactor 'node:path' to 'path' --- .../eslint-plugin/src/rules/no-unnecessary-type-constraint.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts index 77ebe81b107..3e20ab5ccac 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts @@ -1,5 +1,4 @@ -import { extname } from 'node:path'; - +import { extname } from 'path'; import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import * as semver from 'semver'; From 3a04d71a4d9e63b85482d2c33a957bf54377ff51 Mon Sep 17 00:00:00 2001 From: Sean Wood Date: Fri, 21 Apr 2023 17:17:52 +0100 Subject: [PATCH 7/7] fix: linting --- .../eslint-plugin/src/rules/no-unnecessary-type-constraint.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts index 3e20ab5ccac..6ae314c51aa 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts @@ -1,6 +1,6 @@ -import { extname } from 'path'; import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; +import { extname } from 'path'; import * as semver from 'semver'; import * as ts from 'typescript';