Skip to content

Commit

Permalink
test: add more tests and runtime checks
Browse files Browse the repository at this point in the history
Throw error when ControllerExtension.use(...) is not called with one
parameter.
  • Loading branch information
akudev committed Jul 12, 2024
1 parent 399b507 commit d608a51
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 4 deletions.
34 changes: 34 additions & 0 deletions packages/plugin/__test__/__snapshots__/test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -1670,12 +1670,35 @@ exports[`typescript ts-class-controller-extension-extended.ts 1`] = `
"use strict";

const MyExtendedController = Controller.extend("test.controller.MyExtendedController", {
constructor: function constructor() {
Controller.prototype.constructor.apply(this, arguments);
this.routing2 = Routing.use(Routing.override({}));
this.routing3 = Controller.use(Routing.override({}));
},
routing: Routing.override({})
});
return MyExtendedController;
});"
`;

exports[`typescript ts-class-controller-extension-extended-error-1.ts 1`] = `
"ControllerExtension.use() must be called with exactly one argument but has 0
  7 |  */
  8 | export default class MyExtendedController extends Controller {
> 9 | routing = ControllerExtension.use(); // should throw an error
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  10 | }"
`;

exports[`typescript ts-class-controller-extension-extended-error-2.ts 1`] = `
"ControllerExtension.use() must be called with exactly one argument but has 2
  7 |  */
  8 | export default class MyExtendedController extends Controller {
> 9 | routing = ControllerExtension.use(1, 2); // should throw an error
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  10 | }"
`;

exports[`typescript ts-class-controller-extension-extended-explicit-type.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";
Expand All @@ -1691,6 +1714,17 @@ exports[`typescript ts-class-controller-extension-extended-explicit-type.ts 1`]
});"
`;

exports[`typescript ts-class-controller-extension-extended-renamed.ts 1`] = `
"sap.ui.define(["sap/ui/core/mvc/Controller", "sap/ui/core/mvc/ControllerExtension", "sap/fe/core/controllerextensions/Routing"], function (Controller, CtrlEx, Routing) {
"use strict";

const MyExtendedController = Controller.extend("test.controller.MyExtendedController", {
routing: Routing.override({})
});
return MyExtendedController;
});"
`;

exports[`typescript ts-class-controller-extension-usage.ts 1`] = `
"sap.ui.define(["sap/ui/core/mvc/Controller", "sap/fe/core/controllerextensions/Routing", "sap/fe/core/controllerextensions/OtherExtension", "sap/fe/core/controllerextensions/ThirdExtension", "sap/fe/core/controllerextensions/DoubleExportExtension", "sap/fe/core/controllerextensions/ManyExtensions"], function (Controller, Routing, sap_fe_core_controllerextensions_OtherExtension, ThirdExtension, sap_fe_core_controllerextensions_DoubleExportExtension, extensionCollection) {
"use strict";
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import Controller from "sap/ui/core/mvc/Controller";
import ControllerExtension from "sap/ui/core/mvc/ControllerExtension";
import Routing from "sap/fe/core/controllerextensions/Routing";

/**
* @namespace test.controller
*/
export default class MyExtendedController extends Controller {
routing = ControllerExtension.use(); // should throw an error
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import Controller from "sap/ui/core/mvc/Controller";
import ControllerExtension from "sap/ui/core/mvc/ControllerExtension";
import Routing from "sap/fe/core/controllerextensions/Routing";

/**
* @namespace test.controller
*/
export default class MyExtendedController extends Controller {
routing = ControllerExtension.use(1, 2); // should throw an error
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import Controller from "sap/ui/core/mvc/Controller";
import CtrlEx from "sap/ui/core/mvc/ControllerExtension";
import Routing from "sap/fe/core/controllerextensions/Routing";

/**
* @namespace test.controller
*/
export default class MyExtendedController extends Controller {
routing = /* comment */ CtrlEx.use(
/* comment */
Routing.override(
/* comment */
{
/* comment */
}
));
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,6 @@ import Routing from "sap/fe/core/controllerextensions/Routing";
*/
export default class MyExtendedController extends Controller {
routing = ControllerExtension.use(Routing.override({}));
routing2 = (Routing as any).use(Routing.override({})); // should not even try to handle this
routing3 = Controller.use(Routing.override({})); // should not even try to handle this
}
19 changes: 15 additions & 4 deletions packages/plugin/src/classes/helpers/classes.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ export function convertClassToUI5Extend(

// 3. restore the import in case it was run already and removed the import
const neededImportDeclaration = getImportDeclaration(
memberPath.hub.file.opts.filename,
memberPath?.hub?.file?.opts?.filename,
typeName
);
if (
Expand Down Expand Up @@ -231,14 +231,25 @@ export function convertClassToUI5Extend(
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"...
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 ===
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
Expand Down
4 changes: 4 additions & 0 deletions packages/plugin/src/classes/helpers/imports.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ export const saveImports = (file) => {

// can be called from visitor to access previously present declarations
export function getImportDeclaration(filename, typeName) {
if (!filename || !typeName) {
return null;
}

const typeNameParts = typeName.split(".");

// find the declaration importing the typeName among the collected import declarations in this file
Expand Down

0 comments on commit d608a51

Please sign in to comment.