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

Consider merging the readonly-* rules #10

Closed
jonaskello opened this issue Apr 5, 2017 · 5 comments
Closed

Consider merging the readonly-* rules #10

jonaskello opened this issue Apr 5, 2017 · 5 comments

Comments

@jonaskello
Copy link
Owner

Instead of having separate readonly-interface, readonly-indexer, and readonly-array rules we could merge them into one rule readonly which could have options for ignore-interface, ignore-indexer, ignore-array. This would probably speed things up a bit as we only need to traverse the AST once. Also having fewer rules could be a considered an advantage.

@jonaskello jonaskello changed the title Consider merging the readonly-* rules to readonly Consider merging the readonly-* rules Apr 5, 2017
@jonaskello
Copy link
Owner Author

It would probably make more sense to merge readonly-interface and readonly-indexer since both are based on the readonly keyword. But readonly-array is based on the ReadonlyArray<T> type so it should probably be its own rule.

@jonaskello
Copy link
Owner Author

jonaskello commented May 13, 2017

So we could have a readonly-keyword rule that would enforce the use of the readonly keyword in all places it is allowed. So the first thing is to find all places wherereadonly is allowed. However I cannot find an updated version of the typescript language specification that includes this. So I'll try to collect them here:

Interface members (including indexers):

interface Foo {
  readonly bar: string,
  readonly [key: string]: string,
}

Object type alias members (including indexers):

type Foo = {
  readonly bar: string,
  readonly [key: string]: string,
}

Object type literals (including indexers):

let foo: { readonly bar: string, readonly [key: string]: string}; 

Class members (we don't want to support this?)

class Circle {
  readonly radius: number;
}

@jonaskello
Copy link
Owner Author

It seems that in all of the above cases, the type of nodes allowing readonly are the same. They are either PropertySignature or IndexSignature.

So maybe a check for readonly on all PropertySignature and IndexSignature nodes will work. And perhaps it can be done regardless of where those nodes are found.

@jonaskello
Copy link
Owner Author

jonaskello commented May 13, 2017

This is the only official mention of where readonly is allowed I can find. It seems to support the theory that readonly is allowed on PropertySignature and IndexSignature nodes:

A property or index signature can now be declared with the readonly modifier

@jonaskello
Copy link
Owner Author

Continued in #31.

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

No branches or pull requests

1 participant