From 1830d7a7c7692cf65815551544703a97ed03e860 Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Sun, 4 Dec 2022 23:40:56 -0500 Subject: [PATCH 01/37] add debug command for new rule --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index c1044f1ea..df572a495 100644 --- a/package.json +++ b/package.json @@ -27,6 +27,7 @@ "posttest": "eslint . && npm run update:eslint-docs -- --check", "mocha": "cross-env BABEL_ENV=test nyc mocha", "tests-only": "npm run mocha tests/src", + "debug": "DEBUG=eslint-plugin-import/typescript-flow npm run mocha tests/src/rules/typescript-flow", "test": "npm run tests-only", "test-compiled": "npm run prepublish && BABEL_ENV=testCompiled mocha --compilers js:babel-register tests/src", "test-all": "node --require babel-register ./scripts/testAll", From 5684be81c602fd443dd83b8ed4bc38ce6499ab9c Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Sun, 4 Dec 2022 23:41:21 -0500 Subject: [PATCH 02/37] draft files for the rule, test, docs. --- docs/rules/typescript-flow.md | 0 src/rules/typescript-flow.js | 53 ++++++++++++++++++++++++ tests/src/rules/typescript-flow.js | 66 ++++++++++++++++++++++++++++++ 3 files changed, 119 insertions(+) create mode 100644 docs/rules/typescript-flow.md create mode 100644 src/rules/typescript-flow.js create mode 100644 tests/src/rules/typescript-flow.js diff --git a/docs/rules/typescript-flow.md b/docs/rules/typescript-flow.md new file mode 100644 index 000000000..e69de29bb diff --git a/src/rules/typescript-flow.js b/src/rules/typescript-flow.js new file mode 100644 index 000000000..7755d1c15 --- /dev/null +++ b/src/rules/typescript-flow.js @@ -0,0 +1,53 @@ +'use strict'; + +import debug from 'debug'; +import docsUrl from '../docsUrl'; + +const log = debug('eslint-plugin-import/typescript-flow'); + +module.exports = { + meta: { + type: 'suggestion', + docs: { + category: 'Style guide', + description: 'Prefer a default export if module exports a single name or multiple names.', + url: docsUrl('prefer-default-export'), + }, + schema: [{ + type: 'object', + properties:{ + target: { + type: 'string', + enum: ['single', 'any'], + default: 'single', + }, + }, + additionalProperties: false, + }], + }, + + create(context) { + // if TS is < 3.8 => we can just name import it. + + // works from typescript > 3.8 + // import type { Person, Cache } from "./foo" // ImportDeclaration importKind = 'type', ImportSpecifier.importKind = 'value' + + // works only on typescript >4.5 + // import { type Person, Cache } from "./foo"; // ImportDeclaration importKind = 'value', ImportSpecifier.importKind = 'type' + + log('parser'); + console.log(context.parserPath); + + return { + 'ImportDeclaration': function (node) { + // check the options flow + if (node.importKind === 'type') { + log('IF'); + console.log(node); + } else { + context.report(node, 'BOOM'); + } + }, + }; + }, +}; diff --git a/tests/src/rules/typescript-flow.js b/tests/src/rules/typescript-flow.js new file mode 100644 index 000000000..f64667c5e --- /dev/null +++ b/tests/src/rules/typescript-flow.js @@ -0,0 +1,66 @@ + +import { test, getTSParsers, parsers } from '../utils'; + +import { RuleTester } from 'eslint'; +import semver from 'semver'; + +const ruleTester = new RuleTester(); +const rule = require('rules/typescript-flow'); + +const message = 'Do not use import syntax to configure webpack loaders.'; + +context('TypeScript', function () { + getTSParsers().forEach((parser) => { + const parserConfig = { + parser, + settings: { + 'import/parsers': { [parser]: ['.ts'] }, + 'import/resolver': { 'eslint-import-resolver-typescript': true }, + }, + }; + + console.log(parser); + ruleTester.run('prefer-default-export', rule, { + valid: [ + test({ + code: `import type { MyType } from "./typescript.ts"`, + parser, + settings: { + 'import/parsers': { [parser]: ['.ts'] }, + 'import/resolver': { 'eslint-import-resolver-typescript': true }, + }, + // parserConfig, + }), + ], + invalid: [ + test({ + code: `import { MyType } from "./typescript.ts"`, + // parserConfig, + parser, + settings: { + 'import/parsers': { [parser]: ['.ts'] }, + 'import/resolver': { 'eslint-import-resolver-typescript': true }, + }, + errors: [{ + message: 'BOOM', + }], + }), + ] } ); + // @typescript-eslint/parser@5+ throw error for invalid module specifiers at parsing time. + // https://github.com/typescript-eslint/typescript-eslint/releases/tag/v5.0.0 + // if (!(parser === parsers.TS_NEW && semver.satisfies(require('@typescript-eslint/parser/package.json').version, '>= 5'))) { + // ruleTester.run('no-webpack-loader-syntax', rule, { + // valid: [ + // test(Object.assign({ + // code: 'import { foo } from\nalert()', + // }, parserConfig)), + // test(Object.assign({ + // code: 'import foo from\nalert()', + // }, parserConfig)), + // ], + // invalid: [], + // }); + // } + + }); +}); From 7d1ebbd44ef3d00b5601f81963afc2ef6a9c46a7 Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Mon, 5 Dec 2022 01:59:25 -0500 Subject: [PATCH 03/37] add 2 test cases for each option --- tests/src/rules/typescript-flow.js | 73 ++++++++++++++++-------------- 1 file changed, 38 insertions(+), 35 deletions(-) diff --git a/tests/src/rules/typescript-flow.js b/tests/src/rules/typescript-flow.js index f64667c5e..28c2397db 100644 --- a/tests/src/rules/typescript-flow.js +++ b/tests/src/rules/typescript-flow.js @@ -1,14 +1,11 @@ -import { test, getTSParsers, parsers } from '../utils'; +import { test, getTSParsers } from '../utils'; import { RuleTester } from 'eslint'; -import semver from 'semver'; const ruleTester = new RuleTester(); const rule = require('rules/typescript-flow'); - -const message = 'Do not use import syntax to configure webpack loaders.'; - +// TODO: add exports from .TSX files to the test cases context('TypeScript', function () { getTSParsers().forEach((parser) => { const parserConfig = { @@ -19,48 +16,54 @@ context('TypeScript', function () { }, }; + const settings = { + 'import/parsers': { [parser]: ['.ts'] }, + 'import/resolver': { 'eslint-import-resolver-typescript': true }, + }; + console.log(parser); ruleTester.run('prefer-default-export', rule, { valid: [ test({ code: `import type { MyType } from "./typescript.ts"`, - parser, - settings: { - 'import/parsers': { [parser]: ['.ts'] }, - 'import/resolver': { 'eslint-import-resolver-typescript': true }, - }, - // parserConfig, + parser: parserConfig.parser, + settings, + options: [{ + prefer: 'separate', + }], + }), + test({ + code: `import { type MyType } from "./typescript.ts"`, + parser: parserConfig.parser, + settings, + options: [{ + prefer: 'modifier', + }], }), ], invalid: [ test({ - code: `import { MyType } from "./typescript.ts"`, - // parserConfig, - parser, - settings: { - 'import/parsers': { [parser]: ['.ts'] }, - 'import/resolver': { 'eslint-import-resolver-typescript': true }, - }, + code: `import type { MyType } from "./typescript.ts"`, + parser, + settings, + options: [{ + prefer: 'modifier', + }], errors: [{ message: 'BOOM', - }], + }], + }), + test({ + code: `import { type MyType } from "./typescript.ts"`, + parser, + settings, + options: [{ + prefer: 'separate', + }], + errors: [{ + message: 'BOOM', + }], }), ] } ); - // @typescript-eslint/parser@5+ throw error for invalid module specifiers at parsing time. - // https://github.com/typescript-eslint/typescript-eslint/releases/tag/v5.0.0 - // if (!(parser === parsers.TS_NEW && semver.satisfies(require('@typescript-eslint/parser/package.json').version, '>= 5'))) { - // ruleTester.run('no-webpack-loader-syntax', rule, { - // valid: [ - // test(Object.assign({ - // code: 'import { foo } from\nalert()', - // }, parserConfig)), - // test(Object.assign({ - // code: 'import foo from\nalert()', - // }, parserConfig)), - // ], - // invalid: [], - // }); - // } - }); }); From 6141499a153a4a006714f38dccd86fe627b47dcd Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Mon, 5 Dec 2022 02:00:45 -0500 Subject: [PATCH 04/37] add typescript-flow alpha version added options, TS version check, and the logic for two simple cases where we prefer separate or type modifier import --- src/rules/typescript-flow.js | 57 ++++++++++++++++++++++++++---------- 1 file changed, 42 insertions(+), 15 deletions(-) diff --git a/src/rules/typescript-flow.js b/src/rules/typescript-flow.js index 7755d1c15..6e19c7cc7 100644 --- a/src/rules/typescript-flow.js +++ b/src/rules/typescript-flow.js @@ -1,10 +1,29 @@ 'use strict'; -import debug from 'debug'; import docsUrl from '../docsUrl'; +import debug from 'debug'; const log = debug('eslint-plugin-import/typescript-flow'); +// comes from tests/src/utils file, TODO?: import directly from there +import typescriptPkg from 'typescript/package.json'; +import semver from 'semver'; + +function tsVersionSatisfies(specifier) { + return semver.satisfies(typescriptPkg.version, specifier); +} + +// It seems unfortunate to have a rule that is only useful for typescript, but perhaps a rule that detects your TS (or flow) version, +// and can be configured for an order of preferences - ie, between "types as separate import statements", +// "types mixed with values, but marked with a type modifier", you could prefer one over the other, +// and it would fall back to the next one if the preferred one wasn't supported, and if neither are supported, the rule would noop. + +// 3 options of prefering importing types: none, separate, modifier. +// none => no use of word 'type' preferred. +// separate: types as separate import statements +// modifier: types mixed with values, but marked with a type modifier + +// To think about: if separate is preferred but none is used => need to check every import file. maybe exclude the none? module.exports = { meta: { type: 'suggestion', @@ -16,10 +35,10 @@ module.exports = { schema: [{ type: 'object', properties:{ - target: { + prefer: { type: 'string', - enum: ['single', 'any'], - default: 'single', + enum: ['none', 'separate', 'modifier'], + default: 'modifier', }, }, additionalProperties: false, @@ -29,22 +48,30 @@ module.exports = { create(context) { // if TS is < 3.8 => we can just name import it. - // works from typescript > 3.8 - // import type { Person, Cache } from "./foo" // ImportDeclaration importKind = 'type', ImportSpecifier.importKind = 'value' + // works from typescript >= 3.8 + // import type { Person, Cache } from "./foo" + // ImportDeclaration importKind = 'type', ImportSpecifier.importKind = 'value' + + // works only on typescript >= 4.5 + // import { type Person, Cache } from "./foo"; + // ImportDeclaration importKind = 'value', ImportSpecifier.importKind = 'type' - // works only on typescript >4.5 - // import { type Person, Cache } from "./foo"; // ImportDeclaration importKind = 'value', ImportSpecifier.importKind = 'type' + // get Rule options. Default: separate + const { prefer = 'separate' } = context.options[0] || {}; - log('parser'); - console.log(context.parserPath); + const supportsTypeImport = tsVersionSatisfies('>=3.8'); // separate `import type { a } from 'x'` was introduced in TS 3.8 + const supportsTypeModifier = tsVersionSatisfies('>=4.5'); // type modifiers were introduced in TS 4.5. return { 'ImportDeclaration': function (node) { - // check the options flow - if (node.importKind === 'type') { - log('IF'); - console.log(node); - } else { + // case where we want type modifier but we got separate import type + if (supportsTypeModifier && prefer === 'modifier' && node.importKind === 'type') { + context.report(node, 'BOOM'); + } + }, + 'ImportSpecifier': function (node) { + // we want separate import, but have type modifier + if (supportsTypeImport && prefer === 'separate' && node.importKind === 'type') { context.report(node, 'BOOM'); } }, From daaf8aa7754223a1ebfa5f0a4b47fae79ebbdf5c Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Wed, 7 Dec 2022 01:06:06 -0500 Subject: [PATCH 05/37] add test cases for single option none --- tests/src/rules/typescript-flow.js | 115 ++++++++++++++++++----------- 1 file changed, 73 insertions(+), 42 deletions(-) diff --git a/tests/src/rules/typescript-flow.js b/tests/src/rules/typescript-flow.js index 28c2397db..642f05444 100644 --- a/tests/src/rules/typescript-flow.js +++ b/tests/src/rules/typescript-flow.js @@ -5,65 +5,96 @@ import { RuleTester } from 'eslint'; const ruleTester = new RuleTester(); const rule = require('rules/typescript-flow'); -// TODO: add exports from .TSX files to the test cases +// TODO: add exports from .TSX files to the test cases, import default context('TypeScript', function () { getTSParsers().forEach((parser) => { - const parserConfig = { - parser, - settings: { - 'import/parsers': { [parser]: ['.ts'] }, - 'import/resolver': { 'eslint-import-resolver-typescript': true }, - }, - }; + // const parserConfig = { + // parser, + // settings: { + // 'import/parsers': { [parser]: ['.ts'] }, + // 'import/resolver': { 'eslint-import-resolver-typescript': true }, + // }, + // }; const settings = { 'import/parsers': { [parser]: ['.ts'] }, 'import/resolver': { 'eslint-import-resolver-typescript': true }, }; - console.log(parser); - ruleTester.run('prefer-default-export', rule, { + ruleTester.run('typescript-flow', rule, { valid: [ - test({ - code: `import type { MyType } from "./typescript.ts"`, - parser: parserConfig.parser, - settings, - options: [{ - prefer: 'separate', - }], - }), - test({ - code: `import { type MyType } from "./typescript.ts"`, - parser: parserConfig.parser, - settings, - options: [{ - prefer: 'modifier', - }], - }), + // test({ + // code: `import type { MyType } from "./typescript.ts"`, + // parser, + // settings, + // options: ['separate'], + // }), + // test({ + // code: `import { MyType } from "./typescript.ts"`, + // parser, + // settings, + // options: [{ + // prefer: 'modifier', + // }], + // }), ], invalid: [ + // test({ + // code: `import type { MyType, Bar } from "./typescript.ts"`, + // parser, + // settings, + // options: ['none'], + // errors: [{ + // message: 'BOOM', + // }], + // output: 'import { MyType, Bar } from "./typescript.ts"', + // }), + // test({ + // code: 'import { type MyType, Bar } from "./typescript.ts"', + // parser, + // settings, + // options: ['none'], + // errors: [{ + // message: 'BOOM', + // }], + // output: 'import { MyType, Bar } from "./typescript.ts"', + // }), + // test({ + // code: 'import { MyType, type Bar } from "./typescript.ts"', + // parser, + // settings, + // options: ['none'], + // errors: [{ + // message: 'BOOM', + // }], + // output: 'import { MyType, Bar } from "./typescript.ts"', + // }), + // test({ + // code: 'import { type MyType, type Bar } from "./typescript.ts"', + // parser, + // settings, + // options: ['none'], + // errors: [{ + // message: 'BOOM', + // }, + // { + // message: 'BOOM', + // }], + // output: 'import { MyType, Bar } from "./typescript.ts"', + // }), + + // Single Config option which is separate test({ - code: `import type { MyType } from "./typescript.ts"`, - parser, - settings, - options: [{ - prefer: 'modifier', - }], - errors: [{ - message: 'BOOM', - }], - }), - test({ - code: `import { type MyType } from "./typescript.ts"`, + code: 'import { type MyType, Bar } from "./typescript.ts"', parser, settings, - options: [{ - prefer: 'separate', - }], + options: ['separate'], errors: [{ message: 'BOOM', }], + output: 'import type { MyType, Bar } from "./typescript.ts"', }), - ] } ); + ] }, + ); }); }); From 50ce08e16e4bb9531fc7b9b51f0ab917e0b60b7a Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Wed, 7 Dec 2022 01:06:33 -0500 Subject: [PATCH 06/37] add blank docs for new rule --- docs/rules/typescript-flow.md | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/docs/rules/typescript-flow.md b/docs/rules/typescript-flow.md index e69de29bb..3b5eb8b26 100644 --- a/docs/rules/typescript-flow.md +++ b/docs/rules/typescript-flow.md @@ -0,0 +1,24 @@ +This rules helps to define type import patterns for the typescript. + +## Rule details + +##### rule schema: + +```javascript +"import/prefer-default-export": [ + ( "off" | "warn" | "error" ), + { "prefer": "none" | "separate" | "modifier" } // default is ??? +] +``` +### Config Options + +There are three avaiable options: `none`, `separate`, `modifier`. + +```javascript +// import-stars.js + +// The remote module is not inspected. +import * from './other-module' +import * as b from './other-module' + +``` \ No newline at end of file From 33215654f1d128926f5646ac2d4297f548ac2aa4 Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Wed, 7 Dec 2022 01:07:57 -0500 Subject: [PATCH 07/37] change export map to include word 'type' as a key for namespace when the namespace is exported as type= TsTypeAliasDefinition --- src/ExportMap.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ExportMap.js b/src/ExportMap.js index d95fdb7a7..c06b8f559 100644 --- a/src/ExportMap.js +++ b/src/ExportMap.js @@ -623,6 +623,9 @@ ExportMap.parse = function (path, content, context) { // capture declaration if (n.declaration != null) { switch (n.declaration.type) { + case 'TSTypeAliasDeclaration': + m.namespace.set(n.declaration.id.name, 'type'); + break; case 'FunctionDeclaration': case 'ClassDeclaration': case 'TypeAlias': // flowtype with babel-eslint parser @@ -630,7 +633,6 @@ ExportMap.parse = function (path, content, context) { case 'DeclareFunction': case 'TSDeclareFunction': case 'TSEnumDeclaration': - case 'TSTypeAliasDeclaration': case 'TSInterfaceDeclaration': case 'TSAbstractClassDeclaration': case 'TSModuleDeclaration': From 8bf855662364446f01294481ca4b005156e47ce7 Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Wed, 7 Dec 2022 01:08:50 -0500 Subject: [PATCH 08/37] add new schema, comments, and handle single config option when it is none --- src/rules/typescript-flow.js | 202 ++++++++++++++++++++++++++++++++--- 1 file changed, 186 insertions(+), 16 deletions(-) diff --git a/src/rules/typescript-flow.js b/src/rules/typescript-flow.js index 6e19c7cc7..7843aae5b 100644 --- a/src/rules/typescript-flow.js +++ b/src/rules/typescript-flow.js @@ -1,6 +1,7 @@ 'use strict'; import docsUrl from '../docsUrl'; +import Exports from '../ExportMap'; import debug from 'debug'; const log = debug('eslint-plugin-import/typescript-flow'); @@ -12,6 +13,55 @@ import semver from 'semver'; function tsVersionSatisfies(specifier) { return semver.satisfies(typescriptPkg.version, specifier); } +// no need to have separate function, or doing it before the program start running => need to incorporate inside of the function +function processBodyStatement(context, typeExports, node) { + if (node.type !== 'ImportDeclaration') return; + + if (node.specifiers.length === 0) return; + + const imports = Exports.get(node.source.value, context); + // log('imports'); + // console.log(imports.namespace); + if (imports == null) return null; + + if (imports.errors.length > 0) { + imports.reportErrors(context, node); + return; + } + + imports.namespace.forEach((value, key) =>{ + if (value === 'type') { + typeExports.add(key); + } + }); + + + + + // node.specifiers.forEach((specifier) => { + // switch (specifier.type) { + // case 'ImportNamespaceSpecifier': + // if (!imports.size) { + // context.report( + // specifier, + // `No exported names found in module '${node.source.value}'.`, + // ); + // } + // namespaces.set(specifier.local.name, imports); + // break; + // case 'ImportDefaultSpecifier': + // case 'ImportSpecifier': { + // const meta = imports.get( + // // default to 'default' for default https://i.imgur.com/nj6qAWy.jpg + // specifier.imported ? (specifier.imported.name || specifier.imported.value) : 'default', + // ); + // if (!meta || !meta.namespace) { break; } + // namespaces.set(specifier.local.name, meta.namespace); + // break; + // } + // } + // }); +} // It seems unfortunate to have a rule that is only useful for typescript, but perhaps a rule that detects your TS (or flow) version, // and can be configured for an order of preferences - ie, between "types as separate import statements", @@ -24,27 +74,42 @@ function tsVersionSatisfies(specifier) { // modifier: types mixed with values, but marked with a type modifier // To think about: if separate is preferred but none is used => need to check every import file. maybe exclude the none? +// Scenario: user imports exported types without explicitly saying type: TODO: check every import file if it's a type export -> use ExportMaps +// What is import namespaces => omit? + module.exports = { meta: { - type: 'suggestion', + type: 'suggestion', // Layout? docs: { category: 'Style guide', - description: 'Prefer a default export if module exports a single name or multiple names.', - url: docsUrl('prefer-default-export'), + description: 'Prefer a default export if module exports a single name or multiple names.', // TODO: change + url: docsUrl('prefer-default-export'), // TODO: change }, + fixable: 'code', schema: [{ - type: 'object', - properties:{ - prefer: { - type: 'string', - enum: ['none', 'separate', 'modifier'], - default: 'modifier', + 'anyOf': [ + { + 'type': 'array', + 'items': { + 'type': 'string', + 'enum': ['none', 'separate', 'inline'], + }, + 'minItems': 1, + 'maxItems': 3, + 'uniqueItems': true, }, - }, - additionalProperties: false, + { + 'type': 'string', + 'enum': ['none', 'separate', 'inline'], + }, + ], }], }, + + + + create(context) { // if TS is < 3.8 => we can just name import it. @@ -56,24 +121,129 @@ module.exports = { // import { type Person, Cache } from "./foo"; // ImportDeclaration importKind = 'value', ImportSpecifier.importKind = 'type' - // get Rule options. Default: separate - const { prefer = 'separate' } = context.options[0] || {}; + // get Rule options. + const config = context.options[0]; + + // check the config + const configSingle = typeof(context.options[0]) === 'string' ? true : false; + + const prefer = 'separate'; + // allow single value or array of values. When list of available values not possible => message: unfixable problem. When there is multiple problems, finds first value in the list and makes it happen. const supportsTypeImport = tsVersionSatisfies('>=3.8'); // separate `import type { a } from 'x'` was introduced in TS 3.8 const supportsTypeModifier = tsVersionSatisfies('>=4.5'); // type modifiers were introduced in TS 4.5. + // TODO: check the array on TS version to leave possible options + // If ! supportsTypeModifier => remove type modifier from the array + // if ! supportsTypeImport => check if options array has 'none', if not => flag an error + // useful if we can just remove as. + const typeExports = new Set(); + + // TODO: filter down available preferences: find the first one available then use it as preferred. If none is option: find all type key words and auto fixer => remove them. + // return 3 rule objets? return { + // pick up all type imports at body entry time, to properly respect hoisting + Program({ body }) { + body.forEach(x => processBodyStatement(context, typeExports, x)); + }, 'ImportDeclaration': function (node) { + + if (typeof(context.options[0]) === 'string' || config.length === 1) { + // only one config + + // we have none, but have word type + if (config === 'none' && node.importKind === 'type') { + log('inside of if'); + context.report({ + node, + message: 'BOOM', + fix(fixer) { + const sourceCode = context.getSourceCode(); + // log('tokens -> need to find the one we need'); + const token = sourceCode.getTokens(node)[1]; + return fixer.replaceTextRange([token.range[0], token.range[1]+1], ''); + }, + }); + // log('fixed node'); + // console.log(node);: after fix, importKind for ImportDeclaration is still "type" + + } + + // Question: maybe we do not need to check the condition below because typescript checks for us? + } else if ((config === 'separate' || config[0] === 'separate') && node.importKind === 'type' && !supportsTypeImport) { + context.report({ + node, + message: 'BOOM', // You version of Typescript does not support import type statements + fix(fixer) { + const sourceCode = context.getSourceCode(); + // log('tokens -> need to find the one we need'); + const token = sourceCode.getTokens(node)[1]; + return fixer.replaceTextRange([token.range[0], token.range[1]+1], ''); + }, + }); + + // Question TODO: If user wants inline, but we have separate, then which named imports should be autofixed to have 'type' next to them: + // only exported types or also exported interaces ? + + } + // case where we want type modifier but we got separate import type if (supportsTypeModifier && prefer === 'modifier' && node.importKind === 'type') { context.report(node, 'BOOM'); } + + // create a map for all imports? }, 'ImportSpecifier': function (node) { - // we want separate import, but have type modifier - if (supportsTypeImport && prefer === 'separate' && node.importKind === 'type') { - context.report(node, 'BOOM'); + + // cases when we have only one config option + if (typeof(context.options[0]) === 'string' || config.length === 1) { + // only one config + + // we have none, but have word type + if ((config === 'none' || config[0] === 'none') && node.importKind === 'type') { + context.report({ + node, + message: 'BOOM', + fix(fixer) { + const sourceCode = context.getSourceCode(); + const token = sourceCode.getTokens(node)[0]; + // log('source code tokens'); + // console.log(sourceCode.getTokens(node)); + return fixer.replaceTextRange([token.range[0], token.range[1]+1], ''); + }, + }); + } + + // wanted separate but got inline case. Question: How to make two fixes? + if ((config === 'separate' || config[0] === 'separate') && node.importKind === 'type') { + context.report({ + node, + message: 'BOOM', + fix(fixer) { + const sourceCode = context.getSourceCode(); + const token = sourceCode.getTokens(node); + log('source code tokens'); + console.log(token); + + const tokenParent = sourceCode.getTokens(node.parent); + log('source code tokens -- PARENT'); + console.log(tokenParent); + return fixer.replaceTextRange([token.range[0], token.range[1]+1], '') ; + }, + }); + } + } + + // we want separate import, but have type modifier + // if (supportsTypeImport && prefer === 'separate' && node.importKind === 'type') { + // context.report(node, 'BOOM'); + // } + + // if none => remove all type key words ? + + // TODO: import Cache from "./foo"; import Default Specifier type? }, }; }, From bb172d67838e3c81b8ab425928b2f745f6c88887 Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Thu, 8 Dec 2022 01:12:31 -0500 Subject: [PATCH 09/37] delete none option and its code --- src/rules/typescript-flow.js | 45 ++++++------------------------------ 1 file changed, 7 insertions(+), 38 deletions(-) diff --git a/src/rules/typescript-flow.js b/src/rules/typescript-flow.js index 7843aae5b..f735a2797 100644 --- a/src/rules/typescript-flow.js +++ b/src/rules/typescript-flow.js @@ -92,15 +92,15 @@ module.exports = { 'type': 'array', 'items': { 'type': 'string', - 'enum': ['none', 'separate', 'inline'], + 'enum': ['separate', 'inline'], }, 'minItems': 1, - 'maxItems': 3, + 'maxItems': 2, 'uniqueItems': true, }, { 'type': 'string', - 'enum': ['none', 'separate', 'inline'], + 'enum': ['separate', 'inline'], }, ], }], @@ -151,23 +151,6 @@ module.exports = { if (typeof(context.options[0]) === 'string' || config.length === 1) { // only one config - // we have none, but have word type - if (config === 'none' && node.importKind === 'type') { - log('inside of if'); - context.report({ - node, - message: 'BOOM', - fix(fixer) { - const sourceCode = context.getSourceCode(); - // log('tokens -> need to find the one we need'); - const token = sourceCode.getTokens(node)[1]; - return fixer.replaceTextRange([token.range[0], token.range[1]+1], ''); - }, - }); - // log('fixed node'); - // console.log(node);: after fix, importKind for ImportDeclaration is still "type" - - } // Question: maybe we do not need to check the condition below because typescript checks for us? } else if ((config === 'separate' || config[0] === 'separate') && node.importKind === 'type' && !supportsTypeImport) { @@ -187,6 +170,10 @@ module.exports = { } + // Question TODO: iterate over all import specifiers and get a list of all inline type imports + // If option is separate, but we got none as input, how do we determine which named imports get to the new line? + // how to we deal when we have two fixers? + // case where we want type modifier but we got separate import type if (supportsTypeModifier && prefer === 'modifier' && node.importKind === 'type') { context.report(node, 'BOOM'); @@ -201,19 +188,6 @@ module.exports = { // only one config // we have none, but have word type - if ((config === 'none' || config[0] === 'none') && node.importKind === 'type') { - context.report({ - node, - message: 'BOOM', - fix(fixer) { - const sourceCode = context.getSourceCode(); - const token = sourceCode.getTokens(node)[0]; - // log('source code tokens'); - // console.log(sourceCode.getTokens(node)); - return fixer.replaceTextRange([token.range[0], token.range[1]+1], ''); - }, - }); - } // wanted separate but got inline case. Question: How to make two fixes? if ((config === 'separate' || config[0] === 'separate') && node.importKind === 'type') { @@ -236,11 +210,6 @@ module.exports = { } - // we want separate import, but have type modifier - // if (supportsTypeImport && prefer === 'separate' && node.importKind === 'type') { - // context.report(node, 'BOOM'); - // } - // if none => remove all type key words ? // TODO: import Cache from "./foo"; import Default Specifier type? From 803dde0635af3236245bbaf15e4a8d26206843e6 Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Sat, 10 Dec 2022 19:04:35 -0500 Subject: [PATCH 10/37] refactor and finish fixer for strict separate optiop --- src/rules/typescript-flow.js | 178 +++++++++++++++++++++++++---------- 1 file changed, 129 insertions(+), 49 deletions(-) diff --git a/src/rules/typescript-flow.js b/src/rules/typescript-flow.js index f735a2797..9351c51f6 100644 --- a/src/rules/typescript-flow.js +++ b/src/rules/typescript-flow.js @@ -35,9 +35,6 @@ function processBodyStatement(context, typeExports, node) { } }); - - - // node.specifiers.forEach((specifier) => { // switch (specifier.type) { // case 'ImportNamespaceSpecifier': @@ -139,47 +136,130 @@ module.exports = { // useful if we can just remove as. const typeExports = new Set(); + //const typeImports = new Set(); + const valueImports = new Set(); + + const fixerArray = []; // TODO: filter down available preferences: find the first one available then use it as preferred. If none is option: find all type key words and auto fixer => remove them. // return 3 rule objets? return { - // pick up all type imports at body entry time, to properly respect hoisting - Program({ body }) { - body.forEach(x => processBodyStatement(context, typeExports, x)); + // // pick up all type imports at body entry time, to properly respect hoisting + // Program({ body }) { + // body.forEach(x => processBodyStatement(context, typeExports, x)); + // }, + 'ImportNamespaceSpecifier': function (){ + return; }, - 'ImportDeclaration': function (node) { + 'ImportDeclaration': function (node){ + // 3 cases: strict cases: separate, inline. 3rd case is array of preference. The only thing to check is if arr[0] === inline => + // check if it can be supported. If not, fall back on separate. + // If arr[0] === separate => just do separate ? Then what is really a logic? Taking care of user worrying about TS version? + + // only one config. Since we determined that rule will fall back on option that works. if (typeof(context.options[0]) === 'string' || config.length === 1) { - // only one config - + // strict config: separate, but we got inline: + if (config === 'separate' && node.importKind !== 'type') { + const typeImports = []; + const inlineValueImports =[]; + const importRangesToBeRemoved = []; + // identify importSpecifiers that have inline type imports as well as value imports + node.specifiers.forEach((specifier) => { + // catch all inline imports and add them to the set + if (specifier.importKind === 'type') { + log('specifier'); + console.log(specifier); + + if (specifier.local.name !== specifier.imported.name) { + typeImports.push(`${specifier.imported.name} as ${specifier.local.name}`); + } else { + typeImports.push(specifier.local.name); + } + importRangesToBeRemoved.push(specifier.range); + // typeImports.add(specifier); + } else { + if (specifier.local.name !== specifier.imported.name) { + inlineValueImports.push(`${specifier.imported.name} as ${specifier.local.name}`); + } else { + inlineValueImports.push(specifier.local.name); + } + importRangesToBeRemoved.push(specifier.range); + // valueImports.add(specifier); + } + }); + // no inline type imports + if (typeImports.size === 0) { + return; + } - // Question: maybe we do not need to check the condition below because typescript checks for us? - } else if ((config === 'separate' || config[0] === 'separate') && node.importKind === 'type' && !supportsTypeImport) { - context.report({ - node, - message: 'BOOM', // You version of Typescript does not support import type statements - fix(fixer) { - const sourceCode = context.getSourceCode(); - // log('tokens -> need to find the one we need'); - const token = sourceCode.getTokens(node)[1]; - return fixer.replaceTextRange([token.range[0], token.range[1]+1], ''); - }, - }); - - // Question TODO: If user wants inline, but we have separate, then which named imports should be autofixed to have 'type' next to them: - // only exported types or also exported interaces ? + //remove inline type imports + add them as separate import statements + context.report({ + node, + message: 'BOOM', + fix(fixer) { - } + const sourceCode = context.getSourceCode(); + const tokens = sourceCode.getTokens(node); + // log('tokens'); + // console.log(tokens); + + // get import source + const importPath = tokens[tokens.length-1].value; + // log('import path'); + // console.log(importPath); + + importRangesToBeRemoved.forEach((range)=>{ + // log('ranges to remove'); + // console.log(range); + fixerArray.push(fixer.removeRange([range[0], range[1]])); + }); + // get the named type imports and remove them from the initial code + // typeImports.forEach((element)=> { + // newTypeImports.push(element.local.name); + // fixerArray.push(fixer.removeRange([element.range[0], element.range[1]])); + // }); + // // remove inline value imports => maybe remove all at once by removing everything between { and }? TODO + // valueImports.forEach((element)=> { + // if (element.local.name !== element.imported.name) { + // inlineValueImports.push(`${element.imported.name} as ${element.local.name}`); + // } else { + // inlineValueImports.push(element.local.name); + // } + // fixerArray.push(fixer.removeRange([element.range[0], element.range[1]])); + // }); + + let namedImportStart = undefined; + for (let i = 0; i < tokens.length; i++) { + if (tokens[i].value === '{') { + namedImportStart = tokens[i]; + } + if (tokens[i].value === ',' && tokens[i].type === 'Punctuator') { + fixerArray.push(fixer.remove(tokens[i])); + } + } + + + // add inline value imports back + fixerArray.push(fixer.insertTextAfter(namedImportStart, inlineValueImports.join(', '))); + // add new line with separate type import + fixerArray.push(fixer.insertTextAfter(node, `\nimport type { ${typeImports.join(', ')} } from ${importPath}`)); + return fixerArray; + }, + }); + } - // Question TODO: iterate over all import specifiers and get a list of all inline type imports - // If option is separate, but we got none as input, how do we determine which named imports get to the new line? - // how to we deal when we have two fixers? + // log('inlineTypeImports'); + // console.log(typeImports); - // case where we want type modifier but we got separate import type - if (supportsTypeModifier && prefer === 'modifier' && node.importKind === 'type') { - context.report(node, 'BOOM'); + + } + + // case where we want type modifier but we got separate import type + // if (supportsTypeModifier && prefer === 'modifier' && node.importKind === 'type') { + // context.report(node, 'BOOM'); + // } - // create a map for all imports? }, 'ImportSpecifier': function (node) { @@ -190,23 +270,23 @@ module.exports = { // we have none, but have word type // wanted separate but got inline case. Question: How to make two fixes? - if ((config === 'separate' || config[0] === 'separate') && node.importKind === 'type') { - context.report({ - node, - message: 'BOOM', - fix(fixer) { - const sourceCode = context.getSourceCode(); - const token = sourceCode.getTokens(node); - log('source code tokens'); - console.log(token); - - const tokenParent = sourceCode.getTokens(node.parent); - log('source code tokens -- PARENT'); - console.log(tokenParent); - return fixer.replaceTextRange([token.range[0], token.range[1]+1], '') ; - }, - }); - } + // if ((config === 'separate' || config[0] === 'separate') && node.importKind === 'type') { + // context.report({ + // node, + // message: 'BOOM', + // fix(fixer) { + // const sourceCode = context.getSourceCode(); + // const token = sourceCode.getTokens(node); + // log('source code tokens'); + // console.log(token); + + // const tokenParent = sourceCode.getTokens(node.parent); + // log('source code tokens -- PARENT'); + // console.log(tokenParent); + // return fixer.replaceTextRange([token.range[0], token.range[1]+1], '') ; + // }, + // }); + // } } From 74ba6d31ff5e9093081c4cafcd3155f118152e20 Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Sat, 10 Dec 2022 19:05:15 -0500 Subject: [PATCH 11/37] add namespace test cases to invalid strict separate --- tests/src/rules/typescript-flow.js | 46 +++++++++++++++++++++++------- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/tests/src/rules/typescript-flow.js b/tests/src/rules/typescript-flow.js index 642f05444..1835af609 100644 --- a/tests/src/rules/typescript-flow.js +++ b/tests/src/rules/typescript-flow.js @@ -5,7 +5,7 @@ import { RuleTester } from 'eslint'; const ruleTester = new RuleTester(); const rule = require('rules/typescript-flow'); -// TODO: add exports from .TSX files to the test cases, import default +// TODO: add exports from .TSX files to the test cases, import default, import * as b statement context('TypeScript', function () { getTSParsers().forEach((parser) => { // const parserConfig = { @@ -29,14 +29,14 @@ context('TypeScript', function () { // settings, // options: ['separate'], // }), - // test({ - // code: `import { MyType } from "./typescript.ts"`, - // parser, - // settings, - // options: [{ - // prefer: 'modifier', - // }], - // }), + // test({ + // code: `import { MyType } from "./typescript.ts"`, + // parser, + // settings, + // options: [{ + // prefer: 'modifier', + // }], + // }), ], invalid: [ // test({ @@ -85,14 +85,38 @@ context('TypeScript', function () { // Single Config option which is separate test({ - code: 'import { type MyType, Bar } from "./typescript.ts"', + code: 'import {type MyType,Bar} from "./typescript.ts"', + parser, + settings, + options: ['separate'], + errors: [{ + message: 'BOOM', + }], + // Question: Do we just remove the problematic node, what about comma? // import { type MyType, Bar, type Foo } => mport { , Bar, } + output: 'import {Bar} from "./typescript.ts"\nimport type { MyType } from "./typescript.ts"', + //output: 'import type { Bar } from "./typescript.ts"\nimport type { MyType } from "./typescript.ts"', + }), + test({ + code: 'import {type MyType,Bar,type Foo} from "./typescript.ts"', + parser, + settings, + options: ['separate'], + errors: [{ + message: 'BOOM', + }], + // Question: Do we just remove the problematic node, what about comma? // import { type MyType, Bar, type Foo } => mport { , Bar, } + output: 'import {Bar} from "./typescript.ts"\nimport type { MyType, Foo } from "./typescript.ts"', + }), + test({ + code: 'import {type MyType,Bar as Namespace} from "./typescript.ts"', parser, settings, options: ['separate'], errors: [{ message: 'BOOM', }], - output: 'import type { MyType, Bar } from "./typescript.ts"', + output: 'import {Bar as Namespace} from "./typescript.ts"\nimport type { MyType } from "./typescript.ts"', + //output: 'import type { Bar } from "./typescript.ts"\nimport type { MyType } from "./typescript.ts"', }), ] }, ); From 06705cca683e2f7ab74a3de013a31818e13a073c Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Sat, 10 Dec 2022 21:30:55 -0500 Subject: [PATCH 12/37] add default type export --- tests/files/typescript.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/files/typescript.ts b/tests/files/typescript.ts index f8fe2e8e0..549fa5f69 100644 --- a/tests/files/typescript.ts +++ b/tests/files/typescript.ts @@ -35,3 +35,8 @@ export namespace MyNamespace { } interface NotExported {} +type DefaultTypeExport = { + name: string, + age: number +} +export default DefaultTypeExport; From bf6063a10a376f1f16a5640578090de9c165e368 Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Sat, 10 Dec 2022 21:31:28 -0500 Subject: [PATCH 13/37] add valid cases for strict separate --- tests/src/rules/typescript-flow.js | 53 +++++++++++++++++++++++++----- 1 file changed, 45 insertions(+), 8 deletions(-) diff --git a/tests/src/rules/typescript-flow.js b/tests/src/rules/typescript-flow.js index 1835af609..ffde8d0a7 100644 --- a/tests/src/rules/typescript-flow.js +++ b/tests/src/rules/typescript-flow.js @@ -23,12 +23,31 @@ context('TypeScript', function () { ruleTester.run('typescript-flow', rule, { valid: [ - // test({ - // code: `import type { MyType } from "./typescript.ts"`, - // parser, - // settings, - // options: ['separate'], - // }), + test({ + code: `import type { MyType } from "./typescript.ts"`, + parser, + settings, + options: ['separate'], + }), + test({ + code: `import * as Bar from "./typescript.ts"`, + parser, + settings, + options: ['separate'], + }), + // default import is ignored for now + test({ + code: `import Bar from "./typescript.ts"`, + parser, + settings, + options: ['separate'], + }), + test({ + code: `import type Bar from "./typescript.ts"`, + parser, + settings, + options: ['separate'], + }), // test({ // code: `import { MyType } from "./typescript.ts"`, // parser, @@ -83,7 +102,6 @@ context('TypeScript', function () { // output: 'import { MyType, Bar } from "./typescript.ts"', // }), - // Single Config option which is separate test({ code: 'import {type MyType,Bar} from "./typescript.ts"', parser, @@ -116,7 +134,26 @@ context('TypeScript', function () { message: 'BOOM', }], output: 'import {Bar as Namespace} from "./typescript.ts"\nimport type { MyType } from "./typescript.ts"', - //output: 'import type { Bar } from "./typescript.ts"\nimport type { MyType } from "./typescript.ts"', + }), + test({ + code: 'import {type MyType,type Foo} from "./typescript.ts"', + parser, + settings, + options: ['separate'], + errors: [{ + message: 'BOOM', + }], + output: 'import type { MyType, Foo} from "./typescript.ts"', + }), + test({ + code: 'import {type MyType as Bar,type Foo} from "./typescript.ts"', + parser, + settings, + options: ['separate'], + errors: [{ + message: 'BOOM', + }], + output: 'import type { MyType as Bar, Foo} from "./typescript.ts"', }), ] }, ); From 4a6a9bcc66c79d6eabefe31bb8f15b3f1368502f Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Sat, 10 Dec 2022 21:33:47 -0500 Subject: [PATCH 14/37] finish strict separate option --- src/rules/typescript-flow.js | 274 ++++++++++------------------------- 1 file changed, 75 insertions(+), 199 deletions(-) diff --git a/src/rules/typescript-flow.js b/src/rules/typescript-flow.js index 9351c51f6..f38fc0919 100644 --- a/src/rules/typescript-flow.js +++ b/src/rules/typescript-flow.js @@ -1,7 +1,6 @@ 'use strict'; import docsUrl from '../docsUrl'; -import Exports from '../ExportMap'; import debug from 'debug'; const log = debug('eslint-plugin-import/typescript-flow'); @@ -13,52 +12,6 @@ import semver from 'semver'; function tsVersionSatisfies(specifier) { return semver.satisfies(typescriptPkg.version, specifier); } -// no need to have separate function, or doing it before the program start running => need to incorporate inside of the function -function processBodyStatement(context, typeExports, node) { - if (node.type !== 'ImportDeclaration') return; - - if (node.specifiers.length === 0) return; - - const imports = Exports.get(node.source.value, context); - // log('imports'); - // console.log(imports.namespace); - if (imports == null) return null; - - if (imports.errors.length > 0) { - imports.reportErrors(context, node); - return; - } - - imports.namespace.forEach((value, key) =>{ - if (value === 'type') { - typeExports.add(key); - } - }); - - // node.specifiers.forEach((specifier) => { - // switch (specifier.type) { - // case 'ImportNamespaceSpecifier': - // if (!imports.size) { - // context.report( - // specifier, - // `No exported names found in module '${node.source.value}'.`, - // ); - // } - // namespaces.set(specifier.local.name, imports); - // break; - // case 'ImportDefaultSpecifier': - // case 'ImportSpecifier': { - // const meta = imports.get( - // // default to 'default' for default https://i.imgur.com/nj6qAWy.jpg - // specifier.imported ? (specifier.imported.name || specifier.imported.value) : 'default', - // ); - // if (!meta || !meta.namespace) { break; } - // namespaces.set(specifier.local.name, meta.namespace); - // break; - // } - // } - // }); -} // It seems unfortunate to have a rule that is only useful for typescript, but perhaps a rule that detects your TS (or flow) version, // and can be configured for an order of preferences - ie, between "types as separate import statements", @@ -102,11 +55,6 @@ module.exports = { ], }], }, - - - - - create(context) { // if TS is < 3.8 => we can just name import it. @@ -118,182 +66,110 @@ module.exports = { // import { type Person, Cache } from "./foo"; // ImportDeclaration importKind = 'value', ImportSpecifier.importKind = 'type' - // get Rule options. - const config = context.options[0]; - - // check the config - const configSingle = typeof(context.options[0]) === 'string' ? true : false; - - const prefer = 'separate'; - - // allow single value or array of values. When list of available values not possible => message: unfixable problem. When there is multiple problems, finds first value in the list and makes it happen. - const supportsTypeImport = tsVersionSatisfies('>=3.8'); // separate `import type { a } from 'x'` was introduced in TS 3.8 - const supportsTypeModifier = tsVersionSatisfies('>=4.5'); // type modifiers were introduced in TS 4.5. - // TODO: check the array on TS version to leave possible options - // If ! supportsTypeModifier => remove type modifier from the array - // if ! supportsTypeImport => check if options array has 'none', if not => flag an error + // 3 cases: strict cases: separate, inline. 3rd case is array of preference. The only thing to check is if arr[0] === inline => + // check if it can be supported. If not, fall back on separate. + // If arr[0] === separate => just do separate ? Then what is really a logic? Taking care of user worrying about TS version? - // useful if we can just remove as. - const typeExports = new Set(); - - //const typeImports = new Set(); - const valueImports = new Set(); + const supportInlineTypeImport = tsVersionSatisfies('>=4.5'); // type modifiers were introduced in TS 4.5. + // get Rule options. + let config = context.options[0]; + if (config.length === 2 && config[0] === 'inline' && !supportInlineTypeImport) { + config = 'separate'; + } const fixerArray = []; - // TODO: filter down available preferences: find the first one available then use it as preferred. If none is option: find all type key words and auto fixer => remove them. - // return 3 rule objets? - return { - // // pick up all type imports at body entry time, to properly respect hoisting - // Program({ body }) { - // body.forEach(x => processBodyStatement(context, typeExports, x)); - // }, - 'ImportNamespaceSpecifier': function (){ - return; - }, - 'ImportDeclaration': function (node){ + const typeImports = []; + const valueImports =[]; + const importRangesToBeRemoved = []; + let allImportsSize = 0; - // 3 cases: strict cases: separate, inline. 3rd case is array of preference. The only thing to check is if arr[0] === inline => - // check if it can be supported. If not, fall back on separate. - // If arr[0] === separate => just do separate ? Then what is really a logic? Taking care of user worrying about TS version? - - // only one config. Since we determined that rule will fall back on option that works. - if (typeof(context.options[0]) === 'string' || config.length === 1) { - // strict config: separate, but we got inline: - if (config === 'separate' && node.importKind !== 'type') { - const typeImports = []; - const inlineValueImports =[]; - const importRangesToBeRemoved = []; - // identify importSpecifiers that have inline type imports as well as value imports - node.specifiers.forEach((specifier) => { - // catch all inline imports and add them to the set - if (specifier.importKind === 'type') { - log('specifier'); - console.log(specifier); + return { - if (specifier.local.name !== specifier.imported.name) { - typeImports.push(`${specifier.imported.name} as ${specifier.local.name}`); - } else { - typeImports.push(specifier.local.name); - } - importRangesToBeRemoved.push(specifier.range); - // typeImports.add(specifier); - } else { - if (specifier.local.name !== specifier.imported.name) { - inlineValueImports.push(`${specifier.imported.name} as ${specifier.local.name}`); - } else { - inlineValueImports.push(specifier.local.name); - } - importRangesToBeRemoved.push(specifier.range); - // valueImports.add(specifier); - } - }); - // no inline type imports - if (typeImports.size === 0) { - return; + 'ImportDeclaration': function (node){ + + // identify importSpecifiers that have inline type imports as well as value imports + node.specifiers.forEach((specifier) => { + // Question: do we want our rule to deal with default imports? It does not make sense that rule needs to since we do not know the type of default export. + // For now, ignore default export + if (specifier.type === 'ImportNamespaceSpecifier' || specifier.type === 'ImportDefaultSpecifier') { + return; + } + allImportsSize += 1; + // catch all inline imports and add them to the set + if (specifier.importKind === 'type') { + if (specifier.local.name !== specifier.imported.name) { + typeImports.push(`${specifier.imported.name} as ${specifier.local.name}`); + } else { + typeImports.push(specifier.local.name); } - - //remove inline type imports + add them as separate import statements + importRangesToBeRemoved.push(specifier.range); + } else { + if (specifier.local.name !== specifier.imported.name) { + valueImports.push(`${specifier.imported.name} as ${specifier.local.name}`); + } else { + valueImports.push(specifier.local.name); + } + importRangesToBeRemoved.push(specifier.range); + } + }); + + if (config === 'separate' && node.importKind !== 'type') { + // no inline type imports found + if (typeImports.length === 0) { + return; + } else if (typeImports.length === allImportsSize) { + // all inline imports are type imports => need to change it to separate import statement + context.report({ + node, + message: 'BOOM', + fix(fixer) { + const sourceCode = context.getSourceCode(); + const tokens = sourceCode.getTokens(node); + tokens.forEach(token => { + if (token.value === 'type') { + fixerArray.push(fixer.remove(token)); + } + }); + fixerArray.push(fixer.insertTextAfter(tokens[0], ' type')); + return fixerArray; + }, + }); + } + else { context.report({ node, message: 'BOOM', fix(fixer) { - const sourceCode = context.getSourceCode(); const tokens = sourceCode.getTokens(node); - // log('tokens'); - // console.log(tokens); - - // get import source const importPath = tokens[tokens.length-1].value; - // log('import path'); - // console.log(importPath); - + + // remove all imports importRangesToBeRemoved.forEach((range)=>{ - // log('ranges to remove'); - // console.log(range); fixerArray.push(fixer.removeRange([range[0], range[1]])); }); - // get the named type imports and remove them from the initial code - // typeImports.forEach((element)=> { - // newTypeImports.push(element.local.name); - // fixerArray.push(fixer.removeRange([element.range[0], element.range[1]])); - // }); - // // remove inline value imports => maybe remove all at once by removing everything between { and }? TODO - // valueImports.forEach((element)=> { - // if (element.local.name !== element.imported.name) { - // inlineValueImports.push(`${element.imported.name} as ${element.local.name}`); - // } else { - // inlineValueImports.push(element.local.name); - // } - // fixerArray.push(fixer.removeRange([element.range[0], element.range[1]])); - // }); let namedImportStart = undefined; - for (let i = 0; i < tokens.length; i++) { - if (tokens[i].value === '{') { - namedImportStart = tokens[i]; + // remove all commas + tokens.forEach( element => { + if (element.value === '{') { + namedImportStart = element; } - if (tokens[i].value === ',' && tokens[i].type === 'Punctuator') { - fixerArray.push(fixer.remove(tokens[i])); - } - } - - + if (element.value === ',') { + fixerArray.push(fixer.remove(element)); + } + }); // add inline value imports back - fixerArray.push(fixer.insertTextAfter(namedImportStart, inlineValueImports.join(', '))); + fixerArray.push(fixer.insertTextAfter(namedImportStart, valueImports.join(', '))); // add new line with separate type import fixerArray.push(fixer.insertTextAfter(node, `\nimport type { ${typeImports.join(', ')} } from ${importPath}`)); return fixerArray; }, }); } - - // log('inlineTypeImports'); - // console.log(typeImports); - - - } - - // case where we want type modifier but we got separate import type - // if (supportsTypeModifier && prefer === 'modifier' && node.importKind === 'type') { - // context.report(node, 'BOOM'); - // } }, - 'ImportSpecifier': function (node) { - - // cases when we have only one config option - if (typeof(context.options[0]) === 'string' || config.length === 1) { - // only one config - - // we have none, but have word type - - // wanted separate but got inline case. Question: How to make two fixes? - // if ((config === 'separate' || config[0] === 'separate') && node.importKind === 'type') { - // context.report({ - // node, - // message: 'BOOM', - // fix(fixer) { - // const sourceCode = context.getSourceCode(); - // const token = sourceCode.getTokens(node); - // log('source code tokens'); - // console.log(token); - - // const tokenParent = sourceCode.getTokens(node.parent); - // log('source code tokens -- PARENT'); - // console.log(tokenParent); - // return fixer.replaceTextRange([token.range[0], token.range[1]+1], '') ; - // }, - // }); - // } - - } - - // if none => remove all type key words ? - - // TODO: import Cache from "./foo"; import Default Specifier type? - }, }; }, }; From 08d9832803ee97369d5a2a01367e936235220c97 Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Sat, 10 Dec 2022 22:22:13 -0500 Subject: [PATCH 15/37] add strict inline with only import --- src/rules/typescript-flow.js | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/rules/typescript-flow.js b/src/rules/typescript-flow.js index f38fc0919..05544b404 100644 --- a/src/rules/typescript-flow.js +++ b/src/rules/typescript-flow.js @@ -76,10 +76,14 @@ module.exports = { if (config.length === 2 && config[0] === 'inline' && !supportInlineTypeImport) { config = 'separate'; } + if (config[0]==='separate') { + config = 'separate'; + } const fixerArray = []; const typeImports = []; const valueImports =[]; + const valueNodeImports =[]; const importRangesToBeRemoved = []; let allImportsSize = 0; @@ -104,6 +108,7 @@ module.exports = { } importRangesToBeRemoved.push(specifier.range); } else { + valueNodeImports.push(specifier); if (specifier.local.name !== specifier.imported.name) { valueImports.push(`${specifier.imported.name} as ${specifier.local.name}`); } else { @@ -168,6 +173,34 @@ module.exports = { }); } } + + if (config === 'inline' && node.importKind === 'type') { + // check if there are other imports from the same source, if there are, them add type imports there + + // if there are none => remove type and add "type" to every import + // import type {a,b} from 'x' => import {type a, type b} from 'x' + log('want inline but got separate type import'); + context.report({ + node, + message: 'BOOM', + fix(fixer) { + const sourceCode = context.getSourceCode(); + const tokens = sourceCode.getTokens(node); + log('tokens'); + console.log(tokens); + fixerArray.push(fixer.remove(tokens[1])); + + log('value imports'); + console.log(valueNodeImports); + + valueNodeImports.forEach(element => { + fixerArray.push(fixer.insertTextBefore(element, 'type ')); + }); + return fixerArray; + }, + }); + + } }, }; From 4b5e577a935d19764219147219c35153d947040c Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Sat, 10 Dec 2022 22:24:32 -0500 Subject: [PATCH 16/37] add test cases for strict inline with only one import --- tests/src/rules/typescript-flow.js | 100 ++++++++++++++--------------- 1 file changed, 47 insertions(+), 53 deletions(-) diff --git a/tests/src/rules/typescript-flow.js b/tests/src/rules/typescript-flow.js index ffde8d0a7..e58c301fd 100644 --- a/tests/src/rules/typescript-flow.js +++ b/tests/src/rules/typescript-flow.js @@ -29,13 +29,25 @@ context('TypeScript', function () { settings, options: ['separate'], }), + test({ + code: `import type { MyType, Bar } from "./typescript.ts"`, + parser, + settings, + options: ['separate'], + }), + test({ + code: `import type { MyType as Foo } from "./typescript.ts"`, + parser, + settings, + options: ['separate'], + }), test({ code: `import * as Bar from "./typescript.ts"`, parser, settings, options: ['separate'], }), - // default import is ignored for now + // default imports are ignored test({ code: `import Bar from "./typescript.ts"`, parser, @@ -48,60 +60,20 @@ context('TypeScript', function () { settings, options: ['separate'], }), - // test({ - // code: `import { MyType } from "./typescript.ts"`, - // parser, - // settings, - // options: [{ - // prefer: 'modifier', - // }], - // }), + test({ + code: `import { type MyType } from "./typescript.ts"`, + parser, + settings, + options: ['inline'], + }), + test({ + code: `import { type MyType, Bar, MyEnum } from "./typescript.ts"`, + parser, + settings, + options: ['inline'], + }), ], invalid: [ - // test({ - // code: `import type { MyType, Bar } from "./typescript.ts"`, - // parser, - // settings, - // options: ['none'], - // errors: [{ - // message: 'BOOM', - // }], - // output: 'import { MyType, Bar } from "./typescript.ts"', - // }), - // test({ - // code: 'import { type MyType, Bar } from "./typescript.ts"', - // parser, - // settings, - // options: ['none'], - // errors: [{ - // message: 'BOOM', - // }], - // output: 'import { MyType, Bar } from "./typescript.ts"', - // }), - // test({ - // code: 'import { MyType, type Bar } from "./typescript.ts"', - // parser, - // settings, - // options: ['none'], - // errors: [{ - // message: 'BOOM', - // }], - // output: 'import { MyType, Bar } from "./typescript.ts"', - // }), - // test({ - // code: 'import { type MyType, type Bar } from "./typescript.ts"', - // parser, - // settings, - // options: ['none'], - // errors: [{ - // message: 'BOOM', - // }, - // { - // message: 'BOOM', - // }], - // output: 'import { MyType, Bar } from "./typescript.ts"', - // }), - test({ code: 'import {type MyType,Bar} from "./typescript.ts"', parser, @@ -155,6 +127,28 @@ context('TypeScript', function () { }], output: 'import type { MyType as Bar, Foo} from "./typescript.ts"', }), + // the space is left over when 'type' is removed. Question + test({ + code: 'import type {MyType} from "./typescript.ts"', + parser, + settings, + options: ['inline'], + errors: [{ + message: 'BOOM', + }], + output: 'import {type MyType} from "./typescript.ts"', + }), + + test({ + code: 'import type {MyType, Bar} from "./typescript.ts"', + parser, + settings, + options: ['inline'], + errors: [{ + message: 'BOOM', + }], + output: 'import {type MyType, type Bar} from "./typescript.ts"', + }), ] }, ); }); From 8f7239d024386db8e572451ddfacc74ade322f93 Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Tue, 13 Dec 2022 17:47:25 -0500 Subject: [PATCH 17/37] add invalid default import test cases for inline option --- tests/src/rules/typescript-flow.js | 50 ++++++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 6 deletions(-) diff --git a/tests/src/rules/typescript-flow.js b/tests/src/rules/typescript-flow.js index e58c301fd..d7683b4d9 100644 --- a/tests/src/rules/typescript-flow.js +++ b/tests/src/rules/typescript-flow.js @@ -5,8 +5,9 @@ import { RuleTester } from 'eslint'; const ruleTester = new RuleTester(); const rule = require('rules/typescript-flow'); -// TODO: add exports from .TSX files to the test cases, import default, import * as b statement +// TODO: add imports from .TSX files to the test cases context('TypeScript', function () { + // Do we need to check for different TS parsers? TODO Question getTSParsers().forEach((parser) => { // const parserConfig = { // parser, @@ -47,7 +48,7 @@ context('TypeScript', function () { settings, options: ['separate'], }), - // default imports are ignored + // default imports are ignored for strict option separate test({ code: `import Bar from "./typescript.ts"`, parser, @@ -82,9 +83,7 @@ context('TypeScript', function () { errors: [{ message: 'BOOM', }], - // Question: Do we just remove the problematic node, what about comma? // import { type MyType, Bar, type Foo } => mport { , Bar, } output: 'import {Bar} from "./typescript.ts"\nimport type { MyType } from "./typescript.ts"', - //output: 'import type { Bar } from "./typescript.ts"\nimport type { MyType } from "./typescript.ts"', }), test({ code: 'import {type MyType,Bar,type Foo} from "./typescript.ts"', @@ -94,7 +93,6 @@ context('TypeScript', function () { errors: [{ message: 'BOOM', }], - // Question: Do we just remove the problematic node, what about comma? // import { type MyType, Bar, type Foo } => mport { , Bar, } output: 'import {Bar} from "./typescript.ts"\nimport type { MyType, Foo } from "./typescript.ts"', }), test({ @@ -138,7 +136,6 @@ context('TypeScript', function () { }], output: 'import {type MyType} from "./typescript.ts"', }), - test({ code: 'import type {MyType, Bar} from "./typescript.ts"', parser, @@ -149,6 +146,47 @@ context('TypeScript', function () { }], output: 'import {type MyType, type Bar} from "./typescript.ts"', }), + test({ + code: 'import type {MyType, Bar} from "./typescript.ts";import {MyEnum} from "./typescript.ts"', + parser, + settings, + options: ['inline'], + errors: [{ + message: 'BOOM', + }], + output: 'import {MyEnum, type MyType, type Bar} from "./typescript.ts"', + }), + test({ + code: 'import type {MyType, Bar} from "./typescript.ts";import {default as B} from "./typescript.ts"', + parser, + settings, + options: ['inline'], + errors: [{ + message: 'BOOM', + }], + output: 'import {default as B, type MyType, type Bar} from "./typescript.ts"', + }), + test({ + code: 'import type {MyType, Bar} from "./typescript.ts";import defaultExport from "./typescript.ts"', + parser, + settings, + options: ['inline'], + errors: [{ + message: 'BOOM', + }], + output: 'import defaultExport, { type MyType, type Bar } from "./typescript.ts"', + }), + // // TODO: what to do here? Output: import * as b, { type MyType, type Bar } from "./typescript.ts" ? + // test({ + // code: 'import type {MyType, Bar} from "./typescript.ts";import * as b from "./typescript.ts"', + // parser, + // settings, + // options: ['inline'], + // errors: [{ + // message: 'BOOM', + // }], + // output: 'import defaultExport, { type MyType, type Bar } from "./typescript.ts"', + // }), ] }, ); }); From 7e5388ca1fba1d8f4a41aa9d296ba1ad9610827b Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Tue, 13 Dec 2022 17:48:36 -0500 Subject: [PATCH 18/37] add fixer for inline option with ImportDefault specifier --- src/rules/typescript-flow.js | 191 ++++++++++++++++++++++++----------- 1 file changed, 132 insertions(+), 59 deletions(-) diff --git a/src/rules/typescript-flow.js b/src/rules/typescript-flow.js index 05544b404..01afcb180 100644 --- a/src/rules/typescript-flow.js +++ b/src/rules/typescript-flow.js @@ -13,6 +13,36 @@ function tsVersionSatisfies(specifier) { return semver.satisfies(typescriptPkg.version, specifier); } +function processBodyStatement(importMap, node){ + if (node.type !== 'ImportDeclaration' || node.importKind === 'type') return; + if (node.specifiers.length === 0) return; + + const specifiers = []; + let hasDefaultImport= false; + let hasInlineDefaultImport = false; + let hasNamespaceImport = false; + + node.specifiers.forEach((specifier)=>{ + if (specifier.type === 'ImportNamespaceSpecifier') { + hasNamespaceImport = true; + } + // ignore the case for now. TODO: talk with Jordan and implement the handling of the rule here + if (specifier.type === 'ImportDefaultSpecifier') { + hasDefaultImport = true; + specifiers.push(specifier); + } + if (specifier.type === 'ImportSpecifier') { + // check if its {default as B} + if (specifier.imported.name === 'default') { + hasInlineDefaultImport = true; + } + specifiers.push(specifier); + } + }); + // cache all imports + importMap.set(node.source.value, { specifiers, hasDefaultImport, hasInlineDefaultImport, hasNamespaceImport }); +} + // It seems unfortunate to have a rule that is only useful for typescript, but perhaps a rule that detects your TS (or flow) version, // and can be configured for an order of preferences - ie, between "types as separate import statements", // "types mixed with values, but marked with a type modifier", you could prefer one over the other, @@ -23,10 +53,6 @@ function tsVersionSatisfies(specifier) { // separate: types as separate import statements // modifier: types mixed with values, but marked with a type modifier -// To think about: if separate is preferred but none is used => need to check every import file. maybe exclude the none? -// Scenario: user imports exported types without explicitly saying type: TODO: check every import file if it's a type export -> use ExportMaps -// What is import namespaces => omit? - module.exports = { meta: { type: 'suggestion', // Layout? @@ -70,13 +96,13 @@ module.exports = { // check if it can be supported. If not, fall back on separate. // If arr[0] === separate => just do separate ? Then what is really a logic? Taking care of user worrying about TS version? - const supportInlineTypeImport = tsVersionSatisfies('>=4.5'); // type modifiers were introduced in TS 4.5. + const supportInlineTypeImport = tsVersionSatisfies('>=4.5'); // type modifiers (inline type import) were introduced in TS 4.5. // get Rule options. let config = context.options[0]; if (config.length === 2 && config[0] === 'inline' && !supportInlineTypeImport) { config = 'separate'; } - if (config[0]==='separate') { + if (config[0] === 'separate') { config = 'separate'; } @@ -84,46 +110,46 @@ module.exports = { const typeImports = []; const valueImports =[]; const valueNodeImports =[]; - const importRangesToBeRemoved = []; + const importSpecifierRanges = []; let allImportsSize = 0; + const nonTypeImportDeclarations = new Map(); return { - 'ImportDeclaration': function (node){ - - // identify importSpecifiers that have inline type imports as well as value imports - node.specifiers.forEach((specifier) => { - // Question: do we want our rule to deal with default imports? It does not make sense that rule needs to since we do not know the type of default export. - // For now, ignore default export - if (specifier.type === 'ImportNamespaceSpecifier' || specifier.type === 'ImportDefaultSpecifier') { - return; - } - allImportsSize += 1; - // catch all inline imports and add them to the set - if (specifier.importKind === 'type') { - if (specifier.local.name !== specifier.imported.name) { - typeImports.push(`${specifier.imported.name} as ${specifier.local.name}`); - } else { - typeImports.push(specifier.local.name); + + if (config === 'separate' && node.importKind !== 'type') { + // identify importSpecifiers that have inline type imports as well as value imports + node.specifiers.forEach((specifier) => { + if (specifier.type === 'ImportNamespaceSpecifier' || specifier.type === 'ImportDefaultSpecifier') { + return; } - importRangesToBeRemoved.push(specifier.range); - } else { - valueNodeImports.push(specifier); - if (specifier.local.name !== specifier.imported.name) { - valueImports.push(`${specifier.imported.name} as ${specifier.local.name}`); + // Question: do we want our rule to deal with default imports? It does not make sense that rule needs to since we do not know the type of default export. + allImportsSize = allImportsSize + 1; + // catch all inline imports and add them to the set + if (specifier.importKind === 'type') { + if (specifier.local.name !== specifier.imported.name) { + typeImports.push(`${specifier.imported.name} as ${specifier.local.name}`); + } else { + typeImports.push(specifier.local.name); + } + importSpecifierRanges.push(specifier.range); } else { - valueImports.push(specifier.local.name); + valueNodeImports.push(specifier); + if (specifier.local.name !== specifier.imported.name) { + valueImports.push(`${specifier.imported.name} as ${specifier.local.name}`); + } else { + valueImports.push(specifier.local.name); + } + importSpecifierRanges.push(specifier.range); } - importRangesToBeRemoved.push(specifier.range); - } - }); - - if (config === 'separate' && node.importKind !== 'type') { + }); // no inline type imports found if (typeImports.length === 0) { return; - } else if (typeImports.length === allImportsSize) { - // all inline imports are type imports => need to change it to separate import statement + } + // all inline imports are type imports => need to change it to separate import statement + // import {type X, type Y} form 'x' => import type { X, Y} from 'x' + if (typeImports.length === allImportsSize) { context.report({ node, message: 'BOOM', @@ -140,6 +166,9 @@ module.exports = { }, }); } + + // there is a mix of inline value imports and type imports + // import {type X, type Y, Z} form 'x' => import {Z} form 'x'\nimport type { X, Y } from 'x' else { context.report({ node, @@ -150,7 +179,7 @@ module.exports = { const importPath = tokens[tokens.length-1].value; // remove all imports - importRangesToBeRemoved.forEach((range)=>{ + importSpecifierRanges.forEach((range)=>{ fixerArray.push(fixer.removeRange([range[0], range[1]])); }); @@ -175,30 +204,74 @@ module.exports = { } if (config === 'inline' && node.importKind === 'type') { - // check if there are other imports from the same source, if there are, them add type imports there + + node.specifiers.forEach((specifier) => { + if (specifier.local.name !== specifier.imported.name) { + typeImports.push(`type ${specifier.imported.name} as ${specifier.local.name}`); + } else { + typeImports.push(`type ${specifier.local.name}`); + } + valueNodeImports.push(specifier); + }); + + // function process body statement to find if there are any non-type imports from the same location + node.parent.body.forEach(declaration => processBodyStatement(nonTypeImportDeclarations, declaration)); + + if (nonTypeImportDeclarations.has(node.source.value)) { + // file has non type import from the same source + const declaration = nonTypeImportDeclarations.get(node.source.value); + // get the last specifier + const lastSpecifier = declaration.specifiers[declaration.specifiers.length - 1]; + + // try to insert after the last specifier + context.report({ + node, + message: 'BOOM', + fix(fixer) { + + if (lastSpecifier.type === 'ImportDefaultSpecifier' && declaration.specifiers.length === 1) { + // import defaultExport from 'x' + // import type { X, Y } from 'x' + // => import defaultExport, { type X, type Y } from 'x' + const inlineTypeImportsToInsert = ', { ' + typeImports.join(', ') + ' }'; + fixerArray.push(fixer.insertTextAfter(lastSpecifier, inlineTypeImportsToInsert)); + } else { + // import { namedImport } from 'x' + // import type { X, Y } from 'x' + // => import { namedImport, type X, type Y } from 'x' + const inlineTypeImportsToInsert = ', ' + typeImports.join(', '); + fixerArray.push(fixer.insertTextAfter(lastSpecifier, inlineTypeImportsToInsert)); + } - // if there are none => remove type and add "type" to every import + fixerArray.push(fixer.remove(node)); + return fixerArray; + }, + }); + } else { + // There are no other imports from the same location => remove 'type' next to import statement and add "type" to every named import // import type {a,b} from 'x' => import {type a, type b} from 'x' - log('want inline but got separate type import'); - context.report({ - node, - message: 'BOOM', - fix(fixer) { - const sourceCode = context.getSourceCode(); - const tokens = sourceCode.getTokens(node); - log('tokens'); - console.log(tokens); - fixerArray.push(fixer.remove(tokens[1])); - - log('value imports'); - console.log(valueNodeImports); - - valueNodeImports.forEach(element => { - fixerArray.push(fixer.insertTextBefore(element, 'type ')); - }); - return fixerArray; - }, - }); + + // TODO: check this statement: valueNodeImports => possibly rename it ? + context.report({ + node, + message: 'BOOM', + fix(fixer) { + const sourceCode = context.getSourceCode(); + const tokens = sourceCode.getTokens(node); + // log('tokens'); + // console.log(tokens); + fixerArray.push(fixer.remove(tokens[1])); + // log('value imports'); + // console.log(valueNodeImports); + valueNodeImports.forEach(element => { + fixerArray.push(fixer.insertTextBefore(element, 'type ')); + }); + return fixerArray; + }, + }); + } + + } From 8c7ff3fa6c60ba7846a0b73417dacc7b03c5eba9 Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Tue, 13 Dec 2022 21:51:35 -0500 Subject: [PATCH 19/37] fix one failing test case in default rule The default rule had invalid test case where default import was tried, because I have added default export for typescript-flow cases, the test case broke. I have added new file with no default export and substituted it for ./typescript used in default rule --- tests/files/typescript-no-default-export.ts | 4 ++++ tests/src/rules/default.js | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) create mode 100644 tests/files/typescript-no-default-export.ts diff --git a/tests/files/typescript-no-default-export.ts b/tests/files/typescript-no-default-export.ts new file mode 100644 index 000000000..68ea332c6 --- /dev/null +++ b/tests/files/typescript-no-default-export.ts @@ -0,0 +1,4 @@ +export interface Persona { + name: string, + age: number +}; \ No newline at end of file diff --git a/tests/src/rules/default.js b/tests/src/rules/default.js index eb2028c71..ebb524f87 100644 --- a/tests/src/rules/default.js +++ b/tests/src/rules/default.js @@ -264,13 +264,13 @@ context('TypeScript', function () { invalid: [ test({ - code: `import foobar from "./typescript"`, + code: `import foobar from "./typescript-no-default-export"`, parser, settings: { 'import/parsers': { [parser]: ['.ts'] }, 'import/resolver': { 'eslint-import-resolver-typescript': true }, }, - errors: ['No default export found in imported module "./typescript".'], + errors: ['No default export found in imported module "./typescript-no-default-export".'], }), test({ code: `import React from "./typescript-export-assign-default-namespace"`, From 250a0227e0346ee383984008795b126148ff3a93 Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Tue, 13 Dec 2022 21:59:16 -0500 Subject: [PATCH 20/37] add more named import test cases for strict and inline options --- tests/src/rules/typescript-flow.js | 56 ++++++++++++++++++++++++++++-- 1 file changed, 54 insertions(+), 2 deletions(-) diff --git a/tests/src/rules/typescript-flow.js b/tests/src/rules/typescript-flow.js index d7683b4d9..a394e9af7 100644 --- a/tests/src/rules/typescript-flow.js +++ b/tests/src/rules/typescript-flow.js @@ -48,7 +48,7 @@ context('TypeScript', function () { settings, options: ['separate'], }), - // default imports are ignored for strict option separate + // default imports are ignored for strict option separate. Question. TODO test({ code: `import Bar from "./typescript.ts"`, parser, @@ -67,12 +67,24 @@ context('TypeScript', function () { settings, options: ['inline'], }), + test({ + code: `import { type MyType as Persona } from "./typescript.ts"`, + parser, + settings, + options: ['inline'], + }), test({ code: `import { type MyType, Bar, MyEnum } from "./typescript.ts"`, parser, settings, options: ['inline'], }), + test({ + code: `import { type MyType as Persona, Bar as Bar, MyEnum } from "./typescript.ts"`, + parser, + settings, + options: ['inline'], + }), ], invalid: [ test({ @@ -85,6 +97,16 @@ context('TypeScript', function () { }], output: 'import {Bar} from "./typescript.ts"\nimport type { MyType } from "./typescript.ts"', }), + test({ + code: 'import {type MyType as Persona,Bar} from "./typescript.ts"', + parser, + settings, + options: ['separate'], + errors: [{ + message: 'BOOM', + }], + output: 'import {Bar} from "./typescript.ts"\nimport type { MyType as Persona } from "./typescript.ts"', + }), test({ code: 'import {type MyType,Bar,type Foo} from "./typescript.ts"', parser, @@ -95,6 +117,16 @@ context('TypeScript', function () { }], output: 'import {Bar} from "./typescript.ts"\nimport type { MyType, Foo } from "./typescript.ts"', }), + test({ + code: 'import {type MyType as Persona,Bar,type Foo as Baz} from "./typescript.ts"', + parser, + settings, + options: ['separate'], + errors: [{ + message: 'BOOM', + }], + output: 'import {Bar} from "./typescript.ts"\nimport type { MyType as Persona, Foo as Baz } from "./typescript.ts"', + }), test({ code: 'import {type MyType,Bar as Namespace} from "./typescript.ts"', parser, @@ -125,7 +157,7 @@ context('TypeScript', function () { }], output: 'import type { MyType as Bar, Foo} from "./typescript.ts"', }), - // the space is left over when 'type' is removed. Question + // the space is left over when 'type' is removed. Question. TODO test({ code: 'import type {MyType} from "./typescript.ts"', parser, @@ -156,6 +188,16 @@ context('TypeScript', function () { }], output: 'import {MyEnum, type MyType, type Bar} from "./typescript.ts"', }), + test({ + code: 'import type {MyType as Persona, Bar as Foo} from "./typescript.ts";import {MyEnum} from "./typescript.ts"', + parser, + settings, + options: ['inline'], + errors: [{ + message: 'BOOM', + }], + output: 'import {MyEnum, type MyType as Persona, type Bar as Foo} from "./typescript.ts"', + }), test({ code: 'import type {MyType, Bar} from "./typescript.ts";import {default as B} from "./typescript.ts"', parser, @@ -176,6 +218,16 @@ context('TypeScript', function () { }], output: 'import defaultExport, { type MyType, type Bar } from "./typescript.ts"', }), + test({ + code: 'import type {MyType as Persona, Bar as Foo} from "./typescript.ts";import defaultExport from "./typescript.ts"', + parser, + settings, + options: ['inline'], + errors: [{ + message: 'BOOM', + }], + output: 'import defaultExport, { type MyType as Persona, type Bar as Foo } from "./typescript.ts"', + }), // // TODO: what to do here? Output: import * as b, { type MyType, type Bar } from "./typescript.ts" ? // test({ // code: 'import type {MyType, Bar} from "./typescript.ts";import * as b from "./typescript.ts"', From a63016102e0fe183f2e5a5541d3ac7a018eb7b25 Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Tue, 13 Dec 2022 22:02:54 -0500 Subject: [PATCH 21/37] remove unnecessary comments --- src/rules/typescript-flow.js | 32 +++++--------------------------- 1 file changed, 5 insertions(+), 27 deletions(-) diff --git a/src/rules/typescript-flow.js b/src/rules/typescript-flow.js index 01afcb180..862ce3763 100644 --- a/src/rules/typescript-flow.js +++ b/src/rules/typescript-flow.js @@ -5,7 +5,7 @@ import docsUrl from '../docsUrl'; import debug from 'debug'; const log = debug('eslint-plugin-import/typescript-flow'); -// comes from tests/src/utils file, TODO?: import directly from there +// comes from tests/src/utils file, TODO?: import directly from there, issue: too many layers of folders away import typescriptPkg from 'typescript/package.json'; import semver from 'semver'; @@ -43,16 +43,7 @@ function processBodyStatement(importMap, node){ importMap.set(node.source.value, { specifiers, hasDefaultImport, hasInlineDefaultImport, hasNamespaceImport }); } -// It seems unfortunate to have a rule that is only useful for typescript, but perhaps a rule that detects your TS (or flow) version, -// and can be configured for an order of preferences - ie, between "types as separate import statements", -// "types mixed with values, but marked with a type modifier", you could prefer one over the other, -// and it would fall back to the next one if the preferred one wasn't supported, and if neither are supported, the rule would noop. - -// 3 options of prefering importing types: none, separate, modifier. -// none => no use of word 'type' preferred. -// separate: types as separate import statements -// modifier: types mixed with values, but marked with a type modifier - +// TODO: revert changes in ExportMap file. module.exports = { meta: { type: 'suggestion', // Layout? @@ -70,7 +61,7 @@ module.exports = { 'type': 'string', 'enum': ['separate', 'inline'], }, - 'minItems': 1, + 'minItems': 2, 'maxItems': 2, 'uniqueItems': true, }, @@ -81,20 +72,11 @@ module.exports = { ], }], }, - create(context) { - // if TS is < 3.8 => we can just name import it. - - // works from typescript >= 3.8 - // import type { Person, Cache } from "./foo" - // ImportDeclaration importKind = 'type', ImportSpecifier.importKind = 'value' - - // works only on typescript >= 4.5 - // import { type Person, Cache } from "./foo"; - // ImportDeclaration importKind = 'value', ImportSpecifier.importKind = 'type' + create(context) { // 3 cases: strict cases: separate, inline. 3rd case is array of preference. The only thing to check is if arr[0] === inline => // check if it can be supported. If not, fall back on separate. - // If arr[0] === separate => just do separate ? Then what is really a logic? Taking care of user worrying about TS version? + // If arr[0] === separate => just assume it is separate const supportInlineTypeImport = tsVersionSatisfies('>=4.5'); // type modifiers (inline type import) were introduced in TS 4.5. // get Rule options. @@ -270,11 +252,7 @@ module.exports = { }, }); } - - - } - }, }; }, From 366ac51fc52522b7ac76d54e7438db7be9b53047 Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Tue, 13 Dec 2022 22:44:51 -0500 Subject: [PATCH 22/37] added comment check if inline import is supported --- src/rules/typescript-flow.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/rules/typescript-flow.js b/src/rules/typescript-flow.js index 862ce3763..0d103b810 100644 --- a/src/rules/typescript-flow.js +++ b/src/rules/typescript-flow.js @@ -84,6 +84,9 @@ module.exports = { if (config.length === 2 && config[0] === 'inline' && !supportInlineTypeImport) { config = 'separate'; } + if (config === 'inline' && !supportInlineTypeImport) { + // raise error + } if (config[0] === 'separate') { config = 'separate'; } From 5f79ac68e22301399444b154f6945dd526984493 Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Tue, 13 Dec 2022 22:46:14 -0500 Subject: [PATCH 23/37] add check for TS version support for inline type import --- src/rules/typescript-flow.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/rules/typescript-flow.js b/src/rules/typescript-flow.js index 0d103b810..a4e103216 100644 --- a/src/rules/typescript-flow.js +++ b/src/rules/typescript-flow.js @@ -84,9 +84,6 @@ module.exports = { if (config.length === 2 && config[0] === 'inline' && !supportInlineTypeImport) { config = 'separate'; } - if (config === 'inline' && !supportInlineTypeImport) { - // raise error - } if (config[0] === 'separate') { config = 'separate'; } @@ -102,6 +99,11 @@ module.exports = { return { 'ImportDeclaration': function (node){ + if (config === 'inline' && !supportInlineTypeImport) { + // raise error + context.report(node, 'Type modifiers are not supported by your version of TS.'); + } + if (config === 'separate' && node.importKind !== 'type') { // identify importSpecifiers that have inline type imports as well as value imports node.specifiers.forEach((specifier) => { From 150af6f29ea3d73532c3e0883e73238b403c71d3 Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Tue, 13 Dec 2022 23:29:44 -0500 Subject: [PATCH 24/37] add invalid test case with repeated import with namespace to inline option --- tests/src/rules/typescript-flow.js | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/tests/src/rules/typescript-flow.js b/tests/src/rules/typescript-flow.js index a394e9af7..7767db055 100644 --- a/tests/src/rules/typescript-flow.js +++ b/tests/src/rules/typescript-flow.js @@ -228,16 +228,26 @@ context('TypeScript', function () { }], output: 'import defaultExport, { type MyType as Persona, type Bar as Foo } from "./typescript.ts"', }), - // // TODO: what to do here? Output: import * as b, { type MyType, type Bar } from "./typescript.ts" ? + test({ + code: 'import type {MyType, Bar} from "./typescript.ts";import * as b from "./typescript.ts"', + parser, + settings, + options: ['inline'], + errors: [{ + message: 'BOOM', + }], + output: 'import {type MyType, type Bar} from "./typescript.ts";import * as b from "./typescript.ts"', + }), + // TODO: Fix the one below // test({ - // code: 'import type {MyType, Bar} from "./typescript.ts";import * as b from "./typescript.ts"', + // code: 'import type {MyType, Bar} from "./typescript.ts";import type A from "./typescript.ts"', // parser, // settings, // options: ['inline'], // errors: [{ // message: 'BOOM', // }], - // output: 'import defaultExport, { type MyType, type Bar } from "./typescript.ts"', + // output: 'import {type MyType, type Bar} from "./typescript.ts";import type A from "./typescript.ts"', // }), ] }, ); From 64757e8e50cde9b11f39ba5e705bcc4d1cc4bb66 Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Tue, 13 Dec 2022 23:30:31 -0500 Subject: [PATCH 25/37] add support for namespace in repeated source for inline option --- src/rules/typescript-flow.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/rules/typescript-flow.js b/src/rules/typescript-flow.js index a4e103216..94bad75fe 100644 --- a/src/rules/typescript-flow.js +++ b/src/rules/typescript-flow.js @@ -203,10 +203,12 @@ module.exports = { // function process body statement to find if there are any non-type imports from the same location node.parent.body.forEach(declaration => processBodyStatement(nonTypeImportDeclarations, declaration)); + + // file has non type import from the same source + const declaration = nonTypeImportDeclarations.get(node.source.value); + + if (declaration && !declaration.hasNamespaceImport) { - if (nonTypeImportDeclarations.has(node.source.value)) { - // file has non type import from the same source - const declaration = nonTypeImportDeclarations.get(node.source.value); // get the last specifier const lastSpecifier = declaration.specifiers[declaration.specifiers.length - 1]; From e05f4c2b6408cb2f70ece4acfca0c89c59270995 Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Thu, 15 Dec 2022 12:40:54 -0500 Subject: [PATCH 26/37] add new typescript flow to src/index --- src/index.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/index.js b/src/index.js index 15f98d96f..54ff495d4 100644 --- a/src/index.js +++ b/src/index.js @@ -52,6 +52,9 @@ export const rules = { // deprecated aliases to rules 'imports-first': require('./rules/imports-first'), + + // new typescript flow rule + 'typescript-flow': require('./rules/typescript-flow'), }; export const configs = { From fe60ea12fbd0972080a50ed6284e0075e14f29db Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Sat, 31 Dec 2022 01:56:10 -0800 Subject: [PATCH 27/37] fix default type import with inline option --- src/rules/typescript-flow.js | 14 ++++++-------- tests/src/rules/typescript-flow.js | 21 ++++++++++----------- 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/src/rules/typescript-flow.js b/src/rules/typescript-flow.js index 94bad75fe..6e1db6827 100644 --- a/src/rules/typescript-flow.js +++ b/src/rules/typescript-flow.js @@ -2,8 +2,8 @@ import docsUrl from '../docsUrl'; -import debug from 'debug'; -const log = debug('eslint-plugin-import/typescript-flow'); +// import debug from 'debug'; +// const log = debug('eslint-plugin-import/typescript-flow'); // comes from tests/src/utils file, TODO?: import directly from there, issue: too many layers of folders away import typescriptPkg from 'typescript/package.json'; @@ -191,7 +191,10 @@ module.exports = { } if (config === 'inline' && node.importKind === 'type') { - + // ignore default type import like: import type A from 'x' + if (node.specifiers.length === 1 && node.specifiers[0].type === 'ImportDefaultSpecifier') { + return; + } node.specifiers.forEach((specifier) => { if (specifier.local.name !== specifier.imported.name) { typeImports.push(`type ${specifier.imported.name} as ${specifier.local.name}`); @@ -217,7 +220,6 @@ module.exports = { node, message: 'BOOM', fix(fixer) { - if (lastSpecifier.type === 'ImportDefaultSpecifier' && declaration.specifiers.length === 1) { // import defaultExport from 'x' // import type { X, Y } from 'x' @@ -247,11 +249,7 @@ module.exports = { fix(fixer) { const sourceCode = context.getSourceCode(); const tokens = sourceCode.getTokens(node); - // log('tokens'); - // console.log(tokens); fixerArray.push(fixer.remove(tokens[1])); - // log('value imports'); - // console.log(valueNodeImports); valueNodeImports.forEach(element => { fixerArray.push(fixer.insertTextBefore(element, 'type ')); }); diff --git a/tests/src/rules/typescript-flow.js b/tests/src/rules/typescript-flow.js index 7767db055..35d146fe3 100644 --- a/tests/src/rules/typescript-flow.js +++ b/tests/src/rules/typescript-flow.js @@ -238,17 +238,16 @@ context('TypeScript', function () { }], output: 'import {type MyType, type Bar} from "./typescript.ts";import * as b from "./typescript.ts"', }), - // TODO: Fix the one below - // test({ - // code: 'import type {MyType, Bar} from "./typescript.ts";import type A from "./typescript.ts"', - // parser, - // settings, - // options: ['inline'], - // errors: [{ - // message: 'BOOM', - // }], - // output: 'import {type MyType, type Bar} from "./typescript.ts";import type A from "./typescript.ts"', - // }), + test({ + code: 'import type {MyType, Bar} from "./typescript.ts";import type A from "./typescript.ts"', + parser, + settings, + options: ['inline'], + errors: [{ + message: 'BOOM', + }], + output: 'import {type MyType, type Bar} from "./typescript.ts";import type A from "./typescript.ts"', + }), ] }, ); }); From 944d9677d579734249549b9360a504ab721b320b Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Sat, 31 Dec 2022 02:01:10 -0800 Subject: [PATCH 28/37] add TSX file and export/import type cases for typescript flow --- tests/files/tsx-type-exports.tsx | 30 ++++ tests/src/rules/typescript-flow.js | 228 ++++++++++++++++++++++++++++- 2 files changed, 256 insertions(+), 2 deletions(-) create mode 100644 tests/files/tsx-type-exports.tsx diff --git a/tests/files/tsx-type-exports.tsx b/tests/files/tsx-type-exports.tsx new file mode 100644 index 000000000..b1e8f0883 --- /dev/null +++ b/tests/files/tsx-type-exports.tsx @@ -0,0 +1,30 @@ +export type MyType = string +export enum MyEnum { + Foo, + Bar, + Baz +} +export interface Foo { + native: string | number + typedef: MyType + enum: MyEnum +} + +export abstract class Bar { + abstract foo(): Foo + + method() { + return "foo" + } +} + +export function getFoo() : MyType { + return "foo" +} + + +type DefaultTypeExport = { + name: string, + age: number +} +export default DefaultTypeExport; diff --git a/tests/src/rules/typescript-flow.js b/tests/src/rules/typescript-flow.js index 35d146fe3..c2bec78a3 100644 --- a/tests/src/rules/typescript-flow.js +++ b/tests/src/rules/typescript-flow.js @@ -5,7 +5,6 @@ import { RuleTester } from 'eslint'; const ruleTester = new RuleTester(); const rule = require('rules/typescript-flow'); -// TODO: add imports from .TSX files to the test cases context('TypeScript', function () { // Do we need to check for different TS parsers? TODO Question getTSParsers().forEach((parser) => { @@ -85,6 +84,68 @@ context('TypeScript', function () { settings, options: ['inline'], }), + // the same imports from TSX file + test({ + code: `import type { MyType } from "./tsx-type-exports.tsx"`, + parser, + settings, + options: ['separate'], + }), + test({ + code: `import type { MyType, Bar } from "./tsx-type-exports.tsx"`, + parser, + settings, + options: ['separate'], + }), + test({ + code: `import type { MyType as Foo } from "./tsx-type-exports.tsx"`, + parser, + settings, + options: ['separate'], + }), + test({ + code: `import * as Bar from "./tsx-type-exports.tsx"`, + parser, + settings, + options: ['separate'], + }), + // default imports are ignored for strict option separate. Question. TODO + test({ + code: `import Bar from "./tsx-type-exports.tsx"`, + parser, + settings, + options: ['separate'], + }), + test({ + code: `import type Bar from "./tsx-type-exports.tsx"`, + parser, + settings, + options: ['separate'], + }), + test({ + code: `import { type MyType } from "./tsx-type-exports.tsx"`, + parser, + settings, + options: ['inline'], + }), + test({ + code: `import { type MyType as Persona } from "./tsx-type-exports.tsx"`, + parser, + settings, + options: ['inline'], + }), + test({ + code: `import { type MyType, Bar, MyEnum } from "./tsx-type-exports.tsx"`, + parser, + settings, + options: ['inline'], + }), + test({ + code: `import { type MyType as Persona, Bar as Bar, MyEnum } from "./tsx-type-exports.tsx"`, + parser, + settings, + options: ['inline'], + }), ], invalid: [ test({ @@ -248,7 +309,170 @@ context('TypeScript', function () { }], output: 'import {type MyType, type Bar} from "./typescript.ts";import type A from "./typescript.ts"', }), - ] }, + // TSX cases + test({ + code: 'import {type MyType,Bar} from "./tsx-type-exports.tsx"', + parser, + settings, + options: ['separate'], + errors: [{ + message: 'BOOM', + }], + output: 'import {Bar} from "./tsx-type-exports.tsx"\nimport type { MyType } from "./tsx-type-exports.tsx"', + }), + test({ + code: 'import {type MyType as Persona,Bar} from "./tsx-type-exports.tsx"', + parser, + settings, + options: ['separate'], + errors: [{ + message: 'BOOM', + }], + output: 'import {Bar} from "./tsx-type-exports.tsx"\nimport type { MyType as Persona } from "./tsx-type-exports.tsx"', + }), + test({ + code: 'import {type MyType,Bar,type Foo} from "./tsx-type-exports.tsx"', + parser, + settings, + options: ['separate'], + errors: [{ + message: 'BOOM', + }], + output: 'import {Bar} from "./tsx-type-exports.tsx"\nimport type { MyType, Foo } from "./tsx-type-exports.tsx"', + }), + test({ + code: 'import {type MyType as Persona,Bar,type Foo as Baz} from "./tsx-type-exports.tsx"', + parser, + settings, + options: ['separate'], + errors: [{ + message: 'BOOM', + }], + output: 'import {Bar} from "./tsx-type-exports.tsx"\nimport type { MyType as Persona, Foo as Baz } from "./tsx-type-exports.tsx"', + }), + test({ + code: 'import {type MyType,Bar as Namespace} from "./tsx-type-exports.tsx"', + parser, + settings, + options: ['separate'], + errors: [{ + message: 'BOOM', + }], + output: 'import {Bar as Namespace} from "./tsx-type-exports.tsx"\nimport type { MyType } from "./tsx-type-exports.tsx"', + }), + test({ + code: 'import {type MyType,type Foo} from "./tsx-type-exports.tsx"', + parser, + settings, + options: ['separate'], + errors: [{ + message: 'BOOM', + }], + output: 'import type { MyType, Foo} from "./tsx-type-exports.tsx"', + }), + test({ + code: 'import {type MyType as Bar,type Foo} from "./tsx-type-exports.tsx"', + parser, + settings, + options: ['separate'], + errors: [{ + message: 'BOOM', + }], + output: 'import type { MyType as Bar, Foo} from "./tsx-type-exports.tsx"', + }), + // the space is left over when 'type' is removed. Question. TODO + test({ + code: 'import type {MyType} from "./tsx-type-exports.tsx"', + parser, + settings, + options: ['inline'], + errors: [{ + message: 'BOOM', + }], + output: 'import {type MyType} from "./tsx-type-exports.tsx"', + }), + test({ + code: 'import type {MyType, Bar} from "./tsx-type-exports.tsx"', + parser, + settings, + options: ['inline'], + errors: [{ + message: 'BOOM', + }], + output: 'import {type MyType, type Bar} from "./tsx-type-exports.tsx"', + }), + test({ + code: 'import type {MyType, Bar} from "./tsx-type-exports.tsx";import {MyEnum} from "./tsx-type-exports.tsx"', + parser, + settings, + options: ['inline'], + errors: [{ + message: 'BOOM', + }], + output: 'import {MyEnum, type MyType, type Bar} from "./tsx-type-exports.tsx"', + }), + test({ + code: 'import type {MyType as Persona, Bar as Foo} from "./tsx-type-exports.tsx";import {MyEnum} from "./tsx-type-exports.tsx"', + parser, + settings, + options: ['inline'], + errors: [{ + message: 'BOOM', + }], + output: 'import {MyEnum, type MyType as Persona, type Bar as Foo} from "./tsx-type-exports.tsx"', + }), + test({ + code: 'import type {MyType, Bar} from "./tsx-type-exports.tsx";import {default as B} from "./tsx-type-exports.tsx"', + parser, + settings, + options: ['inline'], + errors: [{ + message: 'BOOM', + }], + output: 'import {default as B, type MyType, type Bar} from "./tsx-type-exports.tsx"', + }), + test({ + code: 'import type {MyType, Bar} from "./tsx-type-exports.tsx";import defaultExport from "./tsx-type-exports.tsx"', + parser, + settings, + options: ['inline'], + errors: [{ + message: 'BOOM', + }], + output: 'import defaultExport, { type MyType, type Bar } from "./tsx-type-exports.tsx"', + }), + test({ + code: 'import type {MyType as Persona, Bar as Foo} from "./tsx-type-exports.tsx";import defaultExport from "./tsx-type-exports.tsx"', + parser, + settings, + options: ['inline'], + errors: [{ + message: 'BOOM', + }], + output: 'import defaultExport, { type MyType as Persona, type Bar as Foo } from "./tsx-type-exports.tsx"', + }), + test({ + code: 'import type {MyType, Bar} from "./tsx-type-exports.tsx";import * as b from "./tsx-type-exports.tsx"', + parser, + settings, + options: ['inline'], + errors: [{ + message: 'BOOM', + }], + output: 'import {type MyType, type Bar} from "./tsx-type-exports.tsx";import * as b from "./tsx-type-exports.tsx"', + }), + test({ + code: 'import type {MyType, Bar} from "./tsx-type-exports.tsx";import type A from "./tsx-type-exports.tsx"', + parser, + settings, + options: ['inline'], + errors: [{ + message: 'BOOM', + }], + output: 'import {type MyType, type Bar} from "./tsx-type-exports.tsx";import type A from "./tsx-type-exports.tsx"', + }), + ], + }, ); }); }); From a8d9ba191d1d270d5434ac74c49a8b47c3b1848e Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Sat, 7 Jan 2023 23:07:33 -0500 Subject: [PATCH 29/37] add error messages --- src/rules/typescript-flow.js | 12 +++--- tests/src/rules/typescript-flow.js | 69 ++++++++++++++++-------------- 2 files changed, 43 insertions(+), 38 deletions(-) diff --git a/src/rules/typescript-flow.js b/src/rules/typescript-flow.js index 6e1db6827..12087b7cd 100644 --- a/src/rules/typescript-flow.js +++ b/src/rules/typescript-flow.js @@ -5,7 +5,6 @@ import docsUrl from '../docsUrl'; // import debug from 'debug'; // const log = debug('eslint-plugin-import/typescript-flow'); -// comes from tests/src/utils file, TODO?: import directly from there, issue: too many layers of folders away import typescriptPkg from 'typescript/package.json'; import semver from 'semver'; @@ -13,6 +12,9 @@ function tsVersionSatisfies(specifier) { return semver.satisfies(typescriptPkg.version, specifier); } +const SEPARATE_ERROR_MESSAGE = 'Type imports should be separately imported.'; +const INLINE_ERROR_MESSAGE = 'Type imports should be imported inline with type modifier.'; + function processBodyStatement(importMap, node){ if (node.type !== 'ImportDeclaration' || node.importKind === 'type') return; if (node.specifiers.length === 0) return; @@ -139,7 +141,7 @@ module.exports = { if (typeImports.length === allImportsSize) { context.report({ node, - message: 'BOOM', + message: SEPARATE_ERROR_MESSAGE, fix(fixer) { const sourceCode = context.getSourceCode(); const tokens = sourceCode.getTokens(node); @@ -159,7 +161,7 @@ module.exports = { else { context.report({ node, - message: 'BOOM', + message: SEPARATE_ERROR_MESSAGE, fix(fixer) { const sourceCode = context.getSourceCode(); const tokens = sourceCode.getTokens(node); @@ -218,7 +220,7 @@ module.exports = { // try to insert after the last specifier context.report({ node, - message: 'BOOM', + message: INLINE_ERROR_MESSAGE, fix(fixer) { if (lastSpecifier.type === 'ImportDefaultSpecifier' && declaration.specifiers.length === 1) { // import defaultExport from 'x' @@ -245,7 +247,7 @@ module.exports = { // TODO: check this statement: valueNodeImports => possibly rename it ? context.report({ node, - message: 'BOOM', + message: INLINE_ERROR_MESSAGE, fix(fixer) { const sourceCode = context.getSourceCode(); const tokens = sourceCode.getTokens(node); diff --git a/tests/src/rules/typescript-flow.js b/tests/src/rules/typescript-flow.js index c2bec78a3..3f0959ffb 100644 --- a/tests/src/rules/typescript-flow.js +++ b/tests/src/rules/typescript-flow.js @@ -5,6 +5,10 @@ import { RuleTester } from 'eslint'; const ruleTester = new RuleTester(); const rule = require('rules/typescript-flow'); + +const SEPARATE_ERROR_MESSAGE = 'Type imports should be separately imported.'; +const INLINE_ERROR_MESSAGE = 'Type imports should be imported inline with type modifier.'; + context('TypeScript', function () { // Do we need to check for different TS parsers? TODO Question getTSParsers().forEach((parser) => { @@ -154,7 +158,7 @@ context('TypeScript', function () { settings, options: ['separate'], errors: [{ - message: 'BOOM', + message: SEPARATE_ERROR_MESSAGE, }], output: 'import {Bar} from "./typescript.ts"\nimport type { MyType } from "./typescript.ts"', }), @@ -164,7 +168,7 @@ context('TypeScript', function () { settings, options: ['separate'], errors: [{ - message: 'BOOM', + message: SEPARATE_ERROR_MESSAGE, }], output: 'import {Bar} from "./typescript.ts"\nimport type { MyType as Persona } from "./typescript.ts"', }), @@ -174,7 +178,7 @@ context('TypeScript', function () { settings, options: ['separate'], errors: [{ - message: 'BOOM', + message: SEPARATE_ERROR_MESSAGE, }], output: 'import {Bar} from "./typescript.ts"\nimport type { MyType, Foo } from "./typescript.ts"', }), @@ -184,7 +188,7 @@ context('TypeScript', function () { settings, options: ['separate'], errors: [{ - message: 'BOOM', + message: SEPARATE_ERROR_MESSAGE, }], output: 'import {Bar} from "./typescript.ts"\nimport type { MyType as Persona, Foo as Baz } from "./typescript.ts"', }), @@ -194,7 +198,7 @@ context('TypeScript', function () { settings, options: ['separate'], errors: [{ - message: 'BOOM', + message: SEPARATE_ERROR_MESSAGE, }], output: 'import {Bar as Namespace} from "./typescript.ts"\nimport type { MyType } from "./typescript.ts"', }), @@ -204,7 +208,7 @@ context('TypeScript', function () { settings, options: ['separate'], errors: [{ - message: 'BOOM', + message: SEPARATE_ERROR_MESSAGE, }], output: 'import type { MyType, Foo} from "./typescript.ts"', }), @@ -214,7 +218,7 @@ context('TypeScript', function () { settings, options: ['separate'], errors: [{ - message: 'BOOM', + message: SEPARATE_ERROR_MESSAGE, }], output: 'import type { MyType as Bar, Foo} from "./typescript.ts"', }), @@ -225,7 +229,7 @@ context('TypeScript', function () { settings, options: ['inline'], errors: [{ - message: 'BOOM', + message: INLINE_ERROR_MESSAGE, }], output: 'import {type MyType} from "./typescript.ts"', }), @@ -235,7 +239,7 @@ context('TypeScript', function () { settings, options: ['inline'], errors: [{ - message: 'BOOM', + message: INLINE_ERROR_MESSAGE, }], output: 'import {type MyType, type Bar} from "./typescript.ts"', }), @@ -245,7 +249,7 @@ context('TypeScript', function () { settings, options: ['inline'], errors: [{ - message: 'BOOM', + message: INLINE_ERROR_MESSAGE, }], output: 'import {MyEnum, type MyType, type Bar} from "./typescript.ts"', }), @@ -255,7 +259,7 @@ context('TypeScript', function () { settings, options: ['inline'], errors: [{ - message: 'BOOM', + message: INLINE_ERROR_MESSAGE, }], output: 'import {MyEnum, type MyType as Persona, type Bar as Foo} from "./typescript.ts"', }), @@ -265,7 +269,7 @@ context('TypeScript', function () { settings, options: ['inline'], errors: [{ - message: 'BOOM', + message: INLINE_ERROR_MESSAGE, }], output: 'import {default as B, type MyType, type Bar} from "./typescript.ts"', }), @@ -275,7 +279,7 @@ context('TypeScript', function () { settings, options: ['inline'], errors: [{ - message: 'BOOM', + message: INLINE_ERROR_MESSAGE, }], output: 'import defaultExport, { type MyType, type Bar } from "./typescript.ts"', }), @@ -285,7 +289,7 @@ context('TypeScript', function () { settings, options: ['inline'], errors: [{ - message: 'BOOM', + message: INLINE_ERROR_MESSAGE, }], output: 'import defaultExport, { type MyType as Persona, type Bar as Foo } from "./typescript.ts"', }), @@ -295,7 +299,7 @@ context('TypeScript', function () { settings, options: ['inline'], errors: [{ - message: 'BOOM', + message: INLINE_ERROR_MESSAGE, }], output: 'import {type MyType, type Bar} from "./typescript.ts";import * as b from "./typescript.ts"', }), @@ -305,7 +309,7 @@ context('TypeScript', function () { settings, options: ['inline'], errors: [{ - message: 'BOOM', + message: INLINE_ERROR_MESSAGE, }], output: 'import {type MyType, type Bar} from "./typescript.ts";import type A from "./typescript.ts"', }), @@ -316,7 +320,7 @@ context('TypeScript', function () { settings, options: ['separate'], errors: [{ - message: 'BOOM', + message: SEPARATE_ERROR_MESSAGE, }], output: 'import {Bar} from "./tsx-type-exports.tsx"\nimport type { MyType } from "./tsx-type-exports.tsx"', }), @@ -326,7 +330,7 @@ context('TypeScript', function () { settings, options: ['separate'], errors: [{ - message: 'BOOM', + message: SEPARATE_ERROR_MESSAGE, }], output: 'import {Bar} from "./tsx-type-exports.tsx"\nimport type { MyType as Persona } from "./tsx-type-exports.tsx"', }), @@ -336,7 +340,7 @@ context('TypeScript', function () { settings, options: ['separate'], errors: [{ - message: 'BOOM', + message: SEPARATE_ERROR_MESSAGE, }], output: 'import {Bar} from "./tsx-type-exports.tsx"\nimport type { MyType, Foo } from "./tsx-type-exports.tsx"', }), @@ -346,7 +350,7 @@ context('TypeScript', function () { settings, options: ['separate'], errors: [{ - message: 'BOOM', + message: SEPARATE_ERROR_MESSAGE, }], output: 'import {Bar} from "./tsx-type-exports.tsx"\nimport type { MyType as Persona, Foo as Baz } from "./tsx-type-exports.tsx"', }), @@ -356,7 +360,7 @@ context('TypeScript', function () { settings, options: ['separate'], errors: [{ - message: 'BOOM', + message: SEPARATE_ERROR_MESSAGE, }], output: 'import {Bar as Namespace} from "./tsx-type-exports.tsx"\nimport type { MyType } from "./tsx-type-exports.tsx"', }), @@ -366,7 +370,7 @@ context('TypeScript', function () { settings, options: ['separate'], errors: [{ - message: 'BOOM', + message: SEPARATE_ERROR_MESSAGE, }], output: 'import type { MyType, Foo} from "./tsx-type-exports.tsx"', }), @@ -376,18 +380,17 @@ context('TypeScript', function () { settings, options: ['separate'], errors: [{ - message: 'BOOM', + message: SEPARATE_ERROR_MESSAGE, }], output: 'import type { MyType as Bar, Foo} from "./tsx-type-exports.tsx"', }), - // the space is left over when 'type' is removed. Question. TODO test({ code: 'import type {MyType} from "./tsx-type-exports.tsx"', parser, settings, options: ['inline'], errors: [{ - message: 'BOOM', + message: INLINE_ERROR_MESSAGE, }], output: 'import {type MyType} from "./tsx-type-exports.tsx"', }), @@ -397,7 +400,7 @@ context('TypeScript', function () { settings, options: ['inline'], errors: [{ - message: 'BOOM', + message: INLINE_ERROR_MESSAGE, }], output: 'import {type MyType, type Bar} from "./tsx-type-exports.tsx"', }), @@ -407,7 +410,7 @@ context('TypeScript', function () { settings, options: ['inline'], errors: [{ - message: 'BOOM', + message: INLINE_ERROR_MESSAGE, }], output: 'import {MyEnum, type MyType, type Bar} from "./tsx-type-exports.tsx"', }), @@ -417,7 +420,7 @@ context('TypeScript', function () { settings, options: ['inline'], errors: [{ - message: 'BOOM', + message: INLINE_ERROR_MESSAGE, }], output: 'import {MyEnum, type MyType as Persona, type Bar as Foo} from "./tsx-type-exports.tsx"', }), @@ -427,7 +430,7 @@ context('TypeScript', function () { settings, options: ['inline'], errors: [{ - message: 'BOOM', + message: INLINE_ERROR_MESSAGE, }], output: 'import {default as B, type MyType, type Bar} from "./tsx-type-exports.tsx"', }), @@ -437,7 +440,7 @@ context('TypeScript', function () { settings, options: ['inline'], errors: [{ - message: 'BOOM', + message: INLINE_ERROR_MESSAGE, }], output: 'import defaultExport, { type MyType, type Bar } from "./tsx-type-exports.tsx"', }), @@ -447,7 +450,7 @@ context('TypeScript', function () { settings, options: ['inline'], errors: [{ - message: 'BOOM', + message: INLINE_ERROR_MESSAGE, }], output: 'import defaultExport, { type MyType as Persona, type Bar as Foo } from "./tsx-type-exports.tsx"', }), @@ -457,7 +460,7 @@ context('TypeScript', function () { settings, options: ['inline'], errors: [{ - message: 'BOOM', + message: INLINE_ERROR_MESSAGE, }], output: 'import {type MyType, type Bar} from "./tsx-type-exports.tsx";import * as b from "./tsx-type-exports.tsx"', }), @@ -467,7 +470,7 @@ context('TypeScript', function () { settings, options: ['inline'], errors: [{ - message: 'BOOM', + message: INLINE_ERROR_MESSAGE, }], output: 'import {type MyType, type Bar} from "./tsx-type-exports.tsx";import type A from "./tsx-type-exports.tsx"', }), From 051efd2b9a8d7b3b98e6eae0eb205012bb015f94 Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Sat, 7 Jan 2023 23:10:37 -0500 Subject: [PATCH 30/37] delete unnecessary comments --- tests/src/rules/typescript-flow.js | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/tests/src/rules/typescript-flow.js b/tests/src/rules/typescript-flow.js index 3f0959ffb..7893dddcf 100644 --- a/tests/src/rules/typescript-flow.js +++ b/tests/src/rules/typescript-flow.js @@ -12,13 +12,6 @@ const INLINE_ERROR_MESSAGE = 'Type imports should be imported inline with type m context('TypeScript', function () { // Do we need to check for different TS parsers? TODO Question getTSParsers().forEach((parser) => { - // const parserConfig = { - // parser, - // settings: { - // 'import/parsers': { [parser]: ['.ts'] }, - // 'import/resolver': { 'eslint-import-resolver-typescript': true }, - // }, - // }; const settings = { 'import/parsers': { [parser]: ['.ts'] }, @@ -51,7 +44,6 @@ context('TypeScript', function () { settings, options: ['separate'], }), - // default imports are ignored for strict option separate. Question. TODO test({ code: `import Bar from "./typescript.ts"`, parser, @@ -113,7 +105,6 @@ context('TypeScript', function () { settings, options: ['separate'], }), - // default imports are ignored for strict option separate. Question. TODO test({ code: `import Bar from "./tsx-type-exports.tsx"`, parser, @@ -222,7 +213,6 @@ context('TypeScript', function () { }], output: 'import type { MyType as Bar, Foo} from "./typescript.ts"', }), - // the space is left over when 'type' is removed. Question. TODO test({ code: 'import type {MyType} from "./typescript.ts"', parser, From b42c4d5e2c0ec7ce853454be25852b105cb29ea1 Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Sun, 8 Jan 2023 00:15:48 -0500 Subject: [PATCH 31/37] add 2 default import type test cases --- tests/src/rules/typescript-flow.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/src/rules/typescript-flow.js b/tests/src/rules/typescript-flow.js index 7893dddcf..266c58da5 100644 --- a/tests/src/rules/typescript-flow.js +++ b/tests/src/rules/typescript-flow.js @@ -56,6 +56,18 @@ context('TypeScript', function () { settings, options: ['separate'], }), + test({ + code: `import type Bar from "./typescript.ts"`, + parser, + settings, + options: ['inline'], + }), + test({ + code: `import Bar from "./typescript.ts"`, + parser, + settings, + options: ['inline'], + }), test({ code: `import { type MyType } from "./typescript.ts"`, parser, From b4cc95a4ea94df6f3006702db158420b647c9289 Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Sun, 8 Jan 2023 00:26:29 -0500 Subject: [PATCH 32/37] add docs --- docs/rules/typescript-flow.md | 164 +++++++++++++++++++++++++++++++--- 1 file changed, 152 insertions(+), 12 deletions(-) diff --git a/docs/rules/typescript-flow.md b/docs/rules/typescript-flow.md index 3b5eb8b26..bfcc917bd 100644 --- a/docs/rules/typescript-flow.md +++ b/docs/rules/typescript-flow.md @@ -1,24 +1,164 @@ + This rules helps to define type import patterns for the typescript. ## Rule details +This rule allows you to specify how type imports should be written in your TypeScript code. There are two options available: inline type imports and separate type import statements. Rule can also autofix your type imports to the preferred style. + +The rule has 2 modes: strict and preference. + +Strict option enforces either inline type imports with type modifiers, or separate type imports. + +If you select the preference mode, the rule will check for the availability of your preferred import style and use that if possible. If not, it will fall back to your second preferred option. + +For example, if you specify the preference as ["inline", "separate"], the rule will try to use inline type imports first. If that is not supported by your TypeScript version, it will use separate type import statements instead. + + +#### Rule schema + +```javascript +// strict mode +"import/typescript-flow": [ 0 | 1 | 2, "inline" | "separate" ] + +// preference mode +"import/typescript-flow": [ 0 | 1 | 2, [ "inline", "separate" ] | [ "separate", "inline" ] ] + +``` + +### Strict mode + +#### separate + +**Definition**: Separate type import statements are written using the import type syntax. They were introduced in TypeScript 3.8 + +The following patterns are *not* considered warnings: + +```javascript +// good1.js + +// Separate import type statement +import type { MyType } from "./typescript.ts" +``` + +```javascript +// good2.js + +// Separate import type statement +import type { MyType as Persona, Bar as Foo } from "./typescript.ts" +``` +```javascript +// good3.js + +// Default type imports are ignored +import type DefaultImport from "./typescript.ts" +``` + +```javascript +// good4.js + +// Default imports are ignored +import DefaultImport from "./typescript.ts" +``` + +The following patterns are considered warnings: + +```javascript +// bad1.js + +// type imports must be imported in separate import type statement +import {type MyType,Bar} from "./typescript.ts" + +// gets fixed to the following: +import {Bar} from "./typescript.ts" +import type { MyType } from "./typescript.ts" +``` -##### rule schema: +```javascript +// bad2.js + +// type imports must be imported in separate import type statement +import {type MyType,type Foo} from "./typescript.ts" + +// gets fixed to the following: +import type { MyType, Foo} from "./typescript.ts" +``` ```javascript -"import/prefer-default-export": [ - ( "off" | "warn" | "error" ), - { "prefer": "none" | "separate" | "modifier" } // default is ??? -] +// bad3.js + +// type imports must be imported in separate import type statement +import {type MyType as Persona,Bar,type Foo as Baz} from "./typescript.ts" + +// gets fixed to the following: +import {Bar} from "./typescript.ts" +import type { MyType as Persona, Foo as Baz } from "./typescript.ts" ``` -### Config Options -There are three avaiable options: `none`, `separate`, `modifier`. +#### inline + +**Definition**: Inline type imports are written as type modifiers within the curly braces of a regular import statement. They were introduced in TypeScript 4.5. + +Patterns that do not raise the warning: ```javascript -// import-stars.js +// good1.js + +// no separate import type statement. inline type import exists +import { type MyType } from "./typescript.ts" +``` + +```javascript +// good2.js + +// no separate import type statement. inline type import exists +import { type MyType, Bar, type Foo as Baz } from "./typescript.ts" +``` + +Patterns are considered warnings and fixed: + +```javascript +// bad1.js + +// type imports must be imported inline +import type {MyType} from "./typescript.ts" + +// gets fixed to the following: +import {type MyType} from "./typescript.ts" +``` + +```javascript +// bad1.js + +// type imports must be imported inline +import type {MyType, Bar} from "./typescript.ts" + +// gets fixed to the following: +import {type MyType, type Bar} from "./typescript.ts" +``` + +```javascript +// bad3.js + +// type imports must be imported inline +import type {MyType, Bar} from "./typescript.ts" +import {Value} from "./typescript.ts" + +// Rule can check if there are any other imports from the same source. If yes, then adds inline type imports there. gets fixed to the following: +import {Value, type MyType, type Bar} from "./typescript.ts" +``` + +### Preference mode + +```javascript +// preference mode where inline comes first +"import/typescript-flow": [ 0 | 1 | 2, [ "inline", "separate" ] ] +``` +Rule checks if `inline` type modifiers are supported by TypeScript version (must be >=4.5). If supported, then rule operates with `inline` option, if not, then it falls back to `separate` option. + +```javascript +// preference mode where strict comes first +"import/typescript-flow": [ 0 | 1 | 2, [ "separate", "inline" ] ] +``` -// The remote module is not inspected. -import * from './other-module' -import * as b from './other-module' +If `separate` comes first in preferred order, then rule will act as if user chose `separate`. -``` \ No newline at end of file +Since `separate` type imports are supported in TypeScript version 3.8 and above, and `inline` type imports are supported in version 4.5 and above, the `separate` option is guaranteed to be supported if the `inline` option is supported. Therefore, there is no need to consider the case where the `separate` option is not supported and the rule falls back to the `inline` option. \ No newline at end of file From 97d20744646442b87a2cf9b8b5d68da60b847642 Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Sun, 8 Jan 2023 02:12:45 -0500 Subject: [PATCH 33/37] remove custom script --- package.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/package.json b/package.json index df572a495..fb33e7c48 100644 --- a/package.json +++ b/package.json @@ -27,7 +27,6 @@ "posttest": "eslint . && npm run update:eslint-docs -- --check", "mocha": "cross-env BABEL_ENV=test nyc mocha", "tests-only": "npm run mocha tests/src", - "debug": "DEBUG=eslint-plugin-import/typescript-flow npm run mocha tests/src/rules/typescript-flow", "test": "npm run tests-only", "test-compiled": "npm run prepublish && BABEL_ENV=testCompiled mocha --compilers js:babel-register tests/src", "test-all": "node --require babel-register ./scripts/testAll", @@ -117,4 +116,4 @@ "resolve": "^1.22.1", "tsconfig-paths": "^3.14.1" } -} +} \ No newline at end of file From fc0c1636eca556162ff128706637114a2da85dc0 Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Sun, 8 Jan 2023 02:15:01 -0500 Subject: [PATCH 34/37] restore ExportMap to the version from main branch --- src/ExportMap.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/ExportMap.js b/src/ExportMap.js index c06b8f559..d95fdb7a7 100644 --- a/src/ExportMap.js +++ b/src/ExportMap.js @@ -623,9 +623,6 @@ ExportMap.parse = function (path, content, context) { // capture declaration if (n.declaration != null) { switch (n.declaration.type) { - case 'TSTypeAliasDeclaration': - m.namespace.set(n.declaration.id.name, 'type'); - break; case 'FunctionDeclaration': case 'ClassDeclaration': case 'TypeAlias': // flowtype with babel-eslint parser @@ -633,6 +630,7 @@ ExportMap.parse = function (path, content, context) { case 'DeclareFunction': case 'TSDeclareFunction': case 'TSEnumDeclaration': + case 'TSTypeAliasDeclaration': case 'TSInterfaceDeclaration': case 'TSAbstractClassDeclaration': case 'TSModuleDeclaration': From c08808156c800f9c5ab268af2e68822ff8a7b0d3 Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Wed, 1 Feb 2023 18:26:39 -0500 Subject: [PATCH 35/37] refactor specifier cache loop to function for config = separate --- src/rules/typescript-flow.js | 150 +++++++++++++++++++++++------------ 1 file changed, 100 insertions(+), 50 deletions(-) diff --git a/src/rules/typescript-flow.js b/src/rules/typescript-flow.js index 12087b7cd..90d3c2093 100644 --- a/src/rules/typescript-flow.js +++ b/src/rules/typescript-flow.js @@ -2,8 +2,8 @@ import docsUrl from '../docsUrl'; -// import debug from 'debug'; -// const log = debug('eslint-plugin-import/typescript-flow'); +import debug from 'debug'; +const log = debug('eslint-plugin-import/typescript-flow'); import typescriptPkg from 'typescript/package.json'; import semver from 'semver'; @@ -91,12 +91,12 @@ module.exports = { } const fixerArray = []; - const typeImports = []; - const valueImports =[]; - const valueNodeImports =[]; + const typeImportSpecifiers = []; + const valueImportSpecifiers =[]; + const valueImportNodes =[]; const importSpecifierRanges = []; let allImportsSize = 0; - const nonTypeImportDeclarations = new Map(); + const importMap = new Map(); return { 'ImportDeclaration': function (node){ @@ -108,37 +108,48 @@ module.exports = { if (config === 'separate' && node.importKind !== 'type') { // identify importSpecifiers that have inline type imports as well as value imports - node.specifiers.forEach((specifier) => { - if (specifier.type === 'ImportNamespaceSpecifier' || specifier.type === 'ImportDefaultSpecifier') { - return; - } - // Question: do we want our rule to deal with default imports? It does not make sense that rule needs to since we do not know the type of default export. - allImportsSize = allImportsSize + 1; - // catch all inline imports and add them to the set - if (specifier.importKind === 'type') { - if (specifier.local.name !== specifier.imported.name) { - typeImports.push(`${specifier.imported.name} as ${specifier.local.name}`); - } else { - typeImports.push(specifier.local.name); - } - importSpecifierRanges.push(specifier.range); - } else { - valueNodeImports.push(specifier); - if (specifier.local.name !== specifier.imported.name) { - valueImports.push(`${specifier.imported.name} as ${specifier.local.name}`); - } else { - valueImports.push(specifier.local.name); - } - importSpecifierRanges.push(specifier.range); - } - }); + + const specifierCache = { + typeImportSpecifiers, valueImportSpecifiers, valueImportNodes, importSpecifierRanges, allImportsSize, + }; + + if (!setImportSpecifierCache(node.specifiers, specifierCache)) { + return; + } + allImportsSize = typeImportSpecifiers.length + valueImportSpecifiers.length; + // for (let i = 0; i < node.specifiers.length; i++) { + // const specifier = node.specifiers[i]; + // if (specifier.type === 'ImportNamespaceSpecifier' || specifier.type === 'ImportDefaultSpecifier') { + // return; + // } + // // Question: do we want our rule to deal with default imports? It does not make sense that rule needs to since we do not know the type of default export. + // allImportsSize = allImportsSize + 1; + // // catch all inline imports and add them to the set + // if (specifier.importKind === 'type') { + // if (specifier.local.name !== specifier.imported.name) { + // typeImportSpecifiers.push(`${specifier.imported.name} as ${specifier.local.name}`); + // } else { + // typeImportSpecifiers.push(specifier.local.name); + // } + // importSpecifierRanges.push(specifier.range); + // } else { + // valueImportNodes.push(specifier); + // if (specifier.local.name !== specifier.imported.name) { + // valueImportSpecifiers.push(`${specifier.imported.name} as ${specifier.local.name}`); + // } else { + // valueImportSpecifiers.push(specifier.local.name); + // } + // importSpecifierRanges.push(specifier.range); + // } + // } // no inline type imports found - if (typeImports.length === 0) { + if (typeImportSpecifiers.length === 0) { return; } + log('all imports size - ', allImportsSize); // all inline imports are type imports => need to change it to separate import statement // import {type X, type Y} form 'x' => import type { X, Y} from 'x' - if (typeImports.length === allImportsSize) { + if (typeImportSpecifiers.length === allImportsSize) { context.report({ node, message: SEPARATE_ERROR_MESSAGE, @@ -183,9 +194,9 @@ module.exports = { } }); // add inline value imports back - fixerArray.push(fixer.insertTextAfter(namedImportStart, valueImports.join(', '))); + fixerArray.push(fixer.insertTextAfter(namedImportStart, valueImportSpecifiers.join(', '))); // add new line with separate type import - fixerArray.push(fixer.insertTextAfter(node, `\nimport type { ${typeImports.join(', ')} } from ${importPath}`)); + fixerArray.push(fixer.insertTextAfter(node, `\nimport type { ${typeImportSpecifiers.join(', ')} } from ${importPath}`)); return fixerArray; }, }); @@ -197,42 +208,47 @@ module.exports = { if (node.specifiers.length === 1 && node.specifiers[0].type === 'ImportDefaultSpecifier') { return; } - node.specifiers.forEach((specifier) => { + + // IMPROVE: take out to separate function + for (let i = 0; i < node.specifiers.length; i++) { + const specifier = node.specifiers[i]; if (specifier.local.name !== specifier.imported.name) { - typeImports.push(`type ${specifier.imported.name} as ${specifier.local.name}`); + typeImportSpecifiers.push(`type ${specifier.imported.name} as ${specifier.local.name}`); } else { - typeImports.push(`type ${specifier.local.name}`); + typeImportSpecifiers.push(`type ${specifier.local.name}`); } - valueNodeImports.push(specifier); - }); - - // function process body statement to find if there are any non-type imports from the same location - node.parent.body.forEach(declaration => processBodyStatement(nonTypeImportDeclarations, declaration)); + valueImportNodes.push(specifier); + } - // file has non type import from the same source - const declaration = nonTypeImportDeclarations.get(node.source.value); + // IMPROVE: separate function to handle it? find if there are any non-type imports from the same location + const body = node.parent.body; + for (let j = 0; j < body.length; j++) { + const element = body[j]; + processBodyStatement(importMap, element); + } + const sameSourceValueImport = importMap.get(node.source.value); - if (declaration && !declaration.hasNamespaceImport) { + if (sameSourceValueImport && !sameSourceValueImport.hasNamespaceImport) { // get the last specifier - const lastSpecifier = declaration.specifiers[declaration.specifiers.length - 1]; + const lastSpecifier = sameSourceValueImport.specifiers[sameSourceValueImport.specifiers.length - 1]; // try to insert after the last specifier context.report({ node, message: INLINE_ERROR_MESSAGE, fix(fixer) { - if (lastSpecifier.type === 'ImportDefaultSpecifier' && declaration.specifiers.length === 1) { + if (lastSpecifier.type === 'ImportDefaultSpecifier' && sameSourceValueImport.specifiers.length === 1) { // import defaultExport from 'x' // import type { X, Y } from 'x' // => import defaultExport, { type X, type Y } from 'x' - const inlineTypeImportsToInsert = ', { ' + typeImports.join(', ') + ' }'; + const inlineTypeImportsToInsert = ', { ' + typeImportSpecifiers.join(', ') + ' }'; fixerArray.push(fixer.insertTextAfter(lastSpecifier, inlineTypeImportsToInsert)); } else { // import { namedImport } from 'x' // import type { X, Y } from 'x' // => import { namedImport, type X, type Y } from 'x' - const inlineTypeImportsToInsert = ', ' + typeImports.join(', '); + const inlineTypeImportsToInsert = ', ' + typeImportSpecifiers.join(', '); fixerArray.push(fixer.insertTextAfter(lastSpecifier, inlineTypeImportsToInsert)); } @@ -252,7 +268,7 @@ module.exports = { const sourceCode = context.getSourceCode(); const tokens = sourceCode.getTokens(node); fixerArray.push(fixer.remove(tokens[1])); - valueNodeImports.forEach(element => { + valueImportNodes.forEach(element => { fixerArray.push(fixer.insertTextBefore(element, 'type ')); }); return fixerArray; @@ -264,3 +280,37 @@ module.exports = { }; }, }; + +function setImportSpecifierCache(specifiers, specifierCache) { + const typeImportSpecifiers = specifierCache.typeImportSpecifiers; + const valueImportSpecifiers = specifierCache.valueImportSpecifiers; + const valueImportNodes = specifierCache.valueImportNodes; + const importSpecifierRanges = specifierCache.importSpecifierRanges; + + for (let i = 0; i < specifiers.length; i++) { + const specifier = specifiers[i]; + if (specifier.type === 'ImportNamespaceSpecifier' || specifier.type === 'ImportDefaultSpecifier') { + return false; + } + // Question: do we want our rule to deal with default imports? It does not make sense that rule needs to since we do not know the type of default export. + specifierCache.allImportsSize = specifierCache.allImportsSize + 1; + // catch all inline imports and add them to the set + if (specifier.importKind === 'type') { + if (specifier.local.name !== specifier.imported.name) { + typeImportSpecifiers.push(`${specifier.imported.name} as ${specifier.local.name}`); + } else { + typeImportSpecifiers.push(specifier.local.name); + } + importSpecifierRanges.push(specifier.range); + } else { + valueImportNodes.push(specifier); + if (specifier.local.name !== specifier.imported.name) { + valueImportSpecifiers.push(`${specifier.imported.name} as ${specifier.local.name}`); + } else { + valueImportSpecifiers.push(specifier.local.name); + } + importSpecifierRanges.push(specifier.range); + } + } + return true; +} From accfa3a63e1b2ae11b74bf5145b510e771723b29 Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Wed, 1 Feb 2023 22:11:15 -0500 Subject: [PATCH 36/37] refactor by adding function for every piece of logic --- src/rules/typescript-flow.js | 401 ++++++++++++++++++----------------- 1 file changed, 209 insertions(+), 192 deletions(-) diff --git a/src/rules/typescript-flow.js b/src/rules/typescript-flow.js index 90d3c2093..e06c653c5 100644 --- a/src/rules/typescript-flow.js +++ b/src/rules/typescript-flow.js @@ -2,8 +2,8 @@ import docsUrl from '../docsUrl'; -import debug from 'debug'; -const log = debug('eslint-plugin-import/typescript-flow'); +// import debug from 'debug'; +// const log = debug('eslint-plugin-import/typescript-flow'); import typescriptPkg from 'typescript/package.json'; import semver from 'semver'; @@ -15,36 +15,6 @@ function tsVersionSatisfies(specifier) { const SEPARATE_ERROR_MESSAGE = 'Type imports should be separately imported.'; const INLINE_ERROR_MESSAGE = 'Type imports should be imported inline with type modifier.'; -function processBodyStatement(importMap, node){ - if (node.type !== 'ImportDeclaration' || node.importKind === 'type') return; - if (node.specifiers.length === 0) return; - - const specifiers = []; - let hasDefaultImport= false; - let hasInlineDefaultImport = false; - let hasNamespaceImport = false; - - node.specifiers.forEach((specifier)=>{ - if (specifier.type === 'ImportNamespaceSpecifier') { - hasNamespaceImport = true; - } - // ignore the case for now. TODO: talk with Jordan and implement the handling of the rule here - if (specifier.type === 'ImportDefaultSpecifier') { - hasDefaultImport = true; - specifiers.push(specifier); - } - if (specifier.type === 'ImportSpecifier') { - // check if its {default as B} - if (specifier.imported.name === 'default') { - hasInlineDefaultImport = true; - } - specifiers.push(specifier); - } - }); - // cache all imports - importMap.set(node.source.value, { specifiers, hasDefaultImport, hasInlineDefaultImport, hasNamespaceImport }); -} - // TODO: revert changes in ExportMap file. module.exports = { meta: { @@ -80,200 +50,61 @@ module.exports = { // check if it can be supported. If not, fall back on separate. // If arr[0] === separate => just assume it is separate - const supportInlineTypeImport = tsVersionSatisfies('>=4.5'); // type modifiers (inline type import) were introduced in TS 4.5. + const isInlineTypeImportSupported = tsVersionSatisfies('>=4.5'); // type modifiers (inline type import) were introduced in TS 4.5. // get Rule options. - let config = context.options[0]; - if (config.length === 2 && config[0] === 'inline' && !supportInlineTypeImport) { - config = 'separate'; - } - if (config[0] === 'separate') { - config = 'separate'; - } + const config = getRuleConfig(context.options[0], isInlineTypeImportSupported); - const fixerArray = []; const typeImportSpecifiers = []; const valueImportSpecifiers =[]; const valueImportNodes =[]; const importSpecifierRanges = []; - let allImportsSize = 0; - const importMap = new Map(); + + + const specifierCache = { + typeImportSpecifiers, valueImportSpecifiers, valueImportNodes, importSpecifierRanges, + }; + return { 'ImportDeclaration': function (node){ - if (config === 'inline' && !supportInlineTypeImport) { - // raise error + if (config === 'inline' && !isInlineTypeImportSupported) { context.report(node, 'Type modifiers are not supported by your version of TS.'); } + if (config === 'separate' && node.importKind !== 'type') { // identify importSpecifiers that have inline type imports as well as value imports - - const specifierCache = { - typeImportSpecifiers, valueImportSpecifiers, valueImportNodes, importSpecifierRanges, allImportsSize, - }; - - if (!setImportSpecifierCache(node.specifiers, specifierCache)) { + if (!setImportSpecifierCacheForSeparate(node.specifiers, specifierCache)) { return; } - allImportsSize = typeImportSpecifiers.length + valueImportSpecifiers.length; - // for (let i = 0; i < node.specifiers.length; i++) { - // const specifier = node.specifiers[i]; - // if (specifier.type === 'ImportNamespaceSpecifier' || specifier.type === 'ImportDefaultSpecifier') { - // return; - // } - // // Question: do we want our rule to deal with default imports? It does not make sense that rule needs to since we do not know the type of default export. - // allImportsSize = allImportsSize + 1; - // // catch all inline imports and add them to the set - // if (specifier.importKind === 'type') { - // if (specifier.local.name !== specifier.imported.name) { - // typeImportSpecifiers.push(`${specifier.imported.name} as ${specifier.local.name}`); - // } else { - // typeImportSpecifiers.push(specifier.local.name); - // } - // importSpecifierRanges.push(specifier.range); - // } else { - // valueImportNodes.push(specifier); - // if (specifier.local.name !== specifier.imported.name) { - // valueImportSpecifiers.push(`${specifier.imported.name} as ${specifier.local.name}`); - // } else { - // valueImportSpecifiers.push(specifier.local.name); - // } - // importSpecifierRanges.push(specifier.range); - // } - // } // no inline type imports found if (typeImportSpecifiers.length === 0) { return; } - log('all imports size - ', allImportsSize); - // all inline imports are type imports => need to change it to separate import statement - // import {type X, type Y} form 'x' => import type { X, Y} from 'x' - if (typeImportSpecifiers.length === allImportsSize) { - context.report({ - node, - message: SEPARATE_ERROR_MESSAGE, - fix(fixer) { - const sourceCode = context.getSourceCode(); - const tokens = sourceCode.getTokens(node); - tokens.forEach(token => { - if (token.value === 'type') { - fixerArray.push(fixer.remove(token)); - } - }); - fixerArray.push(fixer.insertTextAfter(tokens[0], ' type')); - return fixerArray; - }, - }); + if (typeImportSpecifiers.length === typeImportSpecifiers.length + valueImportSpecifiers.length) { + changeInlineImportToSeparate(node, context); } - - // there is a mix of inline value imports and type imports - // import {type X, type Y, Z} form 'x' => import {Z} form 'x'\nimport type { X, Y } from 'x' - else { - context.report({ - node, - message: SEPARATE_ERROR_MESSAGE, - fix(fixer) { - const sourceCode = context.getSourceCode(); - const tokens = sourceCode.getTokens(node); - const importPath = tokens[tokens.length-1].value; - - // remove all imports - importSpecifierRanges.forEach((range)=>{ - fixerArray.push(fixer.removeRange([range[0], range[1]])); - }); - - let namedImportStart = undefined; - // remove all commas - tokens.forEach( element => { - if (element.value === '{') { - namedImportStart = element; - } - if (element.value === ',') { - fixerArray.push(fixer.remove(element)); - } - }); - // add inline value imports back - fixerArray.push(fixer.insertTextAfter(namedImportStart, valueImportSpecifiers.join(', '))); - // add new line with separate type import - fixerArray.push(fixer.insertTextAfter(node, `\nimport type { ${typeImportSpecifiers.join(', ')} } from ${importPath}`)); - return fixerArray; - }, - }); + else { + addSeparateTypeImportDeclaration(node, context, specifierCache); } } + if (config === 'inline' && node.importKind === 'type') { // ignore default type import like: import type A from 'x' if (node.specifiers.length === 1 && node.specifiers[0].type === 'ImportDefaultSpecifier') { return; } - // IMPROVE: take out to separate function - for (let i = 0; i < node.specifiers.length; i++) { - const specifier = node.specifiers[i]; - if (specifier.local.name !== specifier.imported.name) { - typeImportSpecifiers.push(`type ${specifier.imported.name} as ${specifier.local.name}`); - } else { - typeImportSpecifiers.push(`type ${specifier.local.name}`); - } - valueImportNodes.push(specifier); - } + setImportSpecifierCacheForInline(node, specifierCache); - // IMPROVE: separate function to handle it? find if there are any non-type imports from the same location - const body = node.parent.body; - for (let j = 0; j < body.length; j++) { - const element = body[j]; - processBodyStatement(importMap, element); - } - const sameSourceValueImport = importMap.get(node.source.value); + const sameSourceValueImport = getImportDeclarationFromTheSameSource(node); if (sameSourceValueImport && !sameSourceValueImport.hasNamespaceImport) { - - // get the last specifier - const lastSpecifier = sameSourceValueImport.specifiers[sameSourceValueImport.specifiers.length - 1]; - - // try to insert after the last specifier - context.report({ - node, - message: INLINE_ERROR_MESSAGE, - fix(fixer) { - if (lastSpecifier.type === 'ImportDefaultSpecifier' && sameSourceValueImport.specifiers.length === 1) { - // import defaultExport from 'x' - // import type { X, Y } from 'x' - // => import defaultExport, { type X, type Y } from 'x' - const inlineTypeImportsToInsert = ', { ' + typeImportSpecifiers.join(', ') + ' }'; - fixerArray.push(fixer.insertTextAfter(lastSpecifier, inlineTypeImportsToInsert)); - } else { - // import { namedImport } from 'x' - // import type { X, Y } from 'x' - // => import { namedImport, type X, type Y } from 'x' - const inlineTypeImportsToInsert = ', ' + typeImportSpecifiers.join(', '); - fixerArray.push(fixer.insertTextAfter(lastSpecifier, inlineTypeImportsToInsert)); - } - - fixerArray.push(fixer.remove(node)); - return fixerArray; - }, - }); + insertInlineTypeImportsToSameSourceImport(node, context, sameSourceValueImport, typeImportSpecifiers); } else { - // There are no other imports from the same location => remove 'type' next to import statement and add "type" to every named import - // import type {a,b} from 'x' => import {type a, type b} from 'x' - - // TODO: check this statement: valueNodeImports => possibly rename it ? - context.report({ - node, - message: INLINE_ERROR_MESSAGE, - fix(fixer) { - const sourceCode = context.getSourceCode(); - const tokens = sourceCode.getTokens(node); - fixerArray.push(fixer.remove(tokens[1])); - valueImportNodes.forEach(element => { - fixerArray.push(fixer.insertTextBefore(element, 'type ')); - }); - return fixerArray; - }, - }); + changeToInlineTypeImport(node, context, valueImportNodes); } } }, @@ -281,7 +112,21 @@ module.exports = { }, }; -function setImportSpecifierCache(specifiers, specifierCache) { + +function getRuleConfig(config, isInlineTypeImportSupported) { + if (config.length === 2 && config[0] === 'inline' && !isInlineTypeImportSupported) { + config = 'separate'; + } + if (config[0] === 'separate') { + config = 'separate'; + } + return config; +} + +/* + Functions for config === separate +*/ +function setImportSpecifierCacheForSeparate(specifiers, specifierCache) { const typeImportSpecifiers = specifierCache.typeImportSpecifiers; const valueImportSpecifiers = specifierCache.valueImportSpecifiers; const valueImportNodes = specifierCache.valueImportNodes; @@ -314,3 +159,175 @@ function setImportSpecifierCache(specifiers, specifierCache) { } return true; } + +function changeInlineImportToSeparate(node, context){ + // all inline imports are type imports => need to change it to separate import statement + // import {type X, type Y} form 'x' => import type { X, Y} from 'x' + + const fixerArray = []; + context.report({ + node, + message: SEPARATE_ERROR_MESSAGE, + fix(fixer) { + const sourceCode = context.getSourceCode(); + const tokens = sourceCode.getTokens(node); + tokens.forEach(token => { + if (token.value === 'type') { + fixerArray.push(fixer.remove(token)); + } + }); + fixerArray.push(fixer.insertTextAfter(tokens[0], ' type')); + return fixerArray; + }, + }); +} + +function addSeparateTypeImportDeclaration(node, context, specifierCache) { + // there is a mix of inline value imports and type imports + // import {type X, type Y, Z} form 'x' => import {Z} form 'x'\nimport type { X, Y } from 'x' + const fixerArray = []; + const importSpecifierRanges = specifierCache.importSpecifierRanges; + const valueImportSpecifiers = specifierCache.valueImportSpecifiers; + const typeImportSpecifiers = specifierCache.typeImportSpecifiers; + + context.report({ + node, + message: SEPARATE_ERROR_MESSAGE, + fix(fixer) { + const sourceCode = context.getSourceCode(); + const tokens = sourceCode.getTokens(node); + const importPath = tokens[tokens.length-1].value; + + // remove all imports + importSpecifierRanges.forEach((range)=>{ + fixerArray.push(fixer.removeRange([range[0], range[1]])); + }); + + let namedImportStart = undefined; + // remove all commas + tokens.forEach( element => { + if (element.value === '{') { + namedImportStart = element; + } + if (element.value === ',') { + fixerArray.push(fixer.remove(element)); + } + }); + // add inline value imports back + fixerArray.push(fixer.insertTextAfter(namedImportStart, valueImportSpecifiers.join(', '))); + // add new line with separate type import + fixerArray.push(fixer.insertTextAfter(node, `\nimport type { ${typeImportSpecifiers.join(', ')} } from ${importPath}`)); + return fixerArray; + }, + }); +} + + +/* + Functions for config === inline +*/ + +function setImportSpecifierCacheForInline(node, specifierCache) { + const typeImportSpecifiers = specifierCache.typeImportSpecifiers; + const valueImportNodes = specifierCache.valueImportNodes; + + for (let i = 0; i < node.specifiers.length; i++) { + const specifier = node.specifiers[i]; + if (specifier.local.name !== specifier.imported.name) { + typeImportSpecifiers.push(`type ${specifier.imported.name} as ${specifier.local.name}`); + } else { + typeImportSpecifiers.push(`type ${specifier.local.name}`); + } + valueImportNodes.push(specifier); + } +} + + +function getImportDeclarationFromTheSameSource(node) { + const importDeclarationCache = new Map(); + const body = node.parent.body; + for (let j = 0; j < body.length; j++) { + const element = body[j]; + processBodyStatement(importDeclarationCache, element); + } + return importDeclarationCache.get(node.source.value); +} + +function processBodyStatement(importDeclarationCache, node){ + if (node.type !== 'ImportDeclaration' || node.importKind === 'type') return; + if (node.specifiers.length === 0) return; + + const specifiers = []; + let hasDefaultImport= false; + let hasInlineDefaultImport = false; + let hasNamespaceImport = false; + + node.specifiers.forEach((specifier)=>{ + if (specifier.type === 'ImportNamespaceSpecifier') { + hasNamespaceImport = true; + } + // ignore the case for now. TODO: talk with Jordan and implement the handling of the rule here + if (specifier.type === 'ImportDefaultSpecifier') { + hasDefaultImport = true; + specifiers.push(specifier); + } + if (specifier.type === 'ImportSpecifier') { + // check if its {default as B} + if (specifier.imported.name === 'default') { + hasInlineDefaultImport = true; + } + specifiers.push(specifier); + } + }); + // cache all imports + importDeclarationCache.set(node.source.value, { specifiers, hasDefaultImport, hasInlineDefaultImport, hasNamespaceImport }); +} + +function insertInlineTypeImportsToSameSourceImport(node, context, sameSourceValueImport, typeImportSpecifiers) { + const fixerArray = []; + const lastSpecifier = sameSourceValueImport.specifiers[sameSourceValueImport.specifiers.length - 1]; + + context.report({ + node, + message: INLINE_ERROR_MESSAGE, + fix(fixer) { + if (lastSpecifier.type === 'ImportDefaultSpecifier' && sameSourceValueImport.specifiers.length === 1) { + // import defaultExport from 'x' + // import type { X, Y } from 'x' + // => import defaultExport, { type X, type Y } from 'x' + const inlineTypeImportsToInsert = ', { ' + typeImportSpecifiers.join(', ') + ' }'; + fixerArray.push(fixer.insertTextAfter(lastSpecifier, inlineTypeImportsToInsert)); + } else { + // import { namedImport } from 'x' + // import type { X, Y } from 'x' + // => import { namedImport, type X, type Y } from 'x' + const inlineTypeImportsToInsert = ', ' + typeImportSpecifiers.join(', '); + fixerArray.push(fixer.insertTextAfter(lastSpecifier, inlineTypeImportsToInsert)); + } + + fixerArray.push(fixer.remove(node)); + return fixerArray; + }, + }); +} + +function changeToInlineTypeImport(node, context, valueNodeImports) { + // There are no other imports from the same location => remove 'type' next to import statement and add "type" to every named import + // import type {a,b} from 'x' => import {type a, type b} from 'x' + + const fixerArray = []; + + context.report({ + node, + message: INLINE_ERROR_MESSAGE, + fix(fixer) { + const sourceCode = context.getSourceCode(); + const tokens = sourceCode.getTokens(node); + fixerArray.push(fixer.remove(tokens[1])); + valueNodeImports.forEach(element => { + fixerArray.push(fixer.insertTextBefore(element, 'type ')); + }); + return fixerArray; + }, + }); +} From a6246b20d3174fa8ea6da084766d99d591ed2b6a Mon Sep 17 00:00:00 2001 From: Aziz Abdullaev Date: Tue, 14 Feb 2023 10:07:43 -0500 Subject: [PATCH 37/37] remove redundant comments --- src/rules/typescript-flow.js | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/src/rules/typescript-flow.js b/src/rules/typescript-flow.js index e06c653c5..b4bc5f537 100644 --- a/src/rules/typescript-flow.js +++ b/src/rules/typescript-flow.js @@ -75,13 +75,11 @@ module.exports = { if (config === 'separate' && node.importKind !== 'type') { // identify importSpecifiers that have inline type imports as well as value imports - if (!setImportSpecifierCacheForSeparate(node.specifiers, specifierCache)) { - return; - } + if (!setImportSpecifierCacheForSeparate(node.specifiers, specifierCache)) return; + // no inline type imports found - if (typeImportSpecifiers.length === 0) { - return; - } + if (typeImportSpecifiers.length === 0) return; + if (typeImportSpecifiers.length === typeImportSpecifiers.length + valueImportSpecifiers.length) { changeInlineImportToSeparate(node, context); } @@ -96,11 +94,8 @@ module.exports = { if (node.specifiers.length === 1 && node.specifiers[0].type === 'ImportDefaultSpecifier') { return; } - setImportSpecifierCacheForInline(node, specifierCache); - const sameSourceValueImport = getImportDeclarationFromTheSameSource(node); - if (sameSourceValueImport && !sameSourceValueImport.hasNamespaceImport) { insertInlineTypeImportsToSameSourceImport(node, context, sameSourceValueImport, typeImportSpecifiers); } else { @@ -137,9 +132,7 @@ function setImportSpecifierCacheForSeparate(specifiers, specifierCache) { if (specifier.type === 'ImportNamespaceSpecifier' || specifier.type === 'ImportDefaultSpecifier') { return false; } - // Question: do we want our rule to deal with default imports? It does not make sense that rule needs to since we do not know the type of default export. specifierCache.allImportsSize = specifierCache.allImportsSize + 1; - // catch all inline imports and add them to the set if (specifier.importKind === 'type') { if (specifier.local.name !== specifier.imported.name) { typeImportSpecifiers.push(`${specifier.imported.name} as ${specifier.local.name}`); @@ -204,7 +197,6 @@ function addSeparateTypeImportDeclaration(node, context, specifierCache) { }); let namedImportStart = undefined; - // remove all commas tokens.forEach( element => { if (element.value === '{') { namedImportStart = element; @@ -213,9 +205,7 @@ function addSeparateTypeImportDeclaration(node, context, specifierCache) { fixerArray.push(fixer.remove(element)); } }); - // add inline value imports back fixerArray.push(fixer.insertTextAfter(namedImportStart, valueImportSpecifiers.join(', '))); - // add new line with separate type import fixerArray.push(fixer.insertTextAfter(node, `\nimport type { ${typeImportSpecifiers.join(', ')} } from ${importPath}`)); return fixerArray; }, @@ -242,7 +232,6 @@ function setImportSpecifierCacheForInline(node, specifierCache) { } } - function getImportDeclarationFromTheSameSource(node) { const importDeclarationCache = new Map(); const body = node.parent.body; @@ -266,20 +255,17 @@ function processBodyStatement(importDeclarationCache, node){ if (specifier.type === 'ImportNamespaceSpecifier') { hasNamespaceImport = true; } - // ignore the case for now. TODO: talk with Jordan and implement the handling of the rule here if (specifier.type === 'ImportDefaultSpecifier') { hasDefaultImport = true; specifiers.push(specifier); } if (specifier.type === 'ImportSpecifier') { - // check if its {default as B} if (specifier.imported.name === 'default') { hasInlineDefaultImport = true; } specifiers.push(specifier); } }); - // cache all imports importDeclarationCache.set(node.source.value, { specifiers, hasDefaultImport, hasInlineDefaultImport, hasNamespaceImport }); }