Skip to content

Commit

Permalink
feat(eslint-plugin): add preferProtectedState rule (#4488)
Browse files Browse the repository at this point in the history
Closes #4474

Co-authored-by: Rainer Hahnekamp <[email protected]>
  • Loading branch information
@NgDaddy and rainerhahnekamp authored Aug 15, 2024
1 parent 8c499cf commit 32c772d
Show file tree
Hide file tree
Showing 9 changed files with 216 additions and 5 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import type { ESLintUtils, TSESLint } from '@typescript-eslint/utils';
import * as path from 'path';
import rule, {
preferProtectedState,
preferProtectedStateSuggest,
} from '../../../src/rules/signals/prefer-protected-state';
import { ruleTester, fromFixture } from '../../utils';

type MessageIds = ESLintUtils.InferMessageIdsTypeFromRule<typeof rule>;
type Options = readonly ESLintUtils.InferOptionsTypeFromRule<typeof rule>[];
type RunTests = TSESLint.RunTests<MessageIds, Options>;

const valid: () => RunTests['valid'] = () => [
`const mySignalStore = signalStore();`,
`const mySignalStore = signalStore({ protectedState: true });`,
`const mySignalStore = signalStore({ providedIn: 'root' });`,
`const mySignalStore = signalStore({ providedIn: 'root', protectedState: true });`,
];

const invalid: () => RunTests['invalid'] = () => [
fromFixture(
`
const mySignalStore = signalStore({ providedIn: 'root', protectedState: false, });
~~~~~~~~~~~~~~~~~~~~~ [${preferProtectedState} suggest]`,
{
suggestions: [
{
messageId: preferProtectedStateSuggest,
output: `
const mySignalStore = signalStore({ providedIn: 'root', });`,
},
],
}
),
fromFixture(
`
const mySignalStore = signalStore({ providedIn: 'root', protectedState: false , });
~~~~~~~~~~~~~~~~~~~~~ [${preferProtectedState} suggest]`,
{
suggestions: [
{
messageId: preferProtectedStateSuggest,
output: `
const mySignalStore = signalStore({ providedIn: 'root', });`,
},
],
}
),
fromFixture(
`
const mySignalStore = signalStore({ protectedState: false, });
~~~~~~~~~~~~~~~~~~~~~ [${preferProtectedState} suggest]`,
{
suggestions: [
{
messageId: preferProtectedStateSuggest,
output: `
const mySignalStore = signalStore();`,
},
],
}
),
fromFixture(
`
const mySignalStore = signalStore({ protectedState: false, providedIn: 'root' });
~~~~~~~~~~~~~~~~~~~~~ [${preferProtectedState} suggest]`,
{
suggestions: [
{
messageId: preferProtectedStateSuggest,
output: `
const mySignalStore = signalStore({ providedIn: 'root' });`,
},
],
}
),
];

ruleTester().run(path.parse(__filename).name, rule, {
valid: valid(),
invalid: invalid(),
});
1 change: 1 addition & 0 deletions modules/eslint-plugin/src/configs/all.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"@ngrx/prefer-concat-latest-from": "error",
"@ngrx/signal-state-no-arrays-at-root-level": "error",
"@ngrx/signal-store-feature-should-use-generic-type": "error",
"@ngrx/prefer-protected-state": "error",
"@ngrx/with-state-no-arrays-at-root-level": "error",
"@ngrx/avoid-combining-selectors": "error",
"@ngrx/avoid-dispatching-multiple-actions-sequentially": "error",
Expand Down
1 change: 1 addition & 0 deletions modules/eslint-plugin/src/configs/all.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export default (
'@ngrx/prefer-concat-latest-from': 'error',
'@ngrx/signal-state-no-arrays-at-root-level': 'error',
'@ngrx/signal-store-feature-should-use-generic-type': 'error',
'@ngrx/prefer-protected-state': 'error',
'@ngrx/with-state-no-arrays-at-root-level': 'error',
'@ngrx/avoid-combining-selectors': 'error',
'@ngrx/avoid-dispatching-multiple-actions-sequentially': 'error',
Expand Down
1 change: 1 addition & 0 deletions modules/eslint-plugin/src/configs/signals.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"rules": {
"@ngrx/signal-state-no-arrays-at-root-level": "error",
"@ngrx/signal-store-feature-should-use-generic-type": "error",
"@ngrx/prefer-protected-state": "error",
"@ngrx/with-state-no-arrays-at-root-level": "error"
},
"parserOptions": {
Expand Down
1 change: 1 addition & 0 deletions modules/eslint-plugin/src/configs/signals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export default (
rules: {
'@ngrx/signal-state-no-arrays-at-root-level': 'error',
'@ngrx/signal-store-feature-should-use-generic-type': 'error',
'@ngrx/prefer-protected-state': 'error',
'@ngrx/with-state-no-arrays-at-root-level': 'error',
},
},
Expand Down
2 changes: 2 additions & 0 deletions modules/eslint-plugin/src/rules/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import preferConcatLatestFrom from './operators/prefer-concat-latest-from';
import signalStateNoArraysAtRootLevel from './signals/signal-state-no-arrays-at-root-level';
import signalStoreFeatureShouldUseGenericType from './signals/signal-store-feature-should-use-generic-type';
import withStateNoArraysAtRootLevel from './signals/with-state-no-arrays-at-root-level';
import preferProtectedState from './signals/prefer-protected-state';

export const rules = {
// component-store
Expand Down Expand Up @@ -79,5 +80,6 @@ export const rules = {
'signal-state-no-arrays-at-root-level': signalStateNoArraysAtRootLevel,
'signal-store-feature-should-use-generic-type':
signalStoreFeatureShouldUseGenericType,
'prefer-protected-state': preferProtectedState,
'with-state-no-arrays-at-root-level': withStateNoArraysAtRootLevel,
};
78 changes: 78 additions & 0 deletions modules/eslint-plugin/src/rules/signals/prefer-protected-state.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import { type TSESTree } from '@typescript-eslint/utils';
import * as path from 'path';
import { createRule } from '../../rule-creator';

export const preferProtectedState = 'preferProtectedState';
export const preferProtectedStateSuggest = 'preferProtectedStateSuggest';

type MessageIds =
| typeof preferProtectedState
| typeof preferProtectedStateSuggest;
type Options = readonly [];

export default createRule<Options, MessageIds>({
name: path.parse(__filename).name,
meta: {
type: 'suggestion',
hasSuggestions: true,
ngrxModule: 'signals',
docs: {
description: `A Signal Store prefers protected state`,
},
schema: [],
messages: {
[preferProtectedState]:
'{ protectedState: false } should be removed to prevent external state mutations.',
[preferProtectedStateSuggest]: 'Remove `{protectedState: false}`.',
},
},
defaultOptions: [],
create: (context) => {
return {
[`CallExpression[callee.name=signalStore][arguments.length>0] > ObjectExpression[properties.length>0] > Property[key.name=protectedState][value.value=false]`](
node: TSESTree.Property
) {
context.report({
node,
messageId: preferProtectedState,
suggest: [
{
messageId: preferProtectedStateSuggest,
fix: (fixer) => {
const getRangeToBeRemoved = (): Parameters<
typeof fixer.removeRange
>[0] => {
const parentObject = node.parent as TSESTree.ObjectExpression;
const parentObjectHasOnlyOneProperty =
parentObject.properties.length === 1;

if (parentObjectHasOnlyOneProperty) {
/**
* Remove the entire object if it contains only one property - the relevant one
*/
return parentObject.range;
}

const tokenAfter = context.sourceCode.getTokenAfter(node);
const tokenAfterIsComma = tokenAfter?.value?.trim() === ',';
/**
* Remove the specific property if there is more than one property in the parent
*/
return [
node.range[0],
/**
* remove trailing comma as well
*/
tokenAfterIsComma ? tokenAfter.range[1] : node.range[1],
];
};

return fixer.removeRange(getRangeToBeRemoved());
},
},
],
});
},
};
},
});
11 changes: 6 additions & 5 deletions projects/ngrx.io/content/guide/eslint-plugin/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,11 +123,12 @@ module.exports = tseslint.config({

### signals

| Name | Description | Category | Fixable | Has suggestions | Configurable | Requires type information |
| ----------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------- | -------- | ------- | --------------- | ------------ | ------------------------- |
| [@ngrx/signal-state-no-arrays-at-root-level](/guide/eslint-plugin/rules/signal-state-no-arrays-at-root-level) | signalState should accept a record or dictionary as an input argument. | problem | No | No | No | No |
| [@ngrx/signal-store-feature-should-use-generic-type](/guide/eslint-plugin/rules/signal-store-feature-should-use-generic-type) | A custom Signal Store feature that accepts an input should define a generic type. | problem | Yes | No | No | No |
| [@ngrx/with-state-no-arrays-at-root-level](/guide/eslint-plugin/rules/with-state-no-arrays-at-root-level) | withState should accept a record or dictionary as an input argument. | problem | No | No | No | Yes |
| Name | Description | Category | Fixable | Has suggestions | Configurable | Requires type information |
|------------------------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------| ---------- |---------| --------------- | ------------ |--------------------------|
| [@ngrx/signal-state-no-arrays-at-root-level](/guide/eslint-plugin/rules/signal-state-no-arrays-at-root-level) | signalState should accept a record or dictionary as an input argument. | problem | No | No | No | No |
| [@ngrx/signal-store-feature-should-use-generic-type](/guide/eslint-plugin/rules/signal-store-feature-should-use-generic-type) | A custom Signal Store feature that accepts an input should define a generic type. | problem | Yes | No | No | No |
| [@ngrx/prefer-protected-state](/guide/eslint-plugin/rules/prefer-protected-state) | A Signal Store prefers protected state. | suggestion | No | Yes | No | No |
| [@ngrx/with-state-no-arrays-at-root-level](/guide/eslint-plugin/rules/with-state-no-arrays-at-root-level) | withState should accept a record or dictionary as an input argument. | problem | No | No | No | Yes |

### store

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# prefer-protected-state

A Signal Store prefers protected state.

- **Type**: suggestion
- **Fixable**: No
- **Suggestion**: Yes
- **Requires type checking**: No
- **Configurable**: No

<!-- Everything above this generated, do not edit -->
<!-- MANUAL-DOC:START -->

## Rule Details

This rule ensures that state changes are only managed by the Signal Store to prevent unintended modifications and provide clear ownership of where changes occur.

Examples of **incorrect** code for this rule:

```ts
// SUGGESTION ❗
const Store = signalStore(
{ protectedState: false },
~~~~~~~~~~~~~~~~~~~~~ [warning]
withState({}),
);
```

Examples of **correct** code for this rule:

```ts
// GOOD ✅
const Store = signalStore(
withState({}),
);
```

```ts
// GOOD ✅
const Store = signalStore(
{ protectedState: true },
withState({}),
);
```

0 comments on commit 32c772d

Please sign in to comment.