Skip to content

Commit

Permalink
feat: Add new output format markdown (#258)
Browse files Browse the repository at this point in the history
Added new `markdown` option for output as suggested in the issue.

Fixes #178

JIRA: CPOUI5FOUNDATION-890

---------

Co-authored-by: Konrad <konrad.kost@gmx.de>
Co-authored-by: Merlin Beutlberger <m.beutlberger@sap.com>
  • Loading branch information
3 people authored Aug 27, 2024
1 parent 9b4125b commit 7329058
Show file tree
Hide file tree
Showing 9 changed files with 356 additions and 12 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ ui5lint --details

#### `--format`

Choose the output format. Currently, `stylish` (default) and `json` are supported.
Choose the output format. Currently, `stylish` (default), `json` and `markdown` are supported.

**Example:**
```sh
Expand Down
7 changes: 6 additions & 1 deletion src/cli/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import path from "node:path";
import {lintProject} from "../linter/linter.js";
import {Text} from "../formatter/text.js";
import {Json} from "../formatter/json.js";
import {Markdown} from "../formatter/markdown.js";
import {Coverage} from "../formatter/coverage.js";
import {writeFile} from "node:fs/promises";
import baseMiddleware from "./middlewares/base.js";
Expand Down Expand Up @@ -71,7 +72,7 @@ const lintCommand: FixedCommandModule<object, LinterArg> = {
describe: "Set the output format for the linter result",
default: "stylish",
type: "string",
choices: ["stylish", "json"],
choices: ["stylish", "json", "markdown"],
})
.coerce([
// base.js
Expand Down Expand Up @@ -133,6 +134,10 @@ async function handleLint(argv: ArgumentsCamelCase<LinterArg>) {
const jsonFormatter = new Json();
process.stdout.write(jsonFormatter.format(res, details));
process.stdout.write("\n");
} else if (format === "markdown") {
const markdownFormatter = new Markdown();
process.stdout.write(markdownFormatter.format(res, details));
process.stdout.write("\n");
} else if (format === "" || format === "stylish") {
const textFormatter = new Text();
process.stderr.write(textFormatter.format(res, details));
Expand Down
118 changes: 118 additions & 0 deletions src/formatter/markdown.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
import {LintMessageSeverity, LintResult, LintMessage} from "../linter/LinterContext.js";

export class Markdown {
format(lintResults: LintResult[], showDetails: boolean): string {
let totalErrorCount = 0;
let totalWarningCount = 0;
let totalFatalErrorCount = 0;

// Sort by file path
lintResults.sort((a, b) => a.filePath.localeCompare(b.filePath));

let findings = "";
lintResults.forEach(({filePath, messages, errorCount, warningCount, fatalErrorCount}) => {
if (!errorCount && !warningCount) {
// Skip files without errors or warnings
return;
}
// Accumulate totals
totalErrorCount += errorCount;
totalWarningCount += warningCount;
totalFatalErrorCount += fatalErrorCount;

// Add the file path as a section header
findings += `### ${filePath}\n\n`;
if (showDetails === true) {
findings += `| Severity | Line | Message | Details |\n`;
findings += `|----------|------|---------|---------|\n`;
} else {
findings += `| Severity | Line | Message |\n`;
findings += `|----------|------|---------|\n`;
}

// Sort messages by severity (sorting order: fatal-errors, errors, warnings)
messages.sort((a, b) => {
// Handle fatal errors first to push them to the bottom
if (a.fatal !== b.fatal) {
return a.fatal ? -1 : 1; // Fatal errors go to the top
}
// Then, compare by severity
if (a.severity !== b.severity) {
return b.severity - a.severity;
}
// If severity is the same, compare by line number (handling nulls)
if ((a.line ?? 0) !== (b.line ?? 0)) {
return (a.line ?? 0) - (b.line ?? 0);
}
// If both severity and line number are the same, compare by column number (handling nulls)
return (a.column ?? 0) - (b.column ?? 0);
});

// Format each message
messages.forEach((msg) => {
const severity = this.formatSeverity(msg.severity, msg.fatal);
const location = this.formatLocation(msg.line, msg.column);
let details;
if (showDetails) {
details = ` ${this.formatMessageDetails(msg)} |`;
} else {
details = "";
}

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

findings += "\n";
});

let summary = "## Summary\n\n";
summary +=
`> ${totalErrorCount + totalWarningCount} problems ` +
`(${totalErrorCount} errors, ${totalWarningCount} warnings) \n`;
if (totalFatalErrorCount) {
summary += `> **${totalFatalErrorCount} fatal errors**\n`;
}

if (findings) {
findings = `## Findings\n${findings}`;
}

let output = `# UI5 linter Report
${summary}
${findings}`;

// Suggest using the details option if not all details are shown
if (!showDetails && (totalErrorCount + totalWarningCount) > 0) {
output += "**Note:** Use `ui5lint --details` to show more information about the findings.\n";
}
return output;
}

