Skip to content

Commit

Permalink
feat: Add ruleId to Markdown format output
Browse files Browse the repository at this point in the history
The `ruleId` links to the corresponding rule documentation.
  • Loading branch information
maxreichmann authored and matz3 committed Dec 9, 2024
1 parent 0ae2b03 commit 913007c
Show file tree
Hide file tree
Showing 11 changed files with 82 additions and 58 deletions.
7 changes: 3 additions & 4 deletions src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import yargs from "yargs";
import {hideBin} from "yargs/helpers";
import base from "./cli/base.js";
import {fileURLToPath} from "node:url";
import {setVersion} from "./cli/version.js";
import {getFormattedVersion, setVersionInfo} from "./cli/version.js";
import {createRequire} from "module";

export default async function () {
Expand All @@ -17,10 +17,9 @@ export default async function () {
const require = createRequire(import.meta.url);
const pkg = require("../package.json") as {version: string};
const ui5LintJsPath = fileURLToPath(new URL("../bin/ui5lint.js", import.meta.url));
const pkgVersion = `${pkg.version} (from ${ui5LintJsPath})`;

setVersion(pkgVersion);
cli.version(pkgVersion);
setVersionInfo(pkg.version, ui5LintJsPath);
cli.version(getFormattedVersion());

// Explicitly set script name to prevent windows from displaying "ui5-linter.js"
cli.scriptName("ui5lint");
Expand Down
3 changes: 2 additions & 1 deletion src/cli/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import baseMiddleware from "./middlewares/base.js";
import chalk from "chalk";
import {isLogLevelEnabled} from "@ui5/logger";
import ConsoleWriter from "@ui5/logger/writers/Console";
import {getVersion} from "./version.js";

export interface LinterArg {
coverage: boolean;
Expand Down Expand Up @@ -172,7 +173,7 @@ async function handleLint(argv: ArgumentsCamelCase<LinterArg>) {
process.stdout.write("\n");
} else if (format === "markdown") {
const markdownFormatter = new Markdown();
process.stdout.write(markdownFormatter.format(res, details));
process.stdout.write(markdownFormatter.format(res, details, getVersion()));
process.stdout.write("\n");
} else if (format === "" || format === "stylish") {
const textFormatter = new Text(rootDir);
Expand Down
4 changes: 2 additions & 2 deletions src/cli/middlewares/logger.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {setLogLevel, isLogLevelEnabled, getLogger} from "@ui5/logger";
import ConsoleWriter from "@ui5/logger/writers/Console";
import {getVersion} from "../version.js";
import {getFormattedVersion} from "../version.js";
import type {ArgumentsCamelCase} from "yargs";
/**
* Logger middleware to enable logging capabilities
Expand Down Expand Up @@ -28,7 +28,7 @@ export async function initLogger(argv: ArgumentsCamelCase) {
ConsoleWriter.init();
if (isLogLevelEnabled("verbose")) {
const log = getLogger("cli:middlewares:base");
log.verbose(`using ui5lint version ${getVersion()}`);
log.verbose(`using ui5lint version ${getFormattedVersion()}`);
log.verbose(`using node version ${process.version}`);
}
}
7 changes: 6 additions & 1 deletion src/cli/version.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
let version: string;
let formattedVersion: string;

// This module holds the CLI's version information (set via cli.js) for later retrieval (e.g. from middlewares/logger)
export function setVersion(v: string) {
export function setVersionInfo(v: string, p: string) {
version = v;
formattedVersion = `${v} (from ${p})`;
}
export function getVersion(): string {
return version || "";
}
export function getFormattedVersion(): string {
return formattedVersion || "";
}
18 changes: 12 additions & 6 deletions src/formatter/markdown.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {LintResult, LintMessage} from "../linter/LinterContext.js";
import {LintMessageSeverity} from "../linter/messages.js";

export class Markdown {
format(lintResults: LintResult[], showDetails: boolean): string {
format(lintResults: LintResult[], showDetails: boolean, version: string): string {
let totalErrorCount = 0;
let totalWarningCount = 0;
let totalFatalErrorCount = 0;
Expand All @@ -21,11 +21,11 @@ export class Markdown {
// Add the file path as a section header
findings += `### ${filePath}\n\n`;
if (showDetails === true) {
findings += `| Severity | Line | Message | Details |\n`;
findings += `|----------|------|---------|---------|\n`;
findings += `| Severity | Rule | Location | Message | Details |\n`;
findings += `|----------|------|----------|---------|---------|\n`;
} else {
findings += `| Severity | Line | Message |\n`;
findings += `|----------|------|---------|\n`;
findings += `| Severity | Rule | Location | Message |\n`;
findings += `|----------|------|----------|---------|\n`;
}

// Sort messages by severity (sorting order: fatal-errors, errors, warnings)
Expand All @@ -50,14 +50,15 @@ export class Markdown {
messages.forEach((msg) => {
const severity = this.formatSeverity(msg.severity, msg.fatal);
const location = this.formatLocation(msg.line, msg.column);
const rule = this.formatRuleId(msg.ruleId, version);
let details;
if (showDetails) {
details = ` ${this.formatMessageDetails(msg)} |`;
} else {
details = "";
}

findings += `| ${severity} | \`${location}\` | ${msg.message} |${details}\n`;
findings += `| ${severity} | ${rule} | \`${location}\` | ${msg.message} |${details}\n`;
});

findings += "\n";
Expand Down Expand Up @@ -113,4 +114,9 @@ ${findings}`;
// Replace multiple spaces or newlines with a single space for clean output
return `${msg.messageDetails.replace(/\s\s+|\n/g, " ")}`;
}

// Formats the rule of the lint message (ruleId and link to rules.md)
private formatRuleId(ruleId: string, version: string): string {
return `[${ruleId}](https://github.com/SAP/ui5-linter/blob/v${version}/docs/Rules.md#${ruleId})`;
}
}
24 changes: 15 additions & 9 deletions test/lib/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ const test = anyTest as TestFn<{
argv: () => unknown;
};
yargs: SinonStub;
setVersion: SinonStub;
setVersionInfo: SinonStub;
getFormattedVersion: SinonStub;
cliBase: SinonStub;
readdir: SinonStub;
cli: MockFunction;
Expand All @@ -44,13 +45,15 @@ test.beforeEach(async (t) => {

t.context.yargs = sinon.stub().returns(t.context.yargsInstance).named("yargs");

t.context.setVersion = sinon.stub().named("setVersion");
t.context.setVersionInfo = sinon.stub().named("setVersionInfo");
t.context.getFormattedVersion = sinon.stub().returns("1.2.3 (from /path/to/cli.js)").named("getFormattedVersion");
t.context.cliBase = sinon.stub().named("cliBase");

t.context.cli = await esmock.p("../../src/cli.js", {
"yargs": t.context.yargs,
"../../src/cli/version.js": {
setVersion: t.context.setVersion,
setVersionInfo: t.context.setVersionInfo,
getFormattedVersion: t.context.getFormattedVersion,
},
"../../src/cli/base.js": t.context.cliBase,
"module": {
Expand All @@ -68,7 +71,7 @@ test.afterEach.always((t) => {
test.serial("CLI", async (t) => {
const {
cli, argvGetter, yargsInstance, yargs,
setVersion, cliBase,
setVersionInfo, cliBase, getFormattedVersion,
} = t.context;

await cli("module");
Expand All @@ -81,14 +84,17 @@ test.serial("CLI", async (t) => {
"parse-numbers": false,
}]);

t.is(setVersion.callCount, 1);
t.deepEqual(setVersion.getCall(0).args, [
`${pkg.version} (from ${fileURLToPath(new URL("../../bin/ui5lint.js", import.meta.url))})`,
t.is(setVersionInfo.callCount, 1);
t.deepEqual(setVersionInfo.getCall(0).args, [
pkg.version, fileURLToPath(new URL("../../bin/ui5lint.js", import.meta.url)),
]);

t.is(getFormattedVersion.callCount, 1);
t.deepEqual(getFormattedVersion.getCall(0).args, []);

t.is(yargsInstance.version.callCount, 1);
t.deepEqual(yargsInstance.version.getCall(0).args, [
`${pkg.version} (from ${fileURLToPath(new URL("../../bin/ui5lint.js", import.meta.url))})`,
getFormattedVersion.getCall(0).returnValue,
]);

t.is(yargsInstance.scriptName.callCount, 1);
Expand All @@ -111,7 +117,7 @@ test.serial("CLI", async (t) => {
sinon.assert.callOrder(
yargs,
yargsInstance.parserConfiguration,
setVersion,
setVersionInfo,
yargsInstance.version,
yargsInstance.scriptName,
cliBase,
Expand Down
10 changes: 5 additions & 5 deletions test/lib/cli/middlewares/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const test = anyTest as TestFn<{
verboseLogStub: SinonStub;
setLogLevelStub: SinonStub;
isLogLevelEnabledStub: SinonStub;
getVersionStub: SinonStub;
getFormattedVersionStub: SinonStub;
logger: MockFunction & {
initLogger: (args:
{loglevel?: string; verbose?: boolean; perf?: boolean; silent?: boolean}) => Promise<void> | void;
Expand All @@ -17,10 +17,10 @@ test.beforeEach(async (t) => {
t.context.verboseLogStub = sinon.stub();
t.context.setLogLevelStub = sinon.stub();
t.context.isLogLevelEnabledStub = sinon.stub().returns(true);
t.context.getVersionStub = sinon.stub().returns("1.0.0");
t.context.getFormattedVersionStub = sinon.stub().returns("1.0.0");
t.context.logger = await esmock("../../../../src/cli/middlewares/logger.js", {
"../../../../src/cli/version.js": {
getVersion: t.context.getVersionStub,
getFormattedVersion: t.context.getFormattedVersionStub,
},
"@ui5/logger": {
getLogger: () => ({
Expand All @@ -33,13 +33,13 @@ test.beforeEach(async (t) => {
});

test.serial("init logger", async (t) => {
const {logger, setLogLevelStub, isLogLevelEnabledStub, verboseLogStub, getVersionStub} = t.context;
const {logger, setLogLevelStub, isLogLevelEnabledStub, verboseLogStub, getFormattedVersionStub} = t.context;
await logger.initLogger({});
t.is(setLogLevelStub.callCount, 0, "setLevel has not been called");
t.is(isLogLevelEnabledStub.callCount, 1, "isLogLevelEnabled has been called once");
t.is(isLogLevelEnabledStub.firstCall.firstArg, "verbose",
"isLogLevelEnabled has been called with expected argument");
t.is(getVersionStub.callCount, 1, "getVersion has been called once");
t.is(getFormattedVersionStub.callCount, 1, "getFormattedVersion has been called once");
t.is(verboseLogStub.callCount, 2, "log.verbose has been called twice");
t.is(verboseLogStub.firstCall.firstArg, "using ui5lint version 1.0.0",
"log.verbose has been called with expected argument on first call");
Expand Down
17 changes: 12 additions & 5 deletions test/lib/cli/version.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
import test from "ava";
import {setVersion, getVersion} from "../../../src/cli/version.js";
import {setVersionInfo, getFormattedVersion, getVersion} from "../../../src/cli/version.js";

test("Set and get version", (t) => {
const sampleVersion = "1.2.3";
const sampleVersion2 = "4.5.6-foo.bar";
const samplePath = "/path/to/cli.js";

t.is(getFormattedVersion(), "");
t.is(getVersion(), "");

setVersion("1.2.3");
t.is(getVersion(), "1.2.3");
setVersionInfo(sampleVersion, samplePath);
t.is(getFormattedVersion(), `${sampleVersion} (from ${samplePath})`);
t.is(getVersion(), sampleVersion);

setVersion("4.5.6-foo.bar");
t.is(getVersion(), "4.5.6-foo.bar");
setVersionInfo(sampleVersion2, samplePath);
t.is(getFormattedVersion(), `${sampleVersion2} (from ${samplePath})`);
t.is(getVersion(), sampleVersion2);
});
6 changes: 3 additions & 3 deletions test/lib/formatter/markdown.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ test("Default", (t) => {
const {lintResults} = t.context;

const markdownFormatter = new Markdown();
const markdownResult = markdownFormatter.format(lintResults, false);
const markdownResult = markdownFormatter.format(lintResults, false, "1.2.3");

t.snapshot(markdownResult);
});
Expand All @@ -101,14 +101,14 @@ test("Details", (t) => {
const {lintResults} = t.context;

const markdownFormatter = new Markdown();
const markdownResult = markdownFormatter.format(lintResults, true);
const markdownResult = markdownFormatter.format(lintResults, true, "1.2.3");

t.snapshot(markdownResult);
});

test("No findings", (t) => {
const markdownFormatter = new Markdown();
const markdownResult = markdownFormatter.format([], true);
const markdownResult = markdownFormatter.format([], true, "1.2.3");

t.snapshot(markdownResult);
});
44 changes: 22 additions & 22 deletions test/lib/formatter/snapshots/markdown.ts.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,20 @@ Generated by [AVA](https://avajs.dev).
## Findings␊
### webapp/Component.js␊
| Severity | Line | Message |␊
|----------|------|---------|␊
| Error | \`1:1\` | Error message |␊
| Warning | \`2:2\` | Warning message |␊
| Severity | Rule | Location | Message |␊
|----------|------|----------|---------|␊
| Error | [rule1](https://github.com/SAP/ui5-linter/blob/v1.2.3/docs/Rules.md#rule1) | \`1:1\` | Error message |␊
| Warning | [rule2](https://github.com/SAP/ui5-linter/blob/v1.2.3/docs/Rules.md#rule2) | \`2:2\` | Warning message |␊
### webapp/Main.controller.js␊
| Severity | Line | Message |␊
|----------|------|---------|␊
| Fatal Error | \`3:6\` | Another error message |␊
| Fatal Error | \`12:3\` | Another error message |␊
| Error | \`11:2\` | Another error message |␊
| Error | \`11:3\` | Another error message |␊
| Warning | \`12:3\` | Another error message |␊
| Severity | Rule | Location | Message |␊
|----------|------|----------|---------|␊
| Fatal Error | [rule3](https://github.com/SAP/ui5-linter/blob/v1.2.3/docs/Rules.md#rule3) | \`3:6\` | Another error message |␊
| Fatal Error | [rule3](https://github.com/SAP/ui5-linter/blob/v1.2.3/docs/Rules.md#rule3) | \`12:3\` | Another error message |␊
| Error | [rule3](https://github.com/SAP/ui5-linter/blob/v1.2.3/docs/Rules.md#rule3) | \`11:2\` | Another error message |␊
| Error | [rule3](https://github.com/SAP/ui5-linter/blob/v1.2.3/docs/Rules.md#rule3) | \`11:3\` | Another error message |␊
| Warning | [rule3](https://github.com/SAP/ui5-linter/blob/v1.2.3/docs/Rules.md#rule3) | \`12:3\` | Another error message |␊
**Note:** Use \`ui5lint --details\` to show more information about the findings.␊
`
Expand All @@ -48,20 +48,20 @@ Generated by [AVA](https://avajs.dev).
## Findings␊
### webapp/Component.js␊
| Severity | Line | Message | Details |␊
|----------|------|---------|---------|␊
| Error | \`1:1\` | Error message | Message details |␊
| Warning | \`2:2\` | Warning message | Message details |␊
| Severity | Rule | Location | Message | Details |␊
|----------|------|----------|---------|---------|␊
| Error | [rule1](https://github.com/SAP/ui5-linter/blob/v1.2.3/docs/Rules.md#rule1) | \`1:1\` | Error message | Message details |␊
| Warning | [rule2](https://github.com/SAP/ui5-linter/blob/v1.2.3/docs/Rules.md#rule2) | \`2:2\` | Warning message | Message details |␊
### webapp/Main.controller.js␊
| Severity | Line | Message | Details |␊
|----------|------|---------|---------|␊
| Fatal Error | \`3:6\` | Another error message | Message details |␊
| Fatal Error | \`12:3\` | Another error message | Message details |␊
| Error | \`11:2\` | Another error message | Message details |␊
| Error | \`11:3\` | Another error message | Message details |␊
| Warning | \`12:3\` | Another error message | Message details |␊
| Severity | Rule | Location | Message | Details |␊
|----------|------|----------|---------|---------|␊
| Fatal Error | [rule3](https://github.com/SAP/ui5-linter/blob/v1.2.3/docs/Rules.md#rule3) | \`3:6\` | Another error message | Message details |␊
| Fatal Error | [rule3](https://github.com/SAP/ui5-linter/blob/v1.2.3/docs/Rules.md#rule3) | \`12:3\` | Another error message | Message details |␊
| Error | [rule3](https://github.com/SAP/ui5-linter/blob/v1.2.3/docs/Rules.md#rule3) | \`11:2\` | Another error message | Message details |␊
| Error | [rule3](https://github.com/SAP/ui5-linter/blob/v1.2.3/docs/Rules.md#rule3) | \`11:3\` | Another error message | Message details |␊
| Warning | [rule3](https://github.com/SAP/ui5-linter/blob/v1.2.3/docs/Rules.md#rule3) | \`12:3\` | Another error message | Message details |␊
`

Expand Down
Binary file modified test/lib/formatter/snapshots/markdown.ts.snap
Binary file not shown.

0 comments on commit 913007c

Please sign in to comment.