diff --git a/src/dependencies.ts b/src/dependencies.ts index b96c96df..6c9d7ced 100644 --- a/src/dependencies.ts +++ b/src/dependencies.ts @@ -50,7 +50,8 @@ export interface Extender { * * @param {SapUiDefineCall} defineCall the module's define call * @param {object} config is passed along from finder and replacer + * @param {string} [name] is the unique variable name when naming conflict is found * @returns {boolean} whether or not the defineCall was modified */ - extend(defineCall: SapUiDefineCall, config: {}): boolean; + extend(defineCall: SapUiDefineCall, config: {}, name?: string): boolean; } diff --git a/src/tasks/addMissingDependencies.ts b/src/tasks/addMissingDependencies.ts index cea916ea..2e4b0d86 100644 --- a/src/tasks/addMissingDependencies.ts +++ b/src/tasks/addMissingDependencies.ts @@ -58,6 +58,7 @@ function mapToFound(oPath: NodePath, oFound: FoundReplacement): FoundCall { replacerName: oFound.oModuleInner.replacer, extenderName: oFound.oModuleInner.extender, newModulePath: oFound.oModuleInner.newModulePath, + newVariableName: oFound.uniqueVariableName, }, }; } @@ -191,7 +192,11 @@ function isFoundInConfig( oNodePath: NodePath, oModuleTree: { [index: string]: { - [index: string]: {finder: string; newModulePath: string}; + [index: string]: { + finder: string; + newModulePath: string; + newVariableName?: string; + }; }; }, finder: {[name: string]: Finder}, @@ -204,6 +209,66 @@ function isFoundInConfig( for (const sModuleInner in oModule) { if (oModule.hasOwnProperty(sModuleInner)) { const oModuleInner = oModule[sModuleInner]; + + // 1. newVariable is unique + // -> OK, we just add a new import and use the variable name + // 2. newVariableName is already in use - same import path + // -> OK, we just use the existing import and use the variable name + // 3. newVariableName is already in use - different import path + // -> NotOK, we need to make the newVariableName unique and add the import + let candidateName = oModuleInner.newVariableName; + if ( + oModuleInner.newVariableName && + oModuleInner.newModulePath + ) { + let existingModulePath = + defineCall.getImportByParamName( + oModuleInner.newVariableName + ); + + if ( + existingModulePath && + existingModulePath !== oModuleInner.newModulePath + ) { + const pathParts = + oModuleInner.newModulePath.split("/"); + if (pathParts.length >= 2) { + // concatenate name suffix from the module path e.g., sap/ui/core/Element, When variable name "Element" + // is already in use, try with name "CoreElement" + candidateName = + pathParts[pathParts.length - 2] + + pathParts[pathParts.length - 1]; + candidateName = + candidateName + .substring(0, 1) + .toUpperCase() + + candidateName.substring(1); + existingModulePath = + defineCall.getImportByParamName( + candidateName + ); + } + + // if there's still a naming conflict, a digit is added to the end of the candidate name till a non-conflict + // name is found + let suffix = 0; + while ( + existingModulePath && + existingModulePath !== + oModuleInner.newModulePath + ) { + existingModulePath = + defineCall.getImportByParamName( + candidateName + suffix++ + ); + } + + if (suffix !== 0) { + candidateName = candidateName + suffix; + } + } + } + const oFinder: Finder = finder[oModuleInner.finder]; const oResult: FinderResult = oFinder.find( oNode, @@ -217,6 +282,7 @@ function isFoundInConfig( configName: oResult.configName, oModuleInner, newModulePath: oModuleInner.newModulePath, + uniqueVariableName: candidateName, }); } } @@ -236,6 +302,7 @@ interface FoundReplacement { newModulePath?: string; }; newModulePath: string; + uniqueVariableName?: string; } interface ConfigObject { @@ -375,6 +442,8 @@ async function migrate(args: Mod.MigrateArguments): Promise { if (!args.config.replacers) { throw new Error("No replacers configured"); } + + // REPLACER - code modification for (const replacerName in args.config.replacers) { if (args.config.replacers.hasOwnProperty(replacerName)) { const modulePath = path.join( @@ -392,6 +461,8 @@ async function migrate(args: Mod.MigrateArguments): Promise { if (!args.config.extenders) { throw new Error("No extenders configured"); } + + // EXTENDER - modify sap.ui.define dependencies for (const extenderName in args.config.extenders) { if (args.config.extenders.hasOwnProperty(extenderName)) { const modulePath = path.join( @@ -419,7 +490,9 @@ async function migrate(args: Mod.MigrateArguments): Promise { // Try to replace the call try { // retrieve variable name from existing import and use it as name argument of replace call - let variableNameToUse = oReplace.config.newVariableName; + let variableNameToUse = + oReplace.importObj.newVariableName || + oReplace.config.newVariableName; if (oReplace.config.newModulePath) { variableNameToUse = analyseResult.defineCall.getParamNameByImport( @@ -437,7 +510,11 @@ async function migrate(args: Mod.MigrateArguments): Promise { // modify define call const oExtender: Extender = mExtenderFuncs[oReplace.importObj.extenderName]; - oExtender.extend(analyseResult.defineCall, oReplace.config); + oExtender.extend( + analyseResult.defineCall, + oReplace.config, + oReplace.importObj.newVariableName + ); args.reporter.collect("replacementsPerformed", 1); if ( diff --git a/src/tasks/helpers/extenders/AddImport.ts b/src/tasks/helpers/extenders/AddImport.ts index f37b2532..2a62a380 100644 --- a/src/tasks/helpers/extenders/AddImport.ts +++ b/src/tasks/helpers/extenders/AddImport.ts @@ -10,11 +10,12 @@ class AddImport implements Extender { config: { newModulePath: string; newVariableName: string; - } + }, + name: string ): boolean { return defineCall.addDependency( config.newModulePath, - config.newVariableName + name || config.newVariableName ); } } diff --git a/src/tasks/helpers/replacers/Module.ts b/src/tasks/helpers/replacers/Module.ts index 5da7529a..bb260e90 100644 --- a/src/tasks/helpers/replacers/Module.ts +++ b/src/tasks/helpers/replacers/Module.ts @@ -67,6 +67,12 @@ const replaceable: ASTReplaceable = { case Syntax.ReturnStatement: // return MyModule.myField oInsertionPoint[node.name] = oNodeModule; break; + case Syntax.ExpressionStatement: + oInsertionPoint[node.name] = oNodeModule; + break; + case Syntax.IfStatement: + oInsertionPoint[node.name] = oNodeModule; + break; default: throw new Error( "Module: insertion is of an unsupported type " + diff --git a/test/addMissingDependencies/extenders/nameClash.config.json b/test/addMissingDependencies/extenders/nameClash.config.json new file mode 100644 index 00000000..be93eb63 --- /dev/null +++ b/test/addMissingDependencies/extenders/nameClash.config.json @@ -0,0 +1,26 @@ +{ + "modules": { + "GLOBALS": { + "*.control": { + "newModulePath": "sap/ui/core/Element", + "newVariableName": "Element", + "replacer": "Module", + "finder": "JQueryControlCallFinder", + "extender": "AddImport" + } + } + }, + "finders": { + "JQueryControlCallFinder": "tasks/helpers/finders/JQueryControlCallFinder.js" + }, + "extenders": { + "AddImport": "tasks/helpers/extenders/AddImport.js" + }, + "replacers": { + "Module": "tasks/helpers/replacers/Module.js" + }, + "comments": { + "unhandledReplacementComment": "TODO unhandled replacement" + }, + "excludes": [] +} \ No newline at end of file diff --git a/test/addMissingDependencies/extenders/nameClash.expected.js b/test/addMissingDependencies/extenders/nameClash.expected.js new file mode 100644 index 00000000..54bc09ec --- /dev/null +++ b/test/addMissingDependencies/extenders/nameClash.expected.js @@ -0,0 +1,43 @@ +/*! + * ${copyright} + */ + +// A module +sap.ui.define(["sap/me/Element", "sap/ui/core/Element"], + function(Element, CoreElement) { + "use strict"; + + /** + * + * @type {{}} + */ + var A = {}; + + /** + * + * @param oParam + * @param sContent + */ + A.x = function (oParam, iIndex) { + Element.foo(); + if (CoreElement) { + var sKey = "Test." + iconName + oParam.control; + if (iconInfo.resourceBundle.hasText(sKey)) { + CoreElement; + } + var x$ = sKey(); + var oTestControl = CoreElement; + A.controls = x$.control(); + A.control = CoreElement; + A.controlAtPlace = CoreElement; + A.defaultControl = CoreElement; + + var oActionControl = oParam.getAction().control(this.oView); + oActionControl.pause(); + + return oTestControl; + } + }; + + return A; + }, /* bExport= */ true); \ No newline at end of file diff --git a/test/addMissingDependencies/extenders/nameClash.js b/test/addMissingDependencies/extenders/nameClash.js new file mode 100644 index 00000000..eb0ad725 --- /dev/null +++ b/test/addMissingDependencies/extenders/nameClash.js @@ -0,0 +1,43 @@ +/*! + * ${copyright} + */ + +// A module +sap.ui.define(["sap/me/Element"], + function(Element) { + "use strict"; + + /** + * + * @type {{}} + */ + var A = {}; + + /** + * + * @param oParam + * @param sContent + */ + A.x = function (oParam, iIndex) { + Element.foo(); + if (oParam.control(0)) { + var sKey = "Test." + iconName + oParam.control; + if (iconInfo.resourceBundle.hasText(sKey)) { + $(sKey).control()[0]; + } + var x$ = sKey(); + var oTestControl = x$.find(".abc").children().eq(0).control(2, true); + A.controls = x$.control(); + A.control = oTestControl.$().control(0); + A.controlAtPlace = x$.control(iIndex); + A.defaultControl = x$.find(".abc").children().eq(0).control()[0]; + + var oActionControl = oParam.getAction().control(this.oView); + oActionControl.pause(); + + return oTestControl; + } + }; + + return A; + }, /* bExport= */ true); \ No newline at end of file diff --git a/test/addMissingDependenciesTest.ts b/test/addMissingDependenciesTest.ts index 2587f639..c63f0e07 100644 --- a/test/addMissingDependenciesTest.ts +++ b/test/addMissingDependenciesTest.ts @@ -516,5 +516,25 @@ describe("addMissingDependencies", () => { [] ); }); + + it("should add new import without name clash", done => { + const subDir = rootDir + "extenders/"; + const expectedContent = fs.readFileSync( + subDir + "nameClash.expected.js", + "utf8" + ); + const config = JSON.parse( + fs.readFileSync(subDir + "nameClash.config.json", "utf8") + ); + const module = new CustomFileInfo(subDir + "nameClash.js"); + analyseMigrateAndTest( + module, + true, + expectedContent, + config, + done, + [] + ); + }); }); });