Fix ArrayType to produce correct fixes (palantir#1533)

Fixes now parenthesize union, intersection and function types.
jonaskello · Sep 1, 2016 · 7d4c1d6 · 7d4c1d6
1 parent 9feb726
commit 7d4c1d6
Show file tree

Hide file tree

Showing 5 changed files with 52 additions and 6 deletions.
diff --git a/src/rules/arrayTypeRule.ts b/src/rules/arrayTypeRule.ts
@@ -37,10 +37,11 @@ class ArrayTypeWalker extends Lint.RuleWalker {
     public visitArrayType(node: ts.ArrayTypeNode) {
         if (this.hasOption(OPTION_GENERIC)) {
             const typeName = node.elementType;
+            const parens = typeName.kind === ts.SyntaxKind.ParenthesizedType ? 1 : 0;
             const fix = new Lint.Fix(Rule.metadata.ruleName, [
-                this.appendText(typeName.getStart(), "Array<"),
+                this.createReplacement(typeName.getStart(), parens, "Array<"),
                 // Delete the square brackets and replace with an angle bracket
-                this.createReplacement(typeName.getEnd(), node.getEnd() - typeName.getEnd(), ">"),
+                this.createReplacement(typeName.getEnd() - parens, node.getEnd() - typeName.getEnd() + parens, ">"),
             ]);
             this.addFailure(this.createFailure(node.getStart(), node.getWidth(), Rule.FAILURE_STRING_GENERIC, fix));
         }
@@ -59,13 +60,16 @@ class ArrayTypeWalker extends Lint.RuleWalker {
                     this.createReplacement(node.getStart(), node.getWidth(), "any[]"),
                 ]);
             } else if (typeArgs && typeArgs.length === 1) {
-                const typeStart = typeArgs[0].getStart();
-                const typeEnd = typeArgs[0].getEnd();
+                const type = typeArgs[0];
+                const typeStart = type.getStart();
+                const typeEnd = type.getEnd();
+                const parens = type.kind === ts.SyntaxKind.UnionType ||
+                    type.kind === ts.SyntaxKind.FunctionType || type.kind === ts.SyntaxKind.IntersectionType;
                 fix = new Lint.Fix(Rule.metadata.ruleName, [
                     // Delete Array and the first angle bracket
-                    this.deleteText(node.getStart(), typeStart - node.getStart()),
+                    this.createReplacement(node.getStart(), typeStart - node.getStart(), parens ? "(" : ""),
                     // Delete the last angle bracket and replace with square brackets
-                    this.createReplacement(typeEnd, node.getEnd() - typeEnd, "[]"),
+                    this.createReplacement(typeEnd, node.getEnd() - typeEnd, (parens ? ")" : "") + "[]"),
                 ]);
             }
             this.addFailure(this.createFailure(node.getStart(), node.getWidth(), Rule.FAILURE_STRING_ARRAY, fix));

diff --git a/test/rules/array-type/array/test.ts.fix b/test/rules/array-type/array/test.ts.fix
@@ -27,3 +27,12 @@ function barFunction(bar: ArrayClass<String>[]) {
 function bazFunction(baz: Arr<ArrayClass<String>>) {
     return baz.map(e => e.baz);
 }
+
+let fooVar: ((c: number) => number)[];
+let barVar: ((c: number) => number)[];
+
+type fooUnion = (string|number|boolean)[];
+type barUnion = (string|number|boolean)[];
+
+type fooIntersection = (string & number)[];
+type barIntersection = (string & number)[];
diff --git a/test/rules/array-type/array/test.ts.lint b/test/rules/array-type/array/test.ts.lint
@@ -36,3 +36,15 @@ function barFunction(bar: ArrayClass<String>[]) {
 function bazFunction(baz: Arr<ArrayClass<String>>) {
     return baz.map(e => e.baz);
 }
+
+let fooVar: Array<(c: number) => number>;
+            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~  [Array type using 'Array<T>' is forbidden. Use 'T[]' instead.]
+let barVar: ((c: number) => number)[];
+
+type fooUnion = Array<string|number|boolean>;
+                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~  [Array type using 'Array<T>' is forbidden. Use 'T[]' instead.]
+type barUnion = (string|number|boolean)[];
+
+type fooIntersection = Array<string & number>;
+                       ~~~~~~~~~~~~~~~~~~~~~~  [Array type using 'Array<T>' is forbidden. Use 'T[]' instead.]
+type barIntersection = (string & number)[];
diff --git a/test/rules/array-type/generic/test.ts.fix b/test/rules/array-type/generic/test.ts.fix
@@ -27,3 +27,12 @@ function barFunction(bar: Array<ArrayClass<String>>) {
 function bazFunction(baz: Arr<ArrayClass<String>>) {
     return baz.map(e => e.baz);
 }
+
+let fooVar: Array<(c: number) => number>;
+let barVar: Array<(c: number) => number>;
+
+type fooUnion = Array<string|number|boolean>;
+type barUnion = Array<string|number|boolean>;
+
+type fooIntersection = Array<string & number>;
+type barIntersection = Array<string & number>;
diff --git a/test/rules/array-type/generic/test.ts.lint b/test/rules/array-type/generic/test.ts.lint
@@ -34,3 +34,15 @@ function barFunction(bar: ArrayClass<String>[]) {
 function bazFunction(baz: Arr<ArrayClass<String>>) {
     return baz.map(e => e.baz);
 }
+
+let fooVar: Array<(c: number) => number>;
+let barVar: ((c: number) => number)[];
+            ~~~~~~~~~~~~~~~~~~~~~~~~~  [Array type using 'T[]' is forbidden. Use 'Array<T>' instead.]
+
+type fooUnion = Array<string|number|boolean>;
+type barUnion = (string|number|boolean)[];
+                ~~~~~~~~~~~~~~~~~~~~~~~~~  [Array type using 'T[]' is forbidden. Use 'Array<T>' instead.]
+
+type fooIntersection = Array<string & number>;
+type barIntersection = (string & number)[];
+                       ~~~~~~~~~~~~~~~~~~~  [Array type using 'T[]' is forbidden. Use 'Array<T>' instead.]