Merge pull request #3 from granodigital/copilot/fix-ef42709c-d695-4b40-b858-2a62906387e9

Author: villelahdenvuo

__tests__/main.test.ts

@@ -174,11 +174,42 @@ at Tests.Registration.main(Registration.java:202)`,
 		testInputs['max-annotations'] = '1';
 		await main.run();
 		expect(warningMock).toHaveBeenCalledWith(
-			'Maximum number of annotations reached (1)',
+			'Maximum number of annotations reached (1). 2 annotations were not shown.',
 		);
-		expect(setOutputMock).toHaveBeenCalledWith('errors', 0);
+		expect(setOutputMock).toHaveBeenCalledWith('errors', 1);
 		expect(setOutputMock).toHaveBeenCalledWith('warnings', 0);
-		expect(setOutputMock).toHaveBeenCalledWith('notices', 1);
+		expect(setOutputMock).toHaveBeenCalledWith('notices', 0);
+		expect(setOutputMock).toHaveBeenCalledWith('total', 1);
+	});
+
+	it('should prioritize errors over warnings and notices when maxAnnotations is reached', async () => {
+		// This test uses junit-eslint fixture which has both errors and warnings
+		testInputs.reports = ['junit-eslint|fixtures/junit-eslint.xml'];
+		testInputs['max-annotations'] = '1';
+		await main.run();
+
+		// Should show the error first, not the warning
+		expect(errorMock).toHaveBeenCalledWith(
+			'["Bucket"] is better written in dot notation.',
+			{
+				endColumn: undefined,
+				endLine: undefined,
+				file: '/home/runner/work/repo-name/repo-name/cypress/plugins/s3-email-client/s3-utils.ts',
+				startColumn: 28,
+				startLine: 7,
+				title: '@typescript-eslint/dot-notation',
+			},
+		);
+
+		// Warning should not be called since we only allow 1 annotation and error has priority
+		expect(warningMock).not.toHaveBeenCalledWith(
+			'Missing JSDoc comment.',
+			expect.any(Object),
+		);
+
+		expect(setOutputMock).toHaveBeenCalledWith('errors', 1);
+		expect(setOutputMock).toHaveBeenCalledWith('warnings', 0);
+		expect(setOutputMock).toHaveBeenCalledWith('notices', 0);
 		expect(setOutputMock).toHaveBeenCalledWith('total', 1);
 	});
 

dist/index.js

@@ -29,7 +29,7 @@
 		"coverage": "make-coverage-badge --output-path ./badges/coverage.svg",
 		"format:write": "prettier --write **/*.ts",
 		"format:check": "prettier --check **/*.ts",
-		"lint": "npx eslint . -c ./.github/linters/.eslintrc.yml",
+		"lint": "./node_modules/.bin/eslint . -c ./.github/linters/.eslintrc.yml",
 		"package": "ncc build src/index.ts --license licenses.txt",
 		"package:watch": "npm run package -- --watch",
 		"test": "jest",

package.json

@@ -33,6 +33,12 @@ export interface Config {
 
 type AnnotationLevel = 'notice' | 'warning' | 'error' | 'ignore';
 
+interface PendingAnnotation {
+	level: AnnotationLevel;
+	message: string;
+	properties: core.AnnotationProperties;
+}
+
 export interface ReportMatcher {
 	/**
 	 * The format of the report e.g. `xml`
@@ -102,8 +108,7 @@ export async function run(): Promise<void> {
 			core.endGroup();
 		}
 
-		const tally = { errors: 0, warnings: 0, notices: 0, total: 0 };
-		let maxAnnotationsReached = false;
+		const allAnnotations: PendingAnnotation[] = [];
 
 		for (const [matcherName, files] of reportFiles) {
 			const matcher = reportMatchers[matcherName];
@@ -114,30 +119,55 @@ export async function run(): Promise<void> {
 				core.debug(`Parsing ${file}`);
 				switch (matcher.format) {
 					case 'xml':
-						maxAnnotationsReached = await parseXmlReport(
-							file,
-							matcher,
-							tally,
-							config.maxAnnotations,
-						);
+						await parseXmlReport(file, matcher, allAnnotations);
 						break;
 					default:
 						throw new Error(
 							`Unsupported matcher format in ${matcherName}: ${matcher.format}`,
 						);
 				}
-				if (maxAnnotationsReached) break;
 			}
 			core.info(
-				`Parsed ${tally.total} annotation(s) from ${files.size} report(s)`,
+				`Parsed ${allAnnotations.length} annotation(s) from ${files.size} report(s)`,
 			);
 			core.endGroup();
-			if (maxAnnotationsReached) break;
 		}
-		if (maxAnnotationsReached)
+
+		// Sort annotations by priority: errors first, then warnings, then notices
+		// Ignore level annotations are already filtered out during collection
+		const priorityOrder: Record<AnnotationLevel, number> = {
+			error: 0,
+			warning: 1,
+			notice: 2,
+			ignore: 3, // Should not appear in the array, but included for type completeness
+		};
+		allAnnotations.sort(
+			(a, b) => priorityOrder[a.level] - priorityOrder[b.level],
+		);
+
+		// Apply the maxAnnotations limit and create the annotations
+		const annotationsToCreate = allAnnotations.slice(0, config.maxAnnotations);
+		const tally = { errors: 0, warnings: 0, notices: 0, total: 0 };
+
+		for (const annotation of annotationsToCreate) {
+			// Type assertion is safe because we filter out 'ignore' level during collection
+			core[annotation.level as 'error' | 'warning' | 'notice'](
+				annotation.message,
+				annotation.properties,
+			);
+			if (annotation.level === 'error') tally.errors++;
+			if (annotation.level === 'warning') tally.warnings++;
+			if (annotation.level === 'notice') tally.notices++;
+			tally.total++;
+		}
+
+		if (allAnnotations.length > config.maxAnnotations) {
 			core.warning(
-				`Maximum number of annotations reached (${config.maxAnnotations})`,
+				`Maximum number of annotations reached (${config.maxAnnotations}). ${
+					allAnnotations.length - config.maxAnnotations
+				} annotations were not shown.`,
 			);
+		}
 		// Set outputs for other workflow steps to use.
 		core.setOutput('errors', tally.errors);
 		core.setOutput('warnings', tally.warnings);
@@ -204,24 +234,21 @@ async function loadConfig(): Promise<Config> {
 async function parseXmlReport(
 	file: string,
 	matcher: ReportMatcher,
-	tally: Record<'errors' | 'warnings' | 'notices' | 'total', number>,
-	maxAnnotations: number,
-): Promise<boolean> {
+	allAnnotations: PendingAnnotation[],
+): Promise<void> {
 	const report = await readFile(file, 'utf8');
 	core.debug(`Parsing report:\n${report}`);
 	const doc = new DOMParser().parseFromString(report, 'text/xml');
 	let items = select(matcher.item, doc);
 	if (!Array.isArray(items) && isNodeLike(items)) items = [items];
 	if (!isArrayOfNodes(items)) {
 		core.warning(`No items found in ${file}`);
-		return false;
+		return;
 	}
 	core.debug(`Found ${items.length} items in ${file}.`);
 
 	for (const item of items) {
-		core.debug(
-			`Processing item ${tally.total + 1}/${maxAnnotations}: ${item}.`,
-		);
+		core.debug(`Processing item: ${item}.`);
 		const xpath = xpathSelect(item);
 		// Figure out the level of the annotation.
 		let level: AnnotationLevel = 'error';
@@ -239,9 +266,10 @@ async function parseXmlReport(
 			core.debug('Ignoring item.');
 			continue;
 		}
-		// Create the annotation.
+
+		// Create the annotation data.
 		const message = xpath.string(matcher.message);
-		const annotation = {
+		const properties = {
 			title: matcher.title ? xpath.string(matcher.title) : undefined,
 			file: matcher.file ? xpath.string(matcher.file) : undefined,
 			startLine: matcher.startLine
@@ -255,14 +283,8 @@ async function parseXmlReport(
 				? xpath.number(matcher.endColumn)
 				: undefined,
 		} satisfies core.AnnotationProperties;
-		core[level](message, annotation);
 
-		// Count the annotations for the output.
-		if (level === 'error') tally.errors++;
-		if (level === 'warning') tally.warnings++;
-		if (level === 'notice') tally.notices++;
-		tally.total++;
-		if (tally.total >= maxAnnotations) return true;
+		// Collect non-ignore annotations
+		allAnnotations.push({ level, message, properties });
 	}
-	return false;
 }