Skip to content

Commit

Permalink
Clean up array-type and consider 'object' as a simple type (palantir#…
Browse files Browse the repository at this point in the history 
…2353)
  • Loading branch information
@andy-hanson @adidahiya
andy-hanson authored and adidahiya committed Mar 29, 2017
1 parent 31c4f38 commit 0658694
Show file tree
Hide file tree
Showing 8 changed files with 147 additions and 99 deletions.
164 changes: 100 additions & 64 deletions src/rules/arrayTypeRule.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import * as ts from "typescript";

import * as Lint from "../index";

type Option = "array" | "generic" | "array-simple";
const OPTION_ARRAY = "array";
const OPTION_GENERIC = "generic";
const OPTION_ARRAY_SIMPLE = "array-simple";
Expand Down Expand Up @@ -51,83 +52,118 @@ export class Rule extends Lint.Rules.AbstractRule {
public static FAILURE_STRING_GENERIC_SIMPLE = "Array type using 'T[]' is forbidden for non-simple types. Use 'Array<T>' instead.";

public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
const alignWalker = new ArrayTypeWalker(sourceFile, this.getOptions());
return this.applyWithWalker(alignWalker);
return this.applyWithFunction(sourceFile, walk, this.ruleArguments[0]);
}
}

class ArrayTypeWalker extends Lint.RuleWalker {
public visitArrayType(node: ts.ArrayTypeNode) {
const typeName = node.elementType;
if (this.hasOption(OPTION_GENERIC) || this.hasOption(OPTION_ARRAY_SIMPLE) && !this.isSimpleType(typeName)) {
const failureString = this.hasOption(OPTION_GENERIC) ? Rule.FAILURE_STRING_GENERIC : Rule.FAILURE_STRING_GENERIC_SIMPLE;
const parens = typeName.kind === ts.SyntaxKind.ParenthesizedType ? 1 : 0;
// Add a space if the type is preceded by 'as' and the node has no leading whitespace
const space = !parens && node.parent!.kind === ts.SyntaxKind.AsExpression &&
node.getStart() === node.getFullStart() ? " " : "";
const fix = [
this.createReplacement(typeName.getStart(), parens, space + "Array<"),
// Delete the square brackets and replace with an angle bracket
this.createReplacement(typeName.getEnd() - parens, node.getEnd() - typeName.getEnd() + parens, ">"),
];
this.addFailureAtNode(node, failureString, fix);
function walk(ctx: Lint.WalkContext<Option>): void {
const { sourceFile, options: option } = ctx;
return ts.forEachChild(sourceFile, function cb(node): void {
switch (node.kind) {
case ts.SyntaxKind.ArrayType:
checkArrayType(node as ts.ArrayTypeNode);
break;
case ts.SyntaxKind.TypeReference:
checkTypeReference(node as ts.TypeReferenceNode);
break;
}
return ts.forEachChild(node, cb);
});

function checkArrayType(node: ts.ArrayTypeNode): void {
const { elementType, parent } = node;
if (option === "array" || option === "array-simple" && isSimpleType(elementType)) {
return;
}

super.visitArrayType(node);
const failureString = option === "generic" ? Rule.FAILURE_STRING_GENERIC : Rule.FAILURE_STRING_GENERIC_SIMPLE;
const parens = elementType.kind === ts.SyntaxKind.ParenthesizedType ? 1 : 0;
// Add a space if the type is preceded by 'as' and the node has no leading whitespace
const space = !parens && parent!.kind === ts.SyntaxKind.AsExpression && node.getStart() === node.getFullStart();
const fix = [
new Lint.Replacement(elementType.getStart(), parens, (space ? " " : "") + "Array<"),
// Delete the square brackets and replace with an angle bracket
Lint.Replacement.replaceFromTo(elementType.getEnd() - parens, node.getEnd(), ">"),
];
ctx.addFailureAtNode(node, failureString, fix);
}

public visitTypeReference(node: ts.TypeReferenceNode) {
const typeName = node.typeName.getText();
if (typeName === "Array" && (this.hasOption(OPTION_ARRAY) || this.hasOption(OPTION_ARRAY_SIMPLE))) {
const failureString = this.hasOption(OPTION_ARRAY) ? Rule.FAILURE_STRING_ARRAY : Rule.FAILURE_STRING_ARRAY_SIMPLE;
const typeArgs = node.typeArguments;
if (!typeArgs || typeArgs.length === 0) {
// Create an 'any' array
const fix = this.createReplacement(node.getStart(), node.getWidth(), "any[]");
this.addFailureAtNode(node, failureString, fix);
} else if (typeArgs && typeArgs.length === 1 && (!this.hasOption(OPTION_ARRAY_SIMPLE) || this.isSimpleType(typeArgs[0]))) {
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;
const fix = [
// Delete Array and the first angle bracket
this.createReplacement(node.getStart(), typeStart - node.getStart(), parens ? "(" : ""),
// Delete the last angle bracket and replace with square brackets
this.createReplacement(typeEnd, node.getEnd() - typeEnd, (parens ? ")" : "") + "[]"),
];
this.addFailureAtNode(node, failureString, fix);
}
function checkTypeReference(node: ts.TypeReferenceNode): void {
const { typeName, typeArguments } = node;

if (option === "generic" || !isArrayIdentifier(typeName)) {
return;
}

super.visitTypeReference(node);
const failureString = option === "array" ? Rule.FAILURE_STRING_ARRAY : Rule.FAILURE_STRING_ARRAY_SIMPLE;
if (!typeArguments || typeArguments.length === 0) {
// Create an 'any' array
const fix = Lint.Replacement.replaceFromTo(node.getStart(), node.getEnd(), "any[]");
ctx.addFailureAtNode(node, failureString, fix);
return;
}

if (typeArguments.length !== 1 || (option === "array-simple" && !isSimpleType(typeArguments[0]))) {
return;
}

const type = typeArguments[0];
const parens = typeNeedsParentheses(type);
const fix = [
// Delete 'Array<'
Lint.Replacement.replaceFromTo(node.getStart(), type.getStart(), parens ? "(" : ""),
// Delete '>' and replace with '[]
Lint.Replacement.replaceFromTo(type.getEnd(), node.getEnd(), parens ? ")[]" : "[]"),
];
ctx.addFailureAtNode(node, failureString, fix);
}
}

private isSimpleType(nodeType: ts.TypeNode) {
switch (nodeType.kind) {
case ts.SyntaxKind.AnyKeyword:
case ts.SyntaxKind.ArrayType:
case ts.SyntaxKind.BooleanKeyword:
case ts.SyntaxKind.NullKeyword:
case ts.SyntaxKind.UndefinedKeyword:
case ts.SyntaxKind.NumberKeyword:
case ts.SyntaxKind.StringKeyword:
case ts.SyntaxKind.SymbolKeyword:
case ts.SyntaxKind.VoidKeyword:
case ts.SyntaxKind.NeverKeyword:
function typeNeedsParentheses(type: ts.TypeNode): boolean {
switch (type.kind) {
case ts.SyntaxKind.UnionType:
case ts.SyntaxKind.FunctionType:
case ts.SyntaxKind.IntersectionType:
case ts.SyntaxKind.TypeOperator:
return true;
default:
return false;
}
}

function isArrayIdentifier(name: ts.EntityName) {
return name.kind === ts.SyntaxKind.Identifier && name.text === "Array";
}

function isSimpleType(nodeType: ts.TypeNode): boolean {
switch (nodeType.kind) {
case ts.SyntaxKind.AnyKeyword:
case ts.SyntaxKind.ArrayType:
case ts.SyntaxKind.BooleanKeyword:
case ts.SyntaxKind.NullKeyword:
case ts.SyntaxKind.ObjectKeyword:
case ts.SyntaxKind.UndefinedKeyword:
case ts.SyntaxKind.NumberKeyword:
case ts.SyntaxKind.StringKeyword:
case ts.SyntaxKind.SymbolKeyword:
case ts.SyntaxKind.VoidKeyword:
case ts.SyntaxKind.NeverKeyword:
return true;
case ts.SyntaxKind.TypeReference:
// TypeReferences must be non-generic or be another Array with a simple type
const { typeName, typeArguments } = nodeType as ts.TypeReferenceNode;
if (!typeArguments) {
return true;
case ts.SyntaxKind.TypeReference:
// TypeReferences must be non-generic or be another Array with a simple type
const node = nodeType as ts.TypeReferenceNode;
const typeArgs = node.typeArguments;
if (!typeArgs || typeArgs.length === 0 || node.typeName.getText() === "Array" && this.isSimpleType(typeArgs[0])) {
}
switch (typeArguments.length) {
case 0:
return true;
} else {
case 1:
return typeName.kind === ts.SyntaxKind.Identifier && typeName.text === "Array" && isSimpleType(typeArguments[0]);
default:
return false;
}
default:
return false;
}
}
default:
return false;
}
}
2 changes: 2 additions & 0 deletions test/rules/array-type/array-simple/test.ts.fix
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
let x: undefined[] = [undefined] as undefined[];
let xx: object[] = [];
let y: string[] = <string[]>["2"];
let z: any[] = [3, "4"];

Expand Down Expand Up @@ -51,3 +52,4 @@ let w: Array<fooName.BazType<string>> = [["baz"]];
interface FooInterface {
'.bar': {baz: string[];};
}

20 changes: 12 additions & 8 deletions test/rules/array-type/array-simple/test.ts.lint
View file
Original file line number Diff line number Diff line change
@@ -1,28 +1,30 @@
let x: Array<undefined> = [undefined] as undefined[];
~~~~~~~~~~~~~~~~ [Array type using 'Array<T>' is forbidden for simple types. Use 'T[]' instead.]
~~~~~~~~~~~~~~~~ [0]
let xx: Array<object> = [];
~~~~~~~~~~~~~ [0]
let y: string[] = <Array<string>>["2"];
~~~~~~~~~~~~~ [Array type using 'Array<T>' is forbidden for simple types. Use 'T[]' instead.]
~~~~~~~~~~~~~ [0]
let z: Array = [3, "4"];
~~~~~ [Array type using 'Array<T>' is forbidden for simple types. Use 'T[]' instead.]
~~~~~ [0]

let xx: Array<Array<number>> = [[1, 2], [3]];
~~~~~~~~~~~~~~~~~~~~ [Array type using 'Array<T>' is forbidden for simple types. Use 'T[]' instead.]
~~~~~~~~~~~~~ [Array type using 'Array<T>' is forbidden for simple types. Use 'T[]' instead.]
~~~~~~~~~~~~~~~~~~~~ [0]
~~~~~~~~~~~~~ [0]
let yy: number[][] = [[4, 5], [6]];

let ya = [[1, "2"]] as[number, string][];
~~~~~~~~~~~~~~~~~~ [Array type using 'T[]' is forbidden for non-simple types. Use 'Array<T>' instead.]

type Arr<T> = Array<T>;
~~~~~~~~ [Array type using 'Array<T>' is forbidden for simple types. Use 'T[]' instead.]
~~~~~~~~ [0]

// Ignore user defined aliases
let yyyy: Arr<Array<Arr<string>>[]> = [[[["2"]]]];
~~~~~~~~~~~~~~~~~~~~ [Array type using 'T[]' is forbidden for non-simple types. Use 'Array<T>' instead.]

interface ArrayClass<T> {
foo: Array<T>;
~~~~~~~~ [Array type using 'Array<T>' is forbidden for simple types. Use 'T[]' instead.]
~~~~~~~~ [0]
bar: T[];
baz: Arr<T>;
}
Expand Down Expand Up @@ -58,11 +60,13 @@ namespace fooName {
}

let v: Array<fooName.BarType> = [{ bar: "bar" }];
~~~~~~~~~~~~~~~~~~~~~~ [Array type using 'Array<T>' is forbidden for simple types. Use 'T[]' instead.]
~~~~~~~~~~~~~~~~~~~~~~ [0]

let w: fooName.BazType<string>[] = [["baz"]];
~~~~~~~~~~~~~~~~~~~~~~~~~ [Array type using 'T[]' is forbidden for non-simple types. Use 'Array<T>' instead.]

interface FooInterface {
'.bar': {baz: string[];};
}

[0]: Array type using 'Array<T>' is forbidden for simple types. Use 'T[]' instead.
1 change: 1 addition & 0 deletions test/rules/array-type/array/test.ts.fix
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,4 @@ type barIntersection = (string & number)[];
interface FooInterface {
'.bar': {baz: string[];};
}

26 changes: 14 additions & 12 deletions test/rules/array-type/array/test.ts.lint
View file
Original file line number Diff line number Diff line change
@@ -1,33 +1,33 @@
let x: Array<undefined> = [undefined] as undefined[];
~~~~~~~~~~~~~~~~ [Array type using 'Array<T>' is forbidden. Use 'T[]' instead.]
~~~~~~~~~~~~~~~~ [0]
let y: string[] = <Array<string>>["2"];
~~~~~~~~~~~~~ [Array type using 'Array<T>' is forbidden. Use 'T[]' instead.]
~~~~~~~~~~~~~ [0]
let z: Array = [3, "4"];
~~~~~ [Array type using 'Array<T>' is forbidden. Use 'T[]' instead.]
~~~~~ [0]

let xx: Array<Array<number>> = [[1, 2], [3]];
~~~~~~~~~~~~~~~~~~~~ [Array type using 'Array<T>' is forbidden. Use 'T[]' instead.]
~~~~~~~~~~~~~ [Array type using 'Array<T>' is forbidden. Use 'T[]' instead.]
~~~~~~~~~~~~~~~~~~~~ [0]
~~~~~~~~~~~~~ [0]
let yy: number[][] = [[4, 5], [6]];

let ya = [[1, "2"]] as[number, string][];

type Arr<T> = Array<T>;
~~~~~~~~ [Array type using 'Array<T>' is forbidden. Use 'T[]' instead.]
~~~~~~~~ [0]

// Ignore user defined aliases
let yyyy: Arr<Array<Arr<string>>[]> = [[[["2"]]]];
~~~~~~~~~~~~~~~~~~ [Array type using 'Array<T>' is forbidden. Use 'T[]' instead.]
~~~~~~~~~~~~~~~~~~ [0]

interface ArrayClass<T> {
foo: Array<T>;
~~~~~~~~ [Array type using 'Array<T>' is forbidden. Use 'T[]' instead.]
~~~~~~~~ [0]
bar: T[];
baz: Arr<T>;
}

function fooFunction(foo: Array<ArrayClass<string>>) {
~~~~~~~~~~~~~~~~~~~~~~~~~ [Array type using 'Array<T>' is forbidden. Use 'T[]' instead.]
~~~~~~~~~~~~~~~~~~~~~~~~~ [0]
return foo.map(e => e.foo);
}

Expand All @@ -40,17 +40,19 @@ function bazFunction(baz: Arr<ArrayClass<String>>) {
}

let fooVar: Array<(c: number) => number>;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Array type using 'Array<T>' is forbidden. Use 'T[]' instead.]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0]
let barVar: ((c: number) => number)[];

type fooUnion = Array<string|number|boolean>;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Array type using 'Array<T>' is forbidden. Use 'T[]' instead.]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0]
type barUnion = (string|number|boolean)[];

type fooIntersection = Array<string & number>;
~~~~~~~~~~~~~~~~~~~~~~ [Array type using 'Array<T>' is forbidden. Use 'T[]' instead.]
~~~~~~~~~~~~~~~~~~~~~~ [0]
type barIntersection = (string & number)[];

interface FooInterface {
'.bar': {baz: string[];};
}

[0]: Array type using 'Array<T>' is forbidden. Use 'T[]' instead.
1 change: 1 addition & 0 deletions test/rules/array-type/generic/test.ts.fix
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,4 @@ type barIntersection = Array<string & number>;
interface FooInterface {
'.bar': {baz: Array<string>;};
}

26 changes: 14 additions & 12 deletions test/rules/array-type/generic/test.ts.lint
View file
Original file line number Diff line number Diff line change
@@ -1,27 +1,27 @@
let x: Array<number> = [1] as number[];
~~~~~~~~ [Array type using 'T[]' is forbidden. Use 'Array<T>' instead.]
~~~~~~~~ [0]
let y: string[] = <Array<string>>["2"];
~~~~~~~~ [Array type using 'T[]' is forbidden. Use 'Array<T>' instead.]
~~~~~~~~ [0]
let z: Array = [3, "4"];

let xx: Array<Array<number>> = [[1, 2], [3]];
let yy: number[][] = [[4, 5], [6]];
~~~~~~~~~~ [Array type using 'T[]' is forbidden. Use 'Array<T>' instead.]
~~~~~~~~ [Array type using 'T[]' is forbidden. Use 'Array<T>' instead.]
~~~~~~~~~~ [0]
~~~~~~~~ [0]

let ya = [[1, "2"]] as[number, string][];
~~~~~~~~~~~~~~~~~~ [Array type using 'T[]' is forbidden. Use 'Array<T>' instead.]
~~~~~~~~~~~~~~~~~~ [0]

type Arr<T> = Array<T>;

// Ignore user defined aliases
let yyyy: Arr<Array<Arr<string>>[]> = [[[["2"]]]];
~~~~~~~~~~~~~~~~~~~~ [Array type using 'T[]' is forbidden. Use 'Array<T>' instead.]
~~~~~~~~~~~~~~~~~~~~ [0]

interface ArrayClass<T> {
foo: Array<T>;
bar: T[];
~~~ [Array type using 'T[]' is forbidden. Use 'Array<T>' instead.]
~~~ [0]
baz: Arr<T>;
}

Expand All @@ -30,7 +30,7 @@ function fooFunction(foo: Array<ArrayClass<string>>) {
}

function barFunction(bar: ArrayClass<String>[]) {
~~~~~~~~~~~~~~~~~~~~ [Array type using 'T[]' is forbidden. Use 'Array<T>' instead.]
~~~~~~~~~~~~~~~~~~~~ [0]
return bar.map(e => e.bar);
}

Expand All @@ -40,17 +40,19 @@ function bazFunction(baz: Arr<ArrayClass<String>>) {

let fooVar: Array<(c: number) => number>;
let barVar: ((c: number) => number)[];
~~~~~~~~~~~~~~~~~~~~~~~~~ [Array type using 'T[]' is forbidden. Use 'Array<T>' instead.]
~~~~~~~~~~~~~~~~~~~~~~~~~ [0]

type fooUnion = Array<string|number|boolean>;
type barUnion = (string|number|boolean)[];
~~~~~~~~~~~~~~~~~~~~~~~~~ [Array type using 'T[]' is forbidden. Use 'Array<T>' instead.]
~~~~~~~~~~~~~~~~~~~~~~~~~ [0]

type fooIntersection = Array<string & number>;
type barIntersection = (string & number)[];
~~~~~~~~~~~~~~~~~~~ [Array type using 'T[]' is forbidden. Use 'Array<T>' instead.]
~~~~~~~~~~~~~~~~~~~ [0]

interface FooInterface {
'.bar': {baz: string[];};
~~~~~~~~ [Array type using 'T[]' is forbidden. Use 'Array<T>' instead.]
~~~~~~~~ [0]
}

[0]: Array type using 'T[]' is forbidden. Use 'Array<T>' instead.
Loading

0 comments on commit 0658694

Please sign in to comment.