Skip to content

Commit 49f54a8

Browse files
RandomByteRandomByte
committed
feat(autofix): Detect deprecated constructor parameters for non-MO
ManagedObject properties where already detected. This adds support for matching Ui5TypeInfo against regular constructor parameters. Included is a first autofix for the deprecated OData v4 parameter "synchronizationMode". JIRA: CPOUI5FOUNDATION-994
1 parent bd386b6 commit 49f54a8

File tree

10 files changed

+266
-50
lines changed

10 files changed

+266
-50
lines changed

‎src/linter/ui5Types/SourceFileLinter.ts‎

Lines changed: 27 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -701,11 +701,6 @@ export default class SourceFileLinter {
701701
ui5ModuleTypeInfo.name === "sap/ui/core/routing/Router"
702702
) {
703703
this.#analyzeNewCoreRouter(node);
704-
} else if (
705-
ui5ModuleTypeInfo?.kind === Ui5TypeInfoKind.Module &&
706-
ui5ModuleTypeInfo.name === "sap/ui/model/odata/v4/ODataModel"
707-
) {
708-
this.#analyzeNewOdataModelV4(node);
709704
} else if (nodeType.symbol.declarations?.some(
710705
(declaration) => ts.isClassDeclaration(declaration) &&
711706
this.isUi5ClassDeclaration(declaration, "sap/ui/base/ManagedObject"))
@@ -750,21 +745,35 @@ export default class SourceFileLinter {
750745
return;
751746
}
752747
const deprecationInfo = this.getDeprecationInfo(propertySymbol);
753-
if (!deprecationInfo) {
748+
if (deprecationInfo) {
749+
this.#reporter.addMessage(MESSAGE.DEPRECATED_PROPERTY_OF_CLASS,
750+
{
751+
propertyName: propertySymbol.escapedName as string,
752+
className: this.checker.typeToString(nodeType),
753+
details: deprecationInfo.messageDetails,
754+
},
755+
{
756+
node: prop,
757+
ui5TypeInfo: deprecationInfo.ui5TypeInfo,
758+
fix: this.getFix(prop, deprecationInfo.ui5TypeInfo),
759+
}
760+
);
754761
return;
755762
}
756-
this.#reporter.addMessage(MESSAGE.DEPRECATED_PROPERTY_OF_CLASS,
757-
{
758-
propertyName: propertySymbol.escapedName as string,
759-
className: this.checker.typeToString(nodeType),
760-
details: deprecationInfo.messageDetails,
761-
},
762-
{
763-
node: prop,
764-
ui5TypeInfo: deprecationInfo.ui5TypeInfo,
765-
fix: this.getFix(prop, deprecationInfo.ui5TypeInfo),
766-
}
767-
);
763+
let reportMessage;
764+
if (ui5ModuleTypeInfo?.kind === Ui5TypeInfoKind.Module &&
765+
ui5ModuleTypeInfo.name === "sap/ui/model/odata/v4/ODataModel" &&
766+
propertyName === "synchronizationMode") {
767+
reportMessage = MESSAGE.DEPRECATED_ODATA_MODEL_V4_SYNCHRONIZATION_MODE;
768+
}
769+
if (!reportMessage) {
770+
return;
771+
}
772+
const ui5TypeInfo = getUi5TypeInfoFromSymbol(propertySymbol, this.apiExtract);
773+
this.#reporter.addMessage(reportMessage, null, {
774+
node: prop,
775+
fix: this.getFix(prop, ui5TypeInfo),
776+
});
768777
});
769778
});
770779
}
@@ -1254,22 +1263,6 @@ export default class SourceFileLinter {
12541263
}
12551264
}
12561265

