new array-type rule produces the wrong fix in a nested expression

[alexeagle]

Bug Report

• TSLint version: HEAD
• TypeScript version: nightly
• Running TSLint via: Node.js API

TypeScript code being linted

type B<T> = T;
class Before {
    queue: Array<(c: number) => B<any>>;
}

class After {
    queue: (c: number) => B<any>[];
}

let b = new Before().queue;
new After().queue = b;

http://www.typescriptlang.org/play/#src=type%20B%3CT%3E%20%3D%20T%3B%0Aclass%20Before%20%7B%0A%09queue%3A%20Array%3C(c%3A%20number)%20%3D%3E%20B%3Cany%3E%3E%3B%0A%7D%0A%0Aclass%20After%20%7B%0A%09queue%3A%20(c%3A%20number)%20%3D%3E%20B%3Cany%3E%5B%5D%3B%0A%7D%0A%0Alet%20b%20%3D%20new%20Before().queue%3B%0Anew%20After().queue%20%3D%20b%3B

the suggested fix changes it from an array of functions to a function that returns an array.

cc @ScottSWu

[alexeagle]

another example: TableRow<Array<string|number|null>> -> TableRow<string|number|null[]>

[ScottSWu]

Adding parenthesis to union types, intersection types and function types should do it. Are there any others?

[alexeagle]

I can't think of any other cases. Probably a perusal of the grammar would let us prove it.

If you have a commit with a fix, I can patch it and see if I find any other cases in Google's TS code.

[ScottSWu]

Try https://github.com/ScottSWu/tslint/blob/array_type/src/rules/arrayTypeRule.ts

[alexeagle]

Thanks Scott, I don't see any more incorrect replacements.

But with some more discussion here, we don't think these are very readable:

{length: number, deferred: Deferred<any[]>}[]
(Document|string)[]
((event: Event) => void)[]
TableRow<(string|number|null)[]>

Maybe I'll file a suggestion that the array-type check should not fire when the T is more than a dotted-identifier.

[alexeagle]

Filed #1526 which is an alternative to this one.

[jkillian]

Seems to me that it makes sense to fix this bug (by adding parens as mentioned above). Then #1526 could be a new option (and it could also be the default option).