fix: fix review comments - maintain full SequenceExpression

ui5-community · Sep 13, 2024 · aaa6e21 · aaa6e21
1 parent c344794
commit aaa6e21
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 55 deletions.
diff --git a/packages/plugin/__test__/__snapshots__/test.js.snap b/packages/plugin/__test__/__snapshots__/test.js.snap
@@ -1885,9 +1885,10 @@ exports[`typescript ts-class-controller-extension-wrapped.ts 1`] = `
     };
   };
   const MyExtendedController = Controller.extend("test.controller.MyExtendedController", {
-    routing3: Routing.override({}),
-    routing2: Routing.override({}),
-    routing: Routing
+    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;
 });"

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
@@ -14,4 +14,5 @@ export default class MyExtendedController extends Controller {
 	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));
 }
diff --git a/packages/plugin/src/classes/helpers/classes.js b/packages/plugin/src/classes/helpers/classes.js
@@ -222,60 +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.
+      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.isCallExpression(member.value) ||
-        t.isSequenceExpression(member.value)
+        t.isSequenceExpression(rightSide) &&
+        rightSide.expressions.some((expression) =>
+          isCallToControllerExtensionUse(expression, memberPath)
+        )
       ) {
-        let callExpression = member.value;
-
-        // code instrumentation sometimes wraps it like:
-        // this.routing = (cov_1uvvg22e7l().s[5]++, ControllerExtension.use(Routing.override({ … })));
-        if (t.isSequenceExpression(member.value)) {
-          // iterate through the expressions in the sequence
-          for (const expr of member.value.expressions) {
-            if (t.isCallExpression(expr)) {
-              callExpression = expr;
-              break;
-            }
-          }
-        }
-
-        if (t.isCallExpression(callExpression)) {
-          const callee = callExpression.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 (
-                !callExpression.arguments ||
-                callExpression.arguments.length !== 1
-              ) {
-                // exactly one argument must be there
-                throw memberPath.buildCodeFrameError(
-                  `ControllerExtension.use() must be called with exactly one argument but has ${
-                    callExpression.arguments
-                      ? callExpression.arguments.length
-                      : 0
-                  }`
-                );
-              }
-              member.value = callExpression.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
-            }
-          }
-        }
+        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.
@@ -314,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.