1257-
#analyzeNewOdataModelV4(node: ts.NewExpression) {
1258-
if (!node.arguments || node.arguments.length < 1 || !ts.isObjectLiteralExpression(node.arguments[0])) {
1259-
return;
1260-
}
1261-
1262-
const synchronizationModeProb = getPropertyAssignmentInObjectLiteralExpression(
1263-
"synchronizationMode", node.arguments[0]
1264-
);
1265-
1266-
if (synchronizationModeProb) {
1267-
this.#reporter.addMessage(
1268-
MESSAGE.DEPRECATED_ODATA_MODEL_V4_SYNCHRONIZATION_MODE, null, {node: synchronizationModeProb}
1269-
);
1270-
}
1271-
}
1272-
12731266
#analyzeViewCreate(node: ts.CallExpression) {
12741267
if (!node.arguments?.length || !ts.isObjectLiteralExpression(node.arguments[0])) {
12751268
return;

‎src/linter/ui5Types/Ui5TypeInfo.ts‎

Lines changed: 63 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@ import {getPropertyNameText} from "./utils/utils.js";
44

55
export enum Ui5TypeInfoKind {
66
Module,
7-
Class,
87
Namespace,
8+
Class,
9+
Constructor,
10+
ConstructorParameter,
911
ManagedObjectSettings,
1012
MetadataProperty,
1113
MetadataEvent,
@@ -20,9 +22,9 @@ export enum Ui5TypeInfoKind {
2022
EnumMember,
2123
}
2224

23-
export type Ui5TypeInfo = Ui5ModuleTypeInfo | Ui5ClassTypeInfo | Ui5NamespaceTypeInfo |
24-
Ui5MetadataTypeInfo | Ui5FunctionTypeInfo | Ui5MethodTypeInfo | Ui5PropertyTypeInfo |
25-
Ui5EnumTypeInfo | Ui5EnumMemberTypeInfo | Ui5ManagedObjectSettingsTypeInfo;
25+
export type Ui5TypeInfo = Ui5ModuleTypeInfo | Ui5NamespaceTypeInfo | Ui5ClassTypeInfo | Ui5ConstructorTypeInfo |
26+
Ui5ConstructorParameterTypeInfo | Ui5MetadataTypeInfo | Ui5FunctionTypeInfo | Ui5MethodTypeInfo |
27+
Ui5PropertyTypeInfo | Ui5EnumTypeInfo | Ui5EnumMemberTypeInfo | Ui5ManagedObjectSettingsTypeInfo;
2628

2729
export interface BaseUi5TypeInfo {
2830
kind: Ui5TypeInfoKind;
@@ -74,7 +76,7 @@ interface Ui5MethodTypeInfo extends BaseUi5TypeInfo {
7476
interface Ui5PropertyTypeInfo extends BaseUi5TypeInfo {
7577
kind: Ui5TypeInfoKind.Property | Ui5TypeInfoKind.StaticProperty;
7678
name: string;
77-
parent: Ui5ClassTypeInfo;
79+
parent: Ui5ClassTypeInfo | Ui5ConstructorTypeInfo | Ui5ConstructorParameterTypeInfo;
7880
}
7981

8082
interface Ui5EnumTypeInfo extends BaseUi5TypeInfo {
@@ -89,6 +91,18 @@ interface Ui5EnumMemberTypeInfo extends BaseUi5TypeInfo {
8991
parent: Ui5EnumTypeInfo;
9092
}
9193

94+
interface Ui5ConstructorTypeInfo extends BaseUi5TypeInfo {
95+
kind: Ui5TypeInfoKind.Constructor;
96+
name: string;
97+
parent: Ui5ClassTypeInfo;
98+
}
99+
100+
interface Ui5ConstructorParameterTypeInfo extends BaseUi5TypeInfo {
101+
kind: Ui5TypeInfoKind.ConstructorParameter;
102+
name: string; // e.g. "mParameters"
103+
parent: Ui5ConstructorTypeInfo;
104+
}
105+
92106
function isManagedObjectSettingsInterfaceName(name: string): boolean {
93107
// This check is based on the naming convention when generating the Interface for ManagedObject settings:
94108
// $<ClassName>Settings
@@ -257,29 +271,39 @@ function createMangedObjectSettingsTypeInfo(node: ts.Declaration, moduleName: st
257271

258272
function createClassTypeInfo(
259273
node: ts.Declaration, parent: Ui5ModuleTypeInfo | Ui5NamespaceTypeInfo
260-
): Ui5MethodTypeInfo | Ui5PropertyTypeInfo | Ui5ClassTypeInfo | undefined {
274+
): Ui5MethodTypeInfo | Ui5PropertyTypeInfo | Ui5ClassTypeInfo | Ui5ConstructorParameterTypeInfo | undefined {
261275
if (ts.isClassDeclaration(node) && node.name) {
262276
return {
263277
kind: Ui5TypeInfoKind.Class,
264278
name: node.name.text,
265279
parent,
266280
};
267281
}
268-
if (!ts.isInterfaceDeclaration(node.parent) && !ts.isClassDeclaration(node.parent)) {
269-
return;
282+
let classNode = node.parent;
283+
const skippedNodes = [];
284+
while (!ts.isInterfaceDeclaration(classNode) && !ts.isClassDeclaration(classNode)) {
285+
if (!classNode.parent) {
286+
return;
287+
}
288+
skippedNodes.unshift(classNode);
289+
classNode = classNode.parent;
270290
}
271-
if (!node.parent.name) {
291+
if (!classNode.name) {
272292
// Class name can be undefined in `export default class { ... }`. But we generally don't expect
273293
// to encounter this case in the UI5 types so we can safely ignore it and keeps our types greedy.
274294
return;
275295
}
276296
const classTypeInfo: Ui5ClassTypeInfo = {
277297
kind: Ui5TypeInfoKind.Class,
278-
name: node.parent.name.text,
298+
name: classNode.name.text,
279299
parent,
280300
};
281301

282302
if (ts.isMethodSignature(node) || ts.isMethodDeclaration(node)) {
303+
if (skippedNodes.length) {
304+
throw new Error(
305+
`Unexpected nested method declaration: ${skippedNodes.map((n) => n.getText()).join(" -> ")}`);
306+
}
283307
const name = getPropertyNameText(node.name);
284308
if (!name) {
285309
// We need a name for methods and properties
@@ -291,16 +315,43 @@ function createClassTypeInfo(
291315
parent: classTypeInfo,
292316
};
293317
}
294-
if (ts.isPropertySignature(node) || ts.isPropertyDeclaration(node)) {
318+
if (ts.isPropertySignature(node) || ts.isPropertyDeclaration(node) || ts.isParameter(node)) {
319+
let parent = classTypeInfo as Ui5ClassTypeInfo | Ui5ConstructorTypeInfo | Ui5ConstructorParameterTypeInfo;
320+
for (const skippedNode of skippedNodes) {
321+
if (ts.isConstructorDeclaration(skippedNode)) {
322+
parent = {
323+
kind: Ui5TypeInfoKind.Constructor,
324+
name: "constructor",
325+
parent,
326+
} as Ui5ConstructorTypeInfo;
327+
} else if (ts.isParameter(skippedNode)) {
328+
parent = {
329+
kind: Ui5TypeInfoKind.ConstructorParameter,
330+
name: skippedNode.name.getText(),
331+
parent,
332+
} as Ui5ConstructorParameterTypeInfo;
333+
}
334+
}
335+
if (!ts.isPropertyName(node.name)) {
336+
return;
337+
}
295338
const name = getPropertyNameText(node.name);
296339
if (!name) {
297340
// We need a name for methods and properties
298341
return undefined;
299342
}
343+
if (ts.isParameter(node)) {
344+
return {
345+
kind: Ui5TypeInfoKind.ConstructorParameter,
346+
name,
347+
parent: parent as Ui5ConstructorTypeInfo,
348+
};
349+
}
350+
// PropertySignature or PropertyDeclaration
300351
return {
301352
kind: hasStaticModifier(node) ? Ui5TypeInfoKind.StaticProperty : Ui5TypeInfoKind.Property,
302353
name,
303-
parent: classTypeInfo,
354+
parent,
304355
};
305356
}
306357
}

‎src/linter/ui5Types/Ui5TypeInfoMatcher.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,21 @@ export default class Ui5TypeInfoMatcher<ValueType> {
120120
});
121121
}
122122

123+
constr(children?: Node<ValueType>[] | ValueType, value?: ValueType): Node<ValueType> {
124+
return this.createNode(Ui5TypeInfoKind.Constructor, "constructor", children, value);
125+
}
126+
127+
constuctorParameter(name: string, children?: Node<ValueType>[] | ValueType, value?: ValueType): Node<ValueType> {
128+
return this.createNode(Ui5TypeInfoKind.ConstructorParameter, name, children, value);
129+
}
130+
131+
constructorParameters(names: string[], children?: Node<ValueType>[] | ValueType, value?: ValueType
132+
): Node<ValueType>[] {
133+
return names.map((name) => {
134+
return this.createNode(Ui5TypeInfoKind.ConstructorParameter, name, children, value);
135+
});
136+
}
137+
123138
method(name: string, children?: Node<ValueType>[] | ValueType, value?: ValueType): Node<ValueType> {
124139
return this.createNode(Ui5TypeInfoKind.Method, name, children, value);
125140
}

‎src/linter/ui5Types/fix/PropertyAssignmentFix.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export default class PropertyAssignmentFix extends XmlEnabledFix {
2626
}
2727

2828
visitLinterNode(node: ts.Node, sourcePosition: PositionInfo) {
29-
if (!ts.isPropertyAssignment(node) || !ts.isIdentifier(node.name)) {
29+
if (!ts.isPropertyAssignment(node) || (!ts.isIdentifier(node.name) && !ts.isStringLiteralLike(node.name))) {
3030
return false;
3131
}
3232

‎src/linter/ui5Types/fix/collections/sapUiCoreFixes.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
accessExpressionFix,
66
callExpressionFix,
77
callExpressionGeneratorFix,
8+
propertyAssignmentFix,
89
} from "../FixFactory.js";
910
import {FixScope} from "../BaseFix.js";
1011

@@ -479,3 +480,13 @@ t.declareModule("sap/ui/core/Core", [
479480
// ["unregisterPlugin", {}],
480481
]),
481482
]);
483+
484+
t.declareModule("sap/ui/model/odata/v4/ODataModel", [
485+
t.class("ODataModel", [
486+
t.constr([
487+
t.constuctorParameter("mParameters", [
488+
t.property("synchronizationMode", propertyAssignmentFix({})),
489+
]),
490+
]),
491+
]),
492+
]);

‎src/linter/ui5Types/utils/utils.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,29 @@ export function getPropertyNameText(node: ts.PropertyName): string | undefined {
3939
return undefined;
4040
}
4141

42+
/**
43+
* Searches for the symbol of an argument within the given construct signatures.
44+
* The first match is returned.
45+
*
46+
* Returns undefined if the argument is not found in any of the construct signatures.
47+
*
48+
* @param constructSignatures construct signatures to search in
49+
* @param argumentPosition position of the argument in the signature
50+
* @returns symbol of the found property or undefined
51+
*/
52+
export function getSymbolForArgumentInConstructSignatures(
53+
constructSignatures: readonly ts.Signature[],
54+
argumentPosition: number
55+
): ts.Symbol | undefined {
56+
for (const constructSignature of constructSignatures) {
57+
const propertySymbol = constructSignature.getParameters()[argumentPosition];
58+
if (propertySymbol) {
59+
return propertySymbol;
60+
}
61+
}
62+
return undefined;
63+
}
64+
4265
/**
4366
* Searches for the symbol of a property within the given construct signatures.
4467
* The first match is returned.

‎test/fixtures/autofix/ODataModelV4.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
sap.ui.define([
2+
"sap/ui/model/odata/v4/ODataModel",
3+
], function(ODataModel) {
4+
var model = new ODataModel({
5+
serviceUrl: "/odata/v4/service",
6+
synchronizationMode: "None" // Parameter "synchronizationMode" is deprecated since 1.110
7+
});
8+
var model = new ODataModel({
9+
serviceUrl: "/odata/v4/service",
10+
/* Parameter "synchronizationMode" is deprecated since 1.110 */ "synchronizationMode": true
11+
});
12+
});

‎test/lib/autofix/snapshots/autofix.fixtures.ts.md‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2483,6 +2483,36 @@ Generated by [AVA](https://avajs.dev).
24832483
},
24842484
]
24852485

2486+
## General: ODataModelV4.js
2487+
2488+
> AutofixResult iteration #0: /ODataModelV4.js
2489+
2490+
`sap.ui.define([␊
2491+
"sap/ui/model/odata/v4/ODataModel",␊
2492+
], function(ODataModel) {␊
2493+
var model = new ODataModel({␊
2494+
serviceUrl: "/odata/v4/service",␊
2495+
// Parameter "synchronizationMode" is deprecated since 1.110␊
2496+
});␊
2497+
var model = new ODataModel({␊
2498+
serviceUrl: "/odata/v4/service",␊
2499+
/* Parameter "synchronizationMode" is deprecated since 1.110 */ ␊
2500+
});␊
2501+
});`
2502+
2503+
> LintResult: ODataModelV4.js
2504+
2505+
[
2506+
{
2507+
coverageInfo: [],
2508+
errorCount: 0,
2509+
fatalErrorCount: 0,
2510+
filePath: 'ODataModelV4.js',
2511+
messages: [],
2512+
warningCount: 0,
2513+
},
2514+
]
2515+
24862516
## General: ParsingError.js
24872517

24882518
> LintResult: ParsingError.js

‎test/lib/autofix/snapshots/autofix.fixtures.ts.snap‎

160 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)