prefer-number-properties: Fix some edge cases (part 2) (sindresorhus#1176)

Author: fisker

rules/prefer-number-properties.js

@@ -2,6 +2,7 @@
 const getDocumentationUrl = require('./utils/get-documentation-url');
 const isShadowed = require('./utils/is-shadowed');
 const renameIdentifier = require('./utils/rename-identifier');
+const referenceIdentifierSelector = require('./utils/reference-identifier-selector');
 
 const METHOD_ERROR_MESSAGE_ID = 'method-error';
 const METHOD_SUGGESTION_MESSAGE_ID = 'method-suggestion';
@@ -28,36 +29,7 @@ const methodsSelector = [
 	`:matches(${Object.keys(methods).map(name => `[name="${name}"]`).join(', ')})`
 ].join('');
 
-const propertiesSelector = [
-	'Identifier',
-	':matches([name="NaN"],[name="Infinity"])',
-	`:not(${
-		[
-			'MemberExpression[computed=false] > .property',
-			'FunctionDeclaration > .id',
-			'ClassDeclaration > .id',
-			// TODO: remove `ClassProperty` when `babel` and `typescript` support `FieldDefinition`
-			'ClassProperty[computed=false] > .key',
-			'FieldDefinition[computed=false] > .key',
-			'MethodDefinition[computed=false] > .key',
-			'VariableDeclarator > .id',
-			'Property[shorthand=false][computed=false] > .key',
-			'LabeledStatement > .label',
-			'ContinueStatement > .label',
-			'BreakStatement > .label',
-			'ExportSpecifier > .local',
-			'ExportSpecifier > .exported',
-			'ExportAllDeclaration > .exported',
-			'ImportSpecifier > .local',
-			'ImportSpecifier > .imported',
-			'ImportNamespaceSpecifier > .local',
-			'ImportDefaultSpecifier > .local',
-			'TSDeclareFunction > .id',
-			'TSEnumMember > .id',
-			'TSPropertySignature > .key'
-		].join(', ')
-	})`
-].join('');
+const propertiesSelector = referenceIdentifierSelector(['NaN', 'Infinity']);
 
 const isNegative = node => {
 	const {parent} = node;

rules/utils/reference-identifier-selector.js

@@ -0,0 +1,84 @@
+'use strict';
+
+const nonReferenceSelector = [
+	// `foo.Identifier`
+	'MemberExpression[computed=false] > .property',
+	// `function Identifier() {}`
+	'FunctionDeclaration > .id',
+	// `const foo = function Identifier() {}`
+	'FunctionExpression > .id',
+	// `class Identifier() {}`
+	'ClassDeclaration > .id',
+	// `const foo = class Identifier() {}`
+	'ClassExpression > .id',
+	// TODO: remove `ClassProperty` when `babel` and `typescript` support `PropertyDefinition`
+	'ClassProperty[computed=false] > .key',
+	// `class Foo {Identifier = 1}`
+	'PropertyDefinition[computed=false] > .key',
+	// `class Foo {Identifier() {}}`
+	'MethodDefinition[computed=false] > .key',
+	// `const Identifier = 1`
+	'VariableDeclarator > .id',
+	// `const foo = {Identifier: 1}`
+	'ObjectExpression > Property[shorthand=false][computed=false].properties > .key',
+	// `const {Identifier} = {}`
+	// `const {Identifier: foo} = {}`
+	'ObjectPattern > Property[computed=false].properties > .key',
+	// `const {Identifier} = {}`
+	// `const {foo: Identifier} = {}`
+	'ObjectPattern > Property.properties > .value',
+	// `const [Identifier] = []`
+	'ArrayPattern > .elements',
+	// `function foo(Identifier) {}`
+	// `const foo = function(Identifier) {}`
+	// `const foo = (Identifier) => {}`
+	':function > .params',
+	/*
+		```
+		Identifier: for (const foo of bar) {
+			continue Identifier;
+			break Identifier;
+		}
+		```
+	*/
+	'LabeledStatement > .label',
+	'ContinueStatement > .label',
+	'BreakStatement > .label',
+	// `export {Identifier as foo}`
+	'ExportSpecifier > .local',
+	// `export {foo as Identifier}`
+	'ExportSpecifier > .exported',
+	// `export * as Identifier from 'foo'`
+	'ExportAllDeclaration > .exported',
+	// `import {foo as Identifier} from 'foo'`
+	'ImportSpecifier > .local',
+	// `import {Identifier as foo} from 'foo'`
+	'ImportSpecifier > .imported',
+	// `import * as Identifier from 'foo'`
+	'ImportNamespaceSpecifier > .local',
+	// `import Identifier from 'foo'`
+	'ImportDefaultSpecifier > .local',
+
+	// TypeScript
+	'TSDeclareFunction > .id',
+	'TSEnumMember > .id',
+	'TSPropertySignature > .key'
+].join(', ');
+
+function referenceIdentifierSelector(nameOrNames) {
+	const selector = [
+		'Identifier',
+		`:not(${nonReferenceSelector})`
+	];
+
+	/* istanbul ignore else: no rule use single name yet */
+	if (Array.isArray(nameOrNames)) {
+		selector.push(`:matches(${nameOrNames.map(name => `[name="${name}"]`).join(', ')})`);
+	} else {
+		selector.push(`[name="${nameOrNames}"]`);
+	}
+
+	return selector.join('');
+}
+
+module.exports = referenceIdentifierSelector;

test/prefer-number-properties.mjs

@@ -184,8 +184,20 @@ test({
 			}
 		`,
 		'const {NaN} = {};',
+		'const {a: NaN} = {};',
+		'const {[a]: NaN} = {};',
+		'const [NaN] = [];',
 		'function NaN() {}',
+		'const foo = function NaN() {}',
+		'function foo(NaN) {}',
+		'foo = function (NaN) {}',
+		'foo = (NaN) => {}',
+		'function foo({NaN}) {}',
+		'function foo({a: NaN}) {}',
+		'function foo({[a]: NaN}) {}',
+		'function foo([NaN]) {}',
 		'class NaN {}',
+		'const Foo = class NaN {}',
 		'class Foo {NaN(){}}',
 		outdent`
 			NaN: for (const foo of bar) {
@@ -358,6 +370,13 @@ test.snapshot({
 		'const foo = 1 - Infinity;',
 		'const foo = 1 - -Infinity;',
 		'const isPositiveZero = value => value === 0 && 1 / value === Infinity;',
-		'const isNegativeZero = value => value === 0 && 1 / value === -Infinity;'
+		'const isNegativeZero = value => value === 0 && 1 / value === -Infinity;',
+
+		'const {a = NaN} = {};',
+		'const {[NaN]: a = NaN} = {};',
+		'const [a = NaN] = [];',
+		'function foo({a = NaN}) {}',
+		'function foo({[NaN]: a = NaN}) {}',
+		'function foo([a = NaN]) {}'
 	]
 });

test/snapshots/prefer-number-properties.mjs.md

@@ -497,3 +497,113 @@ Generated by [AVA](https://avajs.dev).
     > 1 | const isNegativeZero = value => value === 0 && 1 / value === -Infinity;␊
         |                                                              ^^^^^^^^^ Prefer \`Number.NEGATIVE_INFINITY\` over \`-Infinity\`.␊
     `
+
+## Invalid #31
+      1 | const {a = NaN} = {};
+
+> Output
+
+    `␊
+      1 | const {a = Number.NaN} = {};␊
+    `
+
+> Error 1/1
+
+    `␊
+    > 1 | const {a = NaN} = {};␊
+        |            ^^^ Prefer \`Number.NaN\` over \`NaN\`.␊
+    `
+
+## Invalid #32
+      1 | const {[NaN]: a = NaN} = {};
+
+> Output
+
+    `␊
+      1 | const {[Number.NaN]: a = Number.NaN} = {};␊
+    `
+
+> Error 1/2
+
+    `␊
+    > 1 | const {[NaN]: a = NaN} = {};␊
+        |         ^^^ Prefer \`Number.NaN\` over \`NaN\`.␊
+    `
+
+> Error 2/2
+
+    `␊
+    > 1 | const {[NaN]: a = NaN} = {};␊
+        |                   ^^^ Prefer \`Number.NaN\` over \`NaN\`.␊
+    `
+
+## Invalid #33
+      1 | const [a = NaN] = [];
+
+> Output
+
+    `␊
+      1 | const [a = Number.NaN] = [];␊
+    `
+
+> Error 1/1
+
+    `␊
+    > 1 | const [a = NaN] = [];␊
+        |            ^^^ Prefer \`Number.NaN\` over \`NaN\`.␊
+    `
+
+## Invalid #34
+      1 | function foo({a = NaN}) {}
+
+> Output
+
+    `␊
+      1 | function foo({a = Number.NaN}) {}␊
+    `
+
+> Error 1/1
+
+    `␊
+    > 1 | function foo({a = NaN}) {}␊
+        |                   ^^^ Prefer \`Number.NaN\` over \`NaN\`.␊
+    `
+
+## Invalid #35
+      1 | function foo({[NaN]: a = NaN}) {}
+
+> Output
+
+    `␊
+      1 | function foo({[Number.NaN]: a = Number.NaN}) {}␊
+    `
+
+> Error 1/2
+
+    `␊
+    > 1 | function foo({[NaN]: a = NaN}) {}␊
+        |                ^^^ Prefer \`Number.NaN\` over \`NaN\`.␊
+    `
+
+> Error 2/2
+
+    `␊
+    > 1 | function foo({[NaN]: a = NaN}) {}␊
+        |                          ^^^ Prefer \`Number.NaN\` over \`NaN\`.␊
+    `
+
+## Invalid #36
+      1 | function foo([a = NaN]) {}
+
+> Output
+
+    `␊
+      1 | function foo([a = Number.NaN]) {}␊
+    `
+
+> Error 1/1
+
+    `␊
+    > 1 | function foo([a = NaN]) {}␊
+        |                   ^^^ Prefer \`Number.NaN\` over \`NaN\`.␊
+    `