Keep typeAnnotation when replacing identifier (#608)

sindresorhus · Mar 17, 2020 · b657a23 · b657a23
1 parent 26c1a71
commit b657a23
Show file tree

Hide file tree

Showing 7 changed files with 120 additions and 61 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<boolean> = func();'
+		].map(code => ({
+			code,
+			output: code.replace('prop', 'property'),
+			errors: createErrors()
+		}))
+	]
+});