Skip to content

Commit 5c397d5

Browse files
committed
perf: Only parse files with potential autofixes
1 parent 67680dc commit 5c397d5

File tree

3 files changed

+53
-11
lines changed

3 files changed

+53
-11
lines changed

src/autofix/autofix.ts

Lines changed: 47 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,12 @@ import generateSolutionNoGlobals from "./solutions/noGlobals.js";
77
import {getLogger} from "@ui5/logger";
88
import {addDependencies} from "./solutions/amdImports.js";
99
import {RequireExpression} from "../linter/ui5Types/amdTranspiler/parseRequire.js";
10+
import {Resource} from "@ui5/fs";
1011

1112
const log = getLogger("linter:autofix");
1213

1314
export interface AutofixResource {
14-
content: string;
15+
resource: Resource;
1516
messages: RawLintMessage[];
1617
}
1718

@@ -147,19 +148,57 @@ function getJsErrors(code: string, resourcePath: string) {
147148
});
148149
}
149150

150-
// eslint-disable-next-line @typescript-eslint/require-await
151+
function getAutofixMessages(resource: AutofixResource) {
152+
// Collect modules of which at least one message has the conditional fixHint flag set
153+
const conditionalModuleAccess = new Set<string>();
154+
for (const msg of resource.messages) {
155+
if (msg.fixHints?.moduleName && msg.fixHints?.conditional) {
156+
log.verbose(`Skipping fixes that would import module '${msg.fixHints.moduleName}' ` +
157+
`because of conditional global access within the current file.`);
158+
conditionalModuleAccess.add(msg.fixHints.moduleName);
159+
}
160+
}
161+
162+
// Group messages by id
163+
const messagesById = new Map<MESSAGE, RawLintMessage[]>();
164+
for (const msg of resource.messages) {
165+
if (msg.fixHints?.moduleName && conditionalModuleAccess.has(msg.fixHints.moduleName)) {
166+
// Skip messages with conditional fixHints
167+
continue;
168+
}
169+
if (!messagesById.has(msg.id)) {
170+
messagesById.set(msg.id, []);
171+
}
172+
messagesById.get(msg.id)!.push(msg);
173+
}
174+
175+
return messagesById;
176+
}
177+
151178
export default async function ({
152179
rootDir: _unused1,
153180
namespace: _unused2,
154-
resources,
181+
resources: autofixResources,
155182
context,
156183
}: AutofixOptions): Promise<AutofixResult> {
184+
// Only collect and parse files for which fixes are available
185+
const resources: Resource[] = [];
186+
for (const [_, autofixResource] of autofixResources) {
187+
const messagesById = getAutofixMessages(autofixResource);
188+
// Currently only global access autofixes are supported
189+
// This needs to stay aligned with the applyFixes function
190+
if (messagesById.has(MESSAGE.NO_GLOBALS)) {
191+
resources.push(autofixResource.resource);
192+
}
193+
}
194+
157195
const sourceFiles: SourceFiles = new Map();
158196
const resourcePaths = [];
159-
for (const [resourcePath, resource] of resources) {
197+
for (const resource of resources) {
198+
const resourcePath = resource.getPath();
160199
const sourceFile = ts.createSourceFile(
161-
resourcePath,
162-
resource.content,
200+
resource.getPath(),
201+
await resource.getString(),
163202
{
164203
languageVersion: ts.ScriptTarget.ES2022,
165204
jsDocParsingMode: ts.JSDocParsingMode.ParseNone,
@@ -176,7 +215,7 @@ export default async function ({
176215
const res: AutofixResult = new Map();
177216
for (const [resourcePath, sourceFile] of sourceFiles) {
178217
log.verbose(`Applying autofixes to ${resourcePath}`);
179-
const newContent = applyFixes(checker, sourceFile, resourcePath, resources.get(resourcePath)!);
218+
const newContent = applyFixes(checker, sourceFile, resourcePath, autofixResources.get(resourcePath)!);
180219
if (newContent) {
181220
const jsErrors = getJsErrors(newContent, resourcePath);
182221
if (jsErrors.length) {
@@ -199,7 +238,7 @@ function applyFixes(
199238
checker: ts.TypeChecker, sourceFile: ts.SourceFile, resourcePath: ResourcePath,
200239
resource: AutofixResource
201240
): string | undefined {
202-
const {content} = resource;
241+
const content = sourceFile.getFullText();
203242

204243
// Collect modules of which at least one message has the conditional fixHint flag set
205244
const conditionalModuleAccess = new Set<string>();

src/linter/lintWorkspace.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,8 @@ export default async function lintWorkspace(
4141
log.verbose(`Resource '${filePath}' not found. Skipping autofix for this file.`);
4242
continue;
4343
}
44-
const content = await resource.getString();
4544
autofixResources.set(filePath, {
46-
content,
45+
resource,
4746
messages: rawMessages,
4847
});
4948
}

test/lib/autofix/autofix.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import autofix, {AutofixResource, ChangeAction} from "../../../src/autofix/autof
55
import LinterContext from "../../../src/linter/LinterContext.js";
66
import {MESSAGE} from "../../../src/linter/messages.js";
77
import type {addDependencies} from "../../../src/autofix/solutions/amdImports.js";
8+
import {createResource} from "@ui5/fs/resourceFactory";
89

910
const test = anyTest as TestFn<{
1011
sinon: sinonGlobal.SinonSandbox;
@@ -48,7 +49,10 @@ test("autofix: Parsing error after applying fixes", async (t) => {
4849

4950
const resources = new Map<string, AutofixResource>();
5051
resources.set("/resources/file.js", {
51-
content: "sap.ui.define(() => new sap.m.Button())",
52+
resource: createResource({
53+
path: "/resources/file.js",
54+
string: "sap.ui.define(() => new sap.m.Button())",
55+
}),
5256
messages: [
5357
// Noter: Message details don't need to be correct in this test case
5458
// as we stub the addDependencies function

0 commit comments

Comments
 (0)