// Formats the severity of the lint message using appropriate emoji
private formatSeverity(severity: LintMessageSeverity, fatal: LintMessage["fatal"]): string {
if (fatal === true) {
return "Fatal Error";
} else if (severity === LintMessageSeverity.Warning) {
return "Warning";
} else if (severity === LintMessageSeverity.Error) {
return "Error";
} else {
throw new Error(`Unknown severity: ${LintMessageSeverity[severity]}`);
}
}

// Formats the location of the lint message (line and column numbers)
private formatLocation(line?: number, column?: number): string {
// Default to 0 if line or column are not provided
return `${line ?? 0}:${column ?? 0}`;
}

// Formats additional message details if `showDetails` is true
private formatMessageDetails(msg: LintMessage): string {
if (!msg.messageDetails) {
return "";
}
// Replace multiple spaces or newlines with a single space for clean output
return `${msg.messageDetails.replace(/\s\s+|\n/g, " ")}`;
}
}
13 changes: 3 additions & 10 deletions src/formatter/text.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,17 +56,10 @@ export class Text {

this.#writeln(chalk.inverse(path.resolve(process.cwd(), filePath)));

// Group messages by rule
const rules = new Map<string, LintMessage[]>();
let maxLine = 0; // Needed for formatting
let maxColumn = 0; // Needed for formatting
// Determine maximum line and column for position formatting
let maxLine = 0;
let maxColumn = 0;
messages.forEach((msg) => {
const entry = rules.get(msg.ruleId);
if (entry) {
entry.push(msg);
} else {
rules.set(msg.ruleId, [msg]);
}
if (msg.line && msg.line > maxLine) {
maxLine = msg.line;
}
Expand Down
22 changes: 22 additions & 0 deletions test/lib/cli/base.integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,25 @@ test.serial("ui5lint --format json", async (t) => {
const resultProcessStdoutNL = processStdoutWriteStub.secondCall.firstArg;
t.is(resultProcessStdoutNL, "\n", "second write only adds a single newline");
});

// New test for Markdown format
test.serial("ui5lint --format markdown", async (t) => {
const {cli, consoleLogStub, processCwdStub, processStdoutWriteStub, processExitStub} = t.context;

await cli.parseAsync(["--format", "markdown"]);

t.is(consoleLogStub.callCount, 0, "console.log should not be used");
t.true(processCwdStub.callCount > 0, "process.cwd was called");
t.is(processStdoutWriteStub.callCount, 2, "process.stdout.write was called twice");
t.is(processExitStub.callCount, 0, "process.exit got never called");
t.is(process.exitCode, 1, "process.exitCode was set to 1");
// cleanup: reset exit code in order not to fail the test (it cannot be stubbed with sinon)
process.exitCode = 0;

const resultProcessStdoutMarkdown = processStdoutWriteStub.firstCall.firstArg;
t.true(resultProcessStdoutMarkdown.startsWith("# UI5 linter Report"),
"Output starts with the expected Markdown header");

const resultProcessStdoutNL = processStdoutWriteStub.secondCall.firstArg;
t.is(resultProcessStdoutNL, "\n", "second write only adds a single newline");
});
16 changes: 16 additions & 0 deletions test/lib/cli/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const test = anyTest as TestFn<{
processErrWrite: SinonStub;
formatText: SinonStub;
formatJson: SinonStub;
formatMarkdown: SinonStub;
createExitStub: () => SinonStub;
cli: Argv;
base: typeof Base;
Expand Down Expand Up @@ -47,6 +48,7 @@ test.beforeEach(async (t) => {

t.context.formatText = sinon.stub().returns("");
t.context.formatJson = sinon.stub().returns("");
t.context.formatMarkdown = sinon.stub().returns("");

t.context.base = await esmock.p("../../../src/cli/base.js", {
"../../../src/linter/linter.js": {
Expand All @@ -67,6 +69,11 @@ test.beforeEach(async (t) => {
return {format: t.context.formatJson};
}),
},
"../../../src/formatter/markdown.js": {
Markdown: sinon.stub().callsFake(() => {
return {format: t.context.formatMarkdown};
}),
},
"node:fs/promises": {
writeFile: t.context.writeFile,
},
Expand Down Expand Up @@ -154,6 +161,15 @@ test.serial("ui5lint --format json ", async (t) => {
t.true(formatJson.calledOnce, "JSON formatter has been called");
});

test.serial("ui5lint --format markdown", async (t) => {
const {cli, lintProject, formatMarkdown} = t.context;

await cli.parseAsync(["--format", "markdown"]);

t.true(lintProject.calledOnce, "Linter is called");
t.true(formatMarkdown.calledOnce, "Markdown formatter has been called");
});

test.serial("Yargs error handling", async (t) => {
const {processStdErrWriteStub, consoleWriterStopStub, cli, createExitStub} = t.context;
const processExitStub = createExitStub();
Expand Down
113 changes: 113 additions & 0 deletions test/lib/formatter/markdown.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
import anyTest, {TestFn} from "ava";
import {Markdown} from "../../../src/formatter/markdown.js";
import {LintResult, LintMessageSeverity} from "../../../src/linter/LinterContext.js";

const test = anyTest as TestFn<{
lintResults: LintResult[];
}>;

test.beforeEach((t) => {
t.context.lintResults = [
{
filePath: "webapp/Component.js",
messages: [
{
ruleId: "rule1",
severity: LintMessageSeverity.Error,
line: 1,
column: 1,
message: "Error message",
messageDetails: "Message details",
},
{
ruleId: "rule2",
severity: LintMessageSeverity.Warning,
line: 2,
column: 2,
message: "Warning message",
messageDetails: "Message details",
},
],
coverageInfo: [],
errorCount: 1,
fatalErrorCount: 0,
warningCount: 1,
},
{
filePath: "webapp/Main.controller.js",
messages: [
{
ruleId: "rule3",
severity: LintMessageSeverity.Error,
line: 11,
column: 3,
message: "Another error message",
messageDetails: "Message details",
},
{
ruleId: "rule3",
severity: LintMessageSeverity.Error,
line: 12,
column: 3,
message: "Another error message",
messageDetails: "Message details",
fatal: true,
},
{
ruleId: "rule3",
severity: LintMessageSeverity.Error,
line: 3,
column: 6,
message: "Another error message",
messageDetails: "Message details",
fatal: true,
},
{
ruleId: "rule3",
severity: LintMessageSeverity.Warning,
line: 12,
column: 3,
message: "Another error message",
messageDetails: "Message details",
},
{
ruleId: "rule3",
severity: LintMessageSeverity.Error,
line: 11,
column: 2,
message: "Another error message",
messageDetails: "Message details",
},
],
coverageInfo: [],
errorCount: 4,
fatalErrorCount: 2,
warningCount: 1,
},
];
});

test("Default", (t) => {
const {lintResults} = t.context;

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

t.snapshot(markdownResult);
});

test("Details", (t) => {
const {lintResults} = t.context;

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

t.snapshot(markdownResult);
});

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

t.snapshot(markdownResult);
});
Loading

0 comments on commit 7329058

Please sign in to comment.