add jsx-ignore option to no-magic-numbers rule (#4460)

* add jsx-ignore option

* address feedback

* feedback - parse options, add additional test case

* feedback - fix conditional

* feedback - use continue in loop instead

src/rules/noMagicNumbersRule.ts

@@ -21,6 +21,21 @@ import * as ts from "typescript";
 import * as Lint from "../index";
 import { isNegativeNumberLiteral } from "../language/utils";
 
+interface AllowedNumbersType {
+    "allowed-numbers": number[];
+}
+interface IgnoreJsxType {
+    "ignore-jsx": boolean;
+}
+interface ParsedOptionsType {
+    "allowed-numbers": number[];
+    "ignore-jsx"?: boolean;
+}
+type OptionsType = IgnoreJsxType | AllowedNumbersType;
+
+const IGNORE_JSX_OPTION = "ignore-jsx";
+const ALLOWED_NUMBERS_OPTION = "allowed-numbers";
+
 export class Rule extends Lint.Rules.AbstractRule {
     /* tslint:disable:object-literal-sort-keys */
     public static metadata: Lint.IRuleMetadata = {
@@ -32,15 +47,37 @@ export class Rule extends Lint.Rules.AbstractRule {
             Magic numbers should be avoided as they often lack documentation.
             Forcing them to be stored in variables gives them implicit documentation.
         `,
-        optionsDescription: "A list of allowed numbers.",
+        optionsDescription: Lint.Utils.dedent`
+            Options may either be a list of numbers to ignore (not consider 'magic'), or an object containing up to two properties:
+            * \`${ALLOWED_NUMBERS_OPTION}\` as the list of numbers to ignore.
+            * \`${IGNORE_JSX_OPTION}\` to specify that 'magic' numbers should be allowed as JSX attributes.`,
         options: {
             type: "array",
             items: {
                 type: "number",
             },
+            properties: {
+                type: "object",
+                [ALLOWED_NUMBERS_OPTION]: {
+                    type: "array",
+                },
+                [IGNORE_JSX_OPTION]: {
+                    type: "boolean",
+                },
+            },
             minLength: 1,
         },
-        optionExamples: [true, [true, 1, 2, 3]],
+
+        optionExamples: [
+            [true, 1, 2, 3],
+            [
+                true,
+                {
+                    [ALLOWED_NUMBERS_OPTION]: [1, 2, 3],
+                    [IGNORE_JSX_OPTION]: true,
+                },
+            ],
+        ],
         type: "typescript",
         typescriptOnly: false,
     };
@@ -70,13 +107,37 @@ export class Rule extends Lint.Rules.AbstractRule {
             new NoMagicNumbersWalker(
                 sourceFile,
                 this.ruleName,
-                this.ruleArguments.length > 0 ? this.ruleArguments : Rule.DEFAULT_ALLOWED,
+                this.ruleArguments.length > 0
+                    ? parseOptions(this.ruleArguments)
+                    : parseOptions(Rule.DEFAULT_ALLOWED),
             ),
         );
     }
 }
 
-class NoMagicNumbersWalker extends Lint.AbstractWalker<number[]> {
+function parseOptions(options: Array<OptionsType | number>): ParsedOptionsType {
+    const parsedOptions: ParsedOptionsType = { [ALLOWED_NUMBERS_OPTION]: [] };
+    for (const option of options) {
+        if (typeof option === "number") {
+            parsedOptions[ALLOWED_NUMBERS_OPTION].push(option);
+            continue;
+        }
+        if (option.hasOwnProperty(ALLOWED_NUMBERS_OPTION)) {
+            const numberOptions = (option as AllowedNumbersType)[ALLOWED_NUMBERS_OPTION];
+            if (Array.isArray(numberOptions) && numberOptions.length > 0) {
+                numberOptions.forEach((num: number) =>
+                    parsedOptions[ALLOWED_NUMBERS_OPTION].push(num),
+                );
+            }
+        }
+        if (option.hasOwnProperty(IGNORE_JSX_OPTION)) {
+            parsedOptions[IGNORE_JSX_OPTION] = (option as IgnoreJsxType)[IGNORE_JSX_OPTION];
+        }
+    }
+    return parsedOptions;
+}
+
+class NoMagicNumbersWalker extends Lint.AbstractWalker<ParsedOptionsType | number[]> {
     public walk(sourceFile: ts.SourceFile) {
         const cb = (node: ts.Node): void => {
             if (
@@ -101,11 +162,26 @@ class NoMagicNumbersWalker extends Lint.AbstractWalker<number[]> {
         return ts.forEachChild(sourceFile, cb);
     }
 
+    private isAllowedNumber(num: string): boolean {
+        const numberOptions = (this.options as ParsedOptionsType)[ALLOWED_NUMBERS_OPTION];
+        if (numberOptions.length > 0) {
+            return numberOptions.some((allowedNum: number) =>
+                /* Using Object.is() to differentiate between pos/neg zero */
+                Object.is(allowedNum, parseFloat(num)),
+            );
+        }
+        return false;
+    }
+
     private checkNumericLiteral(node: ts.Node, num: string) {
-        /* Using Object.is() to differentiate between pos/neg zero */
+        const shouldIgnoreJsxExpression =
+            node.parent.kind === ts.SyntaxKind.JsxExpression &&
+            (this.options as ParsedOptionsType)[IGNORE_JSX_OPTION];
+
         if (
             !Rule.ALLOWED_NODES.has(node.parent.kind) &&
-            !this.options.some(allowedNum => Object.is(allowedNum, parseFloat(num)))
+            !this.isAllowedNumber(num) &&
+            !shouldIgnoreJsxExpression
         ) {
             this.addFailureAtNode(node, Rule.FAILURE_STRING(num));
         }

test/rules/no-magic-numbers/allowed-numbers/test.ts.lint

@@ -0,0 +1,23 @@
+parseInt();
+parseInt('123', 2);
+parseInt('123', 8);
+parseInt('123', 10);
+parseInt('123', 16);
+console.log(-0);
+console.log(1337);
+console.log(-1337);
+console.log(- 1337);
+console.log(1337.7);
+console.log(1338);
+            ~~~~                      ['magic numbers' are not allowed: 1338]
+console.log(-1338)
+            ~~~~~                     ['magic numbers' are not allowed: -1338]
+parseInt(foo === 4711 ? bar : baz, 10);
+                 ~~~~                 ['magic numbers' are not allowed: 4711]
+parseInt(foo === -0 ? bar : baz, 10);
+export let x = 1337;
+export let x = -1337;
+export let x = 1337.7;
+export let x = 1338;
+export let x = -1338;
+export let x = -0;

test/rules/no-magic-numbers/allowed-numbers/tslint.json

@@ -0,0 +1,7 @@
+{
+  "rules": {
+    "no-magic-numbers": [true, {
+      "allowed-numbers": [1337, 1337.7, -1337, -0]
+    }]
+  }
+}

test/rules/no-magic-numbers/default-jsx/test.tsx.lint

@@ -0,0 +1,8 @@
+import * as React from "react";
+class App extends React.Component {
+    render() {
+        return <Component width={200} height={200} />;
+                                 ~~~                    ['magic numbers' are not allowed: 200]
+                                              ~~~       ['magic numbers' are not allowed: 200]
+    }
+}

test/rules/no-magic-numbers/default-jsx/tslint.json

@@ -0,0 +1,5 @@
+{
+  "rules": {
+    "no-magic-numbers": true
+  }
+}

test/rules/no-magic-numbers/ignore-jsx/test.tsx.lint

@@ -0,0 +1,10 @@
+import * as React from "react";
+class App extends React.Component {
+    render() {
+        return <Component width={200} height={200} />;
+    }
+}
+
+function something(num) {}
+something(100)
+          ~~~     ['magic numbers' are not allowed: 100]

test/rules/no-magic-numbers/ignore-jsx/tslint.json

@@ -0,0 +1,7 @@
+{
+  "rules": {
+    "no-magic-numbers": [true, {
+      "ignore-jsx": true
+    }]
+  }
+}