From b6baccf2cd0f50491c91265829d256a6f5810ead Mon Sep 17 00:00:00 2001 From: Stefan Dirix Date: Wed, 13 Nov 2024 11:53:28 +0100 Subject: [PATCH] dev: add eslint rule for annotation checks Inversify >=6.1 requires to annotate all constructor parameters of injectable classes. To avoid runtime errors we add a new eslint rule to check for this at lint time. --- configs/errors.eslintrc.json | 1 + dev-packages/private-eslint-plugin/README.md | 5 + dev-packages/private-eslint-plugin/index.js | 1 + .../rules/annotation-check.js | 103 ++++++++++++++++++ 4 files changed, 110 insertions(+) create mode 100644 dev-packages/private-eslint-plugin/rules/annotation-check.js diff --git a/configs/errors.eslintrc.json b/configs/errors.eslintrc.json index 13f2ea5250dda..e7eb84276e76e 100644 --- a/configs/errors.eslintrc.json +++ b/configs/errors.eslintrc.json @@ -135,6 +135,7 @@ } } ], + "@theia/annotation-check": "error", "@theia/localization-check": "error", "@theia/no-src-import": "error", "@theia/runtime-import-check": "error", diff --git a/dev-packages/private-eslint-plugin/README.md b/dev-packages/private-eslint-plugin/README.md index cca75688e1af9..e8cd91259b622 100644 --- a/dev-packages/private-eslint-plugin/README.md +++ b/dev-packages/private-eslint-plugin/README.md @@ -17,6 +17,11 @@ The plugin helps identify problems during development through static analysis in ## Rules +### `annotation-check`: + +Inversify >=6.1 requires to annotate all constructor parameters of injectable classes as otherwise runtime errors are thrown. +The rule checks that all constructor parameters of injectable classes are annotated with `@inject`, `@unmanaged` or `@multiInject`. + ### `localization-check`: The rule prevents the following localization related issues: diff --git a/dev-packages/private-eslint-plugin/index.js b/dev-packages/private-eslint-plugin/index.js index aa082e196ca5b..ea3c8d7470a09 100644 --- a/dev-packages/private-eslint-plugin/index.js +++ b/dev-packages/private-eslint-plugin/index.js @@ -17,6 +17,7 @@ /** @type {{[ruleId: string]: import('eslint').Rule.RuleModule}} */ exports.rules = { + "annotation-check": require('./rules/annotation-check'), "localization-check": require('./rules/localization-check'), "no-src-import": require('./rules/no-src-import'), "runtime-import-check": require('./rules/runtime-import-check'), diff --git a/dev-packages/private-eslint-plugin/rules/annotation-check.js b/dev-packages/private-eslint-plugin/rules/annotation-check.js new file mode 100644 index 0000000000000..c1da1cca9d75a --- /dev/null +++ b/dev-packages/private-eslint-plugin/rules/annotation-check.js @@ -0,0 +1,103 @@ +// @ts-check +// ***************************************************************************** +// Copyright (C) 2024 EclipseSource and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// http://www.eclipse.org/legal/epl-2.0. +// +// This Source Code may also be made available under the following Secondary +// Licenses when the conditions for such availability set forth in the Eclipse +// Public License v. 2.0 are satisfied: GNU General Public License, version 2 +// with the GNU Classpath Exception which is available at +// https://www.gnu.org/software/classpath/license.html. +// +// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0 +// ***************************************************************************** + +/** + * @typedef {import('@typescript-eslint/utils').TSESTree.ClassDeclaration} ClassDeclaration + * @typedef {import('@typescript-eslint/utils').TSESTree.ClassElement} ClassElement + * @typedef {import('@typescript-eslint/utils').TSESTree.Decorator} Decorator + * @typedef {import('@typescript-eslint/utils').TSESTree.MethodDefinition} MethodDefinition + * @typedef {import('@typescript-eslint/utils').TSESTree.Parameter} Parameter + * @typedef {import('estree').Node} Node + * @typedef {import('eslint').Rule.RuleModule} RuleModule + */ + +/** + * Type guard to check if a ClassElement is a MethodDefinition. + * @param {ClassElement} element + * @returns {element is MethodDefinition} + */ +function isMethodDefinition(element) { + return element.type === 'MethodDefinition'; +} + +/** @type {RuleModule} */ +module.exports = { + meta: { + type: 'problem', + docs: { + description: + 'Ensure @injectable classes have annotated constructor parameters', + }, + messages: { + missingAnnotation: 'Constructor parameters in an @injectable class must be annotated with @inject, @unmanaged or @multiInject', + }, + }, + create(context) { + return { + /** + * @param {ClassDeclaration} node + */ + ClassDeclaration(node) { + // Check if the class has a decorator named `injectable` + const hasInjectableDecorator = node.decorators?.some( + (/** @type {Decorator} */ decorator) => + decorator.expression.type === 'CallExpression' && + decorator.expression.callee.type === 'Identifier' && + decorator.expression.callee.name === 'injectable' + ); + + if (hasInjectableDecorator) { + // Find the constructor method within the class body + const constructor = node.body.body.find( + member => + isMethodDefinition(member) && + member.kind === 'constructor' + ); + + if ( + constructor && + // We need to re-apply 'isMethodDefinition' here because the type guard is not properly preserved + isMethodDefinition(constructor) && + constructor.value && + constructor.value.params.length > 0 + ) { + constructor.value.params.forEach( + /** @type {Parameter} */ param => { + // Check if each constructor parameter has a decorator + const hasAnnotation = param.decorators?.some( + (/** @type {Decorator} */ decorator) => + decorator.expression.type === 'CallExpression' && + decorator.expression.callee.type === 'Identifier' && + (decorator.expression.callee.name === 'inject' || + decorator.expression.callee.name === 'unmanaged' || + decorator.expression.callee.name === 'multiInject') + ); + + if (!hasAnnotation) { + context.report({ + node: /** @type Node */ (param), + messageId: 'missingAnnotation', + }); + } + } + ); + } + } + }, + }; + }, +};