Skip to content

Commit

Permalink
fix: support wrapped controller extension assignments
Browse files Browse the repository at this point in the history
Code instrumentation (for coverage measurement) may wrap controller
extension assignments like:
this.routing = (cov_1uvvg22e7l().s[5]++,
ControllerExtension.use(Routing.override({ … })));
  • Loading branch information
akudev committed Sep 13, 2024
1 parent 8db5f32 commit dfa51b6
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 33 deletions.
19 changes: 19 additions & 0 deletions packages/plugin/__test__/__snapshots__/test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -1875,6 +1875,25 @@ exports[`typescript ts-class-controller-extension-usage-new.ts 1`] = `
});"
`;
exports[`typescript ts-class-controller-extension-wrapped.ts 1`] = `
"sap.ui.define(["sap/ui/core/mvc/Controller", "sap/ui/core/mvc/ControllerExtension", "sap/fe/core/controllerextensions/Routing"], function (Controller, ControllerExtension, Routing) {
"use strict";
const cov_1uvvg22e7l = () => {
return {
"s": {}
};
};
const MyExtendedController = Controller.extend("test.controller.MyExtendedController", {
routing4: (cov_1uvvg22e7l().s[5]++, ControllerExtension.use(Routing.override({})), Routing),
routing3: (cov_1uvvg22e7l().s[5]++, cov_1uvvg22e7l().s[5]++, Routing.override({})),
routing2: (cov_1uvvg22e7l().s[5]++, Routing.override({})),
routing: (cov_1uvvg22e7l().s[5]++, Routing)
});
return MyExtendedController;
});"
`;
exports[`typescript ts-class-param-props.ts 1`] = `
"sap.ui.define(["sap/Class"], function (SAPClass) {
"use strict";
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import Controller from "sap/ui/core/mvc/Controller";
import ControllerExtension from "sap/ui/core/mvc/ControllerExtension";
import Routing from "sap/fe/core/controllerextensions/Routing";

const cov_1uvvg22e7l = () => { return { "s": {} }; }; // dummy coverage function

/**
* @namespace test.controller
*/
export default class MyExtendedController extends Controller {

// code could already be instrumented, e.g. for code coverage by istanbul, and look like this:
//this.routing = (cov_1uvvg22e7l().s[5]++, ControllerExtension.use(Routing.override({ … })));
routing = (cov_1uvvg22e7l().s[5]++, ControllerExtension.use(Routing));
routing2 = (cov_1uvvg22e7l().s[5]++, ControllerExtension.use(Routing.override({})));
routing3 = (cov_1uvvg22e7l().s[5]++, cov_1uvvg22e7l().s[5]++, ControllerExtension.use(Routing.override({})));
routing4 = (cov_1uvvg22e7l().s[5]++, ControllerExtension.use(Routing.override({})), ControllerExtension.use(Routing));
}
95 changes: 62 additions & 33 deletions packages/plugin/src/classes/helpers/classes.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,39 +222,27 @@ export function convertClassToUI5Extend(
// @transformControllerExtension marker, because it is not a type assignment. In the resulting code, the
// "ControllerExtension.use(...)" part should be removed and the content of the brackets should be assigned
// directly to the member property.
if (t.isCallExpression(member.value)) {
const callee = member.value.callee;
if (
t.isMemberExpression(callee) &&
t.isIdentifier(callee.object) &&
t.isIdentifier(callee.property) &&
callee.property.name === "use" // we are looking for "ControllerExtension.use(...)"
) {
const importDeclaration = getImportDeclaration(
memberPath?.hub?.file?.opts?.filename,
callee?.object?.name // usually, but not necessarily always: "ControllerExtension"...
);
// ...hence we rather look at the imported module name to be sure
if (
importDeclaration?.source?.value ===
"sap/ui/core/mvc/ControllerExtension"
) {
if (
!member.value.arguments ||
member.value.arguments.length !== 1
) {
// exactly one argument must be there
throw memberPath.buildCodeFrameError(
`ControllerExtension.use() must be called with exactly one argument but has ${
member.value.arguments ? member.value.arguments.length : 0
}`
);
}
member.value = member.value.arguments[0];
extendProps.unshift(buildObjectProperty(member)); // add it to the properties of the extend() config object
continue; // prevent the member from also being added to the constructor
}
}
const rightSide = member.value;
if (isCallToControllerExtensionUse(rightSide, memberPath)) {
member.value = rightSide.arguments[0];
extendProps.unshift(buildObjectProperty(member)); // add it to the properties of the extend() config object
continue; // prevent the member from also being added to the constructor
}

// code instrumentation sometimes wraps ControllerExtension.use() calls like:
// this.routing = (cov_1uvvg22e7l().s[5]++, ControllerExtension.use(Routing.override({ … })));
if (
t.isSequenceExpression(rightSide) &&
isCallToControllerExtensionUse(
rightSide.expressions[rightSide.expressions.length - 1],
memberPath
)
) {
rightSide.expressions[rightSide.expressions.length - 1] =
rightSide.expressions[rightSide.expressions.length - 1].arguments[0];
member.value = rightSide;
extendProps.unshift(buildObjectProperty(member)); // add it to the properties of the extend() config object
continue; // prevent the member from also being added to the constructor
}

// Special handling for TypeScript limitation where metadata, renderer and overrides must be properties.
Expand Down Expand Up @@ -293,6 +281,47 @@ export function convertClassToUI5Extend(
}
}

/**
* Checks whether the given thing is a CallExpression that calls ControllerExtension.use(...)
*
* @param {*} expression the thing to check - does not need to be a CallExpression
* @param {string} memberPath
* @returns true if the given expression is a CallExpression that calls ControllerExtension.use(...)
*/
function isCallToControllerExtensionUse(expression, memberPath) {
if (!t.isCallExpression(expression)) {
return false;
}
const callee = expression.callee;
if (
t.isMemberExpression(callee) &&
t.isIdentifier(callee.object) &&
t.isIdentifier(callee.property) &&
callee.property.name === "use" // we are looking for "ControllerExtension.use(...)"
) {
const importDeclaration = getImportDeclaration(
memberPath?.hub?.file?.opts?.filename,
callee?.object?.name // usually, but not necessarily always: "ControllerExtension"...
);
// ...hence we rather look at the imported module name to be sure
if (
importDeclaration?.source?.value ===
"sap/ui/core/mvc/ControllerExtension"
) {
if (!expression.arguments || expression.arguments.length !== 1) {
// exactly one argument must be there
throw memberPath.buildCodeFrameError(
`ControllerExtension.use() must be called with exactly one argument but has ${
expression.arguments ? expression.arguments.length : 0
}`
);
}
return true;
}
}
return false; // return false if not a match
}

// Arrow function properties need to get moved to the constructor so that
// they're bound properly to the class instance, to align with the spec.
// For controllers, use onInit rather than constructor, since controller constructors don't work.
Expand Down

0 comments on commit dfa51b6

Please sign in to comment.