Skip to content

Commit

Permalink
perf: Only collect coverage info / details when requested
Browse files Browse the repository at this point in the history
  • Loading branch information
matz3 committed Mar 14, 2024
1 parent e01b1e5 commit b2fb10c
Show file tree
Hide file tree
Showing 9 changed files with 74 additions and 46 deletions.
8 changes: 7 additions & 1 deletion src/cli/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,12 @@ const lintCommand: FixedCommandModule<object, LinterArg> = {
.option("coverage", {
describe: "Whether to provide a coverage report",
type: "boolean",
default: false,
})
.option("details", {
describe: "Print complementary information for each finding, if available",
type: "boolean",
default: false,
})
.option("loglevel", {
alias: "log-level",
Expand Down Expand Up @@ -114,12 +116,16 @@ async function handleLint(argv: ArgumentsCamelCase<LinterArg>) {
await profile.start();
}

const reportCoverage = !!(process.env.UI5LINT_COVERAGE_REPORT ?? coverage);

const res = await lintProject({
rootDir: path.join(process.cwd()),
filePaths: filePaths?.map((filePath) => path.resolve(process.cwd(), filePath)),
reportCoverage,
messageDetails: details,
});

if (process.env.UI5LINT_COVERAGE_REPORT ?? coverage) {
if (reportCoverage) {
const coverageFormatter = new Coverage();
await writeFile("ui5lint-report.html", await coverageFormatter.format(res));
}
Expand Down
20 changes: 15 additions & 5 deletions src/detectors/typeChecker/FileLinter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {CoverageCategory, LintMessageSeverity, LintResult} from "../AbstractDete

interface DeprecationInfo {
symbol: ts.Symbol;
messageDetails: string;
messageDetails?: string;
}

export default class FileLinter {
Expand All @@ -13,16 +13,21 @@ export default class FileLinter {
#checker: ts.TypeChecker;
#reporter: Reporter;
#boundVisitNode: (node: ts.Node) => void;
#reportCoverage: boolean;
#messageDetails: boolean;

constructor(
rootDir: string, filePath: string, sourceFile: ts.SourceFile, sourceMap: string | undefined,
checker: ts.TypeChecker
checker: ts.TypeChecker, reportCoverage: boolean | undefined = false,
messageDetails: boolean | undefined = false
) {
this.#filePath = filePath;
this.#sourceFile = sourceFile;
this.#checker = checker;
this.#reporter = new Reporter(rootDir, filePath, sourceFile, sourceMap);
this.#boundVisitNode = this.visitNode.bind(this);
this.#reportCoverage = reportCoverage;
this.#messageDetails = messageDetails;
}

// eslint-disable-next-line @typescript-eslint/require-await
Expand Down Expand Up @@ -128,10 +133,13 @@ export default class FileLinter {
const jsdocTags = symbol.getJsDocTags(this.#checker);
const deprecatedTag = jsdocTags.find((tag) => tag.name === "deprecated");
if (deprecatedTag) {
return {
const deprecationInfo: DeprecationInfo = {
symbol,
messageDetails: this.getDeprecationText(deprecatedTag),
};
if (this.#messageDetails) {
deprecationInfo.messageDetails = this.getDeprecationText(deprecatedTag);
}
return deprecationInfo;
}
}
return null;
Expand All @@ -141,7 +149,9 @@ export default class FileLinter {
const exprNode = node.expression;
const exprType = this.#checker.getTypeAtLocation(exprNode);
if (!(exprType?.symbol && this.isSymbolOfUi5OrThirdPartyType(exprType.symbol))) {
this.handleCallExpressionUnknownType(exprType, node);
if (this.#reportCoverage) {
this.handleCallExpressionUnknownType(exprType, node);
}
return;
}

Expand Down
18 changes: 14 additions & 4 deletions src/detectors/typeChecker/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,11 @@ export class TsProjectDetector extends ProjectBasedDetector {
}));
}

async createReports(filePaths: string[]) {
async createReports(
filePaths: string[],
reportCoverage: boolean | undefined = false,
messageDetails: boolean | undefined = false
) {
const result: LintResult[] = [];
const reader = this.project.getReader({
style: "buildtime",
Expand Down Expand Up @@ -231,7 +235,9 @@ export class TsProjectDetector extends ProjectBasedDetector {
}
const linterDone = taskStart("Lint resource", filePath, true);
const linter = new FileLinter(
this.project.getRootPath(), filePath, sourceFile, sourceMaps.get(sourceFile.fileName), checker);
this.project.getRootPath(),
filePath, sourceFile, sourceMaps.get(sourceFile.fileName), checker, reportCoverage, messageDetails
);
const report = await linter.getReport();
if (resourceMessages.has(sourceFile.fileName)) {
report.messages.push(...resourceMessages.get(sourceFile.fileName)!);
Expand All @@ -251,7 +257,9 @@ export class TsProjectDetector extends ProjectBasedDetector {
}

export class TsFileDetector extends FileBasedDetector {
async createReports(filePaths: string[]) {
async createReports(
filePaths: string[], reportCoverage: boolean | undefined = false, messageDetails: boolean | undefined = false
) {
const options: ts.CompilerOptions = {
...DEFAULT_OPTIONS,
rootDir: this.rootDir,
Expand Down Expand Up @@ -311,7 +319,9 @@ export class TsFileDetector extends FileBasedDetector {
throw new Error(`Failed to get FS path for ${sourceFile.fileName}`);
}
const linter = new FileLinter(
this.rootDir, filePath, sourceFile, sourceMaps.get(sourceFile.fileName), checker);
this.rootDir, filePath, sourceFile,
sourceMaps.get(sourceFile.fileName), checker, reportCoverage, messageDetails
);
const report = await linter.getReport();
if (resourceMessages.has(sourceFile.fileName)) {
report.messages.push(...resourceMessages.get(sourceFile.fileName)!);
Expand Down
14 changes: 10 additions & 4 deletions src/linter/linter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import {ProjectGraph} from "@ui5/project";
interface LinterOptions {
rootDir: string;
filePaths: string[];
reportCoverage?: boolean;
messageDetails?: boolean;
}

async function fsStat(fsPath: string) {
Expand Down Expand Up @@ -85,19 +87,23 @@ async function getProjectGraph(rootDir: string): Promise<ProjectGraph> {
});
}

export async function lintProject({rootDir, filePaths}: LinterOptions): Promise<LintResult[]> {
export async function lintProject({
rootDir, filePaths, reportCoverage, messageDetails,
}: LinterOptions): Promise<LintResult[]> {
const lintEnd = taskStart("Linting Project");
const projectGraphDone = taskStart("Project Graph creation");
const graph = await getProjectGraph(rootDir);
const project = graph.getRoot();
projectGraphDone();
const tsDetector = new TsProjectDetector(project);
const res = await tsDetector.createReports(filePaths);
const res = await tsDetector.createReports(filePaths, reportCoverage, messageDetails);
lintEnd();
return res;
}

export async function lintFile({rootDir, filePaths}: LinterOptions): Promise<LintResult[]> {
export async function lintFile({
rootDir, filePaths, reportCoverage, messageDetails,
}: LinterOptions): Promise<LintResult[]> {
const tsDetector = new TsFileDetector(rootDir);
return await tsDetector.createReports(filePaths);
return await tsDetector.createReports(filePaths, reportCoverage, messageDetails);
}
15 changes: 12 additions & 3 deletions test/lib/cli/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,10 @@ test.serial("ui5lint (default) ", async (t) => {

t.true(lintProject.calledOnce, "Linter is called");
t.is(writeFile.callCount, 0, "Coverage was not called");
t.deepEqual(lintProject.getCall(0).args[0], {rootDir: path.join(process.cwd()), filePaths: undefined});
t.deepEqual(lintProject.getCall(0).args[0], {
rootDir: path.join(process.cwd()), filePaths: undefined,
messageDetails: false, reportCoverage: false,
});
t.is(t.context.consoleLogStub.callCount, 0, "console.log should not be used");
});

Expand All @@ -106,7 +109,10 @@ test.serial("ui5lint --file-paths ", async (t) => {
await cli.parseAsync(["--file-paths", filePaths[0], "--file-paths", filePaths[1]]);

t.true(lintProject.calledOnce, "Linter is called");
t.deepEqual(lintProject.getCall(0).args[0], {rootDir: path.join(process.cwd()), filePaths});
t.deepEqual(lintProject.getCall(0).args[0], {
rootDir: path.join(process.cwd()), filePaths,
messageDetails: false, reportCoverage: false,
});
t.is(t.context.consoleLogStub.callCount, 0, "console.log should not be used");
});

Expand All @@ -117,7 +123,10 @@ test.serial("ui5lint --coverage ", async (t) => {

t.true(lintProject.calledOnce, "Linter is called");
t.is(writeFile.callCount, 1, "Coverage was called");
t.deepEqual(lintProject.getCall(0).args[0], {rootDir: path.join(process.cwd()), filePaths: undefined});
t.deepEqual(lintProject.getCall(0).args[0], {
rootDir: path.join(process.cwd()), filePaths: undefined,
messageDetails: false, reportCoverage: true,
});
t.is(t.context.consoleLogStub.callCount, 0, "console.log should not be used");
});

Expand Down
10 changes: 8 additions & 2 deletions test/lib/linter/_linterHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,14 @@ export async function esmockDeprecationText() {
"../../../src/detectors/typeChecker/FileLinter.js":
function (
rootDir: string, filePath: string, sourceFile: SourceFile,
sourceMap: string | undefined, checker: TypeChecker
sourceMap: string | undefined, checker: TypeChecker,
reportCoverage: boolean | undefined = false,
messageDetails: boolean | undefined = false
) {
// Don't use sinon's stubs as it's hard to clean after them in this case and it leaks memory.
const linter = new FileLinter(rootDir, filePath, sourceFile, sourceMap, checker);
const linter = new FileLinter(
rootDir, filePath, sourceFile, sourceMap, checker, reportCoverage, messageDetails
);
linter.getDeprecationText = () => "Deprecated test message";
return linter;
},
Expand Down Expand Up @@ -85,6 +89,8 @@ export function createTestsForFixtures(fixturesPath: string) {
const res = await lintFile({
rootDir: fixturesPath,
filePaths,
reportCoverage: true,
messageDetails: true,
});
assertExpectedLintResults(t, res, fixturesPath, filePaths);
res.forEach((results: {filePath: string}) => {
Expand Down
6 changes: 5 additions & 1 deletion test/lib/linter/linter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ test.serial("lint: All files of com.ui5.troublesome.app", async (t) => {
let res = await lintProject({
rootDir: projectPath,
filePaths: [],
reportCoverage: true,
messageDetails: true,
});

res = res.sort((a: {filePath: string}, b: {filePath: string}) => {
Expand All @@ -44,7 +46,7 @@ test.serial("lint: All files of com.ui5.troublesome.app", async (t) => {
t.snapshot(res);
});

test.serial("lint: Some files of com.ui5.troublesome.app", async (t) => {
test.serial("lint: Some files of com.ui5.troublesome.app (without details / coverage)", async (t) => {
const projectPath = path.join(fixturesProjectsPath, "com.ui5.troublesome.app");
const filePaths = [
path.join("webapp", "controller", "BaseController.js"),
Expand Down Expand Up @@ -77,6 +79,8 @@ test.serial("lint: All files of library.with.custom.paths", async (t) => {
let res = await lintProject({
rootDir: projectPath,
filePaths: [],
reportCoverage: true,
messageDetails: true,
});

res = res.sort((a: {filePath: string}, b: {filePath: string}) => {
Expand Down
29 changes: 3 additions & 26 deletions test/lib/linter/snapshots/linter.ts.md
Original file line number Diff line number Diff line change
Expand Up @@ -732,7 +732,7 @@ Generated by [AVA](https://avajs.dev).
},
]

## lint: Some files of com.ui5.troublesome.app
## lint: Some files of com.ui5.troublesome.app (without details / coverage)

> Snapshot 1
Expand All @@ -746,14 +746,7 @@ Generated by [AVA](https://avajs.dev).
warningCount: 0,
},
{
coverageInfo: [
{
category: 1,
column: 33,
line: 8,
message: 'Unable to analyze this method call because the type of identifier "getContentDensityClass" in "this.getOwnerComponent().getContentDensityClass()"" could not be determined',
},
],
coverageInfo: [],
errorCount: 1,
fatalErrorCount: 0,
filePath: 'webapp/controller/App.controller.js',
Expand All @@ -763,28 +756,14 @@ Generated by [AVA](https://avajs.dev).
fatal: undefined,
line: 10,
message: 'Call to deprecated function \'attachTap\' of class \'Button\'',
messageDetails: 'Deprecated test message',
ruleId: 'ui5-linter-no-deprecated-api',
severity: 2,
},
],
warningCount: 0,
},
{
coverageInfo: [
{
category: 1,
column: 17,
line: 38,
message: 'Unable to analyze this method call because the type of identifier "getModel" in "this.getOwnerComponent().getModel("i18n")"" could not be determined',
},
{
category: 1,
column: 11,
line: 39,
message: 'Unable to analyze this method call because the type of identifier "getResourceBundle" in "oModel.getResourceBundle()"" could not be determined',
},
],
coverageInfo: [],
errorCount: 2,
fatalErrorCount: 0,
filePath: 'webapp/controller/BaseController.js',
Expand All @@ -794,7 +773,6 @@ Generated by [AVA](https://avajs.dev).
fatal: undefined,
line: 9,
message: 'Use of deprecated property \'blocked\' of class \'Button\'',
messageDetails: 'Deprecated test message',
ruleId: 'ui5-linter-no-deprecated-api',
severity: 2,
},
Expand All @@ -803,7 +781,6 @@ Generated by [AVA](https://avajs.dev).
fatal: undefined,
line: 11,
message: 'Call to deprecated function \'attachTap\' of class \'Button\'',
messageDetails: 'Deprecated test message',
ruleId: 'ui5-linter-no-deprecated-api',
severity: 2,
},
Expand Down
Binary file modified test/lib/linter/snapshots/linter.ts.snap
Binary file not shown.

0 comments on commit b2fb10c

Please sign in to comment.