diff --git a/src/linter/ui5Types/SourceFileLinter.ts b/src/linter/ui5Types/SourceFileLinter.ts index d11d650dd..26c45b725 100644 --- a/src/linter/ui5Types/SourceFileLinter.ts +++ b/src/linter/ui5Types/SourceFileLinter.ts @@ -1,4 +1,4 @@ -import ts, {Identifier, SymbolFlags} from "typescript"; +import ts, {SymbolFlags} from "typescript"; import path from "node:path/posix"; import {getLogger} from "@ui5/logger"; import SourceFileReporter from "./SourceFileReporter.js"; @@ -6,7 +6,7 @@ import LinterContext, {ResourcePath, CoverageCategory} from "../LinterContext.js import {MESSAGE} from "../messages.js"; import analyzeComponentJson from "./asyncComponentFlags.js"; import {deprecatedLibraries, deprecatedThemes} from "../../utils/deprecations.js"; -import {getPropertyName} from "./utils.js"; +import {getPropertyName, getSymbolForPropertyInConstructSignatures} from "./utils.js"; import {taskStart} from "../../utils/perf.js"; import {getPositionsForNode} from "../../utils/nodePosition.js"; import {TraceMap} from "@jridgewell/trace-mapping"; @@ -680,38 +680,47 @@ export default class SourceFileLinter { this.#analyzeNewOdataModelV4(node); } + if (!node.arguments?.length) { + // Nothing to check + return; + } + // There can be multiple and we need to find the right one - const [constructSignature] = classType.getConstructSignatures(); - - node.arguments?.forEach((arg, argIdx) => { - const argumentType = constructSignature.getTypeParameterAtPosition(argIdx); - if (ts.isObjectLiteralExpression(arg)) { - arg.properties.forEach((prop) => { - if (ts.isPropertyAssignment(prop)) { // TODO: Necessary? - const propNameIdentifier = prop.name as Identifier; - const propText = propNameIdentifier.escapedText || propNameIdentifier.text; - const propertySymbol = argumentType.getProperty(propText); - - // this.checker.getContextualType(arg) // same as nodeType - // const propertySymbol = allProps.find((symbol) => { - // return symbol.escapedName === propNameIdentifier; - // }); - if (propertySymbol) { - const deprecationInfo = this.getDeprecationInfo(propertySymbol); - if (deprecationInfo) { - this.#reporter.addMessage(MESSAGE.DEPRECATED_PROPERTY_OF_CLASS, - { - propertyName: propertySymbol.escapedName as string, - className: this.checker.typeToString(nodeType), - details: deprecationInfo.messageDetails, - }, - prop - ); - } - } - } - }); + const allConstructSignatures = classType.getConstructSignatures(); + // We can ignore all signatures that have a different number of parameters + const possibleConstructSignatures = allConstructSignatures.filter((constructSignature) => { + return constructSignature.getParameters().length === node.arguments?.length; + }); + + node.arguments.forEach((arg, argIdx) => { + // Only handle object literals, ignoring the optional first id argument or other unrelated arguments + if (!ts.isObjectLiteralExpression(arg)) { + return; } + arg.properties.forEach((prop) => { + if (!ts.isPropertyAssignment(prop)) { + return; + } + const propertyName = getPropertyName(prop.name); + const propertySymbol = getSymbolForPropertyInConstructSignatures( + possibleConstructSignatures, argIdx, propertyName + ); + if (!propertySymbol) { + return; + } + const deprecationInfo = this.getDeprecationInfo(propertySymbol); + if (!deprecationInfo) { + return; + } + this.#reporter.addMessage(MESSAGE.DEPRECATED_PROPERTY_OF_CLASS, + { + propertyName: propertySymbol.escapedName as string, + className: this.checker.typeToString(nodeType), + details: deprecationInfo.messageDetails, + }, + prop + ); + }); }); } diff --git a/src/linter/ui5Types/utils.ts b/src/linter/ui5Types/utils.ts index 98ec56f5f..5aa2fcb08 100644 --- a/src/linter/ui5Types/utils.ts +++ b/src/linter/ui5Types/utils.ts @@ -7,3 +7,19 @@ export function getPropertyName(node: ts.PropertyName | ts.Expression): string { return node.getText(); } } + +export function getSymbolForPropertyInConstructSignatures( + constructSignatures: readonly ts.Signature[], + argumentPosition: number, + propertyName: string +): ts.Symbol | undefined { + for (const constructSignature of constructSignatures) { + const propertySymbol = constructSignature + .getTypeParameterAtPosition(argumentPosition) + .getProperty(propertyName); + if (propertySymbol) { + return propertySymbol; + } + } + return undefined; +} diff --git a/test/fixtures/linter/rules/NoDeprecatedApi/NoDeprecatedApi.js b/test/fixtures/linter/rules/NoDeprecatedApi/NoDeprecatedApi.js index 78b9a6b2b..a5801731c 100644 --- a/test/fixtures/linter/rules/NoDeprecatedApi/NoDeprecatedApi.js +++ b/test/fixtures/linter/rules/NoDeprecatedApi/NoDeprecatedApi.js @@ -48,4 +48,12 @@ sap.ui.define([ const navigationHandler = new NavigationHandler({}); navigationHandler.storeInnerAppState({}); // Method "storeInnerAppState" is deprecated + + // Detection of deprecated API in constructor when an ID is passed as first argument + var btn2 = new Button("btn2", { + blocked: true, // Property "blocked" is deprecated + tap: () => console.log("Tapped"), // Event "tap" is deprecated + + ...moreArgs // Should be ignored + }); }); diff --git a/test/fixtures/linter/rules/NoDeprecatedApi/NoDeprecatedApiTypeScript.ts b/test/fixtures/linter/rules/NoDeprecatedApi/NoDeprecatedApiTypeScript.ts index f0936fc1d..f2d57408c 100644 --- a/test/fixtures/linter/rules/NoDeprecatedApi/NoDeprecatedApiTypeScript.ts +++ b/test/fixtures/linter/rules/NoDeprecatedApi/NoDeprecatedApiTypeScript.ts @@ -12,7 +12,7 @@ import {InputType} from "sap/m/library"; var dateTimeInput = new DateTimeInput(); // Control is deprecated. A finding only appears for the module dependency, not for the usage. var btn = new Button({ - blocked: true, // Property "blocked" is deprecated + "blocked": true, // Property "blocked" is deprecated tap: () => console.log("Tapped") // Event "tap" is deprecated }); @@ -43,3 +43,11 @@ InputType.Date; // Enum value "InputType.Date" is deprecated const navigationHandler = new NavigationHandler({}); navigationHandler.storeInnerAppState({}); // Method "storeInnerAppState" is deprecated + +// Detection of deprecated API in constructor when an ID is passed as first argument +var btn2 = new Button("btn2", { + blocked: true, // Property "blocked" is deprecated + "tap": () => console.log("Tapped"), // Event "tap" is deprecated + + ...moreArgs // Should be ignored +}); diff --git a/test/lib/linter/rules/snapshots/NoDeprecatedApi.ts.md b/test/lib/linter/rules/snapshots/NoDeprecatedApi.ts.md index c17db1388..76aa81e7c 100644 --- a/test/lib/linter/rules/snapshots/NoDeprecatedApi.ts.md +++ b/test/lib/linter/rules/snapshots/NoDeprecatedApi.ts.md @@ -870,7 +870,7 @@ Generated by [AVA](https://avajs.dev). [ { coverageInfo: [], - errorCount: 23, + errorCount: 25, fatalErrorCount: 0, filePath: 'NoDeprecatedApi.js', messages: [ @@ -1058,6 +1058,22 @@ Generated by [AVA](https://avajs.dev). ruleId: 'no-deprecated-api', severity: 2, }, + { + column: 3, + line: 54, + message: 'Use of deprecated property \'blocked\' of class \'Button\'', + messageDetails: 'Deprecated test message', + ruleId: 'no-deprecated-api', + severity: 2, + }, + { + column: 3, + line: 55, + message: 'Use of deprecated property \'tap\' of class \'Button\'', + messageDetails: 'Deprecated test message', + ruleId: 'no-deprecated-api', + severity: 2, + }, ], warningCount: 0, }, @@ -1070,7 +1086,7 @@ Generated by [AVA](https://avajs.dev). [ { coverageInfo: [], - errorCount: 18, + errorCount: 20, fatalErrorCount: 0, filePath: 'NoDeprecatedApiTypeScript.ts', messages: [ @@ -1218,6 +1234,22 @@ Generated by [AVA](https://avajs.dev). ruleId: 'no-deprecated-api', severity: 2, }, + { + column: 2, + line: 49, + message: 'Use of deprecated property \'blocked\' of class \'Button\'', + messageDetails: 'Deprecated test message', + ruleId: 'no-deprecated-api', + severity: 2, + }, + { + column: 2, + line: 50, + message: 'Use of deprecated property \'tap\' of class \'Button\'', + messageDetails: 'Deprecated test message', + ruleId: 'no-deprecated-api', + severity: 2, + }, ], warningCount: 0, }, diff --git a/test/lib/linter/rules/snapshots/NoDeprecatedApi.ts.snap b/test/lib/linter/rules/snapshots/NoDeprecatedApi.ts.snap index d9d1177f1..99323e584 100644 Binary files a/test/lib/linter/rules/snapshots/NoDeprecatedApi.ts.snap and b/test/lib/linter/rules/snapshots/NoDeprecatedApi.ts.snap differ