Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

array-type rule requires array in places that are less readable #1526

Closed
alexeagle opened this issue Aug 29, 2016 · 2 comments
Closed

array-type rule requires array in places that are less readable #1526

alexeagle opened this issue Aug 29, 2016 · 2 comments

Comments

@alexeagle
Copy link
Contributor

Bug Report

  • TSLint version: HEAD

TypeScript code being linted

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

with tslint.json configuration:

(include if relevant)
{
  "rules": {
    "array-type": [true, "array"]
  }
}

Actual behavior

Replaces with

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

Expected behavior

These are more readable in the original form with Array<T>.

@mprobst explains:
precisely, if your type in the grammar (https://github.com/Microsoft/TypeScript/blob/master/doc/spec.md#a1-types) is not just a PredefinedType (number, string, etc) or a TypeReference (some.namepsace.MyFunkyType), then you should be allowed to use Array.

@jkillian
Copy link
Contributor

Sounds like you'd like to follow the same guidelines that @danvk mentioned here:

Not necessarily something that needs to be implemented in this PR, but our style guide is a little more nuanced than either of these options. For simple (non-generic) types we use T[]. If the type becomes more complex, we switch to Array<> for clarity, e.g. Array<{[key: string]: T}>.

@alexeagle
Copy link
Contributor Author

Yes, exactly, I read that comment on the original issue but didn't think deeply enough. @ScottSWu do you want to do the "array-type": [true, "array-for-simple-types"] or shall I?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants