From 4143df2b093a3de3e973afab67bb69ce80785ab9 Mon Sep 17 00:00:00 2001 From: Ren Yamanashi <96362223+ren-yamanashi@users.noreply.github.com> Date: Sat, 18 Jan 2025 01:14:54 +0900 Subject: [PATCH] fix: check property names during instantiation (#94) * fix: require-passing-this * fix: check property name * chore: delete unwanted diffs * docs: add jsDoc --- .../no-construct-stack-suffix.test.ts | 13 ++++++++++-- .../no-variable-construct-id.test.ts | 17 ++++++++++++++++ .../pascal-case-construct-id.test.ts | 11 ++++++++++ src/__tests__/require-passing-this.test.ts | 17 ++++++++++++++++ src/rules/no-construct-stack-suffix.ts | 5 +++++ src/rules/no-variable-construct-id.ts | 4 ++++ src/rules/pascal-case-construct-id.ts | 5 +++++ src/rules/require-passing-this.ts | 4 ++++ src/utils/parseType.ts | 20 +++++++++++++++++++ 9 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 src/utils/parseType.ts diff --git a/src/__tests__/no-construct-stack-suffix.test.ts b/src/__tests__/no-construct-stack-suffix.test.ts index 3a00dd7..2a54311 100644 --- a/src/__tests__/no-construct-stack-suffix.test.ts +++ b/src/__tests__/no-construct-stack-suffix.test.ts @@ -41,7 +41,7 @@ ruleTester.run("no-construct-stack-suffix", noConstructStackSuffix, { { code: ` class TestClass { - constructor(public id: string) {} + constructor(props: any, id: string) {} } const test = new TestClass("test", "SampleConstruct"); `, @@ -49,7 +49,7 @@ ruleTester.run("no-construct-stack-suffix", noConstructStackSuffix, { { code: ` class TestClass { - constructor(public id: string) {} + constructor(props: any, id: string) {} } class Sample { constructor() { @@ -57,6 +57,15 @@ ruleTester.run("no-construct-stack-suffix", noConstructStackSuffix, { } }`, }, + // WHEN: property name is not `id` + { + code: ` + class TestClass { + constructor(props: any, validId: string) {} + } + const test = new TestClass("test", "SampleConstruct"); + `, + }, ], invalid: [ // WHEN: construct id has "Construct" suffix, and extends Construct diff --git a/src/__tests__/no-variable-construct-id.test.ts b/src/__tests__/no-variable-construct-id.test.ts index 4906599..4f6f4f1 100644 --- a/src/__tests__/no-variable-construct-id.test.ts +++ b/src/__tests__/no-variable-construct-id.test.ts @@ -88,6 +88,23 @@ ruleTester.run("no-variable-construct-id", noVariableConstructId, { } `, }, + // WHEN: property name is not `id` + { + code: ` + class Construct {} + class TargetConstruct extends Construct { + constructor(scope: Construct, validId: string) { + super(scope, validId); + } + } + class SampleConstruct extends Construct { + constructor(scope: Construct, id: string) { + super(scope, id); + new TargetConstruct(this, id); + } + } + `, + }, ], invalid: [ // WHEN: id is variable diff --git a/src/__tests__/pascal-case-construct-id.test.ts b/src/__tests__/pascal-case-construct-id.test.ts index ab74ef0..0055b4f 100644 --- a/src/__tests__/pascal-case-construct-id.test.ts +++ b/src/__tests__/pascal-case-construct-id.test.ts @@ -79,6 +79,17 @@ ruleTester.run("pascal-case-construct-id", pascalCaseConstructId, { } const test = new TestClass('test', 'ValidId');`, }, + // WHEN: property name is not `id` + { + code: ` + class Construct {} + class TestClass extends Construct { + constructor(props: any, validId: string) { + super(props, validId); + } + } + const test = new TestClass("test", "invalid_id");`, + }, ], invalid: [ // WHEN: id is snake_case(double quote) diff --git a/src/__tests__/require-passing-this.test.ts b/src/__tests__/require-passing-this.test.ts index 72ce2bf..bcb52ba 100644 --- a/src/__tests__/require-passing-this.test.ts +++ b/src/__tests__/require-passing-this.test.ts @@ -48,6 +48,23 @@ ruleTester.run("require-passing-this", requirePassingThis, { } `, }, + // WHEN: property name is not `scope` + { + code: ` + class Construct {} + class SampleConstruct extends Construct { + constructor(validProperty: Construct, id: string) { + super(validProperty, id); + } + } + class TestConstruct extends Construct { + constructor(scope: Construct, id: string) { + super(scope, id); + new SampleConstruct(scope, "ValidId"); + } + } + `, + }, ], invalid: [ // WHEN: not passing `this` to a constructor diff --git a/src/rules/no-construct-stack-suffix.ts b/src/rules/no-construct-stack-suffix.ts index c4b50f5..a9d9e71 100644 --- a/src/rules/no-construct-stack-suffix.ts +++ b/src/rules/no-construct-stack-suffix.ts @@ -6,6 +6,7 @@ import { } from "@typescript-eslint/utils"; import { toPascalCase } from "../utils/convertString"; +import { getConstructorPropertyNames } from "../utils/parseType"; import { isConstructOrStackType } from "../utils/typeCheck"; type Context = TSESLint.RuleContext<"noConstructStackSuffix", []>; @@ -38,6 +39,10 @@ export const noConstructStackSuffix = ESLintUtils.RuleCreator.withoutDocs({ if (!isConstructOrStackType(type) || node.arguments.length < 2) { return; } + + const constructorPropertyNames = getConstructorPropertyNames(type); + if (constructorPropertyNames[1] !== "id") return; + validateConstructId(node, context); }, }; diff --git a/src/rules/no-variable-construct-id.ts b/src/rules/no-variable-construct-id.ts index 0fb33aa..5d4cf17 100644 --- a/src/rules/no-variable-construct-id.ts +++ b/src/rules/no-variable-construct-id.ts @@ -5,6 +5,7 @@ import { TSESTree, } from "@typescript-eslint/utils"; +import { getConstructorPropertyNames } from "../utils/parseType"; import { isConstructType, isStackType } from "../utils/typeCheck"; type Context = TSESLint.RuleContext<"noVariableConstructId", []>; @@ -40,6 +41,9 @@ export const noVariableConstructId = ESLintUtils.RuleCreator.withoutDocs({ return; } + const constructorPropertyNames = getConstructorPropertyNames(type); + if (constructorPropertyNames[1] !== "id") return; + validateConstructId(node, context); }, }; diff --git a/src/rules/pascal-case-construct-id.ts b/src/rules/pascal-case-construct-id.ts index 633c4d8..2f98950 100644 --- a/src/rules/pascal-case-construct-id.ts +++ b/src/rules/pascal-case-construct-id.ts @@ -6,6 +6,7 @@ import { } from "@typescript-eslint/utils"; import { toPascalCase } from "../utils/convertString"; +import { getConstructorPropertyNames } from "../utils/parseType"; import { isConstructOrStackType } from "../utils/typeCheck"; const QUOTE_TYPE = { @@ -45,6 +46,10 @@ export const pascalCaseConstructId = ESLintUtils.RuleCreator.withoutDocs({ if (!isConstructOrStackType(type) || node.arguments.length < 2) { return; } + + const constructorPropertyNames = getConstructorPropertyNames(type); + if (constructorPropertyNames[1] !== "id") return; + validateConstructId(node, context); }, }; diff --git a/src/rules/require-passing-this.ts b/src/rules/require-passing-this.ts index 1e46b17..0a72249 100644 --- a/src/rules/require-passing-this.ts +++ b/src/rules/require-passing-this.ts @@ -1,5 +1,6 @@ import { AST_NODE_TYPES, ESLintUtils } from "@typescript-eslint/utils"; +import { getConstructorPropertyNames } from "../utils/parseType"; import { isConstructType, isStackType } from "../utils/typeCheck"; /** @@ -38,6 +39,9 @@ export const requirePassingThis = ESLintUtils.RuleCreator.withoutDocs({ const argument = node.arguments[0]; if (argument.type === AST_NODE_TYPES.ThisExpression) return; + const constructorPropertyNames = getConstructorPropertyNames(type); + if (constructorPropertyNames[0] !== "scope") return; + context.report({ node, messageId: "requirePassingThis", diff --git a/src/utils/parseType.ts b/src/utils/parseType.ts new file mode 100644 index 0000000..81d369a --- /dev/null +++ b/src/utils/parseType.ts @@ -0,0 +1,20 @@ +import { isClassDeclaration, isConstructorDeclaration, Type } from "typescript"; + +/** + * Parses type to get the property names of the class constructor. + * @returns The property names of the class constructor. + */ +export const getConstructorPropertyNames = (type: Type): string[] => { + const declarations = type.symbol?.declarations; + if (!declarations?.length) return []; + + const classDeclaration = declarations[0]; + if (!isClassDeclaration(classDeclaration)) return []; + + const constructor = classDeclaration.members.find((member) => + isConstructorDeclaration(member) + ); + if (!constructor?.parameters.length) return []; + + return constructor.parameters.map((param) => param.name.getText()); +};