Skip to content

Commit

Permalink
feat(eslint-plugin): add signalStoreFeatureShouldUseGenericType rule (#…
Browse files Browse the repository at this point in the history
…4454)

Closes #4438
  • Loading branch information
timdeschryver authored Jul 23, 2024
1 parent b58ea22 commit e2ab916
Show file tree
Hide file tree
Showing 10 changed files with 243 additions and 8 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
import type { ESLintUtils, TSESLint } from '@typescript-eslint/utils';
import * as path from 'path';
import rule, {
messageId,
} from '../../../src/rules/signals/signal-store-feature-should-use-generic-type';
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 withY = <Y>() => signalStoreFeature({ state: type<{ y: Y }>() }, withState({}));`,
`export const withY = <Y>() => signalStoreFeature(type<{ state: { y: Y } }>(), withState({}));`,
`const withY = <_>() => { return signalStoreFeature({ state: type<{ y: number }>() }, withState({})); }`,
`export const withY = <_>() => { return signalStoreFeature(type<{ state: { y: number } }>(), withState({})); }`,
`function withY<Y>() { return signalStoreFeature({ state: type<{ y: Y }>() }, withState({})); }`,
`export function withY<_>() { return signalStoreFeature(type<{ state: { y: number } }>(), withState({})); }`,
`function withY<_>() {
const feature = signalStoreFeature(type<{ state: { y: number } }>(), withState({}));
return feature;
}`,
];

const invalid: () => RunTests['invalid'] = () => [
fromFixture(
`
const withY = () => signalStoreFeature({ state: type<{ y: number }>() }, withState({}));
~~~~~~~~~~~~~~~~~~ [${messageId}]`,
{
output: `
const withY = <_>() => signalStoreFeature({ state: type<{ y: number }>() }, withState({}));`,
}
),
fromFixture(
`
const withY = () => signalStoreFeature(type<{ state: { y: number } }>(), withState({}));
~~~~~~~~~~~~~~~~~~ [${messageId}]`,
{
output: `
const withY = <_>() => signalStoreFeature(type<{ state: { y: number } }>(), withState({}));`,
}
),
fromFixture(
`
const withY = () => { return signalStoreFeature({ state: type<{ y: number }>() }, withState({})); }
~~~~~~~~~~~~~~~~~~ [${messageId}]`,
{
output: `
const withY = <_>() => { return signalStoreFeature({ state: type<{ y: number }>() }, withState({})); }`,
}
),
fromFixture(
`
const withY = () => { return signalStoreFeature(type<{ state: { y: number } }>(), withState({})); }
~~~~~~~~~~~~~~~~~~ [${messageId}]`,
{
output: `
const withY = <_>() => { return signalStoreFeature(type<{ state: { y: number } }>(), withState({})); }`,
}
),
fromFixture(
`
const withY = () => { return signalStoreFeature(type<{ state: { y: number } }>(), withState({})); }
~~~~~~~~~~~~~~~~~~ [${messageId}]`,
{
output: `
const withY = <_>() => { return signalStoreFeature(type<{ state: { y: number } }>(), withState({})); }`,
}
),

fromFixture(
`
function withY() { return signalStoreFeature(type<{ state: { y: number } }>(), withState({})); }
~~~~~~~~~~~~~~~~~~ [${messageId}]`,
{
output: `
function withY<_>() { return signalStoreFeature(type<{ state: { y: number } }>(), withState({})); }`,
}
),
fromFixture(
`
function withY() { return signalStoreFeature({ state: type<{ y: number }>() }, withState({})); }
~~~~~~~~~~~~~~~~~~ [${messageId}]`,
{
output: `
function withY<_>() { return signalStoreFeature({ state: type<{ y: number }>() }, withState({})); }`,
}
),
fromFixture(
`
function withY() {
const feature = signalStoreFeature({ state: type<{ y: number }>() }, withState({}));
~~~~~~~~~~~~~~~~~~ [${messageId}]
return feature;
}`,
{
output: `
function withY<_>() {
const feature = signalStoreFeature({ state: type<{ y: number }>() }, withState({}));
return feature;
}`,
}
),
];

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 @@ -14,6 +14,7 @@
"@ngrx/use-effects-lifecycle-interface": "error",
"@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/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 @@ -42,6 +42,7 @@ export default (
'@ngrx/use-effects-lifecycle-interface': 'error',
'@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/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 @@ -3,6 +3,7 @@
"plugins": ["@ngrx"],
"rules": {
"@ngrx/signal-state-no-arrays-at-root-level": "error",
"@ngrx/signal-store-feature-should-use-generic-type": "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 @@ -31,6 +31,7 @@ export default (
},
rules: {
'@ngrx/signal-state-no-arrays-at-root-level': 'error',
'@ngrx/signal-store-feature-should-use-generic-type': 'error',
'@ngrx/with-state-no-arrays-at-root-level': 'error',
},
},
Expand Down
4 changes: 4 additions & 0 deletions modules/eslint-plugin/src/rules/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ import selectStyle from './store/select-style';
import useConsistentGlobalStoreName from './store/use-consistent-global-store-name';
// operators
import preferConcatLatestFrom from './operators/prefer-concat-latest-from';
// signals
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';

export const rules = {
Expand Down Expand Up @@ -75,5 +77,7 @@ export const rules = {
'prefer-concat-latest-from': preferConcatLatestFrom,
// signals
'signal-state-no-arrays-at-root-level': signalStateNoArraysAtRootLevel,
'signal-store-feature-should-use-generic-type':
signalStoreFeatureShouldUseGenericType,
'with-state-no-arrays-at-root-level': withStateNoArraysAtRootLevel,
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
import { ESLintUtils, type TSESTree } from '@typescript-eslint/utils';
import * as path from 'path';
import { createRule } from '../../rule-creator';
import {
isArrowFunctionExpression,
isCallExpression,
isFunctionDeclaration,
isIdentifier,
} from '../../utils';

export const messageId = 'signalStoreFeatureShouldUseGenericType';

type MessageIds = typeof messageId;
type Options = readonly [];

export default createRule<Options, MessageIds>({
name: path.parse(__filename).name,
meta: {
type: 'problem',
ngrxModule: 'signals',
docs: {
description: `A custom Signal Store feature that accepts an input should define a generic type.`,
},
fixable: 'code',
schema: [],
messages: {
[messageId]: `Add an unused generic type to the function creating the signal store feature.`,
},
},
defaultOptions: [],
create: (context) => {
function report(
signalStoreFeature: TSESTree.CallExpression,
func?: TSESTree.Node
) {
if (
!func ||
(!isFunctionDeclaration(func) && !isArrowFunctionExpression(func))
) {
return;
}
const parentHasGenerics =
func.typeParameters && func.typeParameters.params.length > 0;
if (!parentHasGenerics) {
context.report({
node: signalStoreFeature.callee,
messageId,
fix(fixer) {
if (isFunctionDeclaration(func)) {
if (func.id) {
return fixer.insertTextAfter(func.id, '<_>');
}
}

return fixer.insertTextBefore(func, '<_>');
},
});
}
}

function hasInputAsArgument(node: TSESTree.CallExpression) {
const [inputArg] = node.arguments;
return (
!isCallExpression(inputArg) ||
(isIdentifier(inputArg.callee) && inputArg.callee.name === 'type')
);
}

return {
[`ArrowFunctionExpression > CallExpression[callee.name=signalStoreFeature]`](
node: TSESTree.CallExpression
) {
if (hasInputAsArgument(node)) {
report(node, node.parent);
}
},
[`ArrowFunctionExpression > BlockStatement CallExpression[callee.name=signalStoreFeature]`](
node: TSESTree.CallExpression
) {
if (hasInputAsArgument(node)) {
let parent: TSESTree.Node | undefined = node.parent;
while (parent && !isArrowFunctionExpression(parent)) {
parent = parent.parent;
}
report(node, parent);
}
},
[`FunctionDeclaration > BlockStatement CallExpression[callee.name=signalStoreFeature]`](
node: TSESTree.CallExpression
) {
if (hasInputAsArgument(node)) {
let parent: TSESTree.Node | undefined = node.parent;
while (parent && !isFunctionDeclaration(parent)) {
parent = parent.parent;
}
report(node, parent);
}
},
};
},
});
3 changes: 3 additions & 0 deletions modules/eslint-plugin/src/utils/helper-functions/guards.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ export const isPropertyDefinition = isNodeOfType(
export const isFunctionExpression = isNodeOfType(
AST_NODE_TYPES.FunctionExpression
);
export const isFunctionDeclaration = isNodeOfType(
AST_NODE_TYPES.FunctionDeclaration
);
export const isIdentifier = isNodeOfType(AST_NODE_TYPES.Identifier);
export const isImportDeclaration = isNodeOfType(
AST_NODE_TYPES.ImportDeclaration
Expand Down
17 changes: 9 additions & 8 deletions projects/ngrx.io/content/guide/eslint-plugin/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,11 @@ Optionally override some rules via the `rules` property.
Import the NgRx Plugin via `@ngrx/eslint-plugin/v9` and use one or more predefined [configurations](#configurations) by adding them to the `extends` array.

```ts
const tseslint = require("typescript-eslint");
const ngrx = require("@ngrx/eslint-plugin/v9");
const tseslint = require('typescript-eslint');
const ngrx = require('@ngrx/eslint-plugin/v9');

module.exports = tseslint.config({
files: ["**/*.ts"],
files: ['**/*.ts'],
extends: [
// 👇 Use all rules at once
...ngrx.configs.all,
Expand All @@ -85,7 +85,7 @@ module.exports = tseslint.config({
],
rules: {
// 👇 Configure specific rules
"@ngrx/with-state-no-arrays-at-root-level": "warn",
'@ngrx/with-state-no-arrays-at-root-level': 'warn',
},
});
```
Expand Down Expand Up @@ -123,10 +123,11 @@ 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/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/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,12 @@
# signal-store-feature-should-use-generic-type

A custom Signal Store feature that accepts an input should define a generic type.

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

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

0 comments on commit e2ab916

Please sign in to comment.