From 77675a3c85b12aabde3b5b1a9059deac5163e70c Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Mon, 6 May 2024 13:34:58 +0200 Subject: [PATCH] fix: Improve import declaration deprecation checks WIP --- src/linter/ui5Types/SourceFileLinter.ts | 111 ++++++++++++++---- .../rules/NoDeprecatedApi/NoDeprecatedApi.js | 4 +- 2 files changed, 91 insertions(+), 24 deletions(-) diff --git a/src/linter/ui5Types/SourceFileLinter.ts b/src/linter/ui5Types/SourceFileLinter.ts index a638163a1..e703cc365 100644 --- a/src/linter/ui5Types/SourceFileLinter.ts +++ b/src/linter/ui5Types/SourceFileLinter.ts @@ -419,32 +419,73 @@ export default class SourceFileLinter { return; } const symbol = this.#checker.getSymbolAtLocation(moduleSpecifierNode); - // Only check for the "default" export regardless of what's declared - // as UI5 / AMD only supports importing the default anyways. - // TODO: This needs to be enhanced in future - const defaultExportSymbol = symbol?.exports?.get("default" as ts.__String); - const deprecationInfo = this.getDeprecationInfo(defaultExportSymbol); - if (deprecationInfo) { + + if (this.isSymbolOfPseudoType(symbol)) { this.#reporter.addMessage({ node: moduleSpecifierNode, severity: LintMessageSeverity.Error, - ruleId: "ui5-linter-no-deprecated-api", + ruleId: "ui5-linter-no-pseudo-modules", message: - `Import of deprecated module ` + + `Import of pseudo module ` + `'${moduleSpecifierNode.text}'`, - messageDetails: deprecationInfo.messageDetails, + messageDetails: "Import library and reuse the enum from there", }); + return; } - if (this.isSymbolOfPseudoType(symbol)) { + // Check whether all exports are marked as deprecated, as otherwise we might create a false positive. + // TODO TS: This needs to be enhanced for checks of TypeScript sources to only check for the actual + // imports (default, *, named) + if (!symbol?.exports) { + return; + } + let hasDeprecatedExports = false; + const messageDetails: string[] = []; + for (const exportSymbol of symbol.exports.values()) { + const deprecationInfo = this.getDeprecationInfo(exportSymbol); + if (deprecationInfo) { + hasDeprecatedExports = true; + if (deprecationInfo.messageDetails) { + messageDetails.push(deprecationInfo.messageDetails); + } + continue; + } else if ( + exportSymbol.flags === ts.SymbolFlags.ValueModule || + exportSymbol.flags === ts.SymbolFlags.NamespaceModule + ) { + // A namespace can be a "ValueModule" or a "NamespaceModule". + // The namespace itself is most likely not deprecated, + // but all symbols exported by the namespace might be. + if (exportSymbol.exports) { + for (const namespacedExportSymbol of exportSymbol.exports.values()) { + const namespacedDeprecationInfo = this.getDeprecationInfo(namespacedExportSymbol); + if (namespacedDeprecationInfo) { + hasDeprecatedExports = true; + if (namespacedDeprecationInfo.messageDetails) { + messageDetails.push(namespacedDeprecationInfo.messageDetails); + } + continue; + } + // For now, all exports must be deprecated for the import to be considered deprecated + // See TODO TS above for future enhancements + return; + } + continue; + } + } + // For now, all exports must be deprecated for the import to be considered deprecated + // See TODO TS above for future enhancements + return; + } + if (hasDeprecatedExports) { this.#reporter.addMessage({ node: moduleSpecifierNode, severity: LintMessageSeverity.Error, - ruleId: "ui5-linter-no-pseudo-modules", + ruleId: "ui5-linter-no-deprecated-api", message: - `Import of pseudo module ` + - `'${moduleSpecifierNode.text}'`, - messageDetails: "Import library and reuse the enum from there", + `Import of deprecated module ` + + `'${moduleSpecifierNode.text}'`, + messageDetails: messageDetails.length ? messageDetails.join("\n") : undefined, }); } } @@ -453,9 +494,14 @@ export default class SourceFileLinter { if (symbol.name.startsWith("sap/")) { return true; } else { - const sourceFile = symbol.valueDeclaration?.getSourceFile(); - if (sourceFile?.fileName.match(/@openui5|@sapui5|@ui5/)) { - return true; + const declarations = symbol.getDeclarations(); + if (declarations) { + for (const declaration of declarations) { + const sourceFile = declaration.getSourceFile(); + if (sourceFile.fileName.match(/@openui5|@sapui5|@ui5/)) { + return true; + } + } } } return false; @@ -465,20 +511,41 @@ export default class SourceFileLinter { if (symbol.name.startsWith("sap/")) { return true; } else { - const sourceFile = symbol.valueDeclaration?.getSourceFile(); - if (sourceFile?.fileName.match(/@openui5|@sapui5|@ui5|@types\/jquery/)) { - return true; + const declarations = symbol.getDeclarations(); + if (declarations) { + for (const declaration of declarations) { + const sourceFile = declaration.getSourceFile(); + if (sourceFile.fileName.match(/@openui5|@sapui5|@ui5|@types\/jquery/)) { + return true; + } + } } } return false; } isSymbolOfJquerySapType(symbol: ts.Symbol) { - return symbol.valueDeclaration?.getSourceFile().fileName === "/types/@ui5/linter/overrides/jquery.sap.d.ts"; + const declarations = symbol.getDeclarations(); + if (declarations) { + for (const declaration of declarations) { + const sourceFile = declaration.getSourceFile(); + if (sourceFile.fileName === "/types/@ui5/linter/overrides/jquery.sap.d.ts") { + return true; + } + } + } } isSymbolOfPseudoType(symbol: ts.Symbol | undefined) { - return symbol?.valueDeclaration?.getSourceFile().fileName.startsWith("/types/@ui5/linter/overrides/library/"); + const declarations = symbol?.getDeclarations(); + if (declarations) { + for (const declaration of declarations) { + const sourceFile = declaration.getSourceFile(); + if (sourceFile.fileName.startsWith("/types/@ui5/linter/overrides/library/")) { + return true; + } + } + } } findClassOrInterface(node: ts.Node): ts.Type | undefined { diff --git a/test/fixtures/linter/rules/NoDeprecatedApi/NoDeprecatedApi.js b/test/fixtures/linter/rules/NoDeprecatedApi/NoDeprecatedApi.js index 1f81a8937..58e511c36 100644 --- a/test/fixtures/linter/rules/NoDeprecatedApi/NoDeprecatedApi.js +++ b/test/fixtures/linter/rules/NoDeprecatedApi/NoDeprecatedApi.js @@ -1,7 +1,7 @@ sap.ui.define([ "sap/m/Button", "sap/m/DateTimeInput", "sap/base/util/includes", "sap/ui/Device", "sap/ui/core/library", "sap/ui/generic/app/navigation/service/NavigationHandler", - "sap/ui/table/Table", "sap/ui/table/plugins/MultiSelectionPlugin", "sap/ui/core/Configuration", "sap/m/library" -], function(Button, DateTimeInput, includes, Device, coreLib, NavigationHandler, Table, MultiSelectionPlugin, Configuration, mobileLib) { + "sap/ui/table/Table", "sap/ui/table/plugins/MultiSelectionPlugin", "sap/ui/core/Configuration", "sap/m/library", "sap/ui/commons/library" +], function(Button, DateTimeInput, includes, Device, coreLib, NavigationHandler, Table, MultiSelectionPlugin, Configuration, mobileLib, commonsLib) { var dateTimeInput = new DateTimeInput(); // TODO detect: Control is deprecated