From 90bb5ebb82ad5bf7831d3437c1a2cf353eb8f987 Mon Sep 17 00:00:00 2001 From: Rebecca Stevens Date: Mon, 15 Apr 2024 00:38:20 +1200 Subject: [PATCH] feat(functional-parameters): allow overriding options based on where the function type is declared fix #575 --- README.md | 6 +- docs/rules/functional-parameters.md | 44 +++- package.json | 3 +- pnpm-lock.yaml | 4 + src/rules/functional-parameters.ts | 222 +++++++++++++----- src/utils/common-schemas.ts | 52 ++++ src/utils/tree.ts | 21 +- .../functional-parameters/ts/index.test.ts | 17 ++ .../rules/functional-parameters/ts/invalid.ts | 106 +++++++++ tests/rules/functional-parameters/ts/valid.ts | 86 +++++++ 10 files changed, 490 insertions(+), 71 deletions(-) create mode 100644 src/utils/common-schemas.ts create mode 100644 tests/rules/functional-parameters/ts/index.test.ts create mode 100644 tests/rules/functional-parameters/ts/invalid.ts create mode 100644 tests/rules/functional-parameters/ts/valid.ts diff --git a/README.md b/README.md index bcb13bf2a..a10fe5a17 100644 --- a/README.md +++ b/README.md @@ -105,9 +105,9 @@ The [below section](#rules) gives details on which rules are enabled by each rul ### Currying -| Name | Description | πŸ’Ό | ⚠️ | 🚫 | πŸ”§ | πŸ’‘ | πŸ’­ | ❌ | -| :----------------------------------------------------------- | :----------------------------- | :--------------------------- | :-- | :-- | :-- | :-- | :-- | :-- | -| [functional-parameters](docs/rules/functional-parameters.md) | Enforce functional parameters. | β˜‘οΈ βœ… πŸ”’ ![badge-currying][] | | | | | | | +| Name | Description | πŸ’Ό | ⚠️ | 🚫 | πŸ”§ | πŸ’‘ | πŸ’­ | ❌ | +| :----------------------------------------------------------- | :----------------------------- | :--------------------------- | :-- | :---------------------------- | :-- | :-- | :-- | :-- | +| [functional-parameters](docs/rules/functional-parameters.md) | Enforce functional parameters. | β˜‘οΈ βœ… πŸ”’ ![badge-currying][] | | ![badge-disableTypeChecked][] | | | πŸ’­ | | ### No Exceptions diff --git a/docs/rules/functional-parameters.md b/docs/rules/functional-parameters.md index ad91c8cb6..9b48d861d 100644 --- a/docs/rules/functional-parameters.md +++ b/docs/rules/functional-parameters.md @@ -1,11 +1,15 @@ # Enforce functional parameters (`functional/functional-parameters`) -πŸ’Ό This rule is enabled in the following configs: `currying`, β˜‘οΈ `lite`, βœ… `recommended`, πŸ”’ `strict`. +πŸ’ΌπŸš« This rule is enabled in the following configs: `currying`, β˜‘οΈ `lite`, βœ… `recommended`, πŸ”’ `strict`. This rule is _disabled_ in the `disableTypeChecked` config. + +πŸ’­ This rule requires [type information](https://typescript-eslint.io/linting/typed-linting). Disallow use of rest parameters, the `arguments` keyword and enforces that functions take at least 1 parameter. +Note: type information is only required when using the [overrides](#overrides) option. + ## Rule Details In functions, `arguments` is a special variable that is implicitly available. @@ -67,6 +71,23 @@ type Options = { }; ignoreIdentifierPattern?: string[] | string; ignorePrefixSelector?: string[] | string; + overrides?: Array<{ + match: + | { + from: "file"; + path?: string; + } + | { + from: "lib"; + } + | { + from: "package"; + package?: string; + } + | TypeDeclarationSpecifier[]; + options: Omit; + disable: boolean; + }>; }; ``` @@ -196,3 +217,24 @@ const sum = [1, 2, 3].reduce((carry, current) => current, 0); This option takes a RegExp string or an array of RegExp strings. It allows for the ability to ignore violations based on a function's name. + +### `overrides` + +_Using this option requires type infomation._ + +Allows for applying overrides to the options based on where the function's type is defined. +This can be used to override the settings for types coming from 3rd party libraries. + +Note: Only the first matching override will be used. + +#### `overrides[n].specifiers` + +A specifier, or an array of specifiers to match the function type against. + +#### `overrides[n].options` + +The options to use when a specifiers matches. + +#### `overrides[n].disable` + +If true, when a specifier matches, this rule will not be applied to the matching node. diff --git a/package.json b/package.json index eb1257263..31c5e3560 100644 --- a/package.json +++ b/package.json @@ -74,7 +74,8 @@ "deepmerge-ts": "^5.1.0", "escape-string-regexp": "^4.0.0", "is-immutable-type": "^3.1.0", - "ts-api-utils": "^1.3.0" + "ts-api-utils": "^1.3.0", + "ts-declaration-location": "1.0.0" }, "devDependencies": { "@babel/eslint-parser": "7.24.1", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index be0e5a646..1df0d6de1 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -20,6 +20,9 @@ dependencies: ts-api-utils: specifier: ^1.3.0 version: 1.3.0(typescript@5.4.5) + ts-declaration-location: + specifier: 1.0.0 + version: 1.0.0(typescript@5.4.5) devDependencies: '@babel/eslint-parser': @@ -9127,6 +9130,7 @@ packages: eslint: 9.0.0 is-immutable-type: 3.1.0(eslint@9.0.0)(typescript@5.4.5) ts-api-utils: 1.3.0(typescript@5.4.5) + ts-declaration-location: 1.0.0(typescript@5.4.5) typescript: 5.4.5 transitivePeerDependencies: - supports-color diff --git a/src/rules/functional-parameters.ts b/src/rules/functional-parameters.ts index acf160950..fa0e4f4c4 100644 --- a/src/rules/functional-parameters.ts +++ b/src/rules/functional-parameters.ts @@ -5,6 +5,9 @@ import { } from "@typescript-eslint/utils/json-schema"; import { type RuleContext } from "@typescript-eslint/utils/ts-eslint"; import { deepmerge } from "deepmerge-ts"; +import typeMatchesSpecifier, { + type TypeDeclarationSpecifier, +} from "ts-declaration-location"; import { type IgnoreIdentifierPatternOption, @@ -13,14 +16,17 @@ import { ignorePrefixSelectorOptionSchema, shouldIgnorePattern, } from "#eslint-plugin-functional/options"; +import { typeSpecifiersSchema } from "#eslint-plugin-functional/utils/common-schemas"; import { ruleNameScope } from "#eslint-plugin-functional/utils/misc"; import { type ESFunction } from "#eslint-plugin-functional/utils/node-types"; import { type NamedCreateRuleCustomMeta, type RuleResult, createRuleUsingFunction, + getTypeOfNode, } from "#eslint-plugin-functional/utils/rule"; import { + getEnclosingFunction, isArgument, isGetter, isIIFE, @@ -45,26 +51,82 @@ export const fullName = `${ruleNameScope}/${name}`; */ type ParameterCountOptions = "atLeastOne" | "exactlyOne"; +type CoreOptions = IgnoreIdentifierPatternOption & + IgnorePrefixSelectorOption & { + allowRestParameter: boolean; + allowArgumentsKeyword: boolean; + enforceParameterCount: + | ParameterCountOptions + | false + | { + count: ParameterCountOptions; + ignoreLambdaExpression: boolean; + ignoreIIFE: boolean; + ignoreGettersAndSetters: boolean; + }; + }; + /** * The options this rule can take. */ type Options = [ - IgnoreIdentifierPatternOption & - IgnorePrefixSelectorOption & { - allowRestParameter: boolean; - allowArgumentsKeyword: boolean; - enforceParameterCount: - | ParameterCountOptions - | false + CoreOptions & { + overrides?: Array< + { + specifiers: TypeDeclarationSpecifier | TypeDeclarationSpecifier[]; + } & ( | { - count: ParameterCountOptions; - ignoreLambdaExpression: boolean; - ignoreIIFE: boolean; - ignoreGettersAndSetters: boolean; - }; - }, + options: CoreOptions; + disable?: false; + } + | { + disable: true; + } + ) + >; + }, ]; +const coreOptionsPropertiesSchema: JSONSchema4ObjectSchema["properties"] = { + allowRestParameter: { + type: "boolean", + }, + allowArgumentsKeyword: { + type: "boolean", + }, + enforceParameterCount: { + oneOf: [ + { + type: "boolean", + enum: [false], + }, + { + type: "string", + enum: ["atLeastOne", "exactlyOne"], + }, + { + type: "object", + properties: { + count: { + type: "string", + enum: ["atLeastOne", "exactlyOne"], + }, + ignoreGettersAndSetters: { + type: "boolean", + }, + ignoreLambdaExpression: { + type: "boolean", + }, + ignoreIIFE: { + type: "boolean", + }, + }, + additionalProperties: false, + }, + ], + }, +}; + /** * The schema for the rule options. */ @@ -74,43 +136,25 @@ const schema: JSONSchema4[] = [ properties: deepmerge( ignoreIdentifierPatternOptionSchema, ignorePrefixSelectorOptionSchema, + coreOptionsPropertiesSchema, { - allowRestParameter: { - type: "boolean", - }, - allowArgumentsKeyword: { - type: "boolean", - }, - enforceParameterCount: { - oneOf: [ - { - type: "boolean", - enum: [false], - }, - { - type: "string", - enum: ["atLeastOne", "exactlyOne"], - }, - { - type: "object", - properties: { - count: { - type: "string", - enum: ["atLeastOne", "exactlyOne"], - }, - ignoreGettersAndSetters: { - type: "boolean", - }, - ignoreLambdaExpression: { - type: "boolean", - }, - ignoreIIFE: { - type: "boolean", - }, + overrides: { + type: "array", + items: { + type: "object", + properties: { + specifiers: typeSpecifiersSchema, + options: { + type: "object", + properties: coreOptionsPropertiesSchema, + additionalProperties: false, + }, + disable: { + type: "boolean", }, - additionalProperties: false, }, - ], + additionalProperties: false, + }, }, } satisfies JSONSchema4ObjectSchema["properties"], ), @@ -156,16 +200,50 @@ const meta: NamedCreateRuleCustomMeta = { description: "Enforce functional parameters.", recommended: "recommended", recommendedSeverity: "error", + requiresTypeChecking: true, }, messages: errorMessages, schema, }; +/** + * Get the core options to use, taking into account overrides. + */ +function getCoreOptions( + node: TSESTree.Node, + context: Readonly>, + options: Readonly, +): CoreOptions | null { + const [optionsObject] = options; + + const program = context.sourceCode.parserServices?.program ?? undefined; + if (program === undefined) { + return optionsObject; + } + + const type = getTypeOfNode(node, context); + const found = optionsObject.overrides?.find((override) => + (Array.isArray(override.specifiers) + ? override.specifiers + : [override.specifiers] + ).some((specifier) => typeMatchesSpecifier(program, specifier, type)), + ); + + if (found !== undefined) { + if (found.disable === true) { + return null; + } + return found.options; + } + + return optionsObject; +} + /** * Get the rest parameter violations. */ function getRestParamViolations( - [{ allowRestParameter }]: Readonly, + { allowRestParameter }: Readonly, node: ESFunction, ): RuleResult["descriptors"] { return !allowRestParameter && @@ -184,7 +262,7 @@ function getRestParamViolations( * Get the parameter count violations. */ function getParamCountViolations( - [{ enforceParameterCount }]: Readonly, + { enforceParameterCount }: Readonly, node: ESFunction, ): RuleResult["descriptors"] { if ( @@ -235,8 +313,16 @@ function checkFunction( context: Readonly>, options: Readonly, ): RuleResult { - const [optionsObject] = options; - const { ignoreIdentifierPattern } = optionsObject; + const optionsToUse = getCoreOptions(node, context, options); + + if (optionsToUse === null) { + return { + context, + descriptors: [], + }; + } + + const { ignoreIdentifierPattern } = optionsToUse; if (shouldIgnorePattern(node, context, ignoreIdentifierPattern)) { return { @@ -248,8 +334,8 @@ function checkFunction( return { context, descriptors: [ - ...getRestParamViolations(options, node), - ...getParamCountViolations(options, node), + ...getRestParamViolations(optionsToUse, node), + ...getParamCountViolations(optionsToUse, node), ], }; } @@ -262,8 +348,27 @@ function checkIdentifier( context: Readonly>, options: Readonly, ): RuleResult { - const [optionsObject] = options; - const { ignoreIdentifierPattern } = optionsObject; + if (node.name !== "arguments") { + return { + context, + descriptors: [], + }; + } + + const functionNode = getEnclosingFunction(node); + const optionsToUse = + functionNode === null + ? options[0] + : getCoreOptions(functionNode, context, options); + + if (optionsToUse === null) { + return { + context, + descriptors: [], + }; + } + + const { ignoreIdentifierPattern } = optionsToUse; if (shouldIgnorePattern(node, context, ignoreIdentifierPattern)) { return { @@ -272,15 +377,12 @@ function checkIdentifier( }; } - const { allowArgumentsKeyword } = optionsObject; + const { allowArgumentsKeyword } = optionsToUse; return { context, descriptors: - !allowArgumentsKeyword && - node.name === "arguments" && - !isPropertyName(node) && - !isPropertyAccess(node) + !allowArgumentsKeyword && !isPropertyName(node) && !isPropertyAccess(node) ? [ { node, diff --git a/src/utils/common-schemas.ts b/src/utils/common-schemas.ts new file mode 100644 index 000000000..553ccaa61 --- /dev/null +++ b/src/utils/common-schemas.ts @@ -0,0 +1,52 @@ +import { type JSONSchema4 } from "@typescript-eslint/utils/json-schema"; + +const typeSpecifierSchema: JSONSchema4 = { + oneOf: [ + { + type: "object", + properties: { + from: { + type: "string", + enum: ["file"], + }, + path: { + type: "string", + }, + }, + additionalProperties: false, + }, + { + type: "object", + properties: { + from: { + type: "string", + enum: ["lib"], + }, + }, + additionalProperties: false, + }, + { + type: "object", + properties: { + from: { + type: "string", + enum: ["package"], + }, + package: { + type: "string", + }, + }, + additionalProperties: false, + }, + ], +}; + +export const typeSpecifiersSchema: JSONSchema4 = { + oneOf: [ + { + type: "array", + items: typeSpecifierSchema, + }, + typeSpecifierSchema, + ], +}; diff --git a/src/utils/tree.ts b/src/utils/tree.ts index eab28f17c..692f1f8f8 100644 --- a/src/utils/tree.ts +++ b/src/utils/tree.ts @@ -52,7 +52,21 @@ export function isInFunctionBody( node: TSESTree.Node, async?: boolean, ): boolean { - const functionNode = getAncestorOfType( + const functionNode = getEnclosingFunction(node); + + return ( + functionNode !== null && + (async === undefined || functionNode.async === async) + ); +} + +/** + * Get the function the given node is in. + * + * Will return null if not in a function. + */ +export function getEnclosingFunction(node: TSESTree.Node) { + return getAncestorOfType( ( n, c, @@ -62,11 +76,6 @@ export function isInFunctionBody( | TSESTree.FunctionExpression => isFunctionLike(n) && n.body === c, node, ); - - return ( - functionNode !== null && - (async === undefined || functionNode.async === async) - ); } /** diff --git a/tests/rules/functional-parameters/ts/index.test.ts b/tests/rules/functional-parameters/ts/index.test.ts new file mode 100644 index 000000000..e06e353dc --- /dev/null +++ b/tests/rules/functional-parameters/ts/index.test.ts @@ -0,0 +1,17 @@ +import { + name, + rule, +} from "#eslint-plugin-functional/rules/functional-parameters"; +import { testRule } from "#eslint-plugin-functional/tests/helpers/testers"; + +import invalid from "./invalid"; +import valid from "./valid"; + +const tests = { + valid, + invalid, +}; + +const tester = testRule(name, rule); + +tester.typescript(tests); diff --git a/tests/rules/functional-parameters/ts/invalid.ts b/tests/rules/functional-parameters/ts/invalid.ts new file mode 100644 index 000000000..731c910a7 --- /dev/null +++ b/tests/rules/functional-parameters/ts/invalid.ts @@ -0,0 +1,106 @@ +import { AST_NODE_TYPES } from "@typescript-eslint/utils"; +import dedent from "dedent"; + +import { type rule } from "#eslint-plugin-functional/rules/functional-parameters"; +import { + type InvalidTestCaseSet, + type MessagesOf, + type OptionsOf, +} from "#eslint-plugin-functional/tests/helpers/util"; + +const tests: Array< + InvalidTestCaseSet, OptionsOf> +> = [ + { + code: dedent` + function foo(...bar: string[]) { + console.log(bar); + } + `, + errors: [ + { + messageId: "restParam", + type: AST_NODE_TYPES.RestElement, + line: 1, + column: 14, + }, + ], + optionsSet: [ + [ + { + allowRestParameter: false, + overrides: [ + { + specifiers: { + from: "lib", + }, + disable: true, + }, + ], + }, + ], + [ + { + allowRestParameter: false, + overrides: [ + { + specifiers: { + from: "lib", + }, + options: { + allowRestParameter: true, + }, + }, + ], + }, + ], + ], + }, + { + code: dedent` + function foo(bar: string[]) { + console.log(arguments); + } + `, + errors: [ + { + messageId: "arguments", + type: AST_NODE_TYPES.Identifier, + line: 2, + column: 15, + }, + ], + optionsSet: [ + [ + { + allowArgumentsKeyword: false, + overrides: [ + { + specifiers: { + from: "lib", + }, + disable: true, + }, + ], + }, + ], + [ + { + allowArgumentsKeyword: false, + overrides: [ + { + specifiers: { + from: "lib", + }, + options: { + allowArgumentsKeyword: true, + }, + }, + ], + }, + ], + ], + }, +]; + +export default tests; diff --git a/tests/rules/functional-parameters/ts/valid.ts b/tests/rules/functional-parameters/ts/valid.ts new file mode 100644 index 000000000..a0ef62ae7 --- /dev/null +++ b/tests/rules/functional-parameters/ts/valid.ts @@ -0,0 +1,86 @@ +import dedent from "dedent"; + +import { type rule } from "#eslint-plugin-functional/rules/functional-parameters"; +import { + type OptionsOf, + type ValidTestCaseSet, +} from "#eslint-plugin-functional/tests/helpers/util"; + +const tests: Array>> = [ + { + code: dedent` + function foo(...bar: string[]) { + console.log(bar); + } + `, + optionsSet: [ + [ + { + allowRestParameter: false, + overrides: [ + { + specifiers: { + from: "file", + }, + disable: true, + }, + ], + }, + ], + [ + { + allowRestParameter: false, + overrides: [ + { + specifiers: { + from: "file", + }, + options: { + allowRestParameter: true, + }, + }, + ], + }, + ], + ], + }, + { + code: dedent` + function foo(bar: string[]) { + console.log(arguments); + } + `, + optionsSet: [ + [ + { + allowArgumentsKeyword: false, + overrides: [ + { + specifiers: { + from: "file", + }, + disable: true, + }, + ], + }, + ], + [ + { + allowArgumentsKeyword: false, + overrides: [ + { + specifiers: { + from: "file", + }, + options: { + allowArgumentsKeyword: true, + }, + }, + ], + }, + ], + ], + }, +]; + +export default tests;