Skip to content

Commit 156a02c

Browse files
committed
refactor(autofix): Validate that all required arguments are provided in jQuery fixes
Smoke tests showed cases where applications missed to provide required arguments. This lead to autofix errors in cases where we `.trim()` those arguments: > Cannot read properties of undefined (reading 'trim')
1 parent 30bf19e commit 156a02c

File tree

5 files changed

+274
-149
lines changed

5 files changed

+274
-149
lines changed

src/linter/ui5Types/fix/collections/jqueryFixes.ts

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,12 @@ t.declareModule("jQuery", [
8080
t.namespace("resources", [
8181
t.namespace("isBundle", callExpressionGeneratorFix({ // https://github.com/UI5/linter/issues/657
8282
moduleName: "sap/base/i18n/ResourceBundle",
83+
validateArguments(ctx, _, arg1) {
84+
if (!arg1) {
85+
return false;
86+
}
87+
return true;
88+
},
8389
generator(_ctx, [moduleIdentifier], arg1) {
8490
return `${arg1.trim()} instanceof ${moduleIdentifier}`;
8591
},
@@ -205,12 +211,15 @@ t.declareModule("jQuery", [
205211
*/
206212
t.namespace("extend", callExpressionGeneratorFix({
207213
moduleName: "sap/base/util/merge",
208-
validateArguments(ctx, _, arg1) {
209-
if (arg1.kind !== ts.SyntaxKind.TrueKeyword) {
214+
validateArguments(ctx, _, arg1, arg2) {
215+
if (arg1?.kind !== ts.SyntaxKind.TrueKeyword) {
210216
// If the first argument is not "true" (indicating a deep merge),
211217
// do not apply the fix
212218
return false;
213219
}
220+
if (!arg2) {
221+
return false;
222+
}
214223
return true;
215224
},
216225
generator(ctx, [moduleIdentifierName], _, arg2, arg3) {
@@ -653,6 +662,13 @@ t.declareModule("jQuery", [
653662
},
654663
})),
655664
t.namespace("isEqualNode", callExpressionGeneratorFix({
665+
validateArguments(ctx, _, arg1, arg2) {
666+
if (!arg1 || !arg2) {
667+
// If either argument is missing, do not apply the fix
668+
return false;
669+
}
670+
return true;
671+
},
656672
generator: (ctx, _, arg1, arg2) => {
657673
return `!!${arg1.trim()}?.isEqualNode(${arg2.trim()})`;
658674
},
@@ -725,7 +741,11 @@ t.declareModule("jQuery", [
725741
},
726742
})),
727743
t.namespace("delayedCall", callExpressionGeneratorFix<{isFnString: boolean}>({
728-
validateArguments: (ctx, _, _timeout, _objCtx, fnName) => {
744+
validateArguments: (ctx, _, timeout, objCtx, fnName) => {
745+
if (!timeout || !objCtx || !fnName) {
746+
// If the function name is not provided, do not apply the fix
747+
return false;
748+
}
729749
ctx.isFnString = !!fnName && ts.isStringLiteralLike(fnName);
730750
return true;
731751
},
@@ -745,7 +765,11 @@ t.declareModule("jQuery", [
745765
})),
746766
t.namespace("intervalCall", callExpressionGeneratorFix<{isFnString: boolean}>({
747767
globalName: "setInterval",
748-
validateArguments: (ctx, _, _timeout, _objCtx, fnName) => {
768+
validateArguments: (ctx, _, timeout, objCtx, fnName) => {
769+
if (!timeout || !objCtx || !fnName) {
770+
// If the function name is not provided, do not apply the fix
771+
return false;
772+
}
749773
ctx.isFnString = !!fnName && ts.isStringLiteralLike(fnName);
750774
return true;
751775
},
@@ -768,6 +792,13 @@ t.declareModule("jQuery", [
768792
]), // jQuery.sap
769793
// jQuery
770794
t.namespace("inArray", callExpressionGeneratorFix({
795+
validateArguments(ctx, _, arg1, arg2) {
796+
if (!arg1 || !arg2) {
797+
// If either argument is missing, do not apply the fix
798+
return false;
799+
}
800+
return true;
801+
},
771802
generator: (ctx, _, arg1, arg2) => {
772803
return `(${arg2.trim()} ? Array.prototype.indexOf.call(${arg2.trim()}, ${arg1.trim()}) : -1)`;
773804
},

test/fixtures/autofix/jQueryNativeReplacements.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,14 +63,15 @@ sap.ui.define(["sap/ui/thirdparty/jquery"], async function (jQuery) {
6363
});
6464
});
6565
});
66-
66+
jQuery.sap.delayedCall(50, this); // Invalid: Missing third argument must not be migrated
6767
jQuery.sap.clearDelayedCall(delayedCallId);
6868
var intervalCallId = jQuery.sap.intervalCall(1000, myObject, "myFunction");
6969
var intervalCallId2 = jQuery.sap.intervalCall(1000, myObject, myObject.myFunction, ["myParam1"]);
7070
var intervalCallId3 = jQuery.sap.intervalCall(1000, window, Array.isArray, ["myParam1"]);
7171
var intervalCallId4 = jQuery.sap.intervalCall(1000, this, function () {
7272
var padRight = jQuery.sap.padRight("a", "0", 4); // Also migrate internal content
7373
}, ["myParam1", "myParam2"]);
74+
jQuery.sap.intervalCall(50, this); // Invalid: Missing third argument must not be migrated
7475
jQuery.sap.clearIntervalCall(intervalCallId);
7576

7677
const document = globalThis.document;
@@ -79,6 +80,7 @@ sap.ui.define(["sap/ui/thirdparty/jquery"], async function (jQuery) {
7980
var element3 = jQuery.sap.domById();
8081
const divList = document.getElementsByTagName("div");
8182
var isEqNode = jQuery.sap.isEqualNode(divList[0], divList[0]);
83+
jQuery.sap.isEqualNode(); // Invalid: Missing arguments must not be migrated
8284

8385
var person = {firstname: "Peter", lastname: "Miller" };
8486
var newObj = jQuery.sap.newObject(person);
@@ -89,5 +91,6 @@ sap.ui.define(["sap/ui/thirdparty/jquery"], async function (jQuery) {
8991

9092
var myData = ["a", "b", "c"];
9193
var indexOfEntity = jQuery.inArray("b", myData);
94+
jQuery.inArray(); // Invalid: Missing arguments must not be migrated
9295
var isValueAnArray = jQuery.isArray(myData);
9396
});

test/fixtures/autofix/jQuerySap.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ sap.ui.define(["sap/base/strings/NormalizePolyfill"], async function (NormalizeP
1515
// https://github.com/UI5/linter/issues/521
1616
var myBundle = innerScopeResourceBundle.create({url : "i18n/messagebundle.properties"});
1717
var isBundle = jQuery.sap.resources.isBundle(myBundle);
18+
jQuery.sap.resources.isBundle(); // Invalid: Missing argument must not be migrated
1819
});
1920

2021
// https://github.com/UI5/linter/issues/522
@@ -152,6 +153,7 @@ sap.ui.define(["sap/base/strings/NormalizePolyfill"], async function (NormalizeP
152153
var clone7 = jQuery.sap.extend(undefined, [undefined]); // Do not migrate.
153154
var clone8 = jQuery.sap.extend(var1, var2); // Do not migrate. We don't know the types of var1 and var2
154155
var clone9 = jQuery.sap.extend(12, [undefined]); // Do not migrate. We don't know how to handle this
156+
var clone9 = jQuery.sap.extend(true); // Invalid: Missing second argument must not be migrated
155157
var timestampsnumber = jQuery.sap.now();
156158
var props = jQuery.sap.properties({ url: sap.ui.require.toUrl(sap.ui.require.toUrl("testdata/test.properties")) });
157159
var myUid = jQuery.sap.uid();

0 commit comments

Comments
 (0)