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

feat(eslint-plugin): [prefer-readonly-parameter-types] added an optional type allowlist #4436

Merged
merged 79 commits into from Mar 20, 2023
Merged
Show file tree
Hide file tree
Changes from 62 commits
Commits
Show all changes
79 commits
Select commit Hold shift + click to select a range
453ac54
feat(eslint-plugin): [prefer-readonly-parameter-types] Added an optio…
marekdedic Jan 12, 2022
e0a04d7
chore(eslint-plugin): [prefer-readonly-parameter-types] whitelist -> …
marekdedic Jan 13, 2022
06a0631
Merge branch 'main' into prefer-readonly-parameter-types-whitelist
marekdedic Jan 13, 2022
618fbd1
feat(eslint-plugin): [prefer-readonly-parameter-types] Split the allo…
marekdedic Jan 13, 2022
17d013e
fix(eslint-plugin): [prefer-readonly-parameter-types] Fixed lint issu…
marekdedic Jan 13, 2022
6fd713d
fix(eslint-plugin): [prefer-readonly-parameter-types] Using allowlist…
marekdedic Jan 15, 2022
b8cdd5c
fix(eslint-plugin): [prefer-readonly-parameter-types] Passing interna…
marekdedic Jan 15, 2022
4ab1f5f
fix(eslint-plugin): [prefer-readonly-parameter-types] Decoupled optio…
marekdedic Jan 15, 2022
49713c8
feat(eslint-plugin): [prefer-readonly-parameter-types] Added tests
marekdedic Jan 16, 2022
6d842ec
fix(eslint-plugin): [prefer-readonly-parameter-types] Added missing d…
marekdedic Jan 16, 2022
2734b7a
docs(eslint-plugin): [prefer-readonly-parameter-types] Added docs for…
marekdedic Jan 16, 2022
d4fa56b
Merge branch 'main' into prefer-readonly-parameter-types-whitelist
marekdedic Jan 17, 2022
3df00ed
fix(eslint-plugin): [prefer-readonly-parameter-types] Fixed regressio…
marekdedic Jan 17, 2022
8188084
Merge branch 'main' into prefer-readonly-parameter-types-whitelist
marekdedic Jan 24, 2022
53aac78
feat(eslint-plugin): [prefer-readonly-parameter-types] Merged excepti…
marekdedic Jan 24, 2022
efd4140
feat(eslint-plugin): [prefer-readonly-parameter-types] Added a schema…
marekdedic Jan 24, 2022
0504608
chore(eslint-plugin): [prefer-readonly-parameter-types] Split TypeAll…
marekdedic Jan 24, 2022
7a6c943
docs(eslint-plugin): [prefer-readonly-parameter-types] Updated docs f…
marekdedic Jan 24, 2022
f2acfaa
docs(eslint-plugin): [prefer-readonly-parameter-types] Fixed allowlis…
marekdedic Jan 24, 2022
b178a21
test(eslint-plugin): [prefer-readonly-parameter-types] Added tests fo…
marekdedic Jan 24, 2022
e8af7cb
chore(eslint-plugin): [prefer-readonly-parameter-types] Deduplicated …
marekdedic Jan 25, 2022
5e18240
fix(eslint-plugin): [prefer-readonly-parameter-types] Added back read…
marekdedic Jan 25, 2022
59371e7
chore(eslint-plugin): [prefer-readonly-parameter-types] Removed defau…
marekdedic Jan 26, 2022
3eb89b6
docs(eslint-plugin): [prefer-readonly-parameter-types] Fixed default …
marekdedic Jan 27, 2022
766a2e7
test(eslint-plugin): [prefer-readonly-parameter-types] Not using DOM …
marekdedic Jan 27, 2022
854dcac
chore(eslint-plugin): [prefer-readonly-parameter-types] Using propert…
marekdedic Jan 29, 2022
0e489cb
feat(eslint-plugin): [prefer-readonly-parameter-types] TypeAllowlistI…
marekdedic Feb 4, 2022
6534ef3
docs(eslint-plugin): [prefer-readonly-parameter-types] TypeAllowlistI…
marekdedic Feb 4, 2022
5474d23
Merge branch 'main' into prefer-readonly-parameter-types-whitelist
marekdedic Mar 28, 2022
81b18e3
test(type-utils): [prefer-readonly-parameter-types] Added rudimentary…
marekdedic Mar 28, 2022
164ae2b
test(type-utils): [prefer-readonly-parameter-types] Added test for al…
marekdedic Mar 28, 2022
6bda8a3
Update packages/type-utils/src/TypeAllowListItem.ts to use enum in JS…
marekdedic Apr 10, 2022
abfc236
fix(eslint-plugin): [prefer-readonly-parameter-types] Added trainling…
marekdedic Apr 10, 2022
ff28c54
Merge branch 'main' into prefer-readonly-parameter-types-whitelist
marekdedic Apr 19, 2022
da498ea
Merge branch 'main' into prefer-readonly-parameter-types-whitelist
bradzacher May 18, 2022
bd6b77d
Merge branch 'main' into prefer-readonly-parameter-types-whitelist
marekdedic Oct 27, 2022
bbca206
fix(eslint-plugin) Fixed type imports not being separated
marekdedic Oct 27, 2022
eec8883
Merge branch 'main' into prefer-readonly-parameter-types-whitelist
marekdedic Jan 9, 2023
ffd2aae
feat(type-utils): Added TypeOrValueSpecifier, its schema and test for…
marekdedic Jan 10, 2023
611ca15
Merge branch 'main' into prefer-readonly-parameter-types-whitelist
marekdedic Jan 10, 2023
c4d0a3f
feat(type-utils): Added typeMatchesSpecifier() and switched isTypeRea…
marekdedic Jan 11, 2023
5d74151
fix(eslint-plugin): [prefer-readonly-parameter-types] Fixed tests hav…
marekdedic Jan 11, 2023
fb67f4d
fix(type-utils): Removed unneeded function isTypeExcepted
marekdedic Jan 11, 2023
c4334cc
feat(type-utils): Added source file checking to typeMatchesFileSpecif…
marekdedic Jan 11, 2023
bc3ce11
docs(eslint-plugin): [prefer-readonly-parameter-types] Updated docs t…
marekdedic Jan 11, 2023
1e1bfc2
docs(eslint-plugin): [prefer-readonly-parameter-types] Typo fix
marekdedic Jan 11, 2023
c5c1d70
docs(eslint-plugin): [prefer-readonly-parameter-types] Typo fix 2
marekdedic Jan 11, 2023
22bb891
feat(type-utils): Added tests for typeMatchesSpecifier()
marekdedic Jan 11, 2023
af5cfc0
fix(type-utils): Using node path joining typeMatchesSpecifier()
marekdedic Jan 11, 2023
5c3f37f
feat(type-utils): Removed MultiSourceSpecifier
marekdedic Jan 11, 2023
60dcf36
chore(type-utils): Simplified typeMatchesSpecifier()
marekdedic Jan 11, 2023
e6d81a9
feat(type-utils): Added more tests for typeMatchesSpecifier()
marekdedic Jan 11, 2023
2d242f0
Merge branch 'v6' into prefer-readonly-parameter-types-whitelist
marekdedic Feb 5, 2023
333d008
docs(prefer-readonly-parameter-types) more legible docs
marekdedic Feb 5, 2023
08eebc8
fix(eslint-plugin): [prefer-readonly-parameter-types] Fixed missing e…
marekdedic Feb 5, 2023
eb7aef6
chore(type-utils): Simplified typeDeclaredInFile()
marekdedic Feb 5, 2023
b5b91cb
chore(type-utils): Using unknown instead of any in tests
marekdedic Feb 5, 2023
e41d8be
test(type-utils): grammar fix in test specifications
marekdedic Feb 5, 2023
852ca69
chore: Reset yarn.lock
marekdedic Feb 5, 2023
5407105
chore: renamed readonlyness allowlist to just allow
marekdedic Feb 5, 2023
b9f785c
fix(type-utils): fixed services.program now being optional and not ch…
marekdedic Feb 5, 2023
8626d16
test(type-utils): negative tests for isTypeReadonly
marekdedic Feb 5, 2023
235acd3
fix(eslint-plugin): bracket style array notation
marekdedic Feb 20, 2023
a1f38dd
Merge branch 'v6' into prefer-readonly-parameter-types-whitelist
marekdedic Feb 20, 2023
93fbcd8
Merge branch 'prefer-readonly-parameter-types-whitelist' of github.co…
marekdedic Feb 20, 2023
3ad0e9e
Merge branch 'v6' into prefer-readonly-parameter-types-whitelist
marekdedic Mar 6, 2023
0bd7601
fix(type-utils): Fixed array style
marekdedic Mar 6, 2023
0136e5b
fix(type-utils): Not fetching symbol repeatedly
marekdedic Mar 6, 2023
386fd03
fix(type-utils): Remove ManySpecifiers format from TypeOrValueSpecifi…
marekdedic Mar 7, 2023
ed1e07a
docs(eslint-plugin): [prefer-readonly-parameter-types] described file…
marekdedic Mar 7, 2023
3a89ec5
Merge branch 'v6' into prefer-readonly-parameter-types-whitelist
bradzacher Mar 8, 2023
346fdbc
path and package
JoshuaKGoldberg Mar 13, 2023
609351e
Update docs too
JoshuaKGoldberg Mar 13, 2023
a535fb6
Update docs too (again)
JoshuaKGoldberg Mar 13, 2023
8e3f910
Merge branch 'v6'
JoshuaKGoldberg Mar 13, 2023
95bbaa5
Added test name helpers, and fixed test data
JoshuaKGoldberg Mar 13, 2023
5eae317
test(type-utils): fixed package schema tests
marekdedic Mar 15, 2023
751e2de
test(eslint-plugin): fixed type whitelist schema in prefer-readonly-p…
marekdedic Mar 15, 2023
3d465cc
Applied lowercasing to typeDeclaredInFile
JoshuaKGoldberg Mar 20, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -129,6 +129,103 @@ interface Foo {

## Options

### `allow`

Some complex types cannot easily be made readonly, for example the `HTMLElement` type or the `JQueryStatic` type from `@types/jquery`. This option allows you to globally disable reporting of such types.

Each item must be one of:

- A type defined in a file (`{from: "file", name: "Foo", source: "src/foo-file.ts"}` with `source` being optional)
marekdedic marked this conversation as resolved.
Show resolved Hide resolved
- A type from the default library (`{from: "lib", name: "Foo"}`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

defaultLib would be a better from value as it would make it more explicit that it refers to the TypeScript default libs rather than just a random lib.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, that's were all the conversation about the details is. 🙂

- A type from a package (`{from: "package", name: "Foo", source: "foo-lib"}`, this also works for types defined in a typings package).

Two additional formats are supported:

- Multiple sources, as an array for `from` (`{from: ["file", "package"], name: "Foo"}`)
- A type may be defined just as a simple string, which then matches the type independently of its origin.

Examples of code for this rule with:

```json
{
"allow": [
{ "source": "file", "name": "Foo" },
{ "source": "lib", "name": "HTMLElement" },
{ "from": "package", "name": "Bar", "source": "bar-lib" }
]
}
```

<!--tabs-->

#### ❌ Incorrect

```ts
interface ThisIsMutable {
prop: string;
}

interface Wrapper {
sub: ThisIsMutable;
}

interface WrapperWithOther {
readonly sub: Foo;
otherProp: string;
}

function fn1(arg: ThisIsMutable) {} // Incorrect because ThisIsMutable is not readonly
function fn2(arg: Wrapper) {} // Incorrect because Wrapper.sub is not readonly
function fn3(arg: WrapperWithOther) {} // Incorrect because WrapperWithOther.otherProp is not readonly and not in the allowlist
```

```ts
import { Foo } from 'some-lib';
import { Bar } from 'incorrect-lib';

interface HTMLElement {
prop: string;
}

function fn1(arg: Foo) {} // Incorrect because Foo is not a local type
function fn2(arg: HTMLElement) {} // Incorrect because HTMLElement is not from the default library
function fn3(arg: Bar) {} // Incorrect because Bar is not from "bar-lib"
```

#### ✅ Correct

```ts
interface Foo {
prop: string;
}

interface Wrapper {
readonly sub: Foo;
readonly otherProp: string;
}

function fn1(arg: Foo) {} // Works because Foo is allowed
function fn2(arg: Wrapper) {} // Works even when Foo is nested somewhere in the type, with other properties still being checked
```

```ts
import { Bar } from 'bar-lib';

interface Foo {
prop: string;
}

function fn1(arg: Foo) {} // Works because Foo is a local type
function fn2(arg: HTMLElement) {} // Works because HTMLElement is from the default library
function fn3(arg: Bar) {} // Works because Bar is from "bar-lib"
```

```ts
import { Foo } from './foo';

function fn(arg: Foo) {} // Works because Foo is still a local type - it has to be in the same package
```

### `checkParameterProperties`

This option allows you to enable or disable the checking of parameter properties.
Expand Down
Expand Up @@ -5,9 +5,11 @@ import * as util from '../util';

type Options = [
{
allow?: Array<util.TypeOrValueSpecifier>;
marekdedic marked this conversation as resolved.
Show resolved Hide resolved
checkParameterProperties?: boolean;
ignoreInferredTypes?: boolean;
} & util.ReadonlynessOptions,
treatMethodsAsReadonly?: boolean;
},
];
type MessageIds = 'shouldBeReadonly';

Expand All @@ -26,13 +28,15 @@ export default util.createRule<Options, MessageIds>({
type: 'object',
additionalProperties: false,
properties: {
allow: util.readonlynessOptionsSchema.properties.allow,
checkParameterProperties: {
type: 'boolean',
},
ignoreInferredTypes: {
type: 'boolean',
},
...util.readonlynessOptionsSchema.properties,
treatMethodsAsReadonly:
util.readonlynessOptionsSchema.properties.treatMethodsAsReadonly,
},
},
],
Expand All @@ -42,17 +46,25 @@ export default util.createRule<Options, MessageIds>({
},
defaultOptions: [
{
allow: util.readonlynessOptionsDefaults.allow,
checkParameterProperties: true,
ignoreInferredTypes: false,
...util.readonlynessOptionsDefaults,
treatMethodsAsReadonly:
util.readonlynessOptionsDefaults.treatMethodsAsReadonly,
},
],
create(
context,
[{ checkParameterProperties, ignoreInferredTypes, treatMethodsAsReadonly }],
[
{
allow,
checkParameterProperties,
ignoreInferredTypes,
treatMethodsAsReadonly,
},
],
) {
const services = util.getParserServices(context);
const checker = services.program.getTypeChecker();

return {
[[
Expand Down Expand Up @@ -95,8 +107,9 @@ export default util.createRule<Options, MessageIds>({
}

const type = services.getTypeAtLocation(actualParam);
const isReadOnly = util.isTypeReadonly(checker, type, {
const isReadOnly = util.isTypeReadonly(services.program, type, {
treatMethodsAsReadonly: treatMethodsAsReadonly!,
allow,
});

if (!isReadOnly) {
Expand Down
Expand Up @@ -401,6 +401,83 @@ ruleTester.run('prefer-readonly-parameter-types', rule, {
},
],
},
// Allowlist
{
code: `
interface Foo {
readonly prop: RegExp;
}

function foo(arg: Foo) {}
`,
options: [
{
allow: [{ from: 'lib', name: 'RegExp' }],
},
],
},
{
code: `
interface Foo {
prop: RegExp;
}

function foo(arg: Readonly<Foo>) {}
`,
options: [
{
allow: [{ from: 'lib', name: 'RegExp' }],
},
],
},
{
code: `
interface Foo {
prop: string;
}

function foo(arg: Foo) {}
`,
options: [
{
allow: [{ from: 'file', name: 'Foo' }],
},
],
},
{
code: `
interface Bar {
prop: string;
}
interface Foo {
readonly prop: Bar;
}

function foo(arg: Foo) {}
`,
options: [
{
allow: [{ from: 'file', name: 'Foo' }],
},
],
},
{
code: `
interface Bar {
prop: string;
}
interface Foo {
readonly prop: Bar;
}

function foo(arg: Foo) {}
`,
options: [
{
allow: [{ from: 'file', name: 'Bar' }],
},
],
},
],
invalid: [
// arrays
Expand Down Expand Up @@ -869,5 +946,126 @@ ruleTester.run('prefer-readonly-parameter-types', rule, {
},
],
},
// Allowlist
{
code: `
function foo(arg: RegExp) {}
`,
options: [
{
allow: [{ from: 'file', name: 'Foo' }],
},
],
errors: [
{
messageId: 'shouldBeReadonly',
line: 2,
column: 22,
endColumn: 33,
},
],
},
{
code: `
interface Foo {
readonly prop: RegExp;
}

function foo(arg: Foo) {}
`,
options: [
{
allow: [{ from: 'file', name: 'Bar' }],
},
],
errors: [
{
messageId: 'shouldBeReadonly',
line: 6,
column: 22,
endColumn: 30,
},
],
},
{
code: `
interface Foo {
readonly prop: RegExp;
}

function foo(arg: Foo) {}
`,
options: [
{
allow: [{ from: 'lib', name: 'Foo' }],
},
],
errors: [
{
messageId: 'shouldBeReadonly',
line: 6,
column: 22,
endColumn: 30,
},
],
},
{
code: `
interface Foo {
readonly prop: RegExp;
}

function foo(arg: Foo) {}
`,
options: [
{
allow: [{ from: 'package', name: 'Foo', source: 'foo-lib' }],
},
],
errors: [
{
messageId: 'shouldBeReadonly',
line: 6,
column: 22,
endColumn: 30,
},
],
},
{
code: `
function foo(arg: RegExp) {}
`,
options: [
{
allow: [{ from: 'file', name: 'RegExp' }],
},
],
errors: [
{
messageId: 'shouldBeReadonly',
line: 2,
column: 22,
endColumn: 33,
},
],
},
{
code: `
function foo(arg: RegExp) {}
`,
options: [
{
allow: [{ from: 'package', name: 'RegExp', source: 'regexp-lib' }],
},
],
errors: [
{
messageId: 'shouldBeReadonly',
line: 2,
column: 22,
endColumn: 33,
},
],
},
],
});
1 change: 1 addition & 0 deletions packages/type-utils/package.json
Expand Up @@ -46,6 +46,7 @@
},
"devDependencies": {
"@typescript-eslint/parser": "5.50.0",
"ajv": "^8.12.0",
"typescript": "*"
},
"peerDependencies": {
Expand Down