diff --git a/packages/plugin/__test__/__snapshots__/test.js.snap b/packages/plugin/__test__/__snapshots__/test.js.snap index 4aadd7b..b44f49a 100644 --- a/packages/plugin/__test__/__snapshots__/test.js.snap +++ b/packages/plugin/__test__/__snapshots__/test.js.snap @@ -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({})), ControllerExtension.use(Routing)), + routing3: (cov_1uvvg22e7l().s[5]++, cov_1uvvg22e7l().s[5]++, ControllerExtension.use(Routing.override({}))), + routing2: (cov_1uvvg22e7l().s[5]++, ControllerExtension.use(Routing.override({}))), + routing: (cov_1uvvg22e7l().s[5]++, ControllerExtension.use(Routing)) + }); + return MyExtendedController; +});" +`; + exports[`typescript ts-class-param-props.ts 1`] = ` "sap.ui.define(["sap/Class"], function (SAPClass) { "use strict"; diff --git a/packages/plugin/__test__/fixtures/typescript/ts-class-controller-extension-wrapped.ts b/packages/plugin/__test__/fixtures/typescript/ts-class-controller-extension-wrapped.ts new file mode 100644 index 0000000..852e6db --- /dev/null +++ b/packages/plugin/__test__/fixtures/typescript/ts-class-controller-extension-wrapped.ts @@ -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)); +} \ No newline at end of file diff --git a/packages/plugin/src/classes/helpers/classes.js b/packages/plugin/src/classes/helpers/classes.js index ed37e6f..ffbf10d 100644 --- a/packages/plugin/src/classes/helpers/classes.js +++ b/packages/plugin/src/classes/helpers/classes.js @@ -222,39 +222,24 @@ 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) && + rightSide.expressions.some((expression) => + isCallToControllerExtensionUse(expression, memberPath) + ) + ) { + 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. @@ -293,6 +278,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.