Skip to content

Commit

Permalink
feat: Add new output format markdown
Browse files Browse the repository at this point in the history
  • Loading branch information
marianfoo committed Aug 13, 2024
1 parent fd12041 commit 1673fb3
Show file tree
Hide file tree
Showing 6 changed files with 268 additions and 2 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
98 changes: 98 additions & 0 deletions src/formatter/markdown.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import path from "node:path";
import {LintMessageSeverity, LintResult, LintMessage} from "../linter/LinterContext.js";

export class Markdown {
format(lintResults: LintResult[], showDetails: boolean): string {
let output = "# UI5 Linter Report\n\n";
let totalErrorCount = 0;
let totalWarningCount = 0;
let totalFatalErrorCount = 0;

// Sort files alphabetically
lintResults.sort((a, b) => a.filePath.localeCompare(b.filePath));

// Process each lint result
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
output += `## ${path.resolve(process.cwd(), filePath)}\n\n`;

// Group messages by rule for easier reading
const rules = new Map<string, LintMessage[]>();
messages.forEach((msg) => {
const entry = rules.get(msg.ruleId);
if (entry) {
entry.push(msg);
} else {
rules.set(msg.ruleId, [msg]);
}
});

// Sort messages by line, then by column (falling back to 0 if undefined)
messages.sort((a, b) => (a.line ?? 0) - (b.line ?? 0) || (a.column ?? 0) - (b.column ?? 0));

// Format each message
messages.forEach((msg) => {
const severity = this.formatSeverity(msg.severity);
const location = this.formatLocation(msg.line, msg.column);
const details = this.formatMessageDetails(msg, showDetails);

output += `- ${severity} ${location} ${msg.fatal ? "Fatal error: " : ""}${msg.message}${details}\n`;
});

output += "\n";
});

// Summary section
output += "## Summary\n\n";
output += `- Total problems: ${totalErrorCount + totalWarningCount}\n`;
output += ` - Errors: ${totalErrorCount}\n`;
output += ` - Warnings: ${totalWarningCount}\n`;

// Include fatal errors count if any
if (totalFatalErrorCount) {
output += ` - Fatal errors: ${totalFatalErrorCount}\n`;
}

// Suggest using the details option if not all details are shown
if (!showDetails && (totalErrorCount + totalWarningCount + totalFatalErrorCount) > 0) {
output += "\n**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): string {
if (severity === LintMessageSeverity.Error) {
return "🔴";
} else if (severity === LintMessageSeverity.Warning) {
return "🟡";
} 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, showDetails: boolean): string {
if (!showDetails || !msg.messageDetails) {
return "";
}
// Replace multiple spaces or newlines with a single space for clean output
return `\n - **Details:** ${msg.messageDetails.replace(/\s\s+|\n/g, " ")}`;
}
}
30 changes: 30 additions & 0 deletions test/lib/cli/base.integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,33 @@ 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");
t.true(resultProcessStdoutMarkdown.includes("## Summary"),
"Output includes a Summary section");
t.true(resultProcessStdoutMarkdown.includes("- Total problems:"),
"Output includes the total problem count");
t.true(resultProcessStdoutMarkdown.includes(" - Errors:"),
"Output includes the error count");
t.true(resultProcessStdoutMarkdown.includes(" - Warnings:"),
"Output includes the warning count");

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
117 changes: 117 additions & 0 deletions test/lib/formatter/markdown.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
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: "",
messages: [{
ruleId: "ui5-linter-no-deprecated-api",
severity: LintMessageSeverity.Error,
line: 5,
column: 1,
message: "Call to deprecated function 'attachInit' of class 'Core'",
messageDetails: "(since 1.118) - Please use {@link sap.ui.core.Core.ready Core.ready} instead.",
}],
coverageInfo: [],
errorCount: 1,
fatalErrorCount: 0,
warningCount: 0,
}];
});

test("Test Markdown Formatter (with '--details true')", (t) => {
const {lintResults} = t.context;
const markdownFormatter = new Markdown();
const markdownResult = markdownFormatter.format(lintResults, true);

t.true(markdownResult.includes("# UI5 Linter Report"),
"The Markdown output includes the main header");
t.true(markdownResult.includes("🔴 [5:1] Call to deprecated function 'attachInit' of class 'Core'"),
"The Markdown output includes the error message with correct formatting");
t.true(markdownResult.includes("## Summary"),
"The Markdown output includes a summary section");
t.true(markdownResult.includes("- Total problems: 1"),
"The Markdown output includes the total problem count");
t.true(markdownResult.includes(" - Errors: 1"),
"The Markdown output includes the error count");
t.true(markdownResult.includes(" - Warnings: 0"),
"The Markdown output includes the warning count");
});

test("Test Markdown Formatter (with '--details false')", (t) => {
const {lintResults} = t.context;
const markdownFormatter = new Markdown();
const markdownResult = markdownFormatter.format(lintResults, false);

t.true(markdownResult.includes("🔴 [5:1] Call to deprecated function 'attachInit' of class 'Core'"),
"The Markdown output includes the error message with correct formatting");
t.false(markdownResult.includes("**Details:**"),
"The Markdown output does not include the message details");
t.true(markdownResult.includes("**Note:** Use `ui5lint --details` to show more information about the findings."),
"The Markdown output includes a note about using --details");
});

test("Test Markdown Formatter with multiple files and message types", (t) => {
const lintResults: LintResult[] = [
{
filePath: "",
messages: [
{
ruleId: "rule1",
severity: LintMessageSeverity.Error,
line: 1,
column: 1,
message: "Error message",
},
{
ruleId: "rule2",
severity: LintMessageSeverity.Warning,
line: 2,
column: 2,
message: "Warning message",
},
],
coverageInfo: [],
errorCount: 1,
fatalErrorCount: 0,
warningCount: 1,
},
{
filePath: "",
messages: [
{
ruleId: "rule3",
severity: LintMessageSeverity.Error,
line: 3,
column: 3,
message: "Another error message",
},
],
coverageInfo: [],
errorCount: 1,
fatalErrorCount: 0,
warningCount: 0,
},
];

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

t.true(markdownResult.includes("🔴 [1:1] Error message"),
"The Markdown output includes the error message for file1");
t.true(markdownResult.includes("🟡 [2:2] Warning message"),
"The Markdown output includes the warning message for file1");
t.true(markdownResult.includes("🔴 [3:3] Another error message"),
"The Markdown output includes the error message for file2");
t.true(markdownResult.includes("- Total problems: 3"),
"The Markdown output includes the correct total problem count");
t.true(markdownResult.includes(" - Errors: 2"),
"The Markdown output includes the correct error count");
t.true(markdownResult.includes(" - Warnings: 1"),
"The Markdown output includes the correct warning count");
});

0 comments on commit 1673fb3

Please sign in to comment.