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

[bugfix] Build failing on creation of annotations (#34796) #379

Merged
merged 5 commits into from
Aug 28, 2024
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
995 changes: 411 additions & 584 deletions dist/index.js

Large diffs are not rendered by default.

20 changes: 14 additions & 6 deletions src/action/decorate/summary.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { logger } from '../../helper/logger';
import { joinUrl } from '../../helper/url';
import {
AnalysisResult,
Annotation,
Condition,
ConditionDetails,
ExtendedAnnotation,
Expand Down Expand Up @@ -233,12 +232,13 @@ export function createReviewComments(annotations: ExtendedAnnotation[], changedF
.filter(a => a.blocking?.state !== 'no')
.forEach(annotation => {
const displayCount = annotation.count === 1 ? '' : `(${annotation.count.toString()}x) `;
const title = annotation.instanceName + (annotation.rule ? `: ${annotation.rule}` : '');

if (githubConfig.event.isPullRequest) {
if (changedFiles.find(c => annotation.fullPath.includes(c.filename))) {
const reviewComment = {
blocking: annotation.blocking?.state,
title: `${annotation.instanceName}: ${annotation.rule}`,
title: title,
body: createBody(annotation, displayCount),
path: annotation.path,
line: annotation.line
Expand All @@ -253,7 +253,7 @@ export function createReviewComments(annotations: ExtendedAnnotation[], changedF
} else if (annotation.diffLines?.includes(annotation.line)) {
const reviewComment = {
blocking: annotation.blocking?.state,
title: `${annotation.instanceName}: ${annotation.rule}`,
title: title,
body: createBody(annotation, displayCount),
path: annotation.path,
line: annotation.line
Expand All @@ -270,16 +270,24 @@ export function createReviewComments(annotations: ExtendedAnnotation[], changedF
return { postable: postable, unpostable: unpostable };
}

function createBody(annotation: Annotation, displayCount: string) {
function createBody(annotation: ExtendedAnnotation, displayCount: string) {
let body = '';
if (annotation.blocking?.state === 'yes') {
body += `Blocking${EOL}`;
} else if (annotation.blocking?.state === 'after' && annotation.blocking.after) {
body += `Blocking after: ${format(annotation.blocking.after, 'yyyy-MM-dd')}${EOL}`;
}

const secondLine: string[] = [];
if (annotation.level) {
secondLine.push(`Level: ${annotation.level.toString()}`);
}
if (annotation.category) {
secondLine.push(`Category: ${annotation.category}`);
}

body += `Line: ${annotation.line.toString()}: ${displayCount}${annotation.msg}`;
body += `${EOL}Level: ${annotation.level.toString()}, Category: ${annotation.category}`;
body += secondLine.length > 0 ? `${EOL}${secondLine.join(', ')}` : '';
body += annotation.ruleHelp ? `${EOL}Rule-help: ${annotation.ruleHelp}` : '';

return body;
Expand Down Expand Up @@ -385,7 +393,7 @@ export function createUnpostableAnnotationsDetails(unpostableReviewComments: Ext
} else if (previousPath !== path) {
body += `</table><table><tr><th colspan='4'>${path}</th></tr>`;
}
body += `<tr><td>${icon}</td><td>${blocking}</td><td><b>Line:</b> ${reviewComment.line.toString()} <b>Level:</b> ${reviewComment.level.toString()}<br><b>Category:</b> ${reviewComment.category}</td><td><b>${reviewComment.type} violation:</b> ${reviewComment.rule} <b>${displayCount}</b><br>${reviewComment.msg}</td></tr>`;
body += `<tr><td>${icon}</td><td>${blocking}</td><td><b>Line:</b> ${reviewComment.line.toString()} <b>Level:</b> ${reviewComment.level?.toString() ?? ''}<br><b>Category:</b> ${reviewComment.category ?? ''}</td><td><b>${reviewComment.type} violation:</b> ${reviewComment.rule ?? ''} <b>${displayCount}</b><br>${reviewComment.msg}</td></tr>`;
previousPath = reviewComment.path ? reviewComment.path : '';
});
body += '</table>';
Expand Down
17 changes: 11 additions & 6 deletions src/helper/interfaces.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,16 +150,15 @@ export interface AnnotationType {

export interface Annotation {
fullPath: string;
line: number;
level: number;
category: string;
rule: string;
line?: number;
level?: number;
category?: string;
rule?: string;
msg: string;
supp: boolean;
type: string;
count: number;
count?: number;
gateId?: number;
displayCount?: string;
path?: string;
diffLines?: number[];
ruleHelp?: string;
Expand All @@ -168,9 +167,15 @@ export interface Annotation {
state: 'yes' | 'no' | 'after';
after?: number;
};
complexity?: number;
functionName?: string;
}

export interface ExtendedAnnotation extends Annotation {
msg: string;
line: number;
count: number;
displayCount?: string;
instanceName: string;
}

Expand Down
16 changes: 8 additions & 8 deletions src/tics/analyzer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,19 +97,19 @@ async function buildRunCommand(fileListPath: string): Promise<string> {
* @param data stdout or stderr
*/
function findInStdOutOrErr(data: string): void {
const error = data.toString().match(/\[ERROR.*/g);
if (error && !errorList.find(e => e === error.toString())) {
errorList.push(error.toString());
const error = data.match(/\[ERROR.*/g)?.toString();
if (error && !errorList.find(e => e === error)) {
errorList.push(error);
}

const warning = data.toString().match(/\[WARNING.*/g);
if (warning && !warningList.find(w => w === warning.toString())) {
warningList.push(warning.toString());
const warning = data.match(/\[WARNING.*/g)?.toString();
if (warning && !warningList.find(w => w === warning)) {
warningList.push(warning);
}

const noFilesToAnalyze = data.toString().match(/No files to analyze with option '-changed':/g);
const noFilesToAnalyze = data.match(/No files to analyze with option '-changed':/g);
if (noFilesToAnalyze) {
warningList.push(`[WARNING 5057] ${data.toString()}`);
warningList.push(`[WARNING 5057] ${data}`);
}

const findExplorerUrl = data.match(/\/Explorer.*/g);
Expand Down
18 changes: 17 additions & 1 deletion src/viewer/annotations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,27 @@ export async function getAnnotations(apiLinks: AnnotationApiLink[]): Promise<Ext
const response = await httpClient.get<AnnotationResponse>(annotationsUrl.href);

response.data.data.forEach((annotation: Annotation) => {
if (!annotation.line) {
annotation.line = 1;
const rule = annotation.rule ? ` ${annotation.rule} ` : ' ';
logger.notice(
`No line number reported for ${annotation.type} violation${rule}in file ${annotation.fullPath}. Reporting the annotation on line 1.`
);
}

// In case complexity is given, the annotation does not have a message (should be fixed in newer Viewers #34866).
// Present in Viewers <= 2024.2.0
if (annotation.complexity && !annotation.msg) {
annotation.msg = `Function ${annotation.functionName ?? ''} has a complexity of ${annotation.complexity.toString()}`;
}

const extendedAnnotation: ExtendedAnnotation = {
...annotation,
gateId: index,
line: annotation.line,
count: annotation.count ?? 1,
instanceName: response.data.annotationTypes ? response.data.annotationTypes[annotation.type].instanceName : annotation.type
};
extendedAnnotation.gateId = index;

logger.debug(JSON.stringify(extendedAnnotation));
annotations.push(extendedAnnotation);
Expand Down
4 changes: 2 additions & 2 deletions test/unit/action/decorate/summary.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -311,12 +311,12 @@ describe('createReviewComments', () => {
}
},
{
// testing one without a rule
fullPath: 'c:/src/test.js',
line: 0,
level: 1,
category: 'test',
type: 'test',
rule: 'rule-after',
msg: 'message-after',
count: 1,
supp: false,
Expand All @@ -338,7 +338,7 @@ describe('createReviewComments', () => {
},
{
blocking: 'after',
title: 'test: rule-after',
title: 'test',
path: 'src/test.js',
line: 0,
body: `Blocking after: 2024-02-19${EOL}Line: 0: message-after${EOL}Level: 1, Category: test`
Expand Down
62 changes: 52 additions & 10 deletions test/unit/viewer/annotations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,66 @@ import { ticsConfigMock } from '../../.setup/mock';
describe('getAnnotations', () => {
ticsConfigMock.baseUrl = 'http://base.url';

test('Should return analyzed files from viewer', async () => {
test('Should return annotations from viewer', async () => {
jest.spyOn(httpClient, 'get').mockImplementationOnce((): Promise<any> => Promise.resolve({ data: { data: [{ type: 'CS' }] } }));
jest
.spyOn(httpClient, 'get')
.mockImplementationOnce(
(): Promise<any> =>
Promise.resolve({ data: { data: [{ type: 'CS' }], annotationTypes: { CS: { instanceName: 'Coding Standard Violations' } } } })
);
jest.spyOn(httpClient, 'get').mockImplementationOnce(
(): Promise<any> =>
Promise.resolve({
data: {
data: [{ type: 'CS', line: 10, count: 2 }],
annotationTypes: { CS: { instanceName: 'Coding Standard Violations' } }
}
})
);

const response = await getAnnotations([{ url: 'url?fields=default,blocking' }, { url: 'url' }]);

expect(response).toEqual([
{ type: 'CS', gateId: 0, instanceName: 'CS' },
{ type: 'CS', gateId: 1, instanceName: 'Coding Standard Violations' }
{ type: 'CS', gateId: 0, line: 1, count: 1, instanceName: 'CS' },
{ type: 'CS', gateId: 1, line: 10, count: 2, instanceName: 'Coding Standard Violations' }
]);
});

test('Should return no analyzed files when no urls are given', async () => {
test('Should return complexity annotations from the viewer', async () => {
jest.spyOn(httpClient, 'get').mockImplementationOnce(
(): Promise<any> =>
Promise.resolve({
data: {
data: [
{ type: 'COMPLEXITY', line: 10, complexity: 3, functionName: 'main' },
{ type: 'COMPLEXITY', line: 2, complexity: 2, functionName: 'test', msg: 'testing' }
]
}
})
);

const response = await getAnnotations([{ url: 'url' }]);

expect(response).toEqual([
{
type: 'COMPLEXITY',
complexity: 3,
functionName: 'main',
gateId: 0,
line: 10,
count: 1,
instanceName: 'COMPLEXITY',
msg: 'Function main has a complexity of 3'
},
{
type: 'COMPLEXITY',
complexity: 2,
functionName: 'test',
gateId: 0,
line: 2,
count: 1,
instanceName: 'COMPLEXITY',
msg: 'testing'
}
]);
});

test('Should return no annotations when no urls are given', async () => {
const response = await getAnnotations([]);

expect(response).toEqual([]);
Expand Down