Skip to content

Commit e1bd7d4

Browse files
RandomBytematz3
authored andcommitted
feat(Autofix): UI5 globals
This adds the first version of the autofix feature. A new '--fix' option is added which enables the autofix feature. It supports fixing usages of UI5 globals with their corresponding UI5 module imports. JIRA: CPOUI5FOUNDATION-989
1 parent 4fb76f8 commit e1bd7d4

File tree

86 files changed

+8714
-104
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

86 files changed

+8714
-104
lines changed

README.md

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
- [Options](#options)
2121
- [`--details`](#--details)
2222
- [`--format`](#--format)
23+
- [`--fix`](#--fix)
2324
- [`--ignore-pattern`](#--ignore-pattern)
2425
- [`--config`](#--config)
2526
- [`--ui5-config`](#--ui5-config)
@@ -151,6 +152,29 @@ Choose the output format. Currently, `stylish` (default), `json` and `markdown`
151152
ui5lint --format json
152153
```
153154

155+
#### `--fix`
156+
157+
Attempt to automatically correct certain findings.
158+
159+
**Example::**
160+
```sh
161+
ui5lint --fix
162+
```
163+
164+
Currently, issues of the following rules are fixable:
165+
- no-globals: Usage of globals that are part of UI5 (e.g. `sap.m.Button`) are replaced with the corresponding module import
166+
167+
After applying fixes, the linter runs another pass to detect any remaining issues. Not all findings may be fixable, so those issues may need to be addressed manually.
168+
169+
##### Dry Run Mode
170+
To preview the results without modifying any files, set the UI5LINT_FIX_DRY_RUN environment variable:
171+
172+
```sh
173+
UI5LINT_FIX_DRY_RUN=true ui5lint --fix
174+
```
175+
176+
In this mode, the linter will show the messages after the fixes would have been applied but will not actually change the files.
177+
154178
#### `--ignore-pattern`
155179

156180
Pattern/files that will be ignored during linting. Can also be defined in `ui5lint.config.js`.

npm-shrinkwrap.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,9 @@
4343
"prepare": "node ./.husky/skip.js || husky",
4444
"test": "npm run lint && npm run build-test && npm run coverage && npm run e2e && npm run depcheck && npm run check-licenses",
4545
"unit": "ava",
46-
"e2e": "npm run build && npm run e2e:ui5lint && npm run e2e:test",
47-
"e2e:ui5lint": "TEST_E2E_TMP=$PWD/test/tmp/e2e && npm run clean-test-tmp && mkdir -p $TEST_E2E_TMP && cd test/fixtures/linter/projects/com.ui5.troublesome.app && npm exec ui5lint -- --format=json > $TEST_E2E_TMP/ui5lint-results.json 2> $TEST_E2E_TMP/stderr.log || true",
46+
"e2e": "npm run clean-test-tmp && npm run build && npm run e2e:ui5lint && npm run e2e:ui5lint-fix && npm run e2e:test",
47+
"e2e:ui5lint": "TEST_E2E_TMP=$PWD/test/tmp/e2e && mkdir -p $TEST_E2E_TMP && cd test/fixtures/linter/projects/com.ui5.troublesome.app && npm exec ui5lint -- --format=json > $TEST_E2E_TMP/ui5lint-results.json 2> $TEST_E2E_TMP/stderr.log || true",
48+
"e2e:ui5lint-fix": "TEST_E2E_TMP=$PWD/test/tmp/e2e && mkdir -p $TEST_E2E_TMP && cp -r test/fixtures/linter/projects/com.ui5.troublesome.app $TEST_E2E_TMP && cd $TEST_E2E_TMP/com.ui5.troublesome.app && npm exec ui5lint -- --fix --format=json > $TEST_E2E_TMP/ui5lint-results-fix.json 2> $TEST_E2E_TMP/stderr-fix.log || true",
4849
"e2e:test": "ava --config ava-e2e.config.js",
4950
"e2e:test-update-snapshots": "ava --config ava-e2e.config.js --update-snapshots",
5051
"unit-debug": "ava debug",
@@ -81,6 +82,7 @@
8182
"globals": "^16.0.0",
8283
"he": "^1.2.0",
8384
"json-source-map": "^0.6.1",
85+
"magic-string": "^0.30.17",
8486
"minimatch": "^10.0.1",
8587
"sax-wasm": "^3.0.5",
8688
"typescript": "^5.8.2",

src/autofix/autofix.ts

Lines changed: 264 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,264 @@
1+
import ts from "typescript";
2+
import MagicString from "magic-string";
3+
import LinterContext, {RawLintMessage, ResourcePath} from "../linter/LinterContext.js";
4+
import {MESSAGE} from "../linter/messages.js";
5+
import {ModuleDeclaration} from "../linter/ui5Types/amdTranspiler/parseModuleDeclaration.js";
6+
import generateSolutionNoGlobals from "./solutions/noGlobals.js";
7+
import {getLogger} from "@ui5/logger";
8+
import {addDependencies} from "./solutions/amdImports.js";
9+
10+
const log = getLogger("linter:autofix");
11+
12+
export interface AutofixResource {
13+
content: string;
14+
messages: RawLintMessage[];
15+
}
16+
17+
export interface AutofixOptions {
18+
rootDir: string;
19+
namespace?: string;
20+
resources: Map<ResourcePath, AutofixResource>;
21+
context: LinterContext;
22+
}
23+
24+
export enum ChangeAction {
25+
INSERT = "insert",
26+
REPLACE = "replace",
27+
DELETE = "delete",
28+
}
29+
30+
export type ChangeSet = InsertChange | ReplaceChange | DeleteChange;
31+
32+
interface AbstractChangeSet {
33+
action: ChangeAction;
34+
start: number;
35+
}
36+
37+
interface InsertChange extends AbstractChangeSet {
38+
action: ChangeAction.INSERT;
39+
value: string;
40+
}
41+
42+
interface ReplaceChange extends AbstractChangeSet {
43+
action: ChangeAction.REPLACE;
44+
end: number;
45+
value: string;
46+
}
47+
48+
interface DeleteChange extends AbstractChangeSet {
49+
action: ChangeAction.DELETE;
50+
end: number;
51+
}
52+
53+
export type AutofixResult = Map<ResourcePath, string>;
54+
type SourceFiles = Map<ResourcePath, ts.SourceFile>;
55+
56+
interface Position {
57+
line: number;
58+
column: number;
59+
pos: number;
60+
}
61+
export interface GlobalPropertyAccessNodeInfo {
62+
globalVariableName: string;
63+
namespace: string;
64+
moduleName: string;
65+
exportName?: string;
66+
propertyAccess?: string;
67+
position: Position;
68+
node?: ts.Identifier | ts.PropertyAccessExpression | ts.ElementAccessExpression;
69+
}
70+
71+
export interface DeprecatedApiAccessNode {
72+
apiName: string;
73+
position: Position;
74+
node?: ts.CallExpression | ts.Identifier | ts.PropertyAccessExpression | ts.ElementAccessExpression;
75+
}
76+
77+
export type ImportRequests = Map<string, {
78+
nodeInfos: (DeprecatedApiAccessNode | GlobalPropertyAccessNodeInfo)[];
79+
identifier?: string;
80+
}>;
81+
82+
export type ModuleDeclarationInfo = ExistingModuleDeclarationInfo | NewModuleDeclarationInfo;
83+
84+
export interface ExistingModuleDeclarationInfo {
85+
moduleDeclaration: ModuleDeclaration;
86+
importRequests: ImportRequests;
87+
}
88+
89+
export interface NewModuleDeclarationInfo {
90+
declareCall: ts.CallExpression;
91+
requireCalls: Map<string, ts.CallExpression[]>;
92+
importRequests: ImportRequests;
93+
endPos?: number;
94+
}
95+
96+
function createCompilerHost(sourceFiles: SourceFiles): ts.CompilerHost {
97+
return {
98+
getSourceFile: (fileName) => sourceFiles.get(fileName),
99+
writeFile: () => undefined,
100+
getDefaultLibFileName: () => "lib.d.ts",
101+
useCaseSensitiveFileNames: () => false,
102+
getCanonicalFileName: (fileName) => fileName,
103+
getCurrentDirectory: () => "",
104+
getNewLine: () => "\n",
105+
fileExists: (fileName): boolean => sourceFiles.has(fileName),
106+
readFile: () => "",
107+
directoryExists: () => true,
108+
getDirectories: () => [],
109+
};
110+
}
111+
112+
const compilerOptions: ts.CompilerOptions = {
113+
checkJs: false,
114+
allowJs: true,
115+
skipLibCheck: true,
116+
noCheck: true,
117+
target: ts.ScriptTarget.ES2022,
118+
module: ts.ModuleKind.ES2022,
119+
isolatedModules: true,
120+
sourceMap: true,
121+
suppressOutputPathCheck: true,
122+
noLib: true,
123+
noResolve: true,
124+
allowNonTsExtensions: true,
125+
};
126+
127+
function createProgram(inputFileNames: string[], host: ts.CompilerHost): ts.Program {
128+
return ts.createProgram(inputFileNames, compilerOptions, host);
129+
}
130+
131+
function getJsErrors(code: string, resourcePath: string) {
132+
const sourceFile = ts.createSourceFile(
133+
resourcePath,
134+
code,
135+
ts.ScriptTarget.ES2022,
136+
true,
137+
ts.ScriptKind.JS
138+
);
139+
140+
const host = createCompilerHost(new Map([[resourcePath, sourceFile]]));
141+
const program = createProgram([resourcePath], host);
142+
const diagnostics = ts.getPreEmitDiagnostics(program, sourceFile);
143+
144+
return diagnostics.filter(function (d) {
145+
return d.file === sourceFile && d.category === ts.DiagnosticCategory.Error;
146+
});
147+
}
148+
149+
// eslint-disable-next-line @typescript-eslint/require-await
150+
export default async function ({
151+
rootDir: _unused1,
152+
namespace: _unused2,
153+
resources,
154+
context,
155+
}: AutofixOptions): Promise<AutofixResult> {
156+
const sourceFiles: SourceFiles = new Map();
157+
const resourcePaths = [];
158+
for (const [resourcePath, resource] of resources) {
159+
const sourceFile = ts.createSourceFile(
160+
resourcePath,
161+
resource.content,
162+
{
163+
languageVersion: ts.ScriptTarget.ES2022,
164+
jsDocParsingMode: ts.JSDocParsingMode.ParseNone,
165+
}
166+
);
167+
sourceFiles.set(resourcePath, sourceFile);
168+
resourcePaths.push(resourcePath);
169+
}
170+
171+
const compilerHost = createCompilerHost(sourceFiles);
172+
const program = createProgram(resourcePaths, compilerHost);
173+
174+
const checker = program.getTypeChecker();
175+
const res: AutofixResult = new Map();
176+
for (const [resourcePath, sourceFile] of sourceFiles) {
177+
log.verbose(`Applying autofixes to ${resourcePath}`);
178+
const newContent = applyFixes(checker, sourceFile, resourcePath, resources.get(resourcePath)!);
179+
if (newContent) {
180+
const jsErrors = getJsErrors(newContent, resourcePath);
181+
if (jsErrors.length) {
182+
const message = `Syntax error after applying autofix for '${resourcePath}': ` +
183+
jsErrors.map((d) => d.messageText as string).join(", ");
184+
log.verbose(message);
185+
log.verbose(resourcePath + ":\n" + newContent);
186+
context.addLintingMessage(resourcePath, MESSAGE.PARSING_ERROR, {message});
187+
} else {
188+
log.verbose(`Autofix applied to ${resourcePath}`);
189+
res.set(resourcePath, newContent);
190+
}
191+
}
192+
}
193+
194+
return res;
195+
}
196+
197+
function applyFixes(
198+
checker: ts.TypeChecker, sourceFile: ts.SourceFile, resourcePath: ResourcePath,
199+
resource: AutofixResource
200+
): string | undefined {
201+
const {content} = resource;
202+
203+
// Collect modules of which at least one message has the conditional fixHint flag set
204+
const conditionalModuleAccess = new Set<string>();
205+
for (const msg of resource.messages) {
206+
if (msg.fixHints?.moduleName && msg.fixHints?.conditional) {
207+
log.verbose(`Skipping fixes that would import module '${msg.fixHints.moduleName}' ` +
208+
`because of conditional global access within the current file.`);
209+
conditionalModuleAccess.add(msg.fixHints.moduleName);
210+
}
211+
}
212+
213+
// Group messages by id
214+
const messagesById = new Map<MESSAGE, RawLintMessage[]>();
215+
for (const msg of resource.messages) {
216+
if (msg.fixHints?.moduleName && conditionalModuleAccess.has(msg.fixHints.moduleName)) {
217+
// Skip messages with conditional fixHints
218+
continue;
219+
}
220+
if (!messagesById.has(msg.id)) {
221+
messagesById.set(msg.id, []);
222+
}
223+
messagesById.get(msg.id)!.push(msg);
224+
}
225+
226+
const changeSet: ChangeSet[] = [];
227+
let existingModuleDeclarations = new Map<ts.CallExpression, ExistingModuleDeclarationInfo>();
228+
if (messagesById.has(MESSAGE.NO_GLOBALS)) {
229+
existingModuleDeclarations = generateSolutionNoGlobals(
230+
checker, sourceFile, content,
231+
messagesById.get(MESSAGE.NO_GLOBALS) as RawLintMessage<MESSAGE.NO_GLOBALS>[],
232+
changeSet, []);
233+
}
234+
235+
for (const [defineCall, moduleDeclarationInfo] of existingModuleDeclarations) {
236+
addDependencies(defineCall, moduleDeclarationInfo, changeSet, resourcePath);
237+
}
238+
239+
if (changeSet.length === 0) {
240+
// No modifications needed
241+
return undefined;
242+
}
243+
return applyChanges(content, changeSet);
244+
}
245+
246+
function applyChanges(content: string, changeSet: ChangeSet[]): string {
247+
changeSet.sort((a, b) => b.start - a.start);
248+
const s = new MagicString(content);
249+
250+
for (const change of changeSet) {
251+
switch (change.action) {
252+
case ChangeAction.INSERT:
253+
s.appendRight(change.start, change.value);
254+
break;
255+
case ChangeAction.REPLACE:
256+
s.update(change.start, change.end, change.value);
257+
break;
258+
case ChangeAction.DELETE:
259+
s.remove(change.start, change.end);
260+
break;
261+
}
262+
}
263+
return s.toString();
264+
}

0 commit comments

Comments
 (0)