Skip to content

Commit

Permalink
Support Regexp in rule suppression (#151)
Browse files Browse the repository at this point in the history 
* Create common VCS engine

* Add suppressPattern config, pass config object to gitlab and github directly

* Create VCSEngine config interface to avoid accessing unrelated configs from VCSEngine

* Move bot logic into AnalyzerBot

* Move files around and clean up complex methods

* Remove unused imports

* Change inheritance into composition

* Remove dedicate GitHub and GitLab engine and use VCSEngine directly with different adapter

* Remove unrelated changes from refactoring to make it less confusing

* Add test for AnalyzerBot

* Make analyzer bot injectable to simplify testing

* Create testcases for VCSEngine

* Add suppressRules option

* Add unit test for commentUtil

* Make groupComments function easier to understand

* Support rule suppression in commentUtil

* Fix errors because of additional field

* Pass through suppressRules config from AnalyzerBot

* Update readme

* Support regexp in rule suppression

* Fix misleading regexp

* Make suppressRules non-nullable
  • Loading branch information
@icmpecho
icmpecho authored Jul 27, 2023
1 parent 0894d4a commit 23909bc
Show file tree
Hide file tree
Showing 7 changed files with 73 additions and 30 deletions.
38 changes: 19 additions & 19 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,25 +81,25 @@ Will do the same thing.

> Warning: If both config file and options are supplied, options will override config in file.
| Option | Required | Value | Description |
|---------------------------|---------------------------|------------------------------|----------------------------------------------------------------------------------------|
| `vcs` / `-g` | when `dryRun` is `false` | `github` or `gitlab` | |
| | | | |
| `githubRepoUrl` | when `vcs` is `github` | | Repository's HTTPS URL |
| `githubPr` | when `vcs` is `github` | | Pull request ID |
| `githubToken` | when `vcs` is `github` | | Personal Access Token |
| | | | |
| `gitlabHost` | when `vcs` is `gitlab` | `https://gitlab.company.com` | GitLab Server (Could also be `https://gitlab.com`) |
| `gitlabProjectId` | when `vcs` is `gitlab` | | Project ID |
| `gitlabMrIid` | when `vcs` is `gitlab` | | MergeRequest IID (not to be confused with ID) |
| `gitlabToken` | when `vcs` is `gitlab` | | Access Token |
| | | | |
| `buildLogFile` / `-f` | yes, repeatable | | Read below |
| `output` / `-o` | no | | CodeCoach parsed output for debugging |
| `removeOldComment` / `-r` | no | `true` or `false` | Remove CodeCoach's old comments before adding new one |
| `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-id-1` `rule-id-2` | Rule IDs that CodeCoach will still comment but no longer treated as errors or warnings |
| Option | Required | Value | Description |
|---------------------------|---------------------------|-------------------------------------------|----------------------------------------------------------------------------------------|
| `vcs` / `-g` | when `dryRun` is `false` | `github` or `gitlab` | |
| | | | |
| `githubRepoUrl` | when `vcs` is `github` | | Repository's HTTPS URL |
| `githubPr` | when `vcs` is `github` | | Pull request ID |
| `githubToken` | when `vcs` is `github` | | Personal Access Token |
| | | | |
| `gitlabHost` | when `vcs` is `gitlab` | `https://gitlab.company.com` | GitLab Server (Could also be `https://gitlab.com`) |
| `gitlabProjectId` | when `vcs` is `gitlab` | | Project ID |
| `gitlabMrIid` | when `vcs` is `gitlab` | | MergeRequest IID (not to be confused with ID) |
| `gitlabToken` | when `vcs` is `gitlab` | | Access Token |
| | | | |
| `buildLogFile` / `-f` | yes, repeatable | | Read below |
| `output` / `-o` | no | | CodeCoach parsed output for debugging |
| `removeOldComment` / `-r` | no | `true` or `false` | Remove CodeCoach's old comments before adding new one |
| `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 |


##### `--buildLogFile` or `-f`
Expand Down
5 changes: 1 addition & 4 deletions src/AnalyzerBot/AnalyzerBot.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,7 @@ export class AnalyzerBot implements IAnalyzerBot {
this.touchedFileLog = logs
.filter(onlySeverity(LogSeverity.error, LogSeverity.warning))
.filter(onlyIn(touchedDiff));
this.comments = groupComments(
this.touchedFileLog,
new Set(this.config.suppressRules),
);
this.comments = groupComments(this.touchedFileLog, 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
45 changes: 42 additions & 3 deletions src/AnalyzerBot/utils/commentUtil.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ describe('groupComments', () => {
const logs: LogType[] = [touchFileError, touchFileWarning];

it('returns comments based on lint logs', () => {
const comments = groupComments(logs, new Set<string>());
const comments = groupComments(logs, []);
expect(comments).toEqual([
{
file: mockTouchFile,
Expand Down Expand Up @@ -44,7 +44,7 @@ describe('groupComments', () => {
lineOffset: 33,
},
],
new Set<string>(),
[],
);

expect(comments).toEqual([
Expand Down Expand Up @@ -79,7 +79,46 @@ describe('groupComments', () => {
ruleId: 'UNIMPORTANT_RULE2',
},
],
new Set(['UNIMPORTANT_RULE1', 'UNIMPORTANT_RULE2']),
['UNIMPORTANT_RULE1', 'UNIMPORTANT_RULE2'],
);

expect(comments).toEqual([
{
file: mockTouchFile,
line: file1TouchLine,
errors: 1,
warnings: 0,
suppresses: 1,
text:
':rotating_light: msg1' +
' \n' +
':warning: (SUPPRESSED) additional warning' +
' \n',
},
{
file: mockTouchFile,
line: file2TouchLine,
errors: 0,
warnings: 1,
suppresses: 0,
text: ':warning: msg3' + ' \n',
},
]);
});

it('support regexp in suppressRules', () => {
const comments = groupComments(
[
...logs,
{
...touchFileError,
msg: 'additional warning',
severity: LogSeverity.warning,
lineOffset: 33,
ruleId: 'UNIMPORTANT_RULE/RULE2',
},
],
['UNIMPORTANT_RULE/.*'],
);

expect(comments).toEqual([
Expand Down
11 changes: 8 additions & 3 deletions src/AnalyzerBot/utils/commentUtil.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { LogSeverity, LogType } from '../../Parser';
import { Comment, CommentStructure } from '../@types/CommentTypes';
import { MessageUtil } from './message.util';

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

Expand Down Expand Up @@ -61,12 +61,17 @@ function calculateSuppresses(currentComment: Comment, isSuppressed: boolean): nu
return currentComment.suppresses + (isSuppressed ? 1 : 0);
}

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

function updateComment(
currentComment: Comment,
log: LogType,
suppressRules: Set<string>,
suppressRules: Array<string>,
): Comment {
const isSuppressed = suppressRules.has(log.ruleId);
const isSuppressed = shouldBeSuppressed(log, suppressRules);
return {
text: buildText(currentComment, log, isSuppressed),
errors: calculateErrors(currentComment, log, isSuppressed),
Expand Down
2 changes: 1 addition & 1 deletion src/Config/Config.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ and <cwd> is build root directory (optional (Will use current context as cwd)).
string: true,
number: false,
describe:
'List of rules to suppress, bot will still make comment but mark it as suppressed and not failing the job',
'List of rules to suppress (in RegExp format), bot will still make comment but mark it as suppressed and not failing the job',
default: [],
})
.option('dryRun', {
Expand Down
1 change: 1 addition & 0 deletions src/Provider/GitHub/GitHub.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ class PrServiceMock implements IGitHubPRService {
const configs = {
removeOldComment: false,
failOnWarnings: false,
suppressRules: [] as Array<string>,
} as ConfigArgument;

function createGitHub(service: IGitHubPRService, configs: ConfigArgument) {
Expand Down
1 change: 1 addition & 0 deletions src/Provider/GitLab/GitLab.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ class MrServiceMock implements IGitLabMRService {
const configs = {
removeOldComment: false,
failOnWarnings: false,
suppressRules: [] as Array<string>,
} as ConfigArgument;

function createGitLab(service: IGitLabMRService, configs: ConfigArgument) {
Expand Down

0 comments on commit 23909bc

Please sign in to comment.