Add allow-single-argument option and check argument types (fixes micr…

…osoft#822)
IllusionMH · May 17, 2019 · cd70f46 · cd70f46
1 parent 082cb68
commit cd70f46
Show file tree

Hide file tree

Showing 8 changed files with 277 additions and 31 deletions.
diff --git a/src/preferArrayLiteralRule.ts b/src/preferArrayLiteralRule.ts
@@ -14,10 +14,14 @@ function inRestrictedNamespace(node: ts.NewExpression | ts.CallExpression): bool
     return RESTRICTED_NAMESPACES.indexOf(functionTarget) > -1;
 }
 
+type InvocationType = 'constructor' | 'function';
+
 interface Options {
+    allowSingleArgument: boolean;
     allowTypeParameters: boolean;
 }
-export class Rule extends Lint.Rules.AbstractRule {
+
+export class Rule extends Lint.Rules.OptionallyTypedRule {
     public static metadata: ExtendedMetadata = {
         ruleName: 'prefer-array-literal',
         type: 'maintainability',
@@ -34,15 +38,26 @@ export class Rule extends Lint.Rules.AbstractRule {
     };
 
     public static GENERICS_FAILURE_STRING: string = 'Replace generic-typed Array with array literal: ';
-    public static CONSTRUCTOR_FAILURE_STRING: string = 'Replace Array constructor with an array literal: ';
-    public static FUNCTION_FAILURE_STRING: string = 'Replace Array function with an array literal: ';
+    public static getReplaceFailureString = (type: InvocationType, nodeText: string) =>
+        `Replace Array ${type} with an array literal: ${nodeText}`;
+    public static getSingleParamFailureString = (type: InvocationType, nodeText: string) =>
+        `To create an array of given length you should use non-negative integer. Otherwise replace Array ${type} with an array literal: ${nodeText}`;
 
     public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
-        return this.applyWithFunction(sourceFile, walk, this.parseOptions(this.getOptions()));
+        return this.applyWithProgram(sourceFile, /* program */ undefined);
+    }
+    public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program | undefined): Lint.RuleFailure[] {
+        return this.applyWithFunction(
+            sourceFile,
+            walk,
+            this.parseOptions(this.getOptions()),
+            program ? program.getTypeChecker() : undefined
+        );
     }
 
     private parseOptions(options: Lint.IOptions): Options {
-        let value: boolean = false;
+        let allowSingleArgument: boolean = false;
+        let allowTypeParameters: boolean = false;
         let ruleOptions: any[] = [];
 
         if (options.ruleArguments instanceof Array) {
@@ -55,24 +70,38 @@ export class Rule extends Lint.Rules.AbstractRule {
 
         ruleOptions.forEach((opt: unknown) => {
             if (isObject(opt)) {
-                value = opt['allow-type-parameters'] === true;
+                allowSingleArgument = opt['allow-single-argument'] === true;
+                allowTypeParameters = opt['allow-type-parameters'] === true;
             }
         });
 
         return {
-            allowTypeParameters: value
+            allowSingleArgument,
+            allowTypeParameters
         };
     }
 }
 
-function walk(ctx: Lint.WalkContext<Options>) {
-    const { allowTypeParameters } = ctx.options;
-
-    function checkExpression(failureStart: string, node: ts.CallExpression | ts.NewExpression): void {
+function walk(ctx: Lint.WalkContext<Options>, checker: ts.TypeChecker | undefined) {
+    const { allowTypeParameters, allowSingleArgument } = ctx.options;
+    function checkExpression(type: InvocationType, node: ts.CallExpression | ts.NewExpression): void {
         const functionName = AstUtils.getFunctionName(node);
         if (functionName === 'Array' && inRestrictedNamespace(node)) {
-            const failureString = failureStart + node.getText();
-            ctx.addFailureAt(node.getStart(), node.getWidth(), failureString);
+            const callArguments = node.arguments;
+            if (!allowSingleArgument || !callArguments || callArguments.length !== 1) {
+                const failureString = Rule.getReplaceFailureString(type, node.getText());
+                ctx.addFailureAt(node.getStart(), node.getWidth(), failureString);
+            } else {
+                // When typechecker is not available - allow any call with single expression
+                if (checker) {
+                    const argument = callArguments[0];
+                    const argumentType = checker.getTypeAtLocation(argument);
+                    if (!tsutils.isTypeAssignableToNumber(checker, argumentType)) {
+                        const failureString = Rule.getSingleParamFailureString(type, node.getText());
+                        ctx.addFailureAt(node.getStart(), node.getWidth(), failureString);
+                    }
+                }
+            }
         }
     }
 
@@ -87,11 +116,11 @@ function walk(ctx: Lint.WalkContext<Options>) {
         }
 
         if (tsutils.isNewExpression(node)) {
-            checkExpression(Rule.CONSTRUCTOR_FAILURE_STRING, node);
+            checkExpression('constructor', node);
         }
 
         if (tsutils.isCallExpression(node)) {
-            checkExpression(Rule.FUNCTION_FAILURE_STRING, node);
+            checkExpression('function', node);
         }
 
         return ts.forEachChild(node, cb);

diff --git a/tests/prefer-array-literal/allow-single-argument-typed/test.ts.lint b/tests/prefer-array-literal/allow-single-argument-typed/test.ts.lint
@@ -0,0 +1,73 @@
+let a: string[];
+
+let b: Array<string> = [];
+       ~~~~~~~~~~~~~       [type % ("Array<string>")]
+
+interface C {
+    myArray: Array<string>;
+             ~~~~~~~~~~~~~  [type % ("Array<string>")]
+}
+
+var d: Array<string>;
+       ~~~~~~~~~~~~~  [type % ("Array<string>")]
+
+function e(param: Array<number>) { }
+                  ~~~~~~~~~~~~~      [type % ("Array<number>")]
+
+var f = new Array();
+        ~~~~~~~~~~~  [constructor % ("new Array()")]
+
+var g = new Array(4, 5);
+        ~~~~~~~~~~~~~~~  [constructor % ("new Array(4, 5)")]
+
+var h = new Array(4);
+
+var i = Array(2);
+
+var j = new Array;
+        ~~~~~~~~~  [constructor % ("new Array")]
+
+// calls to Array function/constructor on global objects is forbidden
+var nc1 = window.Array(1);
+var nc2 = global.Array(1, 2);
+          ~~~~~~~~~~~~~~~~~~  [function % ("global.Array(1, 2)")]
+var nc3 = globalThis.Array('3');
+          ~~~~~~~~~~~~~~~~~~~~~  [single-argument % ("function","globalThis.Array('3')")]
+
+var nn1 = new window.Array(1);
+var nn2 = new global.Array(1, 2);
+          ~~~~~~~~~~~~~~~~~~~~~~  [constructor % ("new global.Array(1, 2)")]
+var nn3 = new globalThis.Array('3');
+          ~~~~~~~~~~~~~~~~~~~~~~~~~  [single-argument % ("constructor", "new globalThis.Array('3')")]
+
+// calls to Array function/constructor from namespaces are valid
+import { Types } from 'mongoose';
+export const foo: Types.Array<number> = new Types.Array();
+
+declare var num: number;
+declare var str: string;
+declare var unionA: number | Array<number>;
+                             ~~~~~~~~~~~~~  [type % ("Array<number>")]
+declare var unionF: number | (() => number);
+
+const t1 = Array(num);
+const t2 = Array(str);
+           ~~~~~~~~~~  [single-argument % ("function", "Array(str)")]
+const t3 = Array(unionA);
+           ~~~~~~~~~~~~~  [single-argument % ("function", "Array(unionA)")]
+const t3 = Array(unionF);
+           ~~~~~~~~~~~~~  [single-argument % ("function", "Array(unionF)")]
+const t4 = Array(num + 1);
+const t5 = Array(str + 1);
+           ~~~~~~~~~~~~~~  [single-argument % ("function", "Array(str + 1)")]
+const t6 = Array(10 + 1);
+const t7 = Array(10 + '1');
+           ~~~~~~~~~~~~~~~  [single-argument % ("function", "Array(10 + '1')")]
+const t8 = Array(1.5); // no error - limitation of typed rule
+const t9 = Array(-1); // no error - limitation of typed rule
+const t10 = Array(-num); // no error - limitation of typed rule
+
+[type]: Replace generic-typed Array with array literal: %s
+[constructor]: Replace Array constructor with an array literal: %s
+[function]: Replace Array function with an array literal: %s
+[single-argument]: To create an array of given length you should use non-negative integer. Otherwise replace Array %s with an array literal: %s
diff --git a/tests/prefer-array-literal/allow-single-argument-typed/tsconfig.json b/tests/prefer-array-literal/allow-single-argument-typed/tsconfig.json
@@ -0,0 +1,5 @@
+{
+    "compilerOptions": {
+        "target": "es5"
+    }
+}
diff --git a/tests/prefer-array-literal/allow-single-argument-typed/tslint.json b/tests/prefer-array-literal/allow-single-argument-typed/tslint.json
@@ -0,0 +1,5 @@
+{
+    "rules": {
+        "prefer-array-literal": [true, {"allow-single-argument": true}]
+    }
+}
diff --git a/tests/prefer-array-literal/allow-single-argument/test.ts.lint b/tests/prefer-array-literal/allow-single-argument/test.ts.lint
@@ -0,0 +1,66 @@
+let a: string[];
+
+let b: Array<string> = [];
+       ~~~~~~~~~~~~~       [type % ("Array<string>")]
+
+interface C {
+    myArray: Array<string>;
+             ~~~~~~~~~~~~~  [type % ("Array<string>")]
+}
+
+var d: Array<string>;
+       ~~~~~~~~~~~~~  [type % ("Array<string>")]
+
+function e(param: Array<number>) { }
+                  ~~~~~~~~~~~~~      [type % ("Array<number>")]
+
+var f = new Array();
+        ~~~~~~~~~~~  [constructor % ("new Array()")]
+
+var g = new Array(4, 5);
+        ~~~~~~~~~~~~~~~  [constructor % ("new Array(4, 5)")]
+
+var h = new Array(4);
+
+var i = Array(2);
+
+var j = new Array;
+        ~~~~~~~~~  [constructor % ("new Array")]
+
+// calls to Array function/constructor on global objects is forbidden
+var nc1 = window.Array(1);
+var nc2 = global.Array(1, 2);
+          ~~~~~~~~~~~~~~~~~~  [function % ("global.Array(1, 2)")]
+var nc3 = globalThis.Array('3'); // no error - limitation of untyped rule
+
+var nn1 = new window.Array(1);
+var nn2 = new global.Array(1, 2);
+          ~~~~~~~~~~~~~~~~~~~~~~  [constructor % ("new global.Array(1, 2)")]
+var nn3 = new globalThis.Array('3'); // no error - limitation of untyped rule
+
+// calls to Array function/constructor from namespaces are valid
+import { Types } from 'mongoose';
+export const foo: Types.Array<number> = new Types.Array();
+
+declare var num: number;
+declare var str: string;
+declare var unionA: number | Array<number>;
+                             ~~~~~~~~~~~~~  [type % ("Array<number>")]
+declare var unionF: number | (() => number);
+
+// no errors below - limitation of untyped rule
+const t1 = Array(num);
+const t2 = Array(str);
+const t3 = Array(unionA);
+const t3 = Array(unionF);
+const t4 = Array(num + 1);
+const t5 = Array(str + 1);
+const t6 = Array(10 + 1);
+const t7 = Array(10 + '1');
+const t8 = Array(1.5);
+const t9 = Array(-1);
+const t10 = Array(-num);
+
+[type]: Replace generic-typed Array with array literal: %s
+[constructor]: Replace Array constructor with an array literal: %s
+[function]: Replace Array function with an array literal: %s
diff --git a/tests/prefer-array-literal/allow-single-argument/tslint.json b/tests/prefer-array-literal/allow-single-argument/tslint.json
@@ -0,0 +1,5 @@
+{
+    "rules": {
+        "prefer-array-literal": [true, {"allow-single-argument": true}]
+    }
+}
diff --git a/tests/prefer-array-literal/allow-type-parameters/test.ts.lint b/tests/prefer-array-literal/allow-type-parameters/test.ts.lint
@@ -22,24 +22,55 @@ var h = new Array(4);
 var i = Array(2);
         ~~~~~~~~  [function % ("Array(2)")]
 
+var j = new Array;
+        ~~~~~~~~~  [constructor % ("new Array")]
+
 // calls to Array function/constructor on global objects is forbidden
 var nc1 = window.Array(1);
           ~~~~~~~~~~~~~~~  [function % ("window.Array(1)")]
-var nc2 = global.Array(2);
-          ~~~~~~~~~~~~~~~  [function % ("global.Array(2)")]
-var nc3 = globalThis.Array(3);
-          ~~~~~~~~~~~~~~~~~~~  [function % ("globalThis.Array(3)")]
+var nc2 = global.Array(1, 2);
+          ~~~~~~~~~~~~~~~~~~  [function % ("global.Array(1, 2)")]
+var nc3 = globalThis.Array('3');
+          ~~~~~~~~~~~~~~~~~~~~~  [function % ("globalThis.Array('3')")]
 
 var nn1 = new window.Array(1);
           ~~~~~~~~~~~~~~~~~~~  [constructor % ("new window.Array(1)")]
-var nn2 = new global.Array(2);
-          ~~~~~~~~~~~~~~~~~~~  [constructor % ("new global.Array(2)")]
-var nn3 = new globalThis.Array(3);
-          ~~~~~~~~~~~~~~~~~~~~~~~  [constructor % ("new globalThis.Array(3)")]
+var nn2 = new global.Array(1, 2);
+          ~~~~~~~~~~~~~~~~~~~~~~  [constructor % ("new global.Array(1, 2)")]
+var nn3 = new globalThis.Array('3');
+          ~~~~~~~~~~~~~~~~~~~~~~~~~  [constructor % ("new globalThis.Array('3')")]
 
 // calls to Array function/constructor from namespaces are valid
 import { Types } from 'mongoose';
 export const foo: Types.Array<number> = new Types.Array();
 
+declare var num: number;
+declare var str: string;
+declare var unionA: number | Array<number>;
+declare var unionF: number | (() => number);
+
+const t1 = Array(num);
+           ~~~~~~~~~~  [function % ("Array(num)")]
+const t2 = Array(str);
+           ~~~~~~~~~~  [function % ("Array(str)")]
+const t3 = Array(unionA);
+           ~~~~~~~~~~~~~  [function % ("Array(unionA)")]
+const t3 = Array(unionF);
+           ~~~~~~~~~~~~~  [function % ("Array(unionF)")]
+const t4 = Array(num + 1);
+           ~~~~~~~~~~~~~~  [function % ("Array(num + 1)")]
+const t5 = Array(str + 1);
+           ~~~~~~~~~~~~~~  [function % ("Array(str + 1)")]
+const t6 = Array(10 + 1);
+           ~~~~~~~~~~~~~  [function % ("Array(10 + 1)")]
+const t7 = Array(10 + '1');
+           ~~~~~~~~~~~~~~~  [function % ("Array(10 + '1')")]
+const t8 = Array(1.5);
+           ~~~~~~~~~~  [function % ("Array(1.5)")]
+const t9 = Array(-1);
+           ~~~~~~~~~  [function % ("Array(-1)")]
+const t10 = Array(-num);
+            ~~~~~~~~~~~  [function % ("Array(-num)")]
+
 [constructor]: Replace Array constructor with an array literal: %s
 [function]: Replace Array function with an array literal: %s
diff --git a/tests/prefer-array-literal/default/test.ts.lint b/tests/prefer-array-literal/default/test.ts.lint
@@ -26,25 +26,57 @@ var h = new Array(4);
 var i = Array(2);
         ~~~~~~~~  [function % ("Array(2)")]
 
+var j = new Array;
+        ~~~~~~~~~  [constructor % ("new Array")]
+
 // calls to Array function/constructor on global objects is forbidden
 var nc1 = window.Array(1);
           ~~~~~~~~~~~~~~~  [function % ("window.Array(1)")]
-var nc2 = global.Array(2);
-          ~~~~~~~~~~~~~~~  [function % ("global.Array(2)")]
-var nc3 = globalThis.Array(3);
-          ~~~~~~~~~~~~~~~~~~~  [function % ("globalThis.Array(3)")]
+var nc2 = global.Array(1, 2);
+          ~~~~~~~~~~~~~~~~~~  [function % ("global.Array(1, 2)")]
+var nc3 = globalThis.Array('3');
+          ~~~~~~~~~~~~~~~~~~~~~  [function % ("globalThis.Array('3')")]
 
 var nn1 = new window.Array(1);
           ~~~~~~~~~~~~~~~~~~~  [constructor % ("new window.Array(1)")]
-var nn2 = new global.Array(2);
-          ~~~~~~~~~~~~~~~~~~~  [constructor % ("new global.Array(2)")]
-var nn3 = new globalThis.Array(3);
-          ~~~~~~~~~~~~~~~~~~~~~~~  [constructor % ("new globalThis.Array(3)")]
+var nn2 = new global.Array(1, 2);
+          ~~~~~~~~~~~~~~~~~~~~~~  [constructor % ("new global.Array(1, 2)")]
+var nn3 = new globalThis.Array('3');
+          ~~~~~~~~~~~~~~~~~~~~~~~~~  [constructor % ("new globalThis.Array('3')")]
 
 // calls to Array function/constructor from namespaces are valid
 import { Types } from 'mongoose';
 export const foo: Types.Array<number> = new Types.Array();
 
+declare var num: number;
+declare var str: string;
+declare var unionA: number | Array<number>;
+                             ~~~~~~~~~~~~~  [type % ("Array<number>")]
+declare var unionF: number | (() => number);
+
+const t1 = Array(num);
+           ~~~~~~~~~~  [function % ("Array(num)")]
+const t2 = Array(str);
+           ~~~~~~~~~~  [function % ("Array(str)")]
+const t3 = Array(unionA);
+           ~~~~~~~~~~~~~  [function % ("Array(unionA)")]
+const t3 = Array(unionF);
+           ~~~~~~~~~~~~~  [function % ("Array(unionF)")]
+const t4 = Array(num + 1);
+           ~~~~~~~~~~~~~~  [function % ("Array(num + 1)")]
+const t5 = Array(str + 1);
+           ~~~~~~~~~~~~~~  [function % ("Array(str + 1)")]
+const t6 = Array(10 + 1);
+           ~~~~~~~~~~~~~  [function % ("Array(10 + 1)")]
+const t7 = Array(10 + '1');
+           ~~~~~~~~~~~~~~~  [function % ("Array(10 + '1')")]
+const t8 = Array(1.5);
+           ~~~~~~~~~~  [function % ("Array(1.5)")]
+const t9 = Array(-1);
+           ~~~~~~~~~  [function % ("Array(-1)")]
+const t10 = Array(-num);
+            ~~~~~~~~~~~  [function % ("Array(-num)")]
+
 [type]: Replace generic-typed Array with array literal: %s
 [constructor]: Replace Array constructor with an array literal: %s
 [function]: Replace Array function with an array literal: %s