Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add output file format options #153

Merged
merged 11 commits into from
Sep 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ Will do the same thing.
| `failOnWarnings` | no | `true` or `false` | Fail the job when warnings are found |
| `dryRun` | no | `true` or `false` | Running CodeCoach without reporting to VCS |
| `suppressRules` | no | `rule-group-1/.*` `rule-id-1` `rule-id-2` | Rule IDs that CodeCoach will still comment but no longer treated as errors or warnings |
| `outputFormat` | no | `default`, `gitlab` | Output file format |


##### `--buildLogFile` or `-f`
Expand Down
2 changes: 1 addition & 1 deletion sample/eslint/eslint-output.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
"warningCount": 1,
"fixableErrorCount": 0,
"fixableWarningCount": 0,
"source": "#!/usr/bin/env node\r\n\r\nimport { Config, ProjectType } from './Config';\r\nimport { File } from './File';\r\nimport { Log } from './Logger';\r\nimport { CSharpParser, LogType, Parser, TSLintParser } from './Parser';\r\nimport { GitHub, GitHubPRService, VCS } from './Provider';\r\n\r\nclass App {\r\n private readonly parser: Parser;\r\n private readonly vcs: VCS\r\n\r\n constructor() {\r\n this.parser = App.setProjectType(Config.app.projectType);\r\n const githubPRService = new GitHubPRService(\r\n Config.provider.token,\r\n Config.provider.repoUrl,\r\n Config.provider.prId,\r\n );\r\n this.vcs = new GitHub(githubPRService);\r\n }\r\n\r\n async start() {\r\n const logs = await this.parseBuildData(Config.app.buildLogFiles);\r\n Log.info('Build data parsing completed');\r\n\r\n await this.vcs.report(logs);\r\n Log.info('Report to VCS completed');\r\n\r\n await App.writeLogToFile(logs);\r\n Log.info('Write output completed');\r\n }\r\n\r\n private static setProjectType(type: ProjectType): Parser {\r\n switch (type) {\r\n case ProjectType.csharp:\r\n return new CSharpParser(Config.app.cwd);\r\n case ProjectType.tslint:\r\n return new TSLintParser(Config.app.cwd);\r\n }\r\n }\r\n\r\n private async parseBuildData(files: string[]): Promise<LogType[]> {\r\n const parserTasks = files.map(async (file) => {\r\n const content = await File.readFileHelper(file);\r\n this.parser.withContent(content);\r\n });\r\n\r\n await Promise.all(parserTasks);\r\n\r\n return this.parser.getLogs();\r\n }\r\n\r\n private static async writeLogToFile(logs: LogType[]): Promise<void> {\r\n await File.writeFileHelper(Config.app.logFilePath, JSON.stringify(logs, null, 2));\r\n }\r\n}\r\n\r\nnew App(",
"source": "#!/usr/bin/env node\r\n\r\nimport { Config, ProjectType } from './Config';\r\nimport { File } from './File';\r\nimport { Log } from './Logger';\r\nimport { CSharpParser, LintItem, Parser, TSLintParser } from './Parser';\r\nimport { GitHub, GitHubPRService, VCS } from './Provider';\r\n\r\nclass App {\r\n private readonly parser: Parser;\r\n private readonly vcs: VCS\r\n\r\n constructor() {\r\n this.parser = App.setProjectType(Config.app.projectType);\r\n const githubPRService = new GitHubPRService(\r\n Config.provider.token,\r\n Config.provider.repoUrl,\r\n Config.provider.prId,\r\n );\r\n this.vcs = new GitHub(githubPRService);\r\n }\r\n\r\n async start() {\r\n const logs = await this.parseBuildData(Config.app.buildLogFiles);\r\n Log.info('Build data parsing completed');\r\n\r\n await this.vcs.report(logs);\r\n Log.info('Report to VCS completed');\r\n\r\n await App.writeLogToFile(logs);\r\n Log.info('Write output completed');\r\n }\r\n\r\n private static setProjectType(type: ProjectType): Parser {\r\n switch (type) {\r\n case ProjectType.csharp:\r\n return new CSharpParser(Config.app.cwd);\r\n case ProjectType.tslint:\r\n return new TSLintParser(Config.app.cwd);\r\n }\r\n }\r\n\r\n private async parseBuildData(files: string[]): Promise<LintItem[]> {\r\n const parserTasks = files.map(async (file) => {\r\n const content = await File.readFileHelper(file);\r\n this.parser.withContent(content);\r\n });\r\n\r\n await Promise.all(parserTasks);\r\n\r\n return this.parser.getLogs();\r\n }\r\n\r\n private static async writeLogToFile(items: LintItem[]): Promise<void> {\r\n await File.writeFileHelper(Config.app.logFilePath, JSON.stringify(logs, null, 2));\r\n }\r\n}\r\n\r\nnew App(",
"usedDeprecatedRules": []
}
]
4 changes: 2 additions & 2 deletions sample/git-diff/deletesection.diff
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@

