From 0003e5a18ba95b3bfda9c1232c3a12e56b6bd25a Mon Sep 17 00:00:00 2001 From: Rebecca Stevens Date: Mon, 21 Oct 2024 04:30:39 +1300 Subject: [PATCH] feat: new rule no-class-inheritance fix #886 --- .github/workflows/semantic-pr.yml | 1 + README.md | 11 +- docs/rules/no-class-inheritance.md | 95 +++++++++++ docs/rules/no-classes.md | 2 +- eslint.config.js | 7 + src/configs/lite.ts | 2 + src/rules/index.ts | 3 + src/rules/no-class-inheritance.ts | 157 ++++++++++++++++++ .../no-class-inheritance.test.ts.snap | 124 ++++++++++++++ tests/rules/no-class-inheritance.test.ts | 143 ++++++++++++++++ 10 files changed, 539 insertions(+), 6 deletions(-) create mode 100644 docs/rules/no-class-inheritance.md create mode 100644 src/rules/no-class-inheritance.ts create mode 100644 tests/rules/__snapshots__/no-class-inheritance.test.ts.snap create mode 100644 tests/rules/no-class-inheritance.test.ts diff --git a/.github/workflows/semantic-pr.yml b/.github/workflows/semantic-pr.yml index f6cbbb16f..e28ac0580 100644 --- a/.github/workflows/semantic-pr.yml +++ b/.github/workflows/semantic-pr.yml @@ -32,6 +32,7 @@ jobs: functional-parameters immutable-data no-classes + no-class-inheritance no-conditional-statements no-expression-statements no-let diff --git a/README.md b/README.md index 587076557..ffa3e89f6 100644 --- a/README.md +++ b/README.md @@ -135,11 +135,12 @@ The [below section](#rules) gives details on which rules are enabled by each rul ### No Other Paradigms -| Name                | Description | 💼 | ⚠️ | 🚫 | 🔧 | 💡 | 💭 | ❌ | -| :------------------------------------------------------- | :------------------------------------------------------------------------ | :----------------------------------- | :-- | :---------------------------- | :-- | :-- | :-- | :-- | -| [no-classes](docs/rules/no-classes.md) | Disallow classes. | ☑️ ✅ 🔒 ![badge-noOtherParadigms][] | | | | | | | -| [no-mixed-types](docs/rules/no-mixed-types.md) | Restrict types so that only members of the same kind are allowed in them. | ☑️ ✅ 🔒 ![badge-noOtherParadigms][] | | ![badge-disableTypeChecked][] | | | 💭 | | -| [no-this-expressions](docs/rules/no-this-expressions.md) | Disallow this access. | 🔒 ![badge-noOtherParadigms][] | | ☑️ ✅ | | | | | +| Name                 | Description | 💼 | ⚠️ | 🚫 | 🔧 | 💡 | 💭 | ❌ | +| :--------------------------------------------------------- | :------------------------------------------------------------------------ | :----------------------------------- | :-- | :---------------------------- | :-- | :-- | :-- | :-- | +| [no-class-inheritance](docs/rules/no-class-inheritance.md) | Disallow inheritance in classes. | ☑️ ✅ 🔒 ![badge-noOtherParadigms][] | | | | | | | +| [no-classes](docs/rules/no-classes.md) | Disallow classes. | ✅ 🔒 ![badge-noOtherParadigms][] | | ☑️ | | | | | +| [no-mixed-types](docs/rules/no-mixed-types.md) | Restrict types so that only members of the same kind are allowed in them. | ☑️ ✅ 🔒 ![badge-noOtherParadigms][] | | ![badge-disableTypeChecked][] | | | 💭 | | +| [no-this-expressions](docs/rules/no-this-expressions.md) | Disallow this access. | 🔒 ![badge-noOtherParadigms][] | | ☑️ ✅ | | | | | ### No Statements diff --git a/docs/rules/no-class-inheritance.md b/docs/rules/no-class-inheritance.md new file mode 100644 index 000000000..012812942 --- /dev/null +++ b/docs/rules/no-class-inheritance.md @@ -0,0 +1,95 @@ + + + +# Disallow inheritance in classes (`functional/no-class-inheritance`) + +💼 This rule is enabled in the following configs: ☑️ `lite`, `noOtherParadigms`, ✅ `recommended`, 🔒 `strict`. + + + + + +Disallow use of inheritance for classes. + +## Rule Details + +### ❌ Incorrect + + + +```js +/* eslint functional/no-class-inheritance: "error" */ + +abstract class Animal { + constructor(name, age) { + this.name = name; + this.age = age; + } +} + +class Dog extends Animal { + constructor(name, age) { + super(name, age); + } + + get ageInDogYears() { + return 7 * this.age; + } +} + +const dogA = new Dog("Jasper", 2); + +console.log(`${dogA.name} is ${dogA.ageInDogYears} in dog years.`); +``` + +### ✅ Correct + +```js +/* eslint functional/no-class-inheritance: "error" */ + +class Animal { + constructor(name, age) { + this.name = name; + this.age = age; + } +} + +class Dog { + constructor(name, age) { + this.animal = new Animal(name, age); + } + + get ageInDogYears() { + return 7 * this.animal.age; + } +} + +console.log(`${dogA.name} is ${getAgeInDogYears(dogA.age)} in dog years.`); +``` + +## Options + +This rule accepts an options object of the following type: + +```ts +type Options = { + ignoreIdentifierPattern?: string[] | string; + ignoreCodePattern?: string[] | string; +}; +``` + +### Default Options + +```ts +const defaults = {}; +``` + +### `ignoreIdentifierPattern` + +This option takes a RegExp string or an array of RegExp strings. +It allows for the ability to ignore violations based on the class's name. + +### `ignoreCodePattern` + +This option takes a RegExp string or an array of RegExp strings. +It allows for the ability to ignore violations based on the code itself. diff --git a/docs/rules/no-classes.md b/docs/rules/no-classes.md index 50cceebbd..6a75a5001 100644 --- a/docs/rules/no-classes.md +++ b/docs/rules/no-classes.md @@ -3,7 +3,7 @@ # Disallow classes (`functional/no-classes`) -💼 This rule is enabled in the following configs: ☑️ `lite`, `noOtherParadigms`, ✅ `recommended`, 🔒 `strict`. +💼🚫 This rule is enabled in the following configs: `noOtherParadigms`, ✅ `recommended`, 🔒 `strict`. This rule is _disabled_ in the ☑️ `lite` config. diff --git a/eslint.config.js b/eslint.config.js index 6f1fa7b29..53db9db54 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -123,6 +123,13 @@ const configs = await rsEslint( "jsdoc/require-jsdoc": "off", }, }, + { + files: ["**/*.md/**"], + rules: { + "max-classes-per-file": "off", + "ts/no-extraneous-class": "off", + }, + }, ); // Use our local version of the plugin. diff --git a/src/configs/lite.ts b/src/configs/lite.ts index 295daee87..71309dcc2 100644 --- a/src/configs/lite.ts +++ b/src/configs/lite.ts @@ -2,6 +2,7 @@ import type { FlatConfig } from "@typescript-eslint/utils/ts-eslint"; import * as functionalParameters from "#/rules/functional-parameters"; import * as immutableData from "#/rules/immutable-data"; +import * as noClasses from "#/rules/no-classes"; import * as noConditionalStatements from "#/rules/no-conditional-statements"; import * as noExpressionStatements from "#/rules/no-expression-statements"; import * as preferImmutableTypes from "#/rules/prefer-immutable-types"; @@ -16,6 +17,7 @@ const overrides = { }, ], [immutableData.fullName]: ["error", { ignoreClasses: "fieldsOnly" }], + [noClasses.fullName]: "off", [noConditionalStatements.fullName]: "off", [noExpressionStatements.fullName]: "off", [preferImmutableTypes.fullName]: [ diff --git a/src/rules/index.ts b/src/rules/index.ts index 53f668494..f17defc57 100644 --- a/src/rules/index.ts +++ b/src/rules/index.ts @@ -1,5 +1,6 @@ import * as functionalParameters from "./functional-parameters"; import * as immutableData from "./immutable-data"; +import * as noClassInheritance from "./no-class-inheritance"; import * as noClasses from "./no-classes"; import * as noConditionalStatements from "./no-conditional-statements"; import * as noExpressionStatements from "./no-expression-statements"; @@ -25,6 +26,7 @@ export const rules: Readonly<{ [functionalParameters.name]: typeof functionalParameters.rule; [immutableData.name]: typeof immutableData.rule; [noClasses.name]: typeof noClasses.rule; + [noClassInheritance.name]: typeof noClassInheritance.rule; [noConditionalStatements.name]: typeof noConditionalStatements.rule; [noExpressionStatements.name]: typeof noExpressionStatements.rule; [noLet.name]: typeof noLet.rule; @@ -45,6 +47,7 @@ export const rules: Readonly<{ [functionalParameters.name]: functionalParameters.rule, [immutableData.name]: immutableData.rule, [noClasses.name]: noClasses.rule, + [noClassInheritance.name]: noClassInheritance.rule, [noConditionalStatements.name]: noConditionalStatements.rule, [noExpressionStatements.name]: noExpressionStatements.rule, [noLet.name]: noLet.rule, diff --git a/src/rules/no-class-inheritance.ts b/src/rules/no-class-inheritance.ts new file mode 100644 index 000000000..a4b33e6cd --- /dev/null +++ b/src/rules/no-class-inheritance.ts @@ -0,0 +1,157 @@ +import type { JSONSchema4 } from "@typescript-eslint/utils/json-schema"; +import type { RuleContext } from "@typescript-eslint/utils/ts-eslint"; +import { deepmerge } from "deepmerge-ts"; + +import { + type IgnoreCodePatternOption, + type IgnoreIdentifierPatternOption, + ignoreCodePatternOptionSchema, + ignoreIdentifierPatternOptionSchema, + shouldIgnorePattern, +} from "#/options"; +import { ruleNameScope } from "#/utils/misc"; +import type { ESClass } from "#/utils/node-types"; +import { + type NamedCreateRuleCustomMeta, + type Rule, + type RuleResult, + createRule, +} from "#/utils/rule"; + +/** + * The name of this rule. + */ +export const name = "no-class-inheritance"; + +/** + * The full name of this rule. + */ +export const fullName: `${typeof ruleNameScope}/${typeof name}` = `${ruleNameScope}/${name}`; + +/** + * The options this rule can take. + */ +type Options = [IgnoreIdentifierPatternOption & IgnoreCodePatternOption]; + +/** + * The schema for the rule options. + */ +const schema: JSONSchema4[] = [ + { + type: "object", + properties: deepmerge( + ignoreIdentifierPatternOptionSchema, + ignoreCodePatternOptionSchema, + ), + additionalProperties: false, + }, +]; + +/** + * The default options for the rule. + */ +const defaultOptions: Options = [{}]; + +/** + * The possible error messages. + */ +const errorMessages = { + abstract: "Unexpected abstract class.", + extends: "Unexpected inheritance, use composition instead.", +} as const; + +/** + * The meta data for this rule. + */ +const meta: NamedCreateRuleCustomMeta = { + type: "suggestion", + docs: { + category: "No Other Paradigms", + description: "Disallow inheritance in classes.", + recommended: "recommended", + recommendedSeverity: "error", + requiresTypeChecking: false, + }, + messages: errorMessages, + schema, +}; + +/** + * Check if the given class node violates this rule. + */ +function checkClass( + node: ESClass, + context: Readonly>, + options: Readonly, +): RuleResult { + const [optionsObject] = options; + const { ignoreIdentifierPattern, ignoreCodePattern } = optionsObject; + + const m_descriptors: Array< + RuleResult["descriptors"][number] + > = []; + + if ( + !shouldIgnorePattern( + node, + context, + ignoreIdentifierPattern, + undefined, + ignoreCodePattern, + ) + ) { + if (node.abstract) { + const nodeText = context.sourceCode.getText(node); + const abstractRelativeIndex = nodeText.indexOf("abstract"); + const abstractIndex = + context.sourceCode.getIndexFromLoc(node.loc.start) + + abstractRelativeIndex; + const start = context.sourceCode.getLocFromIndex(abstractIndex); + const end = context.sourceCode.getLocFromIndex( + abstractIndex + "abstract".length, + ); + + m_descriptors.push({ + node, + loc: { + start, + end, + }, + messageId: "abstract", + }); + } + + if (node.superClass !== null) { + const nodeText = context.sourceCode.getText(node); + const extendsRelativeIndex = nodeText.indexOf("extends"); + const extendsIndex = + context.sourceCode.getIndexFromLoc(node.loc.start) + + extendsRelativeIndex; + const start = context.sourceCode.getLocFromIndex(extendsIndex); + const { end } = node.superClass.loc; + + m_descriptors.push({ + node, + loc: { + start, + end, + }, + messageId: "extends", + }); + } + } + + return { + context, + descriptors: m_descriptors, + }; +} + +// Create the rule. +export const rule: Rule = createRule< + keyof typeof errorMessages, + Options +>(name, meta, defaultOptions, { + ClassDeclaration: checkClass, + ClassExpression: checkClass, +}); diff --git a/tests/rules/__snapshots__/no-class-inheritance.test.ts.snap b/tests/rules/__snapshots__/no-class-inheritance.test.ts.snap new file mode 100644 index 000000000..a11213fd1 --- /dev/null +++ b/tests/rules/__snapshots__/no-class-inheritance.test.ts.snap @@ -0,0 +1,124 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`no-class-inheritance > javascript - es latest > ignoreCodePattern > should report class inheritance with non-matching identifiers 1`] = ` +[ + { + "column": 11, + "endColumn": 22, + "endLine": 1, + "line": 1, + "message": "Unexpected inheritance, use composition instead.", + "messageId": "extends", + "nodeType": "ClassDeclaration", + "ruleId": "no-class-inheritance", + "severity": 2, + }, +] +`; + +exports[`no-class-inheritance > javascript - es latest > options > ignoreIdentifierPattern > should report class inheritance with non-matching identifiers 1`] = ` +[ + { + "column": 11, + "endColumn": 22, + "endLine": 1, + "line": 1, + "message": "Unexpected inheritance, use composition instead.", + "messageId": "extends", + "nodeType": "ClassDeclaration", + "ruleId": "no-class-inheritance", + "severity": 2, + }, +] +`; + +exports[`no-class-inheritance > javascript - es latest > reports class inheritance 1`] = ` +[ + { + "column": 11, + "endColumn": 22, + "endLine": 1, + "line": 1, + "message": "Unexpected inheritance, use composition instead.", + "messageId": "extends", + "nodeType": "ClassDeclaration", + "ruleId": "no-class-inheritance", + "severity": 2, + }, +] +`; + +exports[`no-class-inheritance > typescript > ignoreCodePattern > should report class inheritance with non-matching identifiers 1`] = ` +[ + { + "column": 1, + "endColumn": 9, + "endLine": 1, + "line": 1, + "message": "Unexpected abstract class.", + "messageId": "abstract", + "nodeType": "ClassDeclaration", + "ruleId": "no-class-inheritance", + "severity": 2, + }, +] +`; + +exports[`no-class-inheritance > typescript > options > ignoreIdentifierPattern > should report class inheritance with non-matching identifiers 1`] = ` +[ + { + "column": 1, + "endColumn": 9, + "endLine": 1, + "line": 1, + "message": "Unexpected abstract class.", + "messageId": "abstract", + "nodeType": "ClassDeclaration", + "ruleId": "no-class-inheritance", + "severity": 2, + }, +] +`; + +exports[`no-class-inheritance > typescript > reports class inheritance 1`] = ` +[ + { + "column": 1, + "endColumn": 9, + "endLine": 1, + "line": 1, + "message": "Unexpected abstract class.", + "messageId": "abstract", + "nodeType": "ClassDeclaration", + "ruleId": "no-class-inheritance", + "severity": 2, + }, +] +`; + +exports[`no-class-inheritance > typescript > reports class inheritance 2`] = ` +[ + { + "column": 1, + "endColumn": 9, + "endLine": 1, + "line": 1, + "message": "Unexpected abstract class.", + "messageId": "abstract", + "nodeType": "ClassDeclaration", + "ruleId": "no-class-inheritance", + "severity": 2, + }, + { + "column": 20, + "endColumn": 31, + "endLine": 1, + "line": 1, + "message": "Unexpected inheritance, use composition instead.", + "messageId": "extends", + "nodeType": "ClassDeclaration", + "ruleId": "no-class-inheritance", + "severity": 2, + }, +] +`; diff --git a/tests/rules/no-class-inheritance.test.ts b/tests/rules/no-class-inheritance.test.ts new file mode 100644 index 000000000..1b324f888 --- /dev/null +++ b/tests/rules/no-class-inheritance.test.ts @@ -0,0 +1,143 @@ +import dedent from "dedent"; +import { createRuleTester } from "eslint-vitest-rule-tester"; +import { describe, expect, it } from "vitest"; + +import { name, rule } from "#/rules/no-class-inheritance"; + +import { esLatestConfig, typescriptConfig } from "../utils/configs"; + +describe(name, () => { + describe("javascript - es latest", () => { + const { valid, invalid } = createRuleTester({ + name, + rule, + configs: esLatestConfig, + }); + + it("doesn't report non-issues", () => { + valid("class Foo {}"); + }); + + it("reports class inheritance", () => { + const invalidResult1 = invalid({ + code: "class Foo extends Bar {}", + errors: ["extends"], + }); + expect(invalidResult1.messages).toMatchSnapshot(); + }); + + describe("options", () => { + describe("ignoreIdentifierPattern", () => { + it("should not report class inheritance with matching identifiers", () => { + valid({ + code: dedent` + class Foo extends Bar {} + `, + options: [{ ignoreIdentifierPattern: "^Foo$" }], + }); + }); + + it("should report class inheritance with non-matching identifiers", () => { + const invalidResult = invalid({ + code: dedent` + class Bar extends Foo {} + `, + options: [{ ignoreIdentifierPattern: "^Foo$" }], + errors: ["extends"], + }); + expect(invalidResult.messages).toMatchSnapshot(); + }); + }); + }); + + describe("ignoreCodePattern", () => { + it("should not report class inheritance with matching identifiers", () => { + valid({ + code: dedent` + class Foo extends Bar {} + `, + options: [{ ignoreCodePattern: "class Foo" }], + }); + }); + + it("should report class inheritance with non-matching identifiers", () => { + const invalidResult = invalid({ + code: dedent` + class Bar extends Foo {} + `, + options: [{ ignoreCodePattern: "class Foo" }], + errors: ["extends"], + }); + expect(invalidResult.messages).toMatchSnapshot(); + }); + }); + }); + + describe("typescript", () => { + const { valid, invalid } = createRuleTester({ + name, + rule, + configs: typescriptConfig, + }); + + it("reports class inheritance", () => { + const invalidResult1 = invalid({ + code: "abstract class Foo {}", + errors: ["abstract"], + }); + expect(invalidResult1.messages).toMatchSnapshot(); + + const invalidResult2 = invalid({ + code: "abstract class Foo extends Bar {}", + errors: ["abstract", "extends"], + }); + expect(invalidResult2.messages).toMatchSnapshot(); + }); + + describe("options", () => { + describe("ignoreIdentifierPattern", () => { + it("should not report class inheritance with matching identifiers", () => { + valid({ + code: dedent` + abstract class Foo {} + `, + options: [{ ignoreIdentifierPattern: "^Foo$" }], + }); + }); + + it("should report class inheritance with non-matching identifiers", () => { + const invalidResult = invalid({ + code: dedent` + abstract class Bar {} + `, + options: [{ ignoreIdentifierPattern: "^Foo$" }], + errors: ["abstract"], + }); + expect(invalidResult.messages).toMatchSnapshot(); + }); + }); + }); + + describe("ignoreCodePattern", () => { + it("should not report class inheritance with matching identifiers", () => { + valid({ + code: dedent` + abstract class Foo {} + `, + options: [{ ignoreCodePattern: "class Foo" }], + }); + }); + + it("should report class inheritance with non-matching identifiers", () => { + const invalidResult = invalid({ + code: dedent` + abstract class Bar {} + `, + options: [{ ignoreCodePattern: "class Foo" }], + errors: ["abstract"], + }); + expect(invalidResult.messages).toMatchSnapshot(); + }); + }); + }); +});