From b657a23ecd16bd21501d2de917914c799bbd35e2 Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Tue, 17 Mar 2020 15:23:53 +0800 Subject: [PATCH] Keep `typeAnnotation` when replacing `identifier` (#608) --- rules/prevent-abbreviations.js | 65 ++----------------- ...t-pattern-shorthand-property-identifier.js | 11 ++++ rules/utils/is-shorthand-export-identifier.js | 6 ++ rules/utils/is-shorthand-import-identifier.js | 6 ++ .../utils/is-shorthand-property-identifier.js | 8 +++ rules/utils/rename-identifier.js | 37 +++++++++++ test/prevent-abbreviations.js | 48 ++++++++++++++ 7 files changed, 120 insertions(+), 61 deletions(-) create mode 100644 rules/utils/is-assignment-pattern-shorthand-property-identifier.js create mode 100644 rules/utils/is-shorthand-export-identifier.js create mode 100644 rules/utils/is-shorthand-import-identifier.js create mode 100644 rules/utils/is-shorthand-property-identifier.js create mode 100644 rules/utils/rename-identifier.js diff --git a/rules/prevent-abbreviations.js b/rules/prevent-abbreviations.js index 5b98fb634e..314ae579aa 100644 --- a/rules/prevent-abbreviations.js +++ b/rules/prevent-abbreviations.js @@ -6,7 +6,9 @@ const {defaultsDeep, upperFirst, lowerFirst} = require('lodash'); const getDocumentationUrl = require('./utils/get-documentation-url'); const avoidCapture = require('./utils/avoid-capture'); const cartesianProductSamples = require('./utils/cartesian-product-samples'); -const isSameNode = require('./utils/is-same-node'); +const isShorthandPropertyIdentifier = require('./utils/is-shorthand-property-identifier'); +const isShorthandImportIdentifier = require('./utils/is-shorthand-import-identifier'); +const renameIdentifier = require('./utils/rename-identifier'); const isUpperCase = string => string === string.toUpperCase(); const isUpperFirst = string => isUpperCase(string[0]); @@ -398,65 +400,6 @@ const shouldFix = variable => { return !variableIdentifiers(variable).some(isExportedIdentifier); }; -const isShorthandPropertyIdentifier = identifier => { - return ( - identifier.parent.type === 'Property' && - identifier.parent.shorthand && - isSameNode(identifier, identifier.parent.key) - ); -}; - -const isAssignmentPatternShorthandPropertyIdentifier = identifier => { - return ( - identifier.parent.type === 'AssignmentPattern' && - identifier.parent.left === identifier && - identifier.parent.parent.type === 'Property' && - isSameNode(identifier, identifier.parent.parent.key) && - identifier.parent.parent.value === identifier.parent && - identifier.parent.parent.shorthand - ); -}; - -const isShorthandImportIdentifier = identifier => { - return ( - identifier.parent.type === 'ImportSpecifier' && - identifier.parent.imported.name === identifier.name && - identifier.parent.local.name === identifier.name - ); -}; - -const isShorthandExportIdentifier = identifier => { - return ( - identifier.parent.type === 'ExportSpecifier' && - identifier.parent.exported.name === identifier.name && - identifier.parent.local.name === identifier.name - ); -}; - -const fixIdentifier = (fixer, replacement, sourceCode) => identifier => { - if ( - isShorthandPropertyIdentifier(identifier) || - isAssignmentPatternShorthandPropertyIdentifier(identifier) - ) { - return fixer.replaceText(identifier, `${identifier.name}: ${replacement}`); - } - - if (isShorthandImportIdentifier(identifier)) { - return fixer.replaceText(identifier, `${identifier.name} as ${replacement}`); - } - - if (isShorthandExportIdentifier(identifier)) { - return fixer.replaceText(identifier, `${replacement} as ${identifier.name}`); - } - - // `TypeParameter` default value - if (identifier.default) { - return fixer.replaceText(identifier, `${replacement} = ${sourceCode.getText(identifier.default)}`); - } - - return fixer.replaceText(identifier, replacement); -}; - const isDefaultOrNamespaceImportName = identifier => { if ( identifier.parent.type === 'ImportDefaultSpecifier' && @@ -689,7 +632,7 @@ const create = context => { problem.fix = fixer => { return variableIdentifiers(variable) - .map(fixIdentifier(fixer, replacement, sourceCode)); + .map(identifier => renameIdentifier(identifier, replacement, fixer, sourceCode)); }; } diff --git a/rules/utils/is-assignment-pattern-shorthand-property-identifier.js b/rules/utils/is-assignment-pattern-shorthand-property-identifier.js new file mode 100644 index 0000000000..c819b9a77d --- /dev/null +++ b/rules/utils/is-assignment-pattern-shorthand-property-identifier.js @@ -0,0 +1,11 @@ +'use strict'; + +const isSameNode = require('./is-same-node'); + +module.exports = identifier => + identifier.parent.type === 'AssignmentPattern' && + identifier.parent.left === identifier && + identifier.parent.parent.type === 'Property' && + isSameNode(identifier, identifier.parent.parent.key) && + identifier.parent.parent.value === identifier.parent && + identifier.parent.parent.shorthand; diff --git a/rules/utils/is-shorthand-export-identifier.js b/rules/utils/is-shorthand-export-identifier.js new file mode 100644 index 0000000000..6eda048781 --- /dev/null +++ b/rules/utils/is-shorthand-export-identifier.js @@ -0,0 +1,6 @@ +'use strict'; + +module.exports = identifier => + identifier.parent.type === 'ExportSpecifier' && + identifier.parent.exported.name === identifier.name && + identifier.parent.local.name === identifier.name; diff --git a/rules/utils/is-shorthand-import-identifier.js b/rules/utils/is-shorthand-import-identifier.js new file mode 100644 index 0000000000..5b132fd6db --- /dev/null +++ b/rules/utils/is-shorthand-import-identifier.js @@ -0,0 +1,6 @@ +'use strict'; + +module.exports = identifier => + identifier.parent.type === 'ImportSpecifier' && + identifier.parent.imported.name === identifier.name && + identifier.parent.local.name === identifier.name; diff --git a/rules/utils/is-shorthand-property-identifier.js b/rules/utils/is-shorthand-property-identifier.js new file mode 100644 index 0000000000..8763494d8b --- /dev/null +++ b/rules/utils/is-shorthand-property-identifier.js @@ -0,0 +1,8 @@ +'use strict'; + +const isSameNode = require('./is-same-node'); + +module.exports = identifier => + identifier.parent.type === 'Property' && + identifier.parent.shorthand && + isSameNode(identifier, identifier.parent.key); diff --git a/rules/utils/rename-identifier.js b/rules/utils/rename-identifier.js new file mode 100644 index 0000000000..2edb64d23f --- /dev/null +++ b/rules/utils/rename-identifier.js @@ -0,0 +1,37 @@ +'use strict'; + +const isShorthandPropertyIdentifier = require('./is-shorthand-property-identifier'); +const isAssignmentPatternShorthandPropertyIdentifier = require('./is-assignment-pattern-shorthand-property-identifier'); +const isShorthandImportIdentifier = require('./is-shorthand-import-identifier'); +const isShorthandExportIdentifier = require('./is-shorthand-export-identifier'); + +function renameIdentifier(identifier, name, fixer, sourceCode) { + if ( + isShorthandPropertyIdentifier(identifier) || + isAssignmentPatternShorthandPropertyIdentifier(identifier) + ) { + return fixer.replaceText(identifier, `${identifier.name}: ${name}`); + } + + if (isShorthandImportIdentifier(identifier)) { + return fixer.replaceText(identifier, `${identifier.name} as ${name}`); + } + + if (isShorthandExportIdentifier(identifier)) { + return fixer.replaceText(identifier, `${name} as ${identifier.name}`); + } + + // `TypeParameter` default value + if (identifier.default) { + return fixer.replaceText(identifier, `${name} = ${sourceCode.getText(identifier.default)}`); + } + + // `typeAnnotation` + if (identifier.typeAnnotation) { + return fixer.replaceText(identifier, `${name}${sourceCode.getText(identifier.typeAnnotation)}`); + } + + return fixer.replaceText(identifier, name); +} + +module.exports = renameIdentifier; diff --git a/test/prevent-abbreviations.js b/test/prevent-abbreviations.js index 6c821c7309..f88bfe17d4 100644 --- a/test/prevent-abbreviations.js +++ b/test/prevent-abbreviations.js @@ -29,6 +29,10 @@ const babelRuleTester = avaRuleTester(test, { parser: require.resolve('babel-eslint') }); +const typescriptRuleTester = avaRuleTester(test, { + parser: require.resolve('@typescript-eslint/parser') +}); + const noFixingTestCase = test => ({...test, output: test.code}); const createErrors = message => { @@ -1641,6 +1645,31 @@ babelRuleTester.run('prevent-abbreviations', rule, { errors: createErrors() }, + // #347 + { + code: outdent` + function onKeyDown(e: KeyboardEvent) { + if (e.keyCode) {} + } + `, + output: outdent` + function onKeyDown(event: KeyboardEvent) { + if (event.keyCode) {} + } + `, + options: [ + { + extendDefaultReplacements: false, + replacements: { + e: { + event: true + } + } + } + ], + errors: createErrors() + }, + // https://github.com/facebook/relay/blob/597d2a17aa29d401830407b6814a5f8d148f632d/packages/relay-experimental/EntryPointTypes.flow.js#L138 { code: outdent` @@ -1673,3 +1702,22 @@ babelRuleTester.run('prevent-abbreviations', rule, { }) ] }); + +typescriptRuleTester.run('prevent-abbreviations', rule, { + valid: [], + invalid: [ + // Types + ...[ + 'declare const prop: string;', + 'declare const prop: string;', + 'declare var prop: number;', + 'declare let prop: any;', + 'declare class prop {}', + 'const prop: SomeThing = func();' + ].map(code => ({ + code, + output: code.replace('prop', 'property'), + errors: createErrors() + })) + ] +});