Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exception for readonly-array rule with rest parameters #73

Closed
yenbekbay opened this issue Feb 14, 2018 · 2 comments · Fixed by #94
Closed

Exception for readonly-array rule with rest parameters #73

yenbekbay opened this issue Feb 14, 2018 · 2 comments · Fixed by #94

Comments

@yenbekbay
Copy link

yenbekbay commented Feb 14, 2018

I have the readonly-array rule enabled as follows:

{
  "rules": {
    ...
    "readonly-array": true,
    ...
  }
}

For the following piece of code, the rule throws an error:

type SomeFn = (...args: string[]) => void;
                        ^^^
[tslint] Only ReadonlyArray allowed. (readonly-array)

However, if I do fix this error and replace string[] with ReadonlyArray<string>, the code becomes invalid:

type SomeFn = (...args: ReadonlyArray<string>) => void;
                        ^^^
[ts] A rest parameter must be of an array type.

As you can see, the TypeScript compiler requires that rest arguments are typed as plain arrays.

Proposal

Add an exception to the readonly-array rule for this specific case.

Environment

Executable Version
tslint --version 5.9.1
tsc --version 2.7.1
node --version 9.5.0
@jonaskello
Copy link
Owner

It would be interesting to know the typescript team's opinion on this. Is this something they will fix or not. Do you know if there exist an issue about this in the typescript repo?

I cannot see a reason why rest params should need to be mutable so IMO it should be fixed in typescript. If it is going to be fixed, then we should probably add it as an option to the readonly-array rule instead, something like ignore-rest-param. If it is never going to get fixed I guess it could be added by default.

@whatisaphone
Copy link

Here is the corresponding TypeScript issue.

aboyton pushed a commit to aboyton/tslint-immutable that referenced this issue Oct 7, 2018
This fixes jonaskello#73 while we wait for TypeScript to implement a proper fix.

Rather than repeating all the test cases from default that this should not break it creates a new test either way for functions and symlinks the others.
aboyton pushed a commit to aboyton/tslint-immutable that referenced this issue Oct 7, 2018
This fixes jonaskello#73 while we wait for TypeScript to implement a proper fix.

Rather than repeating all the test cases from default that this should not break it creates a new test either way for functions and symlinks the others.
jonaskello pushed a commit that referenced this issue Oct 7, 2018
This fixes #73 while we wait for TypeScript to implement a proper fix.

Rather than repeating all the test cases from default that this should not break it creates a new test either way for functions and symlinks the others.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants