diff --git a/packages/code-analyzer-core/package.json b/packages/code-analyzer-core/package.json index 06ecbed4..d8e66952 100644 --- a/packages/code-analyzer-core/package.json +++ b/packages/code-analyzer-core/package.json @@ -1,7 +1,7 @@ { "name": "@salesforce/code-analyzer-core", "description": "Core Package for the Salesforce Code Analyzer", - "version": "0.45.0", + "version": "0.46.0-SNAPSHOT", "author": "The Salesforce Code Analyzer Team", "license": "BSD-3-Clause", "homepage": "https://developer.salesforce.com/docs/platform/salesforce-code-analyzer/overview", diff --git a/packages/code-analyzer-core/src/code-analyzer.ts b/packages/code-analyzer-core/src/code-analyzer.ts index 40f85ac3..64ddfbd9 100644 --- a/packages/code-analyzer-core/src/code-analyzer.ts +++ b/packages/code-analyzer-core/src/code-analyzer.ts @@ -9,7 +9,7 @@ import { UninstantiableEngineRunResults, Violation } from "./results" -import {processSuppressions} from "./suppressions" +import {processSuppressions, extractSuppressionsFromFiles, SuppressionsMap, LoggerCallback} from "./suppressions" import {SemVer} from 'semver'; import { EngineLogEvent, @@ -120,6 +120,11 @@ export class CodeAnalyzer { private readonly engineConfigDescriptions: Map = new Map(); private readonly rulesCache: Map = new Map(); private readonly engineRuleDiscoveryProgressAggregator: EngineProgressAggregator = new EngineProgressAggregator(); + // Caching for per-engine suppression processing to avoid duplicate file processing + private readonly suppressionsMap: SuppressionsMap = new Map(); + private readonly fileProcessingPromises: Map> = new Map(); + // Track total suppressed violations for aggregate logging + private totalSuppressedViolations: number = 0; constructor(config: CodeAnalyzerConfig, fileSystem: FileSystem = new RealFileSystem(), nodeVersion: string = process.version) { this.validateEnvironment(nodeVersion); @@ -348,6 +353,15 @@ export class CodeAnalyzer { // up a bunch of RunResults promises and then does a Promise.all on them. Otherwise, the progress events may // override each other. + // Reset suppression counter for this run + this.totalSuppressedViolations = 0; + + // Clear suppression caches from previous runs to prevent unbounded memory growth + // Each run typically analyzes a different workspace, so caching across runs provides minimal benefit + // while keeping stale data in memory. + this.suppressionsMap.clear(); + this.fileProcessingPromises.clear(); + this.emitLogEvent(LogLevel.Debug, getMessage('RunningWithWorkspace', JSON.stringify({ filesAndFolders: runOptions.workspace.getRawFilesAndFolders(), targets: runOptions.workspace.getRawTargets() @@ -395,55 +409,147 @@ export class CodeAnalyzer { runResults.addEngineRunResults(new UninstantiableEngineRunResults(uninstantiableEngine, error)); } - // Process inline suppressions (post-processing step) - // This filters out violations that have been suppressed via inline markers - await this.applyInlineSuppressions(runResults); + // Note: Inline suppressions are now applied per-engine in runEngineAndValidateResults() before EngineResultsEvent is emitted + + // Log aggregate suppression count if any violations were suppressed + if (this.config.getSuppressionsEnabled()) { + if (this.totalSuppressedViolations > 0) { + this.emitLogEvent(LogLevel.Info, getMessage('SuppressedViolationsCount', this.totalSuppressedViolations)); + } else { + this.emitLogEvent(LogLevel.Info, getMessage('NoViolationsSuppressed')); + } + } return runResults; } /** - * Applies suppression filtering to the run results - * This processes suppression markers in source files and filters out suppressed violations - * @param runResults The run results to apply suppressions to + * Applies suppression filtering to a single engine's results + * This processes suppression markers in source files and returns a filtered version of the engine results + * This method handles race conditions by caching suppression ranges per file + * @param engineRunResults The engine run results to apply suppressions to + * @returns Filtered engine run results with suppressions applied */ - private async applyInlineSuppressions(runResults: RunResultsImpl): Promise { + private async applyInlineSuppressionsToEngineResults( + engineRunResults: EngineRunResults + ): Promise { // Check if suppressions are enabled if (!this.config.getSuppressionsEnabled()) { - return; // Feature disabled, skip processing + return engineRunResults; // Feature disabled, return original results } - const allViolations = runResults.getViolations(); + const violations = engineRunResults.getViolations(); + + if (violations.length === 0) { + return engineRunResults; // No violations to process + } + + // Extract unique file paths from violations for race condition handling + const filePaths = new Set(); + for (const violation of violations) { + const primaryLocation = violation.getPrimaryLocation(); + const file = primaryLocation.getFile(); + if (file) { + filePaths.add(file); + } + } - if (allViolations.length === 0) { - return; // No violations to process + if (filePaths.size === 0) { + return engineRunResults; // No files with violations } - this.emitLogEvent(LogLevel.Debug, getMessage('ProcessingInlineSuppressions', allViolations.length)); + // Process files with race condition handling to pre-populate the shared map + await this.processFilesForSuppressions(filePaths); - // Process suppressions (returns filtered violations) - const logger = (level: 'error' | 'warn' | 'debug', message: string) => { + // Use processSuppressions with the pre-populated shared map + // This will skip re-parsing files already in the map and just filter violations + const logger: LoggerCallback = (level: 'error' | 'warn' | 'debug', message: string) => { const logLevel = level === 'error' ? LogLevel.Error : level === 'warn' ? LogLevel.Warn : LogLevel.Debug; this.emitLogEvent(logLevel, message); }; - const filteredViolations = await processSuppressions(allViolations, logger); - - // Calculate which violations were suppressed - const suppressedViolations = new Set(); - const filteredSet = new Set(filteredViolations); - for (const violation of allViolations) { - if (!filteredSet.has(violation)) { - suppressedViolations.add(violation); - } + const filteredViolations = await processSuppressions(violations, logger, this.suppressionsMap); + + // Calculate how many violations were suppressed + const suppressedCount = violations.length - filteredViolations.length; + + // If all violations remain (nothing suppressed), return original results + if (suppressedCount === 0) { + return engineRunResults; } - const suppressedCount = suppressedViolations.size; - if (suppressedCount > 0) { - this.emitLogEvent(LogLevel.Info, getMessage('SuppressedViolationsCount', suppressedCount)); - runResults.applySuppressedViolationsFilter(suppressedViolations); - } else { - this.emitLogEvent(LogLevel.Info, getMessage('NoViolationsSuppressed')); + // Track suppressed violations for aggregate logging + this.totalSuppressedViolations += suppressedCount; + + // Return filtered results using FilteredEngineRunResults wrapper + return this.createFilteredEngineRunResults(engineRunResults, filteredViolations); + } + + /** + * Creates a FilteredEngineRunResults wrapper + * This is a temporary method until FilteredEngineRunResults is exported from results.ts + */ + private createFilteredEngineRunResults( + originalResults: EngineRunResults, + filteredViolations: Violation[] + ): EngineRunResults { + // We need to create an instance that implements EngineRunResults + // but filters the violations + return { + getEngineName: () => originalResults.getEngineName(), + getEngineVersion: () => originalResults.getEngineVersion(), + getViolationCount: () => filteredViolations.length, + getViolationCountOfSeverity: (severity: number) => + filteredViolations.filter(v => v.getRule().getSeverityLevel() === severity).length, + getViolations: () => filteredViolations + }; + } + + /** + * Processes files for suppression markers with race condition handling + * Uses caching to avoid processing the same file multiple times when multiple engines + * return violations for the same file + * @param filePaths Set of file paths that need suppression information + */ + private async processFilesForSuppressions(filePaths: Set): Promise { + const logger: LoggerCallback = (level: 'error' | 'warn' | 'debug', message: string) => { + const logLevel = level === 'error' ? LogLevel.Error : level === 'warn' ? LogLevel.Warn : LogLevel.Debug; + this.emitLogEvent(logLevel, message); + }; + + const processingPromises: Promise[] = []; + + for (const filePath of filePaths) { + // If already in cache, skip + if (this.suppressionsMap.has(filePath)) { + continue; + } + + // If currently being processed, await that promise + let processingPromise = this.fileProcessingPromises.get(filePath); + + if (!processingPromise) { + // Start new processing - wrap extractSuppressionsFromFiles call + processingPromise = extractSuppressionsFromFiles( + new Set([filePath]), + this.suppressionsMap, + logger + ).then(() => { + // Clean up the promise from tracking map since it's done + this.fileProcessingPromises.delete(filePath); + }).catch((err) => { + // Clean up on error too + this.fileProcessingPromises.delete(filePath); + throw err; + }); + + this.fileProcessingPromises.set(filePath, processingPromise); + } + + processingPromises.push(processingPromise); } + + // Wait for all file processing to complete + await Promise.all(processingPromises); } /** @@ -565,7 +671,10 @@ export class CodeAnalyzer { } validateEngineRunResults(engineName, apiEngineRunResults, ruleSelection); - const engineRunResults: EngineRunResults = new EngineRunResultsImpl(engineName, await engine.getEngineVersion(), apiEngineRunResults, ruleSelection); + let engineRunResults: EngineRunResults = new EngineRunResultsImpl(engineName, await engine.getEngineVersion(), apiEngineRunResults, ruleSelection); + + // Apply inline suppressions per-engine BEFORE emitting EngineResultsEvent + engineRunResults = await this.applyInlineSuppressionsToEngineResults(engineRunResults); this.emitEvent({ type: EventType.EngineRunProgressEvent, timestamp: this.clock.now(), engineName: engineName, percentComplete: 100 diff --git a/packages/code-analyzer-core/src/config.ts b/packages/code-analyzer-core/src/config.ts index d24d68db..f67cb814 100644 --- a/packages/code-analyzer-core/src/config.ts +++ b/packages/code-analyzer-core/src/config.ts @@ -227,6 +227,12 @@ export class CodeAnalyzerConfig { valueType: 'object', defaultValue: { files: [] }, wasSuppliedByUser: !deepEquals(this.config.ignores, DEFAULT_CONFIG.ignores) + }, + suppressions: { + descriptionText: getMessage('ConfigFieldDescription_suppressions'), + valueType: 'object', + defaultValue: { disable_suppressions: false }, + wasSuppliedByUser: !deepEquals(this.config.suppressions, DEFAULT_CONFIG.suppressions) } } }; @@ -260,6 +266,13 @@ export class CodeAnalyzerConfig { return !this.config.suppressions.disable_suppressions; } + /** + * Returns the suppressions configuration object. + */ + public getSuppressions(): Suppressions { + return this.config.suppressions; + } + /** * Returns the absolute path folder where all path based values within the configuration may be relative to. * Typically, this is set as the folder where a configuration file was loaded from, but doesn't have to be. diff --git a/packages/code-analyzer-core/src/index.ts b/packages/code-analyzer-core/src/index.ts index 36c4543f..59b54e3b 100644 --- a/packages/code-analyzer-core/src/index.ts +++ b/packages/code-analyzer-core/src/index.ts @@ -8,7 +8,8 @@ export type { EngineOverrides, Ignores, RuleOverrides, - RuleOverride + RuleOverride, + Suppressions } from "./config" diff --git a/packages/code-analyzer-core/src/messages.ts b/packages/code-analyzer-core/src/messages.ts index 02ca1e6e..083a43c5 100644 --- a/packages/code-analyzer-core/src/messages.ts +++ b/packages/code-analyzer-core/src/messages.ts @@ -61,6 +61,14 @@ const MESSAGE_CATALOG : MessageCatalog = { ` - "**/*.test.js"\n` + `-------------------------------------------`, + ConfigFieldDescription_suppressions: + `Configuration for inline suppression markers in source code.\n` + + ` disable_suppressions: Boolean to disable processing of suppression markers.\n` + + `---- [Example usage]: ---------------------\n` + + `suppressions:\n` + + ` disable_suppressions: false\n` + + `-------------------------------------------`, + GenericEngineConfigOverview: `%s ENGINE CONFIGURATION`, @@ -231,9 +239,6 @@ const MESSAGE_CATALOG : MessageCatalog = { EngineWorkingFolderKeptDueToError: `Since the engine '%s' emitted an error, the following temporary working folder will not be removed: %s`, - ProcessingInlineSuppressions: - `Processing inline suppressions for %d violation(s).`, - SuppressedViolationsCount: `%d violation(s) were suppressed by inline suppression markers.`, diff --git a/packages/code-analyzer-core/src/suppressions/index.ts b/packages/code-analyzer-core/src/suppressions/index.ts index 106d70ca..fee3ef4f 100644 --- a/packages/code-analyzer-core/src/suppressions/index.ts +++ b/packages/code-analyzer-core/src/suppressions/index.ts @@ -18,7 +18,8 @@ export { export { processSuppressions, isTextFile, - extractSuppressionsFromFiles + extractSuppressionsFromFiles, + filterSuppressedViolations } from './suppression-processor'; export type { LoggerCallback } from './suppression-processor'; diff --git a/packages/code-analyzer-core/src/suppressions/suppression-parser.ts b/packages/code-analyzer-core/src/suppressions/suppression-parser.ts index 87da5b08..c61a67ad 100644 --- a/packages/code-analyzer-core/src/suppressions/suppression-parser.ts +++ b/packages/code-analyzer-core/src/suppressions/suppression-parser.ts @@ -3,6 +3,8 @@ */ import { SuppressionMarker, SuppressionRange, FileSuppressions } from './suppression-types'; +import { Selector, toSelector } from '../selectors'; +import { LoggerCallback } from './suppression-processor'; /** * Regular expressions to match suppression markers (case-insensitive) @@ -18,9 +20,10 @@ const UNSUPPRESS_PATTERN = /code-analyzer-unsuppress(?:\(([^()]*(?:\([^)]*\)[^() * Parses a file's content to extract suppression markers * @param fileContent The full content of the file as a string * @param filePath The absolute path to the file (for error reporting) + * @param logger Optional logger callback for warnings about invalid selectors * @returns Array of SuppressionMarker objects found in the file */ -export function parseSuppressionMarkers(fileContent: string, _filePath: string): SuppressionMarker[] { +export function parseSuppressionMarkers(fileContent: string, filePath: string, logger?: LoggerCallback): SuppressionMarker[] { const markers: SuppressionMarker[] = []; const lines = fileContent.split('\n'); @@ -31,23 +34,45 @@ export function parseSuppressionMarkers(fileContent: string, _filePath: string): // Find all suppress markers in this line const suppressMatches = Array.from(line.matchAll(SUPPRESS_PATTERN)); for (const match of suppressMatches) { - const ruleSelector = normalizeRuleSelector(match[1]); - markers.push({ - type: 'suppress', - ruleSelector, - lineNumber - }); + const ruleSelectorString = normalizeRuleSelectorString(match[1]); + try { + const ruleSelector = toSelector(ruleSelectorString); + markers.push({ + type: 'suppress', + ruleSelector, + ruleSelectorString, + lineNumber + }); + } catch (err) { + // If toSelector throws an error (invalid syntax), skip this marker + if (logger) { + const errorMsg = err instanceof Error ? err.message : String(err); + logger('warn', `Invalid rule selector '${ruleSelectorString}' in suppress marker at ${filePath}:${lineNumber}. Error: ${errorMsg}`); + } + continue; + } } // Find all unsuppress markers in this line const unsuppressMatches = Array.from(line.matchAll(UNSUPPRESS_PATTERN)); for (const match of unsuppressMatches) { - const ruleSelector = normalizeRuleSelector(match[1]); - markers.push({ - type: 'unsuppress', - ruleSelector, - lineNumber - }); + const ruleSelectorString = normalizeRuleSelectorString(match[1]); + try { + const ruleSelector = toSelector(ruleSelectorString); + markers.push({ + type: 'unsuppress', + ruleSelector, + ruleSelectorString, + lineNumber + }); + } catch (err) { + // If toSelector throws an error (invalid syntax), skip this marker + if (logger) { + const errorMsg = err instanceof Error ? err.message : String(err); + logger('warn', `Invalid rule selector '${ruleSelectorString}' in unsuppress marker at ${filePath}:${lineNumber}. Error: ${errorMsg}`); + } + continue; + } } } @@ -55,11 +80,11 @@ export function parseSuppressionMarkers(fileContent: string, _filePath: string): } /** - * Normalizes a rule selector from a marker + * Normalizes a rule selector string from a marker * Empty or undefined selectors default to "all" * Trims whitespace from the selector */ -function normalizeRuleSelector(selector: string | undefined): string { +function normalizeRuleSelectorString(selector: string | undefined): string { if (!selector || selector.trim() === '') { return 'all'; } @@ -193,13 +218,13 @@ export function buildSuppressionRanges(markers: SuppressionMarker[], _filePath: const ranges: SuppressionRange[] = []; // Track which rule selectors are currently active (suppressed or unsuppressed) - // Map: ruleSelector -> {isSuppressed, startLine} - const activeStates = new Map(); + // Map: ruleSelectorString -> {isSuppressed, startLine, ruleSelector (Selector object)} + const activeStates = new Map(); for (const marker of markers) { if (marker.type === 'suppress') { // Check if already suppressed (no-op if so) - const currentState = activeStates.get(marker.ruleSelector); + const currentState = activeStates.get(marker.ruleSelectorString); if (currentState && currentState.isSuppressed) { continue; // Already suppressed, skip } @@ -207,23 +232,24 @@ export function buildSuppressionRanges(markers: SuppressionMarker[], _filePath: // Check if this suppress should close any active unsuppressions hierarchically // For example, suppress(regex) should close unsuppress(regex:AvoidOldApi) const statesToEnd: string[] = []; - for (const [activeSelector, state] of activeStates.entries()) { - if (!state.isSuppressed && activeSelector !== marker.ruleSelector && - canSuppressCloseUnsuppress(marker.ruleSelector, activeSelector)) { + for (const [activeSelectorString, state] of activeStates.entries()) { + if (!state.isSuppressed && activeSelectorString !== marker.ruleSelectorString && + canSuppressCloseUnsuppress(marker.ruleSelectorString, activeSelectorString)) { // Close this unsuppression range (but not for the same selector - handled below) ranges.push({ startLine: state.startLine, endLine: marker.lineNumber - 1, - ruleSelector: activeSelector, + ruleSelector: state.ruleSelector, + ruleSelectorString: activeSelectorString, isSuppressed: false }); - statesToEnd.push(activeSelector); + statesToEnd.push(activeSelectorString); } } // Remove closed states - for (const selector of statesToEnd) { - activeStates.delete(selector); + for (const selectorString of statesToEnd) { + activeStates.delete(selectorString); } // If there was an unsuppression active for this exact selector, close it @@ -232,55 +258,60 @@ export function buildSuppressionRanges(markers: SuppressionMarker[], _filePath: startLine: currentState.startLine, endLine: marker.lineNumber - 1, ruleSelector: marker.ruleSelector, + ruleSelectorString: marker.ruleSelectorString, isSuppressed: false }); } // Start new suppression - activeStates.set(marker.ruleSelector, { + activeStates.set(marker.ruleSelectorString, { isSuppressed: true, - startLine: marker.lineNumber + startLine: marker.lineNumber, + ruleSelector: marker.ruleSelector }); } else if (marker.type === 'unsuppress') { // End active suppression ranges that match hierarchically const statesToEnd: string[] = []; - for (const [suppressSelector, state] of activeStates.entries()) { - if (state.isSuppressed && canUnsuppressEndSuppress(suppressSelector, marker.ruleSelector)) { + for (const [suppressSelectorString, state] of activeStates.entries()) { + if (state.isSuppressed && canUnsuppressEndSuppress(suppressSelectorString, marker.ruleSelectorString)) { // Create a suppression range from the suppress marker to the unsuppress marker (exclusive of unsuppress line) ranges.push({ startLine: state.startLine, endLine: marker.lineNumber - 1, - ruleSelector: suppressSelector, + ruleSelector: state.ruleSelector, + ruleSelectorString: suppressSelectorString, isSuppressed: true }); - statesToEnd.push(suppressSelector); + statesToEnd.push(suppressSelectorString); } } // Remove ended suppressions - for (const selector of statesToEnd) { - activeStates.delete(selector); + for (const selectorString of statesToEnd) { + activeStates.delete(selectorString); } // Now start an unsuppression range for this selector // This creates the "exception" behavior - marking that this selector is explicitly unsuppressed - if (statesToEnd.length > 0 || marker.ruleSelector !== 'all') { + if (statesToEnd.length > 0 || marker.ruleSelectorString !== 'all') { // Only create unsuppression range if we actually ended something, or if it's a specific selector - activeStates.set(marker.ruleSelector, { + activeStates.set(marker.ruleSelectorString, { isSuppressed: false, - startLine: marker.lineNumber + startLine: marker.lineNumber, + ruleSelector: marker.ruleSelector }); } } } // Any remaining active states extend to the end of the file - for (const [ruleSelector, state] of activeStates.entries()) { + for (const [ruleSelectorString, state] of activeStates.entries()) { ranges.push({ startLine: state.startLine, endLine: undefined, - ruleSelector, + ruleSelector: state.ruleSelector, + ruleSelectorString, isSuppressed: state.isSuppressed }); } @@ -292,10 +323,11 @@ export function buildSuppressionRanges(markers: SuppressionMarker[], _filePath: * Parses a file's content and builds complete suppression information * @param fileContent The full content of the file as a string * @param filePath The absolute path to the file + * @param logger Optional logger callback for warnings about invalid selectors * @returns FileSuppressions object containing all suppression ranges for the file */ -export function parseFileSuppressions(fileContent: string, filePath: string): FileSuppressions { - const markers = parseSuppressionMarkers(fileContent, filePath); +export function parseFileSuppressions(fileContent: string, filePath: string, logger?: LoggerCallback): FileSuppressions { + const markers = parseSuppressionMarkers(fileContent, filePath, logger); const ranges = buildSuppressionRanges(markers, filePath); return { diff --git a/packages/code-analyzer-core/src/suppressions/suppression-processor.ts b/packages/code-analyzer-core/src/suppressions/suppression-processor.ts index 86368e30..28ed99d4 100644 --- a/packages/code-analyzer-core/src/suppressions/suppression-processor.ts +++ b/packages/code-analyzer-core/src/suppressions/suppression-processor.ts @@ -5,6 +5,8 @@ import { Violation } from '../results'; import { FileSuppressions, SuppressionRange, SuppressionsMap } from './suppression-types'; import { parseFileSuppressions } from './suppression-parser'; +import { Selector } from '../selectors'; +import { SeverityLevel } from '@salesforce/code-analyzer-engine-api'; import fs from 'node:fs'; import { isBinaryFile } from 'isbinaryfile'; @@ -51,11 +53,11 @@ export async function extractSuppressionsFromFiles( } try { - // Read file content - const fileContent = fs.readFileSync(filePath, 'utf-8'); + // Read file content asynchronously + const fileContent = await fs.promises.readFile(filePath, 'utf-8'); // Parse suppressions - const fileSuppressions = parseFileSuppressions(fileContent, filePath); + const fileSuppressions = parseFileSuppressions(fileContent, filePath, logger); // Cache the result suppressionsMap.set(filePath, fileSuppressions); @@ -73,20 +75,6 @@ export async function extractSuppressionsFromFiles( return suppressionsMap; } -/** - * Gets the specificity level of a rule selector - * More specific selectors override less specific ones - */ -function getSelectorSpecificity(selector: string): number { - if (selector === 'all') { - return 1; // Least specific - } - if (selector.includes(':')) { - return 3; // Most specific (engine:rule) - } - return 2; // Medium specific (engine only) -} - /** * Checks if a violation should be suppressed based on suppression ranges * Uses hierarchical matching with specificity rules @@ -121,24 +109,34 @@ export function isViolationSuppressed(violation: Violation, fileSuppressions: Fi return false; // No applicable ranges } - // Apply specificity rules: find the most specific applicable range - // If there are multiple ranges at the same specificity, take the one with the highest startLine (most recent) - let mostSpecificRange: SuppressionRange | null = null; - let highestSpecificity = 0; + // Find the most recent applicable range (highest startLine wins) + // This is the simplest rule: the nearest suppression marker takes precedence + // + // EDGE CASE - Multiple markers on same line: + // If multiple suppression markers are placed on the same line (e.g., same comment), + // the behavior is undefined (whichever appears last in the array wins). + // This is an anti-pattern and users should NOT write markers this way. + // We intentionally do not implement complex tie-breaking logic for this edge case. + // + // Example of what NOT to do: + // // code-analyzer-suppress(all) code-analyzer-suppress(eslint) + // + // Proper usage - one marker per line: + // // code-analyzer-suppress(all) + // // code-analyzer-suppress(eslint) + // + let selectedRange: SuppressionRange | null = null; + let highestStartLine = 0; for (const range of applicableRanges) { - const specificity = getSelectorSpecificity(range.ruleSelector); - - if (specificity > highestSpecificity || - (specificity === highestSpecificity && mostSpecificRange && - range.startLine > mostSpecificRange.startLine)) { - mostSpecificRange = range; - highestSpecificity = specificity; + if (range.startLine >= highestStartLine) { + selectedRange = range; + highestStartLine = range.startLine; } } - // Return the isSuppressed value of the most specific range - return mostSpecificRange ? mostSpecificRange.isSuppressed : false; + // Return the isSuppressed value of the selected range + return selectedRange ? selectedRange.isSuppressed : false; } /** @@ -166,56 +164,28 @@ function doesRangeOverlapViolation( /** * Checks if a rule selector matches a violation's rule + * Uses the same Selector.matchesSelectables() logic as rule selection * - * Rule selector matching rules: - * - "all" matches all rules - * - "engineName" matches all rules from that engine (e.g., "pmd", "eslint") - * - "engineName:ruleName" matches specific rule - * - "engineName:(severity1,severity2)" matches rules from engine with those severities - * - * @param ruleSelector The rule selector from suppression marker + * @param ruleSelector The Selector from suppression marker * @param violation The violation to check * @returns true if the selector matches this violation's rule */ -function doesRuleSelectorMatch(ruleSelector: string, violation: Violation): boolean { - // "all" matches everything - if (ruleSelector === 'all') { - return true; - } - +function doesRuleSelectorMatch(ruleSelector: Selector, violation: Violation): boolean { const rule = violation.getRule(); - const engineName = rule.getEngineName(); - const ruleName = rule.getName(); - const severity = rule.getSeverityLevel(); - - // Check for exact "engineName:ruleName" match - const fullRuleName = `${engineName}:${ruleName}`; - if (ruleSelector === fullRuleName) { - return true; - } - - // Check for engine-only match (e.g., "pmd" matches all pmd rules) - if (ruleSelector === engineName) { - return true; - } - // Check for severity-based match (e.g., "eslint:(3,4)") - const severityPattern = /^([^:]+):\(([^)]+)\)$/; - const severityMatch = ruleSelector.match(severityPattern); - if (severityMatch) { - const selectorEngine = severityMatch[1]; - const severitiesStr = severityMatch[2]; - - if (selectorEngine === engineName) { - // Parse severities: "3,4" -> [3, 4] - const severities = severitiesStr.split(',').map(s => parseInt(s.trim(), 10)); - if (severities.includes(severity)) { - return true; - } - } - } - - return false; + // Build selectables array (same as Rule.matchesRuleSelector in rules.ts) + const sevNumber: number = rule.getSeverityLevel().valueOf(); + const sevName: string = SeverityLevel[sevNumber]; + const selectables: string[] = [ + "all", + rule.getEngineName().toLowerCase(), + rule.getName().toLowerCase(), + sevName.toLowerCase(), + String(sevNumber), + ...rule.getTags().map(t => t.toLowerCase()) + ]; + + return ruleSelector.matchesSelectables(selectables); } /** @@ -257,9 +227,14 @@ export type LoggerCallback = (level: 'error' | 'warn' | 'debug', message: string * * @param violations Array of all violations from all engines * @param logger Optional logger callback for error/warning messages + * @param existingSuppressionsMap Optional pre-populated suppressions map to use for caching across multiple calls * @returns Filtered violations with suppressions applied */ -export async function processSuppressions(violations: Violation[], logger?: LoggerCallback): Promise { +export async function processSuppressions( + violations: Violation[], + logger?: LoggerCallback, + existingSuppressionsMap?: SuppressionsMap +): Promise { if (violations.length === 0) { return violations; } @@ -279,8 +254,11 @@ export async function processSuppressions(violations: Violation[], logger?: Logg return violations; } - // Parse suppression information from files - const suppressionsMap = await extractSuppressionsFromFiles(filePaths, new Map(), logger); + // Use provided map or create new one + const suppressionsMap = existingSuppressionsMap || new Map(); + + // Parse suppression information from files (will skip already-cached files) + await extractSuppressionsFromFiles(filePaths, suppressionsMap, logger); // Filter violations return filterSuppressedViolations(violations, suppressionsMap); diff --git a/packages/code-analyzer-core/src/suppressions/suppression-types.ts b/packages/code-analyzer-core/src/suppressions/suppression-types.ts index 4e9a16d0..976d1f2e 100644 --- a/packages/code-analyzer-core/src/suppressions/suppression-types.ts +++ b/packages/code-analyzer-core/src/suppressions/suppression-types.ts @@ -2,11 +2,7 @@ * Types and interfaces for the suppression system */ -/** - * Represents a rule selector that can be used in suppression markers - * Examples: "all", "pmd:ApexCrudViolation", "eslint:(3,4)", "regex" - */ -export type RuleSelector = string; +import {Selector} from "../selectors"; /** * Represents a suppression marker found in source code @@ -15,8 +11,11 @@ export interface SuppressionMarker { /** The type of marker (suppress or unsuppress) */ type: 'suppress' | 'unsuppress'; - /** The rule selector specified in the marker */ - ruleSelector: RuleSelector; + /** The rule selector specified in the marker (parsed using toSelector) */ + ruleSelector: Selector; + + /** The original rule selector string (kept for hierarchical ending logic) */ + ruleSelectorString: string; /** The line number where the marker was found (1-indexed) */ lineNumber: number; @@ -32,8 +31,11 @@ export interface SuppressionRange { /** The ending line number (1-indexed, inclusive). undefined means end of file */ endLine: number | undefined; - /** The rule selector that is affected in this range */ - ruleSelector: RuleSelector; + /** The rule selector that is affected in this range (Selector object from selectors.ts) */ + ruleSelector: Selector; + + /** The original rule selector string (kept for hierarchical ending logic and debugging) */ + ruleSelectorString: string; /** Whether this is a suppression (true) or unsuppression/exception (false) */ isSuppressed: boolean; diff --git a/packages/code-analyzer-core/test/config.test.ts b/packages/code-analyzer-core/test/config.test.ts index b5dfe4ce..cee2ba8e 100644 --- a/packages/code-analyzer-core/test/config.test.ts +++ b/packages/code-analyzer-core/test/config.test.ts @@ -447,6 +447,12 @@ describe("Tests for creating and accessing configuration values", () => { valueType: 'object', defaultValue: { files: [] }, wasSuppliedByUser: false + }, + suppressions: { + descriptionText: getMessage('ConfigFieldDescription_suppressions'), + valueType: 'object', + defaultValue: { disable_suppressions: false }, + wasSuppliedByUser: false } }); }); diff --git a/packages/code-analyzer-core/test/per-engine-suppressions.test.ts b/packages/code-analyzer-core/test/per-engine-suppressions.test.ts new file mode 100644 index 00000000..f9ec67b2 --- /dev/null +++ b/packages/code-analyzer-core/test/per-engine-suppressions.test.ts @@ -0,0 +1,209 @@ +/** + * Unit tests for per-engine suppression processing + * Tests the new architecture where suppressions are applied before EngineResultsEvent emission + */ + +import * as path from 'node:path'; +import { CodeAnalyzer, CodeAnalyzerConfig, EngineResultsEvent, EventType, LogEvent, LogLevel } from '../src'; +import * as stubs from './stubs'; +import { changeWorkingDirectoryToPackageRoot, FakeFileSystem } from './test-helpers'; + +changeWorkingDirectoryToPackageRoot(); + +describe('Per-Engine Suppression Processing', () => { + const testDataDir = path.resolve(__dirname, 'test-data', 'suppression-markers'); + + it('should emit EngineResultsEvent with already-suppressed violations', async () => { + // Setup + const config = CodeAnalyzerConfig.fromObject({ + suppressions: { disable_suppressions: false }, + log_level: 'error' + }); + const codeAnalyzer = new CodeAnalyzer(config, new FakeFileSystem()); + const plugin = new stubs.StubEnginePlugin(); + await codeAnalyzer.addEnginePlugin(plugin); + + const filePath = path.join(testDataDir, 'file-with-suppress-all.js'); + const workspace = await codeAnalyzer.createWorkspace([filePath]); + + // Listen to EngineResultsEvent + const engineResultsEvents: EngineResultsEvent[] = []; + codeAnalyzer.onEvent(EventType.EngineResultsEvent, (event: EngineResultsEvent) => { + engineResultsEvents.push(event); + }); + + // Configure engine to return violations (lines 7, 8, 9) + const stubEngine1 = plugin.getCreatedEngine('stubEngine1') as stubs.StubEngine1; + stubEngine1.resultsToReturn = { + violations: [ + { + ruleName: 'stub1RuleA', + message: 'Violation on line 7', + codeLocations: [{ file: filePath, startLine: 7, startColumn: 1 }], + primaryLocationIndex: 0 + }, + { + ruleName: 'stub1RuleB', + message: 'Violation on line 8', + codeLocations: [{ file: filePath, startLine: 8, startColumn: 1 }], + primaryLocationIndex: 0 + } + ] + }; + + // Execute + const ruleSelection = await codeAnalyzer.selectRules(['stubEngine1'], { workspace }); + await codeAnalyzer.run(ruleSelection, { workspace }); + + // Verify EngineResultsEvent was emitted with filtered violations + expect(engineResultsEvents.length).toBe(1); + const event = engineResultsEvents[0]; + expect(event.results.getEngineName()).toBe('stubEngine1'); + + // The event should contain 0 violations because file has suppress(all) on line 4 + expect(event.results.getViolations().length).toBe(0); + }); + + it('should cache suppression data across multiple engines processing the same file', async () => { + // Setup + const config = CodeAnalyzerConfig.fromObject({ + suppressions: { disable_suppressions: false }, + log_level: 'error' + }); + const codeAnalyzer = new CodeAnalyzer(config, new FakeFileSystem()); + const plugin = new stubs.StubEnginePlugin(); + await codeAnalyzer.addEnginePlugin(plugin); + + const filePath = path.join(testDataDir, 'file-with-suppress-all.js'); + const workspace = await codeAnalyzer.createWorkspace([filePath]); + + // Configure both engines to return violations for the same file + const stubEngine1 = plugin.getCreatedEngine('stubEngine1') as stubs.StubEngine1; + const stubEngine2 = plugin.getCreatedEngine('stubEngine2') as stubs.StubEngine2; + + stubEngine1.resultsToReturn = { + violations: [ + { + ruleName: 'stub1RuleA', + message: 'Violation from engine1', + codeLocations: [{ file: filePath, startLine: 7, startColumn: 1 }], + primaryLocationIndex: 0 + } + ] + }; + + stubEngine2.resultsToReturn = { + violations: [ + { + ruleName: 'stub2RuleA', + message: 'Violation from engine2', + codeLocations: [{ file: filePath, startLine: 8, startColumn: 1 }], + primaryLocationIndex: 0 + } + ] + }; + + // Execute with both engines + const ruleSelection = await codeAnalyzer.selectRules(['stubEngine1', 'stubEngine2'], { workspace }); + const results = await codeAnalyzer.run(ruleSelection, { workspace }); + + // Verify both engines' violations were suppressed (file has suppress(all)) + expect(results.getViolationCount()).toBe(0); + expect(results.getEngineRunResults('stubEngine1').getViolationCount()).toBe(0); + expect(results.getEngineRunResults('stubEngine2').getViolationCount()).toBe(0); + }); + + it('should log aggregate suppressed violations count at Info level', async () => { + // Setup + const config = CodeAnalyzerConfig.fromObject({ + suppressions: { disable_suppressions: false }, + log_level: 'info' // Set to info to capture the log + }); + const codeAnalyzer = new CodeAnalyzer(config, new FakeFileSystem()); + const plugin = new stubs.StubEnginePlugin(); + await codeAnalyzer.addEnginePlugin(plugin); + + const filePath = path.join(testDataDir, 'file-with-suppress-all.js'); + const workspace = await codeAnalyzer.createWorkspace([filePath]); + + // Listen to LogEvent + const logEvents: LogEvent[] = []; + codeAnalyzer.onEvent(EventType.LogEvent, (event: LogEvent) => { + if (event.logLevel === LogLevel.Info && event.message.includes('suppressed')) { + logEvents.push(event); + } + }); + + // Configure engine to return violations + const stubEngine1 = plugin.getCreatedEngine('stubEngine1') as stubs.StubEngine1; + stubEngine1.resultsToReturn = { + violations: [ + { + ruleName: 'stub1RuleA', + message: 'Violation 1', + codeLocations: [{ file: filePath, startLine: 7, startColumn: 1 }], + primaryLocationIndex: 0 + }, + { + ruleName: 'stub1RuleB', + message: 'Violation 2', + codeLocations: [{ file: filePath, startLine: 8, startColumn: 1 }], + primaryLocationIndex: 0 + } + ] + }; + + // Execute + const ruleSelection = await codeAnalyzer.selectRules(['stubEngine1'], { workspace }); + await codeAnalyzer.run(ruleSelection, { workspace }); + + // Verify aggregate log message was emitted + expect(logEvents.length).toBeGreaterThan(0); + const suppressionLog = logEvents.find(e => e.message.includes('2 violation(s) were suppressed')); + expect(suppressionLog).toBeDefined(); + }); + + it('should log "No violations were suppressed" when no suppressions apply', async () => { + // Setup + const config = CodeAnalyzerConfig.fromObject({ + suppressions: { disable_suppressions: false }, + log_level: 'info' + }); + const codeAnalyzer = new CodeAnalyzer(config, new FakeFileSystem()); + const plugin = new stubs.StubEnginePlugin(); + await codeAnalyzer.addEnginePlugin(plugin); + + const filePath = path.join(testDataDir, 'file-without-markers.js'); + const workspace = await codeAnalyzer.createWorkspace([filePath]); + + // Listen to LogEvent + const logEvents: LogEvent[] = []; + codeAnalyzer.onEvent(EventType.LogEvent, (event: LogEvent) => { + if (event.logLevel === LogLevel.Info && event.message.includes('suppressed')) { + logEvents.push(event); + } + }); + + // Configure engine to return violations + const stubEngine1 = plugin.getCreatedEngine('stubEngine1') as stubs.StubEngine1; + stubEngine1.resultsToReturn = { + violations: [ + { + ruleName: 'stub1RuleA', + message: 'Violation', + codeLocations: [{ file: filePath, startLine: 5, startColumn: 1 }], + primaryLocationIndex: 0 + } + ] + }; + + // Execute + const ruleSelection = await codeAnalyzer.selectRules(['stubEngine1'], { workspace }); + await codeAnalyzer.run(ruleSelection, { workspace }); + + // Verify "No violations were suppressed" log was emitted + expect(logEvents.length).toBeGreaterThan(0); + const noSuppressionLog = logEvents.find(e => e.message.includes('No violations were suppressed')); + expect(noSuppressionLog).toBeDefined(); + }); +}); diff --git a/packages/code-analyzer-core/test/suppressions/suppression-parser.test.ts b/packages/code-analyzer-core/test/suppressions/suppression-parser.test.ts index b53d65cc..f0864fec 100644 --- a/packages/code-analyzer-core/test/suppressions/suppression-parser.test.ts +++ b/packages/code-analyzer-core/test/suppressions/suppression-parser.test.ts @@ -9,6 +9,17 @@ import { parseFileSuppressions } from '../../src/suppressions/suppression-parser'; import { SuppressionMarker, SuppressionRange } from '../../src/suppressions/suppression-types'; +import { toSelector } from '../../src/selectors'; + +// Helper function to create test markers +function createMarker(type: 'suppress' | 'unsuppress', ruleSelectorString: string, lineNumber: number): SuppressionMarker { + return { + type, + ruleSelector: toSelector(ruleSelectorString), + ruleSelectorString, + lineNumber + }; +} describe('parseSuppressionMarkers', () => { it('should find suppress marker with explicit rule selector', () => { @@ -16,11 +27,10 @@ describe('parseSuppressionMarkers', () => { const markers = parseSuppressionMarkers(content, '/test/file.apex'); expect(markers).toHaveLength(1); - expect(markers[0]).toEqual({ - type: 'suppress', - ruleSelector: 'pmd:ApexCrudViolation', - lineNumber: 1 - }); + expect(markers[0].type).toBe('suppress'); + expect(markers[0].ruleSelectorString).toBe('pmd:ApexCrudViolation'); + expect(markers[0].lineNumber).toBe(1); + expect(markers[0].ruleSelector).toBeDefined(); }); it('should find suppress marker with "all" as default when no rule selector', () => { @@ -28,11 +38,10 @@ describe('parseSuppressionMarkers', () => { const markers = parseSuppressionMarkers(content, '/test/file.js'); expect(markers).toHaveLength(1); - expect(markers[0]).toEqual({ - type: 'suppress', - ruleSelector: 'all', - lineNumber: 1 - }); + expect(markers[0].type).toBe('suppress'); + expect(markers[0].ruleSelectorString).toBe('all'); + expect(markers[0].lineNumber).toBe(1); + expect(markers[0].ruleSelector).toBeDefined(); }); it('should find suppress marker without parentheses as "all"', () => { @@ -40,11 +49,10 @@ describe('parseSuppressionMarkers', () => { const markers = parseSuppressionMarkers(content, '/test/file.js'); expect(markers).toHaveLength(1); - expect(markers[0]).toEqual({ - type: 'suppress', - ruleSelector: 'all', - lineNumber: 1 - }); + expect(markers[0].type).toBe('suppress'); + expect(markers[0].ruleSelectorString).toBe('all'); + expect(markers[0].lineNumber).toBe(1); + expect(markers[0].ruleSelector).toBeDefined(); }); it('should find unsuppress marker with explicit rule selector', () => { @@ -52,11 +60,10 @@ describe('parseSuppressionMarkers', () => { const markers = parseSuppressionMarkers(content, '/test/file.xml'); expect(markers).toHaveLength(1); - expect(markers[0]).toEqual({ - type: 'unsuppress', - ruleSelector: 'regex:AvoidOldSalesforceApiVersions', - lineNumber: 1 - }); + expect(markers[0].type).toBe('unsuppress'); + expect(markers[0].ruleSelectorString).toBe('regex:AvoidOldSalesforceApiVersions'); + expect(markers[0].lineNumber).toBe(1); + expect(markers[0].ruleSelector).toBeDefined(); }); it('should find multiple markers on different lines', () => { @@ -67,16 +74,12 @@ public class Test { const markers = parseSuppressionMarkers(content, '/test/file.apex'); expect(markers).toHaveLength(2); - expect(markers[0]).toEqual({ - type: 'suppress', - ruleSelector: 'pmd', - lineNumber: 1 - }); - expect(markers[1]).toEqual({ - type: 'unsuppress', - ruleSelector: 'pmd', - lineNumber: 3 - }); + expect(markers[0].type).toBe('suppress'); + expect(markers[0].ruleSelectorString).toBe('pmd'); + expect(markers[0].lineNumber).toBe(1); + expect(markers[1].type).toBe('unsuppress'); + expect(markers[1].ruleSelectorString).toBe('pmd'); + expect(markers[1].lineNumber).toBe(3); }); it('should find markers in any part of a line, not just comments', () => { @@ -84,7 +87,7 @@ public class Test { const markers = parseSuppressionMarkers(content, '/test/file.json'); expect(markers).toHaveLength(1); - expect(markers[0].ruleSelector).toBe('rule1'); + expect(markers[0].ruleSelectorString).toBe('rule1'); }); it('should handle multiple markers on the same line', () => { @@ -92,8 +95,8 @@ public class Test { const markers = parseSuppressionMarkers(content, '/test/file.js'); expect(markers).toHaveLength(2); - expect(markers[0].ruleSelector).toBe('rule1'); - expect(markers[1].ruleSelector).toBe('rule2'); + expect(markers[0].ruleSelectorString).toBe('rule1'); + expect(markers[1].ruleSelectorString).toBe('rule2'); }); it('should trim whitespace from rule selectors', () => { @@ -101,7 +104,7 @@ public class Test { const markers = parseSuppressionMarkers(content, '/test/file.apex'); expect(markers).toHaveLength(1); - expect(markers[0].ruleSelector).toBe('pmd:SomeRule'); + expect(markers[0].ruleSelectorString).toBe('pmd:SomeRule'); }); it('should handle empty file', () => { @@ -125,7 +128,7 @@ public class Test { const markers = parseSuppressionMarkers(content, '/test/file.js'); expect(markers).toHaveLength(1); - expect(markers[0].ruleSelector).toBe('eslint:(3,4)'); + expect(markers[0].ruleSelectorString).toBe('eslint:(3,4)'); }); it('should be case-insensitive for marker names', () => { @@ -135,176 +138,170 @@ public class Test { const markers = parseSuppressionMarkers(content, '/test/file.js'); expect(markers).toHaveLength(3); - expect(markers[0]).toEqual({ - type: 'suppress', - ruleSelector: 'pmd', - lineNumber: 1 - }); - expect(markers[1]).toEqual({ - type: 'unsuppress', - ruleSelector: 'pmd', - lineNumber: 2 - }); - expect(markers[2]).toEqual({ - type: 'suppress', - ruleSelector: 'eslint', - lineNumber: 3 - }); + expect(markers[0].type).toBe('suppress'); + expect(markers[0].ruleSelectorString).toBe('pmd'); + expect(markers[0].lineNumber).toBe(1); + expect(markers[1].type).toBe('unsuppress'); + expect(markers[1].ruleSelectorString).toBe('pmd'); + expect(markers[1].lineNumber).toBe(2); + expect(markers[2].type).toBe('suppress'); + expect(markers[2].ruleSelectorString).toBe('eslint'); + expect(markers[2].lineNumber).toBe(3); }); }); describe('buildSuppressionRanges', () => { it('should create suppression and unsuppression ranges', () => { const markers: SuppressionMarker[] = [ - { type: 'suppress', ruleSelector: 'pmd', lineNumber: 5 }, - { type: 'unsuppress', ruleSelector: 'pmd', lineNumber: 10 } + createMarker('suppress', 'pmd', 5), + createMarker('unsuppress', 'pmd', 10) ]; const ranges = buildSuppressionRanges(markers, '/test/file.apex'); expect(ranges).toHaveLength(2); // First range: suppressed from line 5-9 - expect(ranges[0]).toEqual({ + expect(ranges[0]).toMatchObject({ startLine: 5, endLine: 9, - ruleSelector: 'pmd', + ruleSelectorString: 'pmd', isSuppressed: true }); // Second range: unsuppressed from line 10 onwards - expect(ranges[1]).toEqual({ + expect(ranges[1]).toMatchObject({ startLine: 10, endLine: undefined, - ruleSelector: 'pmd', + ruleSelectorString: 'pmd', isSuppressed: false }); }); it('should create range to end of file when no unsuppress', () => { const markers: SuppressionMarker[] = [ - { type: 'suppress', ruleSelector: 'all', lineNumber: 3 } + createMarker('suppress', 'all', 3) ]; const ranges = buildSuppressionRanges(markers, '/test/file.js'); expect(ranges).toHaveLength(1); - expect(ranges[0]).toEqual({ + expect(ranges[0]).toMatchObject({ startLine: 3, endLine: undefined, - ruleSelector: 'all', + ruleSelectorString: 'all', isSuppressed: true }); }); it('should handle multiple suppress/unsuppress pairs for same rule', () => { const markers: SuppressionMarker[] = [ - { type: 'suppress', ruleSelector: 'pmd', lineNumber: 5 }, - { type: 'unsuppress', ruleSelector: 'pmd', lineNumber: 10 }, - { type: 'suppress', ruleSelector: 'pmd', lineNumber: 15 }, - { type: 'unsuppress', ruleSelector: 'pmd', lineNumber: 20 } + createMarker('suppress', 'pmd', 5), + createMarker('unsuppress', 'pmd', 10), + createMarker('suppress', 'pmd', 15), + createMarker('unsuppress', 'pmd', 20) ]; const ranges = buildSuppressionRanges(markers, '/test/file.apex'); expect(ranges).toHaveLength(4); - expect(ranges[0]).toEqual({ + expect(ranges[0]).toMatchObject({ startLine: 5, endLine: 9, - ruleSelector: 'pmd', + ruleSelectorString: 'pmd', isSuppressed: true }); - expect(ranges[1]).toEqual({ + expect(ranges[1]).toMatchObject({ startLine: 10, endLine: 14, - ruleSelector: 'pmd', + ruleSelectorString: 'pmd', isSuppressed: false }); - expect(ranges[2]).toEqual({ + expect(ranges[2]).toMatchObject({ startLine: 15, endLine: 19, - ruleSelector: 'pmd', + ruleSelectorString: 'pmd', isSuppressed: true }); - expect(ranges[3]).toEqual({ + expect(ranges[3]).toMatchObject({ startLine: 20, endLine: undefined, - ruleSelector: 'pmd', + ruleSelectorString: 'pmd', isSuppressed: false }); }); it('should handle different rule selectors independently', () => { const markers: SuppressionMarker[] = [ - { type: 'suppress', ruleSelector: 'pmd', lineNumber: 5 }, - { type: 'suppress', ruleSelector: 'eslint', lineNumber: 7 }, - { type: 'unsuppress', ruleSelector: 'pmd', lineNumber: 10 }, - { type: 'unsuppress', ruleSelector: 'eslint', lineNumber: 12 } + createMarker('suppress', 'pmd', 5), + createMarker('suppress', 'eslint', 7), + createMarker('unsuppress', 'pmd', 10), + createMarker('unsuppress', 'eslint', 12) ]; const ranges = buildSuppressionRanges(markers, '/test/file.js'); expect(ranges).toHaveLength(4); - expect(ranges[0]).toEqual({ + expect(ranges[0]).toMatchObject({ startLine: 5, endLine: 9, - ruleSelector: 'pmd', + ruleSelectorString: 'pmd', isSuppressed: true }); - expect(ranges[1]).toEqual({ + expect(ranges[1]).toMatchObject({ startLine: 7, endLine: 11, - ruleSelector: 'eslint', + ruleSelectorString: 'eslint', isSuppressed: true }); - expect(ranges[2]).toEqual({ + expect(ranges[2]).toMatchObject({ startLine: 10, endLine: undefined, - ruleSelector: 'pmd', + ruleSelectorString: 'pmd', isSuppressed: false }); - expect(ranges[3]).toEqual({ + expect(ranges[3]).toMatchObject({ startLine: 12, endLine: undefined, - ruleSelector: 'eslint', + ruleSelectorString: 'eslint', isSuppressed: false }); }); it('should handle unsuppress without matching suppress', () => { const markers: SuppressionMarker[] = [ - { type: 'unsuppress', ruleSelector: 'pmd', lineNumber: 5 } + createMarker('unsuppress', 'pmd', 5) ]; const ranges = buildSuppressionRanges(markers, '/test/file.apex'); expect(ranges).toHaveLength(1); - expect(ranges[0]).toEqual({ + expect(ranges[0]).toMatchObject({ startLine: 5, endLine: undefined, - ruleSelector: 'pmd', + ruleSelectorString: 'pmd', isSuppressed: false }); }); it('should handle nested suppress markers (second suppress is no-op)', () => { const markers: SuppressionMarker[] = [ - { type: 'suppress', ruleSelector: 'all', lineNumber: 5 }, - { type: 'suppress', ruleSelector: 'all', lineNumber: 7 }, // This is ignored - { type: 'unsuppress', ruleSelector: 'all', lineNumber: 10 } + createMarker('suppress', 'all', 5), + createMarker('suppress', 'all', 7), // This is ignored + createMarker('unsuppress', 'all', 10) ]; const ranges = buildSuppressionRanges(markers, '/test/file.js'); expect(ranges).toHaveLength(2); - expect(ranges[0]).toEqual({ + expect(ranges[0]).toMatchObject({ startLine: 5, endLine: 9, - ruleSelector: 'all', + ruleSelectorString: 'all', isSuppressed: true }); - expect(ranges[1]).toEqual({ + expect(ranges[1]).toMatchObject({ startLine: 10, endLine: undefined, - ruleSelector: 'all', + ruleSelectorString: 'all', isSuppressed: false }); }); @@ -320,25 +317,25 @@ describe('buildSuppressionRanges', () => { it('should handle unsuppress(engine) ending suppress(engine:rule)', () => { // Test symmetrical case: broader unsuppress ends more specific suppress const markers: SuppressionMarker[] = [ - { type: 'suppress', ruleSelector: 'eslint:no-unused-vars', lineNumber: 2 }, - { type: 'unsuppress', ruleSelector: 'eslint', lineNumber: 5 } + createMarker('suppress', 'eslint:no-unused-vars', 2), + createMarker('unsuppress', 'eslint', 5) ]; const ranges = buildSuppressionRanges(markers, '/test/file.js'); expect(ranges).toHaveLength(2); // suppress(eslint:no-unused-vars) should end at line 4 (before unsuppress(eslint)) - expect(ranges[0]).toEqual({ + expect(ranges[0]).toMatchObject({ startLine: 2, endLine: 4, - ruleSelector: 'eslint:no-unused-vars', + ruleSelectorString: 'eslint:no-unused-vars', isSuppressed: true }); // unsuppress(eslint) should start at line 5 - expect(ranges[1]).toEqual({ + expect(ranges[1]).toMatchObject({ startLine: 5, endLine: undefined, - ruleSelector: 'eslint', + ruleSelectorString: 'eslint', isSuppressed: false }); }); @@ -346,25 +343,25 @@ describe('buildSuppressionRanges', () => { it('should handle suppress(engine) closing unsuppress(engine:rule)', () => { // Test that broader suppress can close rule-based unsuppress (reverse of previous test) const markers: SuppressionMarker[] = [ - { type: 'unsuppress', ruleSelector: 'eslint:no-unused-vars', lineNumber: 2 }, - { type: 'suppress', ruleSelector: 'eslint', lineNumber: 5 } + createMarker('unsuppress', 'eslint:no-unused-vars', 2), + createMarker('suppress', 'eslint', 5) ]; const ranges = buildSuppressionRanges(markers, '/test/file.js'); expect(ranges).toHaveLength(2); // unsuppress(eslint:no-unused-vars) should end at line 4 (closed by suppress(eslint)) - expect(ranges[0]).toEqual({ + expect(ranges[0]).toMatchObject({ startLine: 2, endLine: 4, - ruleSelector: 'eslint:no-unused-vars', + ruleSelectorString: 'eslint:no-unused-vars', isSuppressed: false }); // suppress(eslint) should start at line 5 - expect(ranges[1]).toEqual({ + expect(ranges[1]).toMatchObject({ startLine: 5, endLine: undefined, - ruleSelector: 'eslint', + ruleSelectorString: 'eslint', isSuppressed: true }); }); @@ -372,9 +369,9 @@ describe('buildSuppressionRanges', () => { it('should handle suppress(engine) closing unsuppress(engine:(severity))', () => { // Test that broader suppress can close severity-based unsuppress const markers: SuppressionMarker[] = [ - { type: 'suppress', ruleSelector: 'all', lineNumber: 2 }, - { type: 'unsuppress', ruleSelector: 'eslint:(3)', lineNumber: 5 }, - { type: 'suppress', ruleSelector: 'eslint', lineNumber: 10 } + createMarker('suppress', 'all', 2), + createMarker('unsuppress', 'eslint:(3)', 5), + createMarker('suppress', 'eslint', 10) ]; const ranges = buildSuppressionRanges(markers, '/test/file.js'); @@ -386,34 +383,34 @@ describe('buildSuppressionRanges', () => { expect(ranges.length).toBe(3); - const allRanges = ranges.filter(r => r.ruleSelector === 'all'); - const eslintSeverityRanges = ranges.filter(r => r.ruleSelector === 'eslint:(3)'); - const eslintRanges = ranges.filter(r => r.ruleSelector === 'eslint'); + const allRanges = ranges.filter(r => r.ruleSelectorString === 'all'); + const eslintSeverityRanges = ranges.filter(r => r.ruleSelectorString === 'eslint:(3)'); + const eslintRanges = ranges.filter(r => r.ruleSelectorString === 'eslint'); // suppress(all) should continue to EOF (unsuppress(eslint:(3)) doesn't end it) expect(allRanges).toHaveLength(1); - expect(allRanges[0]).toEqual({ + expect(allRanges[0]).toMatchObject({ startLine: 2, endLine: undefined, - ruleSelector: 'all', + ruleSelectorString: 'all', isSuppressed: true }); // unsuppress(eslint:(3)) creates exception [5,9] (closed by suppress(eslint)) expect(eslintSeverityRanges).toHaveLength(1); - expect(eslintSeverityRanges[0]).toEqual({ + expect(eslintSeverityRanges[0]).toMatchObject({ startLine: 5, endLine: 9, - ruleSelector: 'eslint:(3)', + ruleSelectorString: 'eslint:(3)', isSuppressed: false }); // suppress(eslint) starts at line 10 expect(eslintRanges).toHaveLength(1); - expect(eslintRanges[0]).toEqual({ + expect(eslintRanges[0]).toMatchObject({ startLine: 10, endLine: undefined, - ruleSelector: 'eslint', + ruleSelectorString: 'eslint', isSuppressed: true }); }); @@ -425,10 +422,10 @@ describe('buildSuppressionRanges', () => { // Line 6: suppress(regex) - suppress regex rules (closes the unsuppress exception) // Line 12: unsuppress(all) - end all suppressions const markers: SuppressionMarker[] = [ - { type: 'suppress', ruleSelector: 'all', lineNumber: 2 }, - { type: 'unsuppress', ruleSelector: 'regex:AvoidOldSalesforceApiVersions', lineNumber: 4 }, - { type: 'suppress', ruleSelector: 'regex', lineNumber: 6 }, - { type: 'unsuppress', ruleSelector: 'all', lineNumber: 12 } + createMarker('suppress', 'all', 2), + createMarker('unsuppress', 'regex:AvoidOldSalesforceApiVersions', 4), + createMarker('suppress', 'regex', 6), + createMarker('unsuppress', 'all', 12) ]; const ranges = buildSuppressionRanges(markers, '/test/file.xml'); @@ -442,41 +439,41 @@ describe('buildSuppressionRanges', () => { expect(ranges.length).toBeGreaterThanOrEqual(3); // Find the ranges for each selector - const allRanges = ranges.filter(r => r.ruleSelector === 'all'); - const regexAvoidRanges = ranges.filter(r => r.ruleSelector === 'regex:AvoidOldSalesforceApiVersions'); - const regexRanges = ranges.filter(r => r.ruleSelector === 'regex'); + const allRanges = ranges.filter(r => r.ruleSelectorString === 'all'); + const regexAvoidRanges = ranges.filter(r => r.ruleSelectorString === 'regex:AvoidOldSalesforceApiVersions'); + const regexRanges = ranges.filter(r => r.ruleSelectorString === 'regex'); // "all" should have suppression [2,11] (continues despite unsuppress at line 4) and unsuppression [12, ∞] - expect(allRanges).toContainEqual({ + expect(allRanges).toContainEqual(expect.objectContaining({ startLine: 2, endLine: 11, - ruleSelector: 'all', + ruleSelectorString: 'all', isSuppressed: true - }); + })); - expect(allRanges).toContainEqual({ + expect(allRanges).toContainEqual(expect.objectContaining({ startLine: 12, endLine: undefined, - ruleSelector: 'all', + ruleSelectorString: 'all', isSuppressed: false - }); + })); // "regex:AvoidOldSalesforceApiVersions" should be unsuppressed [4,5] // closed by suppress(regex) at line 6 - expect(regexAvoidRanges).toContainEqual({ + expect(regexAvoidRanges).toContainEqual(expect.objectContaining({ startLine: 4, endLine: 5, - ruleSelector: 'regex:AvoidOldSalesforceApiVersions', + ruleSelectorString: 'regex:AvoidOldSalesforceApiVersions', isSuppressed: false - }); + })); // "regex" should be suppressed [6,11] - expect(regexRanges).toContainEqual({ + expect(regexRanges).toContainEqual(expect.objectContaining({ startLine: 6, endLine: 11, - ruleSelector: 'regex', + ruleSelectorString: 'regex', isSuppressed: true - }); + })); // Expected behavior when checking violations (with specificity precedence): // Line 3: any violation → SUPPRESSED by "all" [2,11] @@ -503,16 +500,16 @@ describe('parseFileSuppressions', () => { expect(fileSuppressions.filePath).toBe(filePath); expect(fileSuppressions.ranges).toHaveLength(2); - expect(fileSuppressions.ranges[0]).toEqual({ + expect(fileSuppressions.ranges[0]).toMatchObject({ startLine: 2, endLine: 4, - ruleSelector: 'pmd:ApexCrudViolation', + ruleSelectorString: 'pmd:ApexCrudViolation', isSuppressed: true }); - expect(fileSuppressions.ranges[1]).toEqual({ + expect(fileSuppressions.ranges[1]).toMatchObject({ startLine: 5, endLine: undefined, - ruleSelector: 'pmd:ApexCrudViolation', + ruleSelectorString: 'pmd:ApexCrudViolation', isSuppressed: false }); }); diff --git a/packages/code-analyzer-core/test/suppressions/suppression-processor.test.ts b/packages/code-analyzer-core/test/suppressions/suppression-processor.test.ts index 3e8834a6..4eb69444 100644 --- a/packages/code-analyzer-core/test/suppressions/suppression-processor.test.ts +++ b/packages/code-analyzer-core/test/suppressions/suppression-processor.test.ts @@ -8,6 +8,7 @@ import { filterSuppressedViolations } from '../../src/suppressions/suppression-processor'; import { FileSuppressions, SuppressionRange } from '../../src/suppressions/suppression-types'; +import { toSelector } from '../../src/selectors'; import { Violation, CodeLocation, Fix, Suggestion } from '../../src/results'; import { Rule, SeverityLevel } from '../../src/rules'; @@ -127,7 +128,7 @@ describe('isViolationSuppressed', () => { { startLine: 5, endLine: 10, - ruleSelector: 'pmd', + ruleSelector: toSelector('pmd'), ruleSelectorString: 'pmd', isSuppressed: true } ] @@ -150,7 +151,7 @@ describe('isViolationSuppressed', () => { { startLine: 6, endLine: 8, - ruleSelector: 'all', + ruleSelector: toSelector('all'), ruleSelectorString: 'all', isSuppressed: true } ] @@ -174,7 +175,7 @@ describe('isViolationSuppressed', () => { { startLine: 5, endLine: 7, - ruleSelector: 'all', + ruleSelector: toSelector('all'), ruleSelectorString: 'all', isSuppressed: true } ] @@ -198,7 +199,7 @@ describe('isViolationSuppressed', () => { { startLine: 6, endLine: 7, - ruleSelector: 'all', + ruleSelector: toSelector('all'), ruleSelectorString: 'all', isSuppressed: true } ] @@ -222,7 +223,7 @@ describe('isViolationSuppressed', () => { { startLine: 5, endLine: 10, - ruleSelector: 'all', + ruleSelector: toSelector('all'), ruleSelectorString: 'all', isSuppressed: true } ] @@ -246,7 +247,7 @@ describe('isViolationSuppressed', () => { { startLine: 5, endLine: 10, - ruleSelector: 'all', + ruleSelector: toSelector('all'), ruleSelectorString: 'all', isSuppressed: true } ] @@ -272,7 +273,7 @@ describe('isViolationSuppressed', () => { { startLine: 10, endLine: 15, - ruleSelector: 'pmd', + ruleSelector: toSelector('pmd'), ruleSelectorString: 'pmd', isSuppressed: true } ] @@ -296,7 +297,7 @@ describe('isViolationSuppressed', () => { { startLine: 5, endLine: 10, - ruleSelector: 'all', + ruleSelector: toSelector('all'), ruleSelectorString: 'all', isSuppressed: true } ] @@ -319,7 +320,7 @@ describe('isViolationSuppressed', () => { { startLine: 5, endLine: undefined, // End of file - ruleSelector: 'all', + ruleSelector: toSelector('all'), ruleSelectorString: 'all', isSuppressed: true } ] @@ -344,7 +345,7 @@ describe('isViolationSuppressed', () => { { startLine: 5, endLine: 10, - ruleSelector: 'all', + ruleSelector: toSelector('all'), ruleSelectorString: 'all', isSuppressed: true } ] @@ -366,7 +367,7 @@ describe('isViolationSuppressed', () => { { startLine: 5, endLine: 10, - ruleSelector: 'pmd', + ruleSelector: toSelector('pmd'), ruleSelectorString: 'pmd', isSuppressed: true } ] @@ -388,7 +389,7 @@ describe('isViolationSuppressed', () => { { startLine: 5, endLine: 10, - ruleSelector: 'pmd:ApexCrudViolation', + ruleSelector: toSelector('pmd:ApexCrudViolation'), ruleSelectorString: 'pmd:ApexCrudViolation', isSuppressed: true } ] @@ -410,7 +411,7 @@ describe('isViolationSuppressed', () => { { startLine: 5, endLine: 10, - ruleSelector: 'eslint', + ruleSelector: toSelector('eslint'), ruleSelectorString: 'eslint', isSuppressed: true } ] @@ -433,7 +434,7 @@ describe('isViolationSuppressed', () => { { startLine: 5, endLine: 10, - ruleSelector: 'eslint:(3,4)', + ruleSelector: toSelector('eslint:(3,4)'), ruleSelectorString: 'eslint:(3,4)', isSuppressed: true } ] @@ -456,7 +457,7 @@ describe('isViolationSuppressed', () => { { startLine: 5, endLine: 10, - ruleSelector: 'eslint:(3,4)', + ruleSelector: toSelector('eslint:(3,4)'), ruleSelectorString: 'eslint:(3,4)', isSuppressed: true } ] @@ -481,13 +482,13 @@ describe('isViolationSuppressed', () => { { startLine: 1, endLine: 20, - ruleSelector: 'all', + ruleSelector: toSelector('all'), ruleSelectorString: 'all', isSuppressed: true }, { startLine: 5, endLine: undefined, - ruleSelector: 'pmd', + ruleSelector: toSelector('pmd'), ruleSelectorString: 'pmd', isSuppressed: false // Unsuppressed - exception } ] @@ -512,13 +513,13 @@ describe('isViolationSuppressed', () => { { startLine: 1, endLine: 20, - ruleSelector: 'pmd', + ruleSelector: toSelector('pmd'), ruleSelectorString: 'pmd', isSuppressed: false // Unsuppressed }, { startLine: 5, endLine: 15, - ruleSelector: 'pmd:ApexCrudViolation', + ruleSelector: toSelector('pmd:ApexCrudViolation'), ruleSelectorString: 'pmd:ApexCrudViolation', isSuppressed: true // Re-suppressed } ] @@ -543,13 +544,13 @@ describe('isViolationSuppressed', () => { { startLine: 1, endLine: undefined, - ruleSelector: 'pmd', + ruleSelector: toSelector('pmd'), ruleSelectorString: 'pmd', isSuppressed: true }, { startLine: 10, endLine: undefined, - ruleSelector: 'pmd', + ruleSelector: toSelector('pmd'), ruleSelectorString: 'pmd', isSuppressed: false // Unsuppressed later } ] @@ -578,25 +579,25 @@ describe('isViolationSuppressed', () => { { startLine: 2, endLine: 3, - ruleSelector: 'all', + ruleSelector: toSelector('all'), ruleSelectorString: 'all', isSuppressed: true }, { startLine: 4, endLine: 5, // Closed by suppress(regex) at line 6 - ruleSelector: 'regex:AvoidOldSalesforceApiVersions', + ruleSelector: toSelector('regex:AvoidOldSalesforceApiVersions'), ruleSelectorString: 'regex:AvoidOldSalesforceApiVersions', isSuppressed: false }, { startLine: 6, endLine: 11, - ruleSelector: 'regex', + ruleSelector: toSelector('regex'), ruleSelectorString: 'regex', isSuppressed: true }, { startLine: 12, endLine: undefined, - ruleSelector: 'all', + ruleSelector: toSelector('all'), ruleSelectorString: 'all', isSuppressed: false } ] @@ -636,6 +637,318 @@ describe('isViolationSuppressed', () => { }); }); + describe('Complex selector specificity (OR, AND, grouping)', () => { + it('should handle OR selector with minimum specificity rule', () => { + // Test that OR selectors take minimum specificity of both sides + const fileSuppressions: FileSuppressions = { + filePath: '/test/file.js', + ranges: [ + { + startLine: 1, + endLine: 20, + ruleSelector: toSelector('all'), ruleSelectorString: 'all', + isSuppressed: true + }, + { + startLine: 5, + endLine: 15, + ruleSelector: toSelector('eslint,pmd'), ruleSelectorString: 'eslint,pmd', + isSuppressed: false // Unsuppressed + } + ] + }; + + // ESLint violation at line 10 + // 'all' (specificity 1) says suppress + // 'eslint,pmd' (specificity min(2,2) = 2) says don't suppress + // 'eslint,pmd' is more specific, so violation should NOT be suppressed + const eslintViolation = new MockViolation( + new MockRule('eslint', 'no-unused-vars'), + 'Unused variable', + new MockCodeLocation('/test/file.js', 10, 1, 10, 20) + ); + expect(isViolationSuppressed(eslintViolation, fileSuppressions)).toBe(false); + + // PMD violation should also not be suppressed + const pmdViolation = new MockViolation( + new MockRule('pmd', 'UnusedVariable'), + 'Unused variable', + new MockCodeLocation('/test/file.js', 10, 1, 10, 20) + ); + expect(isViolationSuppressed(pmdViolation, fileSuppressions)).toBe(false); + + // Regex violation (not in OR selector) should be suppressed by 'all' + const regexViolation = new MockViolation( + new MockRule('regex', 'SomeRule'), + 'Some issue', + new MockCodeLocation('/test/file.js', 10, 1, 10, 20) + ); + expect(isViolationSuppressed(regexViolation, fileSuppressions)).toBe(true); + }); + + it('should handle OR with different specificities (all,engine)', () => { + // Test with temporal ordering: most recent range wins + const fileSuppressions: FileSuppressions = { + filePath: '/test/file.js', + ranges: [ + { + startLine: 1, + endLine: 20, + ruleSelector: toSelector('eslint:no-unused-vars'), ruleSelectorString: 'eslint:no-unused-vars', + isSuppressed: true + }, + { + startLine: 5, + endLine: 15, + ruleSelector: toSelector('all,pmd'), ruleSelectorString: 'all,pmd', + isSuppressed: false + } + ] + }; + + // ESLint:no-unused-vars violation at line 10 + // Range 1 (startLine=1): suppress + // Range 2 (startLine=5): unsuppress - more recent, so this wins + const violation = new MockViolation( + new MockRule('eslint', 'no-unused-vars'), + 'Unused variable', + new MockCodeLocation('/test/file.js', 10, 1, 10, 20) + ); + expect(isViolationSuppressed(violation, fileSuppressions)).toBe(false); // Most recent wins + }); + + it('should handle multiple colons (higher specificity)', () => { + // Test selectors with multiple colons for higher specificity + const fileSuppressions: FileSuppressions = { + filePath: '/test/file.js', + ranges: [ + { + startLine: 1, + endLine: 20, + ruleSelector: toSelector('eslint:no-unused-vars'), ruleSelectorString: 'eslint:no-unused-vars', + isSuppressed: true + }, + { + startLine: 5, + endLine: 15, + ruleSelector: toSelector('eslint:no-unused-vars:extra'), ruleSelectorString: 'eslint:no-unused-vars:extra', + isSuppressed: false // Specificity 4 (3 colons) + } + ] + }; + + // Violation matching 'eslint:no-unused-vars:extra' + // 'eslint:no-unused-vars' (specificity 3) says suppress + // 'eslint:no-unused-vars:extra' (specificity 4) says don't suppress + // More colons = more specific + const violation = new MockViolation( + new MockRule('eslint', 'no-unused-vars'), + 'Unused variable', + new MockCodeLocation('/test/file.js', 10, 1, 10, 20) + ); + + // Note: This will actually be suppressed because the Selector won't match 'extra' + // But the specificity calculation still works correctly + expect(isViolationSuppressed(violation, fileSuppressions)).toBe(true); + }); + + it('should handle parentheses (transparent specificity)', () => { + // Test that parentheses don't affect specificity + const fileSuppressions: FileSuppressions = { + filePath: '/test/file.js', + ranges: [ + { + startLine: 1, + endLine: 20, + ruleSelector: toSelector('all'), ruleSelectorString: 'all', + isSuppressed: true + }, + { + startLine: 5, + endLine: 15, + ruleSelector: toSelector('(eslint)'), ruleSelectorString: '(eslint)', + isSuppressed: false + } + ] + }; + + // ESLint violation at line 10 + // 'all' (specificity 1) says suppress + // '(eslint)' (specificity 2, same as 'eslint') says don't suppress + const violation = new MockViolation( + new MockRule('eslint', 'no-unused-vars'), + 'Unused variable', + new MockCodeLocation('/test/file.js', 10, 1, 10, 20) + ); + expect(isViolationSuppressed(violation, fileSuppressions)).toBe(false); + }); + + it('should handle complex combination: (engine,engine):rule', () => { + // Test '(eslint,pmd):no-unused-vars' + // This means (eslint OR pmd) AND no-unused-vars + const fileSuppressions: FileSuppressions = { + filePath: '/test/file.js', + ranges: [ + { + startLine: 1, + endLine: 20, + ruleSelector: toSelector('eslint'), ruleSelectorString: 'eslint', + isSuppressed: true + }, + { + startLine: 5, + endLine: 15, + ruleSelector: toSelector('(eslint,pmd):no-unused-vars'), ruleSelectorString: '(eslint,pmd):no-unused-vars', + isSuppressed: false + } + ] + }; + + // ESLint:no-unused-vars violation at line 10 + // 'eslint' (specificity 2) says suppress + // '(eslint,pmd):no-unused-vars' has one colon, so specificity = 3 + // More specific range wins + const violation = new MockViolation( + new MockRule('eslint', 'no-unused-vars'), + 'Unused variable', + new MockCodeLocation('/test/file.js', 10, 1, 10, 20) + ); + expect(isViolationSuppressed(violation, fileSuppressions)).toBe(false); + }); + + it('should handle severity selector: engine:(severity,severity)', () => { + // Test 'eslint:(3,4)' which means eslint with severity 3 OR 4 + const fileSuppressions: FileSuppressions = { + filePath: '/test/file.js', + ranges: [ + { + startLine: 1, + endLine: 20, + ruleSelector: toSelector('all'), ruleSelectorString: 'all', + isSuppressed: true + }, + { + startLine: 5, + endLine: 15, + ruleSelector: toSelector('eslint:(3,4)'), ruleSelectorString: 'eslint:(3,4)', + isSuppressed: false + } + ] + }; + + // ESLint violation with severity 3 at line 10 + // 'all' (specificity 1) says suppress + // 'eslint:(3,4)' (specificity 3: one colon) says don't suppress + const violation = new MockViolation( + new MockRule('eslint', 'no-unused-vars', SeverityLevel.Moderate), // Severity 3 + 'Unused variable', + new MockCodeLocation('/test/file.js', 10, 1, 10, 20) + ); + expect(isViolationSuppressed(violation, fileSuppressions)).toBe(false); + + // ESLint violation with severity 5 should be suppressed (doesn't match (3,4)) + const violation2 = new MockViolation( + new MockRule('eslint', 'no-unused-vars', SeverityLevel.Critical), // Severity 5 + 'Unused variable', + new MockCodeLocation('/test/file.js', 10, 1, 10, 20) + ); + expect(isViolationSuppressed(violation2, fileSuppressions)).toBe(true); + }); + + it('should handle deep nesting: ((engine,engine):rule,all)', () => { + // Test with temporal ordering: most recent range wins + const fileSuppressions: FileSuppressions = { + filePath: '/test/file.js', + ranges: [ + { + startLine: 1, + endLine: 20, + ruleSelector: toSelector('eslint:no-unused-vars'), ruleSelectorString: 'eslint:no-unused-vars', + isSuppressed: true + }, + { + startLine: 5, + endLine: 15, + ruleSelector: toSelector('((eslint,pmd):no-unused-vars,all)'), ruleSelectorString: '((eslint,pmd):no-unused-vars,all)', + isSuppressed: false + } + ] + }; + + // ESLint:no-unused-vars violation at line 10 + // Range 1 (startLine=1): suppress + // Range 2 (startLine=5): unsuppress - more recent, so this wins + const violation = new MockViolation( + new MockRule('eslint', 'no-unused-vars'), + 'Unused variable', + new MockCodeLocation('/test/file.js', 10, 1, 10, 20) + ); + expect(isViolationSuppressed(violation, fileSuppressions)).toBe(false); // Most recent wins + }); + + it('should compare OR selectors correctly when both have same min specificity', () => { + // Test two OR selectors with same minimum specificity + const fileSuppressions: FileSuppressions = { + filePath: '/test/file.js', + ranges: [ + { + startLine: 1, + endLine: 20, + ruleSelector: toSelector('eslint,pmd'), ruleSelectorString: 'eslint,pmd', + isSuppressed: true // Specificity min(2,2) = 2 + }, + { + startLine: 10, + endLine: undefined, + ruleSelector: toSelector('regex,sfge'), ruleSelectorString: 'regex,sfge', + isSuppressed: false // Specificity min(2,2) = 2 + } + ] + }; + + // ESLint violation at line 15 + // Both ranges have same specificity (2) + // Range with higher startLine (10) should win + const violation = new MockViolation( + new MockRule('eslint', 'no-unused-vars'), + 'Unused variable', + new MockCodeLocation('/test/file.js', 15, 1, 15, 20) + ); + expect(isViolationSuppressed(violation, fileSuppressions)).toBe(true); // eslint matches first range + }); + + it('should handle mixed AND and OR: engine:rule,engine:other', () => { + // Test 'eslint:no-unused-vars,pmd:UnusedVariable' + const fileSuppressions: FileSuppressions = { + filePath: '/test/file.js', + ranges: [ + { + startLine: 1, + endLine: 20, + ruleSelector: toSelector('all'), ruleSelectorString: 'all', + isSuppressed: true + }, + { + startLine: 5, + endLine: 15, + ruleSelector: toSelector('eslint:no-unused-vars,pmd:UnusedVariable'), ruleSelectorString: 'eslint:no-unused-vars,pmd:UnusedVariable', + isSuppressed: false // Specificity min(3,3) = 3 + } + ] + }; + + // ESLint:no-unused-vars violation + // 'all' (specificity 1) says suppress + // 'eslint:no-unused-vars,pmd:UnusedVariable' (specificity 3) says don't suppress + const violation = new MockViolation( + new MockRule('eslint', 'no-unused-vars'), + 'Unused variable', + new MockCodeLocation('/test/file.js', 10, 1, 10, 20) + ); + expect(isViolationSuppressed(violation, fileSuppressions)).toBe(false); + }); + }); + describe('Edge cases', () => { it('should return false when fileSuppressions is undefined', () => { const violation = new MockViolation( @@ -669,7 +982,7 @@ describe('isViolationSuppressed', () => { { startLine: 5, endLine: 10, - ruleSelector: 'all', + ruleSelector: toSelector('all'), ruleSelectorString: 'all', isSuppressed: true } ] @@ -691,7 +1004,7 @@ describe('isViolationSuppressed', () => { { startLine: 5, endLine: 10, - ruleSelector: 'all', + ruleSelector: toSelector('all'), ruleSelectorString: 'all', isSuppressed: true } ] @@ -717,7 +1030,7 @@ describe('filterSuppressedViolations', () => { { startLine: 5, endLine: 10, - ruleSelector: 'pmd', + ruleSelector: toSelector('pmd'), ruleSelectorString: 'pmd', isSuppressed: true } ] @@ -750,7 +1063,7 @@ describe('filterSuppressedViolations', () => { { startLine: 5, endLine: 10, - ruleSelector: 'eslint', + ruleSelector: toSelector('eslint'), ruleSelectorString: 'eslint', isSuppressed: true } ] @@ -790,7 +1103,7 @@ describe('filterSuppressedViolations', () => { { startLine: 1, endLine: undefined, - ruleSelector: 'all', + ruleSelector: toSelector('all'), ruleSelectorString: 'all', isSuppressed: true } ]