From 02bfd27527743266b946733da2c059ed925d6a2e Mon Sep 17 00:00:00 2001 From: ren-yamanashi Date: Thu, 26 Dec 2024 23:19:40 +0900 Subject: [PATCH] refactor: rule codes --- src/__tests__/no-class-in-interface.test.ts | 4 +- ...ce-props.mts => no-class-in-interface.mts} | 6 +- src/no-construct-stack-suffix.mts | 23 ++---- src/no-import-private.mts | 30 ++++---- src/no-mutable-props-interface.mts | 4 +- src/no-mutable-public-fields.mts | 11 +-- src/no-parent-name-construct-id-match.mts | 19 +++-- src/no-public-class-fields.mts | 74 ++++++------------- src/no-variable-construct-id.mts | 28 +++---- src/pascal-case-construct-id.mts | 44 +++++------ src/require-passing-this.mts | 22 +++--- 11 files changed, 99 insertions(+), 166 deletions(-) rename src/{no-class-in-interface-props.mts => no-class-in-interface.mts} (85%) diff --git a/src/__tests__/no-class-in-interface.test.ts b/src/__tests__/no-class-in-interface.test.ts index 312d082..6bd47b6 100644 --- a/src/__tests__/no-class-in-interface.test.ts +++ b/src/__tests__/no-class-in-interface.test.ts @@ -1,6 +1,6 @@ import { RuleTester } from "@typescript-eslint/rule-tester"; -import { noClassInInterfaceProps } from "../no-class-in-interface-props.mjs"; +import { noClassInInterface } from "../no-class-in-interface.mjs"; const ruleTester = new RuleTester({ languageOptions: { @@ -12,7 +12,7 @@ const ruleTester = new RuleTester({ }, }); -ruleTester.run("no-class-in-interface", noClassInInterfaceProps, { +ruleTester.run("no-class-in-interface", noClassInInterface, { valid: [ // WHEN: property type is string { diff --git a/src/no-class-in-interface-props.mts b/src/no-class-in-interface.mts similarity index 85% rename from src/no-class-in-interface-props.mts rename to src/no-class-in-interface.mts index c36d893..725f11c 100644 --- a/src/no-class-in-interface-props.mts +++ b/src/no-class-in-interface.mts @@ -7,7 +7,7 @@ import { SymbolFlags } from "typescript"; * @returns An object containing the AST visitor functions * @see {@link https://eslint-cdk-plugin.dev/rules/no-class-in-interface} - Documentation */ -export const noClassInInterfaceProps = ESLintUtils.RuleCreator.withoutDocs({ +export const noClassInInterface = ESLintUtils.RuleCreator.withoutDocs({ meta: { type: "problem", docs: { @@ -22,7 +22,6 @@ export const noClassInInterfaceProps = ESLintUtils.RuleCreator.withoutDocs({ defaultOptions: [], create(context) { const parserServices = ESLintUtils.getParserServices(context); - const typeChecker = parserServices.program.getTypeChecker(); return { TSInterfaceDeclaration(node) { for (const property of node.body.body) { @@ -34,8 +33,7 @@ export const noClassInInterfaceProps = ESLintUtils.RuleCreator.withoutDocs({ continue; } - const tsNode = parserServices.esTreeNodeToTSNodeMap.get(property); - const type = typeChecker.getTypeAtLocation(tsNode); + const type = parserServices.getTypeAtLocation(property); if (!type.symbol) continue; // NOTE: check class type diff --git a/src/no-construct-stack-suffix.mts b/src/no-construct-stack-suffix.mts index 7dedc36..9002567 100644 --- a/src/no-construct-stack-suffix.mts +++ b/src/no-construct-stack-suffix.mts @@ -32,19 +32,13 @@ export const noConstructStackSuffix = ESLintUtils.RuleCreator.withoutDocs({ defaultOptions: [], create(context) { const parserServices = ESLintUtils.getParserServices(context); - const typeChecker = parserServices.program.getTypeChecker(); return { NewExpression(node) { - const type = typeChecker.getTypeAtLocation( - parserServices.esTreeNodeToTSNodeMap.get(node) - ); - if (!isConstructOrStackType(type)) { + const type = parserServices.getTypeAtLocation(node); + if (!isConstructOrStackType(type) || node.arguments.length < 2) { return; } - - if (node.arguments.length < 2) return; - - validateConstructId(node, context, node); + validateConstructId(node, context); }, }; }, @@ -54,14 +48,11 @@ export const noConstructStackSuffix = ESLintUtils.RuleCreator.withoutDocs({ * Validate that construct ID does not end with "Construct" or "Stack" */ const validateConstructId = ( - node: TSESTree.Node, - context: Context, - expression: TSESTree.NewExpression + node: TSESTree.NewExpression, + context: Context ): void => { - if (expression.arguments.length < 2) return; - // NOTE: Treat the second argument as ID - const secondArg = expression.arguments[1]; + const secondArg = node.arguments[1]; if ( secondArg.type !== AST_NODE_TYPES.Literal || typeof secondArg.value !== "string" @@ -69,7 +60,7 @@ const validateConstructId = ( return; } - const formattedConstructId = toPascalCase(secondArg.value as string); + const formattedConstructId = toPascalCase(secondArg.value); if (formattedConstructId.endsWith("Construct")) { context.report({ diff --git a/src/no-import-private.mts b/src/no-import-private.mts index 8beaf03..8c19fcd 100644 --- a/src/no-import-private.mts +++ b/src/no-import-private.mts @@ -28,24 +28,22 @@ export const noImportPrivate: Rule.RuleModule = { const currentFilePath = context.filename; const currentDirPath = path.dirname(currentFilePath); - if (importPath.includes("/private")) { - const absoluteCurrentDirPath = path.resolve(currentDirPath); - const absoluteImportPath = path.resolve(currentDirPath, importPath); + if (!importPath.includes("/private")) return; + const absoluteCurrentDirPath = path.resolve(currentDirPath); + const absoluteImportPath = path.resolve(currentDirPath, importPath); - // NOTE: Get the directory from the import path up to the private directory - const importDirBeforePrivate = - absoluteImportPath.split("/private")[0]; + // NOTE: Get the directory from the import path up to the private directory + const importDirBeforePrivate = absoluteImportPath.split("/private")[0]; - const currentDirSegments = getDirSegments(absoluteCurrentDirPath); - const importDirSegments = getDirSegments(importDirBeforePrivate); - if ( - currentDirSegments.length !== importDirSegments.length || - currentDirSegments.some( - (segment, index) => segment !== importDirSegments[index] - ) - ) { - context.report({ node, messageId: "noImportPrivate" }); - } + const currentDirSegments = getDirSegments(absoluteCurrentDirPath); + const importDirSegments = getDirSegments(importDirBeforePrivate); + if ( + currentDirSegments.length !== importDirSegments.length || + currentDirSegments.some( + (segment, index) => segment !== importDirSegments[index] + ) + ) { + context.report({ node, messageId: "noImportPrivate" }); } }, }; diff --git a/src/no-mutable-props-interface.mts b/src/no-mutable-props-interface.mts index 9a4a039..2916000 100644 --- a/src/no-mutable-props-interface.mts +++ b/src/no-mutable-props-interface.mts @@ -21,10 +21,10 @@ export const noMutablePropsInterface = ESLintUtils.RuleCreator.withoutDocs({ }, defaultOptions: [], create(context) { - const sourceCode = context.sourceCode; - return { TSInterfaceDeclaration(node) { + const sourceCode = context.sourceCode; + // NOTE: Interface name check for "Props" if (!node.id.name.endsWith("Props")) return; diff --git a/src/no-mutable-public-fields.mts b/src/no-mutable-public-fields.mts index a021205..8a8a225 100644 --- a/src/no-mutable-public-fields.mts +++ b/src/no-mutable-public-fields.mts @@ -24,17 +24,12 @@ export const noMutablePublicFields = ESLintUtils.RuleCreator.withoutDocs({ defaultOptions: [], create(context) { const parserServices = ESLintUtils.getParserServices(context); - const typeChecker = parserServices.program.getTypeChecker(); - const sourceCode = context.sourceCode; return { ClassDeclaration(node) { - const type = typeChecker.getTypeAtLocation( - parserServices.esTreeNodeToTSNodeMap.get(node) - ); - if (!isConstructOrStackType(type)) { - return; - } + const sourceCode = context.sourceCode; + const type = parserServices.getTypeAtLocation(node); + if (!isConstructOrStackType(type)) return; for (const member of node.body.body) { // NOTE: check property definition diff --git a/src/no-parent-name-construct-id-match.mts b/src/no-parent-name-construct-id-match.mts index cfac6af..94fb78f 100644 --- a/src/no-parent-name-construct-id-match.mts +++ b/src/no-parent-name-construct-id-match.mts @@ -299,15 +299,14 @@ const validateConstructId = ({ const formattedConstructId = toPascalCase(secondArg.value as string); const formattedParentClassName = toPascalCase(parentClassName); + if (formattedParentClassName !== formattedConstructId) return; - if (formattedParentClassName === formattedConstructId) { - context.report({ - node, - messageId: "noParentNameConstructIdMatch", - data: { - constructId: secondArg.value, - parentConstructName: parentClassName, - }, - }); - } + context.report({ + node, + messageId: "noParentNameConstructIdMatch", + data: { + constructId: secondArg.value, + parentConstructName: parentClassName, + }, + }); }; diff --git a/src/no-public-class-fields.mts b/src/no-public-class-fields.mts index f624234..3e63c91 100644 --- a/src/no-public-class-fields.mts +++ b/src/no-public-class-fields.mts @@ -5,7 +5,7 @@ import { TSESLint, TSESTree, } from "@typescript-eslint/utils"; -import { SymbolFlags, TypeChecker } from "typescript"; +import { SymbolFlags } from "typescript"; import { isConstructOrStackType } from "./utils/typeCheck.mjs"; @@ -32,23 +32,13 @@ export const noPublicClassFields = ESLintUtils.RuleCreator.withoutDocs({ defaultOptions: [], create(context) { const parserServices = ESLintUtils.getParserServices(context); - const typeChecker = parserServices.program.getTypeChecker(); return { ClassDeclaration(node) { - const type = typeChecker.getTypeAtLocation( - parserServices.esTreeNodeToTSNodeMap.get(node) - ); - if (!isConstructOrStackType(type)) { - return; - } + const type = parserServices.getTypeAtLocation(node); + if (!isConstructOrStackType(type)) return; // NOTE: Check class members - validateClassMember({ - node, - context, - parserServices, - typeChecker, - }); + validateClassMember(node, context, parserServices); // NOTE: Check constructor parameter properties const constructor = node.body.body.find( @@ -62,12 +52,12 @@ export const noPublicClassFields = ESLintUtils.RuleCreator.withoutDocs({ ) { return; } - validateConstructorParameterProperty({ + + validateConstructorParameterProperty( constructor, context, - parserServices, - typeChecker, - }); + parserServices + ); }, }; }, @@ -77,17 +67,11 @@ export const noPublicClassFields = ESLintUtils.RuleCreator.withoutDocs({ * check the public variable of the class * - if it is a class type, report an error */ -const validateClassMember = ({ - node, - context, - parserServices, - typeChecker, -}: { - node: TSESTree.ClassDeclaration; - context: Context; - parserServices: ParserServicesWithTypeInformation; - typeChecker: TypeChecker; -}) => { +const validateClassMember = ( + node: TSESTree.ClassDeclaration, + context: Context, + parserServices: ParserServicesWithTypeInformation +) => { for (const member of node.body.body) { if ( member.type !== AST_NODE_TYPES.PropertyDefinition || @@ -102,13 +86,9 @@ const validateClassMember = ({ } // NOTE: Skip fields without type annotation - if (!member.typeAnnotation) { - continue; - } - - const tsNode = parserServices.esTreeNodeToTSNodeMap.get(member); - const type = typeChecker.getTypeAtLocation(tsNode); + if (!member.typeAnnotation) continue; + const type = parserServices.getTypeAtLocation(member); if (!type.symbol) continue; const isClass = type.symbol.flags === SymbolFlags.Class; @@ -129,17 +109,11 @@ const validateClassMember = ({ * check the constructor parameter property * - if it is a class type, report an error */ -const validateConstructorParameterProperty = ({ - constructor, - context, - parserServices, - typeChecker, -}: { - constructor: TSESTree.MethodDefinition; - context: Context; - parserServices: ParserServicesWithTypeInformation; - typeChecker: TypeChecker; -}) => { +const validateConstructorParameterProperty = ( + constructor: TSESTree.MethodDefinition, + context: Context, + parserServices: ParserServicesWithTypeInformation +) => { for (const param of constructor.value.params) { if ( param.type !== AST_NODE_TYPES.TSParameterProperty || @@ -154,13 +128,9 @@ const validateConstructorParameterProperty = ({ } // NOTE: Skip parameters without type annotation - if (!param.parameter.typeAnnotation) { - continue; - } - - const tsNode = parserServices.esTreeNodeToTSNodeMap.get(param); - const type = typeChecker.getTypeAtLocation(tsNode); + if (!param.parameter.typeAnnotation) continue; + const type = parserServices.getTypeAtLocation(param); if (!type.symbol) continue; const isClass = type.symbol.flags === SymbolFlags.Class; diff --git a/src/no-variable-construct-id.mts b/src/no-variable-construct-id.mts index 9514de9..5dad20d 100644 --- a/src/no-variable-construct-id.mts +++ b/src/no-variable-construct-id.mts @@ -28,20 +28,19 @@ export const noVariableConstructId = ESLintUtils.RuleCreator.withoutDocs({ defaultOptions: [], create(context) { const parserServices = ESLintUtils.getParserServices(context); - const typeChecker = parserServices.program.getTypeChecker(); - return { NewExpression(node) { - const type = typeChecker.getTypeAtLocation( - parserServices.esTreeNodeToTSNodeMap.get(node) - ); - if (!isConstructType(type) || isStackType(type)) { + const type = parserServices.getTypeAtLocation(node); + + if ( + !isConstructType(type) || + isStackType(type) || + node.arguments.length < 2 + ) { return; } - if (node.arguments.length < 2) return; - - validateConstructId(node, context, node); + validateConstructId(node, context); }, }; }, @@ -51,16 +50,13 @@ export const noVariableConstructId = ESLintUtils.RuleCreator.withoutDocs({ * Check if the construct ID is a literal string */ const validateConstructId = ( - node: TSESTree.Node, - context: Context, - expression: TSESTree.NewExpression + node: TSESTree.NewExpression, + context: Context ) => { - if (expression.arguments.length < 2 || isInsideLoop(node)) { - return; - } + if (node.arguments.length < 2 || isInsideLoop(node)) return; // NOTE: Treat the second argument as ID - const secondArg = expression.arguments[1]; + const secondArg = node.arguments[1]; // NOTE: When id is string literal, it's OK if ( diff --git a/src/pascal-case-construct-id.mts b/src/pascal-case-construct-id.mts index 6f3c11b..41ed750 100644 --- a/src/pascal-case-construct-id.mts +++ b/src/pascal-case-construct-id.mts @@ -39,19 +39,13 @@ export const pascalCaseConstructId = ESLintUtils.RuleCreator.withoutDocs({ defaultOptions: [], create(context) { const parserServices = ESLintUtils.getParserServices(context); - const typeChecker = parserServices.program.getTypeChecker(); return { NewExpression(node) { - const type = typeChecker.getTypeAtLocation( - parserServices.esTreeNodeToTSNodeMap.get(node) - ); - if (!isConstructOrStackType(type)) { + const type = parserServices.getTypeAtLocation(node); + if (!isConstructOrStackType(type) || node.arguments.length < 2) { return; } - - if (node.arguments.length < 2) return; - - validateConstructId(node, context, node); + validateConstructId(node, context); }, }; }, @@ -70,14 +64,13 @@ const isPascalCase = (str: string) => { * Check the construct ID is PascalCase */ const validateConstructId = ( - node: TSESTree.Node, - context: Context, - expression: TSESTree.NewExpression + node: TSESTree.NewExpression, + context: Context ) => { - if (expression.arguments.length < 2) return; + if (node.arguments.length < 2) return; // NOTE: Treat the second argument as ID - const secondArg = expression.arguments[1]; + const secondArg = node.arguments[1]; if ( secondArg.type !== AST_NODE_TYPES.Literal || typeof secondArg.value !== "string" @@ -89,17 +82,14 @@ const validateConstructId = ( ? QUOTE_TYPE.DOUBLE : QUOTE_TYPE.SINGLE; - if (!isPascalCase(secondArg.value)) { - context.report({ - node, - messageId: "pascalCaseConstructId", - fix: (fixer) => { - const pascalCaseValue = toPascalCase(secondArg.value as string); - return fixer.replaceText( - secondArg, - `${quote}${pascalCaseValue}${quote}` - ); - }, - }); - } + if (isPascalCase(secondArg.value)) return; + + context.report({ + node, + messageId: "pascalCaseConstructId", + fix: (fixer) => { + const pascalCaseValue = toPascalCase(secondArg.value as string); + return fixer.replaceText(secondArg, `${quote}${pascalCaseValue}${quote}`); + }, + }); }; diff --git a/src/require-passing-this.mts b/src/require-passing-this.mts index 4d0ad84..0851434 100644 --- a/src/require-passing-this.mts +++ b/src/require-passing-this.mts @@ -23,12 +23,9 @@ export const requirePassingThis = ESLintUtils.RuleCreator.withoutDocs({ defaultOptions: [], create(context) { const parserServices = ESLintUtils.getParserServices(context); - const typeChecker = parserServices.program.getTypeChecker(); return { NewExpression(node) { - const type = typeChecker.getTypeAtLocation( - parserServices.esTreeNodeToTSNodeMap.get(node) - ); + const type = parserServices.getTypeAtLocation(node); if ( !isConstructType(type) || @@ -39,16 +36,15 @@ export const requirePassingThis = ESLintUtils.RuleCreator.withoutDocs({ } const argument = node.arguments[0]; + if (argument.type === AST_NODE_TYPES.ThisExpression) return; - if (argument.type !== AST_NODE_TYPES.ThisExpression) { - context.report({ - node, - messageId: "requirePassingThis", - fix: (fixer) => { - return fixer.replaceText(argument, "this"); - }, - }); - } + context.report({ + node, + messageId: "requirePassingThis", + fix: (fixer) => { + return fixer.replaceText(argument, "this"); + }, + }); }, }; },