Log.debug(`Commit SHA ${commitId}`);
Log.debug('Touched files', touchedFiles);
- Log.debug('Touched file log', touchedFileLog);
- Log.debug('Touched file log', touchedFileItem);

const reviewResults = await Promise.all(
touchedFileLog.map((log) => this.toCreateReviewComment(commitId, log)),
touchedFileItem.map((log) => this.toCreateReviewComment(commitId, log)),
@@ -83,10 +82,12 @@ ${issuesTableContent}
log.source,
log.line,
Expand Down
2 changes: 1 addition & 1 deletion sample/git-diff/multi.diff
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import { getRelativePath } from '../Provider/utils/path.util';
-import { LogSeverity } from './@enums/log.severity.enum';
import { Parser } from './@interfaces/parser.interface';
import { LogType } from './@types';
import { LintItem } from './@types';
+import { mapSeverity } from './utils/dotnetSeverityMap';
import { splitByLine } from './utils/lineBreak.util';

Expand Down
6 changes: 3 additions & 3 deletions src/AnalyzerBot/@interfaces/IAnalyzerBot.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import { LogType } from '../../Parser';
import { LintItem } from '../../Parser';
import { Comment } from '../@types/CommentTypes';
import { Diff } from '../../Git/@types/PatchTypes';

export interface IAnalyzerBot {
touchedFileLog: LogType[];
touchedFileItem: LintItem[];
comments: Comment[];
nError: number;
nWarning: number;

analyze(logs: LogType[], touchedDiff: Diff[]): void;
analyze(items: LintItem[], touchedDiff: Diff[]): void;

shouldGenerateOverview(): boolean;

Expand Down
4 changes: 2 additions & 2 deletions src/AnalyzerBot/AnalyzerBot.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ describe('AnalyzerBot', () => {
const diff = [mockTouchDiff];
const analyzer = new AnalyzerBot(config);

describe('.touchedFileLog', () => {
describe('.touchedFileItem', () => {
it('should return only logs that are in touchedDiff', () => {
analyzer.analyze(logs, diff);
expect(analyzer.touchedFileLog).toEqual([touchFileError, touchFileWarning]);
expect(analyzer.touchedFileItem).toEqual([touchFileError, touchFileWarning]);
});
});

Expand Down
12 changes: 6 additions & 6 deletions src/AnalyzerBot/AnalyzerBot.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { LogSeverity, LogType } from '../Parser';
import { LintSeverity, LintItem } from '../Parser';
import { Diff } from '../Git/@types/PatchTypes';
import { onlyIn, onlySeverity } from './utils/filter.util';
import { groupComments } from './utils/commentUtil';
Expand All @@ -8,18 +8,18 @@ import { Comment } from './@types/CommentTypes';
import { IAnalyzerBot } from './@interfaces/IAnalyzerBot';

export class AnalyzerBot implements IAnalyzerBot {
touchedFileLog: LogType[];
touchedFileItem: LintItem[];
comments: Comment[];
nError: number;
nWarning: number;

constructor(private readonly config: AnalyzerBotConfig) {}

analyze(logs: LogType[], touchedDiff: Diff[]): void {
this.touchedFileLog = logs
.filter(onlySeverity(LogSeverity.error, LogSeverity.warning))
analyze(items: LintItem[], touchedDiff: Diff[]): void {
this.touchedFileItem = items
.filter(onlySeverity(LintSeverity.error, LintSeverity.warning))
.filter(onlyIn(touchedDiff));
this.comments = groupComments(this.touchedFileLog, this.config.suppressRules);
this.comments = groupComments(this.touchedFileItem, this.config.suppressRules);
this.nError = this.comments.reduce((sum, comment) => sum + comment.errors, 0);
this.nWarning = this.comments.reduce((sum, comment) => sum + comment.warnings, 0);
}
Expand Down
22 changes: 11 additions & 11 deletions src/AnalyzerBot/utils/commentUtil.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { LogSeverity, LogType } from '../../Parser';
import { LintSeverity, LintItem } from '../../Parser';
import {
file1TouchLine,
file2TouchLine,
Expand All @@ -9,10 +9,10 @@ import {
import { groupComments } from './commentUtil';

describe('groupComments', () => {
const logs: LogType[] = [touchFileError, touchFileWarning];
const items: LintItem[] = [touchFileError, touchFileWarning];

it('returns comments based on lint logs', () => {
const comments = groupComments(logs, []);
it('returns comments based on lint items', () => {
const comments = groupComments(items, []);
expect(comments).toEqual([
{
file: mockTouchFile,
Expand All @@ -35,14 +35,14 @@ describe('groupComments', () => {
]);
});

it('group multiple logs on the same line to the same comment', () => {
it('group multiple items on the same line to the same comment', () => {
const comments = groupComments(
[
...logs,
...items,
{
...touchFileError,
msg: 'additional warning',
severity: LogSeverity.warning,
severity: LintSeverity.warning,
lineOffset: 33,
},
],
Expand Down Expand Up @@ -74,11 +74,11 @@ describe('groupComments', () => {
it('suppress errors and warnings according to provided suppressRules', () => {
const comments = groupComments(
[
...logs,
...items,
{
...touchFileError,
msg: 'additional warning',
severity: LogSeverity.warning,
severity: LintSeverity.warning,
lineOffset: 33,
ruleId: 'UNIMPORTANT_RULE2',
},
Expand Down Expand Up @@ -115,11 +115,11 @@ describe('groupComments', () => {
it('support regexp in suppressRules', () => {
const comments = groupComments(
[
...logs,
...items,
{
...touchFileError,
msg: 'additional warning',
severity: LogSeverity.warning,
severity: LintSeverity.warning,
lineOffset: 33,
ruleId: 'UNIMPORTANT_RULE/RULE2',
},
Expand Down
47 changes: 27 additions & 20 deletions src/AnalyzerBot/utils/commentUtil.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
import { LogSeverity, LogType } from '../../Parser';
import { LintSeverity, LintItem } from '../../Parser';
import { Comment, CommentStructure } from '../@types/CommentTypes';
import { MessageUtil } from './message.util';

export function groupComments(logs: LogType[], suppressRules: Array<string>): Comment[] {
const commentMap = logs.reduce((state: CommentStructure, log) => {
const { source: file, line, nLines } = log;
export function groupComments(
items: LintItem[],
suppressRules: Array<string>,
): Comment[] {
const commentMap = items.reduce((state: CommentStructure, item) => {
const { source: file, line, nLines } = item;

if (!line) return state;

const currentComment = getOrInitComment(state, file, line, nLines);
const updatedComment = updateComment(currentComment, log, suppressRules);
const updatedComment = updateComment(currentComment, item, suppressRules);
return updateCommentStructure(state, updatedComment);
}, {});

Expand All @@ -35,8 +38,12 @@ function getOrInitComment(
);
}

function buildText(currentComment: Comment, log: LogType, isSuppressed: boolean): string {
const { severity, msg } = log;
function buildText(
currentComment: Comment,
item: LintItem,
isSuppressed: boolean,
): string {
const { severity, msg } = item;
const { text: currentText } = currentComment;
const msgWithSuppression = isSuppressed ? `(SUPPRESSED) ${msg}` : msg;
const text = MessageUtil.createMessageWithEmoji(msgWithSuppression, severity);
Expand All @@ -45,43 +52,43 @@ function buildText(currentComment: Comment, log: LogType, isSuppressed: boolean)

function calculateErrors(
currentComment: Comment,
log: LogType,
item: LintItem,
isSuppressed: boolean,
): number {
if (isSuppressed) return currentComment.errors;
const { severity } = log;
return currentComment.errors + (severity === LogSeverity.error ? 1 : 0);
const { severity } = item;
return currentComment.errors + (severity === LintSeverity.error ? 1 : 0);
}

function calculateWarnings(
currentComment: Comment,
log: LogType,
item: LintItem,
isSuppressed: boolean,
): number {
if (isSuppressed) return currentComment.warnings;
const { severity } = log;
return currentComment.warnings + (severity === LogSeverity.warning ? 1 : 0);
const { severity } = item;
return currentComment.warnings + (severity === LintSeverity.warning ? 1 : 0);
}

function calculateSuppresses(currentComment: Comment, isSuppressed: boolean): number {
return currentComment.suppresses + (isSuppressed ? 1 : 0);
}

function shouldBeSuppressed(log: LogType, suppressRules: Array<string>): boolean {
function shouldBeSuppressed(item: LintItem, suppressRules: Array<string>): boolean {
const suppressRegexps: Array<RegExp> = suppressRules.map((rule) => new RegExp(rule));
return suppressRegexps.some((regexp) => regexp.test(log.ruleId));
return suppressRegexps.some((regexp) => regexp.test(item.ruleId));
}

function updateComment(
currentComment: Comment,
log: LogType,
item: LintItem,
suppressRules: Array<string>,
): Comment {
const isSuppressed = shouldBeSuppressed(log, suppressRules);
const isSuppressed = shouldBeSuppressed(item, suppressRules);
return {
text: buildText(currentComment, log, isSuppressed),
errors: calculateErrors(currentComment, log, isSuppressed),
warnings: calculateWarnings(currentComment, log, isSuppressed),
text: buildText(currentComment, item, isSuppressed),
errors: calculateErrors(currentComment, item, isSuppressed),
warnings: calculateWarnings(currentComment, item, isSuppressed),
suppresses: calculateSuppresses(currentComment, isSuppressed),
file: currentComment.file,
line: currentComment.line,
Expand Down
13 changes: 7 additions & 6 deletions src/AnalyzerBot/utils/filter.util.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { LogSeverity, LogType } from '../../Parser';
import { LintSeverity, LintItem } from '../../Parser';
import { Diff } from '../../Git/@types/PatchTypes';

export const onlyIn = (diffs: Diff[]) => (log: LogType): boolean =>
export const onlyIn = (diffs: Diff[]) => (item: LintItem): boolean =>
diffs.some(
(d) =>
d.file === log.source &&
d.patch.some((p) => !log.line || (log.line >= p.from && log.line <= p.to)),
d.file === item.source &&
d.patch.some((p) => !item.line || (item.line >= p.from && item.line <= p.to)),
);
export const onlySeverity = (...severities: LogSeverity[]) => (log: LogType): boolean =>
severities.includes(log.severity);
export const onlySeverity = (...severities: LintSeverity[]) => (
item: LintItem,
): boolean => severities.includes(item.severity);
6 changes: 3 additions & 3 deletions src/AnalyzerBot/utils/message.util.spec.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import { LogSeverity } from '../../Parser';
import { LintSeverity } from '../../Parser';
import { MessageUtil } from './message.util';

describe('createMessageWithEmoji', () => {
it('should parsed log message to Emoji correctly', () => {
// ¯\_(ツ)_/¯
const msg = 'test';

expect(MessageUtil.createMessageWithEmoji(msg, LogSeverity.error)).toBe(
expect(MessageUtil.createMessageWithEmoji(msg, LintSeverity.error)).toBe(
`:rotating_light: ${msg}`,
);
expect(MessageUtil.createMessageWithEmoji(msg, LogSeverity.warning)).toBe(
expect(MessageUtil.createMessageWithEmoji(msg, LintSeverity.warning)).toBe(
`:warning: ${msg}`,
);
});
Expand Down
12 changes: 6 additions & 6 deletions src/AnalyzerBot/utils/message.util.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
import { LogSeverity } from '../../Parser';
import { LintSeverity } from '../../Parser';

const EMOJI_ERROR = ':rotating_light:';
const EMOJI_WARNING = ':warning:';

export class MessageUtil {
static createMessageWithEmoji(msg: string, severity: LogSeverity): string {
static createMessageWithEmoji(msg: string, severity: LintSeverity): string {
let emoji = '';
switch (severity) {
case LogSeverity.error:
case LintSeverity.error:
emoji = EMOJI_ERROR;
break;
case LogSeverity.warning:
case LintSeverity.warning:
emoji = EMOJI_WARNING;
break;
}
Expand All @@ -24,11 +24,11 @@ export class MessageUtil {
const issueCountMsg = this.pluralize('issue', nOfErrors + nOfWarnings);
const errorMsg = MessageUtil.createMessageWithEmoji(
MessageUtil.pluralize('error', nOfErrors),
LogSeverity.error,
LintSeverity.error,
);
const warningMsg = MessageUtil.createMessageWithEmoji(
MessageUtil.pluralize('warning', nOfWarnings),
LogSeverity.warning,
LintSeverity.warning,
);

return `## CodeCoach reports ${issueCountMsg}
Expand Down
4 changes: 4 additions & 0 deletions src/Config/@enums/outputFormat.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export enum OutputFormat {
default = 'default',
gitlab = 'gitlab',
}
1 change: 1 addition & 0 deletions src/Config/@types/configArgument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export type ConfigArgument = {
gitlabToken: string;
buildLogFile: BuildLogFile[];
output: string; // =logFilePath
outputFormat: 'default' | 'gitlab';
removeOldComment: boolean;
failOnWarnings: boolean;
suppressRules: string[];
Expand Down
6 changes: 3 additions & 3 deletions src/Config/Config.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ const mockGitLabProjectId = 1234;
const mockGitLabMrIid = 69;
const mockGitLabToken = 'mockGitLabToken';

const mockLogType = 'dotnetbuild';
const mockLintItem = 'dotnetbuild';
const mockLogFile = './sample/dotnetbuild/build.content';
const mockLogCwd = '/repo/src';
const mockBuildLogFile = `${mockLogType};${mockLogFile};${mockLogCwd}`;
const mockBuildLogFile = `${mockLintItem};${mockLogFile};${mockLogCwd}`;
const mockOutput = './tmp/out.json';

const GITHUB_ENV_ARGS = [
Expand Down Expand Up @@ -64,7 +64,7 @@ describe('Config parsing Test', () => {

const validateBuildLog = (buildLog: BuildLogFile[]) => {
expect(buildLog).toHaveLength(1);
expect(buildLog[0].type).toBe(mockLogType);
expect(buildLog[0].type).toBe(mockLintItem);
expect(buildLog[0].path).toBe(mockLogFile);
expect(buildLog[0].cwd).toBe(mockLogCwd);
};
Expand Down
5 changes: 5 additions & 0 deletions src/Config/Config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ and <cwd> is build root directory (optional (Will use current context as cwd)).
type: 'string',
default: DEFAULT_OUTPUT_FILE,
})
.option('outputFormat', {
describe: 'Output format',
choices: ['default', 'gitlab'],
default: 'default',
})
.option('removeOldComment', {
alias: 'r',
type: 'boolean',
Expand Down
Loading