Skip to content

Commit 4319306

Browse files
matz3matz3
committed
fix: Rework AMD import solutions
1 parent c0a8715 commit 4319306

File tree

5 files changed

+136
-102
lines changed

5 files changed

+136
-102
lines changed

‎src/autofix/autofix.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ function applyFixes(
228228
}
229229

230230
function applyChanges(content: string, changeSet: ChangeSet[]): string {
231-
changeSet.sort((a, b) => a.start - b.start);
231+
changeSet.sort((a, b) => b.start - a.start);
232232
const s = new MagicString(content);
233233

234234
for (const change of changeSet) {

‎src/autofix/solutions/amdImports.ts‎

Lines changed: 128 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,49 @@ import {ChangeAction, ImportRequests, ChangeSet, ExistingModuleDeclarationInfo}
33
import {collectModuleIdentifiers} from "../utils.js";
44
import {resolveUniqueName} from "../../linter/ui5Types/utils/utils.js";
55

6+
function createDependencyMap(dependencies: ts.NodeArray<ts.Expression> | undefined) {
7+
const dependencyMap = new Map<string, {node: ts.StringLiteralLike; index: number}>();
8+
dependencies?.forEach((dependency, index) => {
9+
if (!ts.isStringLiteralLike(dependency)) {
10+
return;
11+
}
12+
// In case of duplicate imports, we only use the first one.
13+
// As the map is only used for a lookup for reusing existing imports and not
14+
// as a exhaustive list of dependencies, this is fine.
15+
if (!dependencyMap.has(dependency.text)) {
16+
dependencyMap.set(dependency.text, {node: dependency, index});
17+
}
18+
});
19+
return dependencyMap;
20+
}
21+
22+
function getParameterDeclarationText(param: ts.ParameterDeclaration): string | undefined {
23+
if (!ts.isIdentifier(param.name)) {
24+
return;
25+
}
26+
return param.name.text;
27+
}
28+
29+
function getParameterSyntax(
30+
node: ts.ArrowFunction | ts.FunctionDeclaration | ts.FunctionExpression
31+
): {syntaxList: ts.SyntaxList; hasParens: boolean} {
32+
let syntaxList: ts.SyntaxList | undefined;
33+
let hasParens = false;
34+
for (const child of node.getChildren()) {
35+
if (child.kind === ts.SyntaxKind.SyntaxList) {
36+
syntaxList = child as ts.SyntaxList;
37+
}
38+
if (child.kind === ts.SyntaxKind.OpenParenToken || child.kind === ts.SyntaxKind.CloseParenToken) {
39+
hasParens = true;
40+
}
41+
}
42+
if (syntaxList) {
43+
return {syntaxList, hasParens};
44+
} else {
45+
throw new Error("SyntaxList not found in factory function");
46+
}
47+
}
48+
649
export function addDependencies(
750
defineCall: ts.CallExpression, moduleDeclarationInfo: ExistingModuleDeclarationInfo,
851
changeSet: ChangeSet[]
@@ -13,122 +56,114 @@ export function addDependencies(
1356
return;
1457
}
1558

16-
const declaredIdentifiers = collectModuleIdentifiers(moduleDeclaration.factory);
17-
18-
const defineCallArgs = defineCall.arguments;
19-
const existingImportModules = defineCall.arguments && ts.isArrayLiteralExpression(defineCallArgs[0]) ?
20-
defineCallArgs[0].elements.map((el) => ts.isStringLiteralLike(el) ? el.text : "") :
21-
[];
22-
2359
if (!ts.isFunctionLike(moduleDeclaration.factory)) {
2460
throw new Error("Invalid factory function");
2561
}
26-
const existingIdentifiers = moduleDeclaration.factory
27-
.parameters.map((param: ts.ParameterDeclaration) => (param.name as ts.Identifier).text);
28-
const existingIdentifiersLength = existingIdentifiers.length;
29-
30-
const imports = [...importRequests.keys()];
31-
32-
const identifiersForExistingImports: string[] = [];
33-
let existingIdentifiersCut = 0;
34-
existingImportModules.forEach((existingModule, index) => {
35-
const indexOf = imports.indexOf(existingModule);
36-
const identifierName = existingIdentifiers[index] ||
37-
resolveUniqueName(existingModule, declaredIdentifiers);
38-
declaredIdentifiers.add(identifierName);
39-
identifiersForExistingImports.push(identifierName);
40-
if (indexOf !== -1) {
41-
// If there are defined dependencies, but identifiers for them are missing,
42-
// and those identifiers are needed in the code, then we need to find out
43-
// up to which index we need to build identifiers and cut the rest.
44-
existingIdentifiersCut = index > existingIdentifiersCut ? (index + 1) : existingIdentifiersCut;
45-
imports.splice(indexOf, 1);
46-
importRequests.get(existingModule)!.identifier = identifierName;
47-
}
48-
});
4962

50-
// Cut identifiers that are already there
51-
identifiersForExistingImports.splice(existingIdentifiersCut);
52-
53-
const dependencies = imports.map((i) => `"${i}"`);
54-
const identifiers = [
55-
...identifiersForExistingImports,
56-
...imports.map((i) => {
57-
const identifier = resolveUniqueName(i, declaredIdentifiers);
58-
declaredIdentifiers.add(identifier);
59-
importRequests.get(i)!.identifier = identifier;
60-
return identifier;
61-
})];
62-
63-
if (dependencies.length) {
64-
// Add dependencies
65-
if (moduleDeclaration.dependencies) {
66-
const depsNode = defineCall.arguments[0];
67-
const depElementNodes = depsNode && ts.isArrayLiteralExpression(depsNode) ? depsNode.elements : [];
68-
const insertAfterElement = depElementNodes[existingIdentifiersLength - 1] ??
69-
depElementNodes[depElementNodes.length - 1];
70-
71-
if (insertAfterElement) {
72-
changeSet.push({
73-
action: ChangeAction.INSERT,
74-
start: insertAfterElement.getEnd(),
75-
value: (existingImportModules.length ? ", " : "") + dependencies.join(", "),
76-
});
63+
const declaredIdentifiers = collectModuleIdentifiers(moduleDeclaration.factory);
64+
65+
const dependencies = moduleDeclaration.dependencies?.elements;
66+
const dependencyMap = createDependencyMap(dependencies);
67+
68+
const parameters = moduleDeclaration.factory.parameters;
69+
const parameterSyntax = getParameterSyntax(moduleDeclaration.factory);
70+
71+
const newDependencies: {moduleName: string; identifier: string}[] = [];
72+
73+
// Calculate after which index we can add new dependencies
74+
const insertAfterIndex = Math.min(parameters.length, dependencies?.length ?? 0) - 1;
75+
76+
// Check whether requested imports are already available in the list of dependencies
77+
for (const [requestedModuleName, importRequest] of importRequests) {
78+
const existingDependency = dependencyMap.get(requestedModuleName);
79+
if (existingDependency) {
80+
// Reuse the existing dependency
81+
// Check whether a parameter name already exists for this dependency
82+
// Lookup needs to be based on the same index of the parameter in the factory function
83+
const existingParameter = parameters[existingDependency.index];
84+
if (existingParameter) {
85+
// Existing parameter can be reused
86+
importRequest.identifier = getParameterDeclarationText(existingParameter);
87+
continue;
7788
} else {
89+
// Dependency exist, but without a function parameter.
90+
// Remove the dependency so that it will be handled together with the new dependencies
7891
changeSet.push({
79-
action: ChangeAction.REPLACE,
80-
start: depsNode.getFullStart(),
81-
end: depsNode.getEnd(),
82-
value: `[${dependencies.join(", ")}]`,
92+
action: ChangeAction.DELETE,
93+
// Also remove the comma before the dependency
94+
start: existingDependency.node.getFullStart() - (existingDependency.index > 0 ? 1 : 0),
95+
end: existingDependency.node.getEnd(),
8396
});
97+
dependencyMap.delete(requestedModuleName);
8498
}
85-
} else {
86-
changeSet.push({
87-
action: ChangeAction.INSERT,
88-
start: defineCall.arguments[0].getFullStart(),
89-
value: `[${dependencies.join(", ")}], `,
90-
});
9199
}
100+
// Add a completely new module dependency
101+
const identifier = resolveUniqueName(requestedModuleName, declaredIdentifiers);
102+
declaredIdentifiers.add(identifier);
103+
importRequest.identifier = identifier;
104+
newDependencies.push({
105+
moduleName: requestedModuleName,
106+
identifier,
107+
});
92108
}
93109

94-
if (identifiers.length) {
95-
const closeParenToken = moduleDeclaration.factory.getChildren()
96-
.find((c) => c.kind === ts.SyntaxKind.CloseParenToken);
97-
// Factory arguments
98-
const syntaxList = moduleDeclaration.factory.getChildren()
99-
.find((c) => c.kind === ts.SyntaxKind.SyntaxList);
100-
if (!syntaxList) {
101-
throw new Error("Invalid factory syntax");
102-
}
103-
104-
// Patch factory arguments
105-
const value = (existingIdentifiersLength ? ", " : "") + identifiers.join(", ");
106-
if (!closeParenToken) {
110+
// Add new dependencies
111+
if (newDependencies.length) {
112+
const newDependencyValue = newDependencies.map((newDependency) => {
113+
return `"${newDependency.moduleName}"`;
114+
}).join(", ");
115+
116+
const insertAfterDependencyElement = dependencies?.[insertAfterIndex];
117+
if (insertAfterDependencyElement || (dependencies && insertAfterIndex === -1)) {
118+
const existingDependenciesLeft = insertAfterIndex > -1 && dependencyMap.size > 0;
119+
const existingDependenciesRight = insertAfterIndex === -1 && dependencyMap.size > 0;
120+
let value = existingDependenciesLeft ? ", " + newDependencyValue : newDependencyValue;
121+
value += existingDependenciesRight ? ", " : "";
122+
const start = insertAfterDependencyElement?.getEnd() ?? dependencies.pos;
107123
changeSet.push({
108124
action: ChangeAction.INSERT,
109-
start: syntaxList.getStart(),
110-
value: "(",
125+
start,
126+
value,
111127
});
128+
} else if (moduleDeclaration.dependencies) {
129+
const start = moduleDeclaration.dependencies.getStart();
130+
const end = moduleDeclaration.dependencies.getEnd();
112131
changeSet.push({
113-
action: ChangeAction.INSERT,
114-
start: syntaxList.getEnd(),
115-
value: `${value})`,
132+
action: ChangeAction.REPLACE,
133+
start,
134+
end,
135+
value: `[${newDependencyValue}]`,
116136
});
117137
} else {
118-
let start = syntaxList.getEnd();
138+
changeSet.push({
139+
action: ChangeAction.INSERT,
140+
// TODO: is this correct if the module name is defined as first argument?
141+
start: defineCall.arguments[0].getFullStart(),
142+
value: `[${newDependencyValue}], `,
143+
});
144+
}
119145

120-
// Existing trailing comma: Insert new args before it, to keep the trailing comma
121-
const lastSyntaxListChild = syntaxList.getChildAt(syntaxList.getChildCount() - 1);
122-
if (lastSyntaxListChild?.kind === ts.SyntaxKind.CommaToken) {
123-
start = lastSyntaxListChild.getStart();
124-
}
146+
let newParametersValue = newDependencies.map((newDependency) => {
147+
return newDependency.identifier;
148+
}).join(", ");
125149

150+
if (!parameterSyntax.hasParens) {
126151
changeSet.push({
127152
action: ChangeAction.INSERT,
128-
start,
129-
value,
153+
start: parameterSyntax.syntaxList.getStart(),
154+
value: "(",
130155
});
156+
newParametersValue += ")";
131157
}
158+
159+
const insertAfterParameterDeclaration = parameters?.[insertAfterIndex];
160+
const start = insertAfterParameterDeclaration?.getEnd() ?? parameterSyntax.syntaxList.getStart();
161+
const value = parameters.length ? ", " + newParametersValue : newParametersValue;
162+
changeSet.push({
163+
action: ChangeAction.INSERT,
164+
start,
165+
value,
166+
});
132167
}
133168

134169
// Patch identifiers

‎test/fixtures/autofix/GlobalsExistingDefineWithDeps_SpecialNames.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ sap.ui.define([
44
"./../../library",
55
"../other/library",
66
"./library",
7-
"sap/m/Button",
7+
"sap/m/Button"
88
], function() {
99
const button = new sap.m.Button({
1010
text: "Hello"

‎test/lib/autofix/snapshots/autofix.ts.md‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -471,14 +471,13 @@ Generated by [AVA](https://avajs.dev).
471471

472472
> AutofixResult: /GlobalsExistingDefineWithDeps_SpecialNames.js
473473
474-
`sap.ui.define([␊
474+
`sap.ui.define(["sap/m/Button", ␊
475475
"sap/ui/thirdparty/sinon-qunit",␊
476476
"./controller/App.controller",␊
477477
"./../../library",␊
478478
"../other/library",␊
479-
"./library",␊
480-
"sap/m/Button",␊
481-
], function(sinonQunit, AppController, library, otherLibrary, _Library, Button) {␊
479+
"./library"␊
480+
], function(Button) {␊
482481
const button = new Button({␊
483482
text: "Hello"␊
484483
});␊
@@ -502,7 +501,7 @@ Generated by [AVA](https://avajs.dev).
502501

503502
> AutofixResult: /GlobalsExistingDefineWithExistingDepsMultiple.js
504503
505-
`sap.ui.define(["sap/m/Button", "sap/m/Input"], function(Button, Input, Button, Input) {␊
504+
`sap.ui.define(["sap/m/Button", "sap/m/Input"], function(Button, Input) {␊
506505
"use strict";␊
507506
␊
508507
new Input();␊
@@ -550,7 +549,7 @@ Generated by [AVA](https://avajs.dev).
550549

551550
> AutofixResult: /GlobalsExistingDefineWithExistingUnusedDepsMultiple.js
552551
553-
`sap.ui.define(["sap/m/Button", "sap/m/Input"], function(Button, Input) {␊
552+
`sap.ui.define(["sap/m/Input", "sap/m/Button"], function(Input) {␊
554553
"use strict";␊
555554
␊
556555
new Input();␊
@@ -574,7 +573,7 @@ Generated by [AVA](https://avajs.dev).
574573

575574
> AutofixResult: /GlobalsExistingDefineWithExistingUnusedDepsSingle.js
576575
577-
`sap.ui.define(["sap/m/Button"], function() {␊
576+
`sap.ui.define(["sap/m/Button"], function(Button) {␊
578577
"use strict";␊
579578
␊
580579
new Button();␊

‎test/lib/autofix/snapshots/autofix.ts.snap‎

-33 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)