Skip to content

Commit

Permalink
fix: Detect deprecations in new expressions with ID
Browse files Browse the repository at this point in the history
This solves the gap of not detecting usage of deprecated UI5 metadata
(e.g. properties, aggregations, events) when using the constructor
with an ID as first argument.
  • Loading branch information
matz3 committed Dec 4, 2024
1 parent 4eb2758 commit 156e747
Show file tree
Hide file tree
Showing 6 changed files with 108 additions and 35 deletions.
73 changes: 41 additions & 32 deletions src/linter/ui5Types/SourceFileLinter.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
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";
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";
Expand Down Expand Up @@ -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
);
});
});
}

Expand Down
16 changes: 16 additions & 0 deletions src/linter/ui5Types/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
8 changes: 8 additions & 0 deletions test/fixtures/linter/rules/NoDeprecatedApi/NoDeprecatedApi.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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
});

Expand Down Expand Up @@ -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
});
36 changes: 34 additions & 2 deletions test/lib/linter/rules/snapshots/NoDeprecatedApi.ts.md
Original file line number Diff line number Diff line change
Expand Up @@ -870,7 +870,7 @@ Generated by [AVA](https://avajs.dev).
[
{
coverageInfo: [],
errorCount: 23,
errorCount: 25,
fatalErrorCount: 0,
filePath: 'NoDeprecatedApi.js',
messages: [
Expand Down Expand Up @@ -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,
},
Expand All @@ -1070,7 +1086,7 @@ Generated by [AVA](https://avajs.dev).
[
{
coverageInfo: [],
errorCount: 18,
errorCount: 20,
fatalErrorCount: 0,
filePath: 'NoDeprecatedApiTypeScript.ts',
messages: [
Expand Down Expand Up @@ -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,
},
Expand Down
Binary file modified test/lib/linter/rules/snapshots/NoDeprecatedApi.ts.snap
Binary file not shown.

0 comments on commit 156e747

Please sign in to comment.