Skip to content

Commit

Permalink
Add output file format options (#153)
Browse files Browse the repository at this point in the history
* Add new config: output format

* Add output formatter

* Add fingerprint

* code style update

* Update doc

* Add ref

* Rename log -> lintItem

* Add test yay

* Rename LogSeverity -> LintSeverity

* Fix naming for AndroidLintStyleParser

* fix styling

---------

Co-authored-by: pdujtipiya <[email protected]>
  • Loading branch information
encX and pdujtipiya authored Sep 19, 2023
1 parent ae8e86d commit c7c9102
Show file tree
Hide file tree
Showing 48 changed files with 365 additions and 228 deletions.
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

0 comments on commit c7c9102

Please sign in to comment.