From 0453d2806e3d432167bcc49f39d6878eac049775 Mon Sep 17 00:00:00 2001 From: Namrata Gupta Date: Fri, 10 Apr 2026 13:20:49 +0530 Subject: [PATCH 01/16] Adding changes for bulk suppressions in core --- .../code-analyzer-core/src/code-analyzer.ts | 112 ++- packages/code-analyzer-core/src/config.ts | 86 +- packages/code-analyzer-core/src/messages.ts | 11 +- .../bulk-suppression-processor.ts | 247 ++++++ .../src/suppressions/index.ts | 9 + .../test/bulk-suppressions-e2e.test.ts | 290 +++++++ .../code-analyzer-core/test/config.test.ts | 10 + packages/code-analyzer-core/test/stubs.ts | 4 + .../test/suppressions-e2e.test.ts | 495 +++++++++++ .../bulk-suppression-processor.test.ts | 791 ++++++++++++++++++ .../bulk-suppression-workspace-root.test.ts | 356 ++++++++ .../suppressions/suppression-parser.test.ts | 35 + .../code-analyzer.yml | 17 + .../bulk-suppressions-workspace/file1.js | 12 + .../bulk-suppressions-workspace/file2.js | 4 + .../src/controllers/controller1.js | 8 + .../src/controllers/controller2.js | 4 + 17 files changed, 2474 insertions(+), 17 deletions(-) create mode 100644 packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts create mode 100644 packages/code-analyzer-core/test/bulk-suppressions-e2e.test.ts create mode 100644 packages/code-analyzer-core/test/suppressions-e2e.test.ts create mode 100644 packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts create mode 100644 packages/code-analyzer-core/test/suppressions/bulk-suppression-workspace-root.test.ts create mode 100644 packages/code-analyzer-core/test/test-data/bulk-suppressions-workspace/code-analyzer.yml create mode 100644 packages/code-analyzer-core/test/test-data/bulk-suppressions-workspace/file1.js create mode 100644 packages/code-analyzer-core/test/test-data/bulk-suppressions-workspace/file2.js create mode 100644 packages/code-analyzer-core/test/test-data/bulk-suppressions-workspace/src/controllers/controller1.js create mode 100644 packages/code-analyzer-core/test/test-data/bulk-suppressions-workspace/src/controllers/controller2.js diff --git a/packages/code-analyzer-core/src/code-analyzer.ts b/packages/code-analyzer-core/src/code-analyzer.ts index 3c592284..6ff954c6 100644 --- a/packages/code-analyzer-core/src/code-analyzer.ts +++ b/packages/code-analyzer-core/src/code-analyzer.ts @@ -10,6 +10,7 @@ import { Violation } from "./results" import {processSuppressions, extractSuppressionsFromFiles, SuppressionsMap, LoggerCallback} from "./suppressions" +import {applyBulkSuppressions, BulkSuppressionQuotas} from "./suppressions/bulk-suppression-processor" import {SemVer} from 'semver'; import { EngineLogEvent, @@ -48,6 +49,14 @@ export interface Workspace { */ getWorkspaceId(): string + /** + * Returns the longest root folder that contains all the workspace paths or null if one does not exist. + * For example, if the workspace was constructed with "/some/folder/subFolder/file1.txt" and + * "/some/folder/file2.txt", then the workspace root folder would be equal to "/some/folder". + * Returns null if a root folder does not exist (e.g., paths from different drives). + */ + getWorkspaceRoot(): string | null + /** * Returns the unique list of files and folders that were used to construct the workspace. */ @@ -123,8 +132,13 @@ export class CodeAnalyzer { // 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; + // Track suppressed violations for aggregate logging (separated by type) + private totalInlineSuppressedViolations: number = 0; + private totalBulkSuppressedViolations: number = 0; + // Bulk suppression quota tracking (shared across files, scoped to config paths) + private readonly bulkSuppressionQuotas: BulkSuppressionQuotas = new Map(); + // Current workspace root (set during run, used for bulk suppression path resolution) + private currentWorkspaceRoot: string | null = null; constructor(config: CodeAnalyzerConfig, fileSystem: FileSystem = new RealFileSystem(), nodeVersion: string = process.version) { this.validateEnvironment(nodeVersion); @@ -353,14 +367,20 @@ 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; + // Reset suppression counters for this run + this.totalInlineSuppressedViolations = 0; + this.totalBulkSuppressedViolations = 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.bulkSuppressionQuotas.clear(); + + // Store workspace root for bulk suppression path resolution (consistent with ignores feature) + // Falls back to config root if workspace root is null (e.g., files from different drives) + this.currentWorkspaceRoot = runOptions.workspace.getWorkspaceRoot() || this.config.getConfigRoot(); this.emitLogEvent(LogLevel.Debug, getMessage('RunningWithWorkspace', JSON.stringify({ filesAndFolders: runOptions.workspace.getRawFilesAndFolders(), @@ -408,12 +428,16 @@ export class CodeAnalyzer { for (const [uninstantiableEngine, error] of this.uninstantiableEnginesMap.entries()) { runResults.addEngineRunResults(new UninstantiableEngineRunResults(uninstantiableEngine, error)); } - - // 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() && this.totalSuppressedViolations > 0) { - this.emitLogEvent(LogLevel.Info, getMessage('SuppressedViolationsCount', this.totalSuppressedViolations)); + if (!this.config.getSuppressionsEnabled()) { + return runResults; + } + // Note: Inline and bulk suppressions are now applied per-engine in runEngineAndValidateResults() before EngineResultsEvent is emitted + // Log aggregate suppression counts if any violations were suppressed (separate messages for inline vs bulk) + if (this.totalInlineSuppressedViolations > 0) { + this.emitLogEvent(LogLevel.Info, getMessage('InlineSuppressedViolationsCount', this.totalInlineSuppressedViolations)); + } + if (this.totalBulkSuppressedViolations > 0) { + this.emitLogEvent(LogLevel.Info, getMessage('BulkSuppressedViolationsCount', this.totalBulkSuppressedViolations)); } return runResults; @@ -473,8 +497,8 @@ export class CodeAnalyzer { return engineRunResults; } - // Track suppressed violations for aggregate logging - this.totalSuppressedViolations += suppressedCount; + // Track inline suppressed violations for aggregate logging + this.totalInlineSuppressedViolations += suppressedCount; // Return filtered results using FilteredEngineRunResults wrapper return this.createFilteredEngineRunResults(engineRunResults, filteredViolations); @@ -500,6 +524,63 @@ export class CodeAnalyzer { }; } + /** + * Applies bulk suppression filtering to a single engine's results + * This processes bulk suppression rules from config and returns a filtered version of the engine results + * @param engineRunResults The engine run results to apply bulk suppressions to + * @returns Filtered engine run results with bulk suppressions applied + */ + private applyBulkSuppressionsToEngineResults( + engineRunResults: EngineRunResults + ): EngineRunResults { + // Check if suppressions are enabled + if (!this.config.getSuppressionsEnabled()) { + return engineRunResults; + } + + const violations = engineRunResults.getViolations(); + if (violations.length === 0) { + return engineRunResults; + } + + const bulkConfig = this.config.getBulkSuppressions(); + if (Object.keys(bulkConfig).length === 0) { + return engineRunResults; // No bulk suppressions configured + } + + // Use workspace root for path resolution (consistent with ignores feature) + // This is set during run() and should never be null at this point + const workspaceRoot = this.currentWorkspaceRoot || this.config.getConfigRoot(); + + // Create logger for bulk suppression debug output + 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 bulkResult = applyBulkSuppressions( + violations, + bulkConfig, + this.bulkSuppressionQuotas, + workspaceRoot, + logger + ); + + const suppressedCount = bulkResult.suppressedCount; + + // If nothing was suppressed, return original results + if (suppressedCount === 0) { + return engineRunResults; + } + + // Track bulk suppressed violations for aggregate logging + this.totalBulkSuppressedViolations += suppressedCount; + + // Return filtered results + return this.createFilteredEngineRunResults(engineRunResults, bulkResult.unsuppressedViolations); + } + /** * Processes files for suppression markers with race condition handling * Uses caching to avoid processing the same file multiple times when multiple engines @@ -672,6 +753,9 @@ export class CodeAnalyzer { // Apply inline suppressions per-engine BEFORE emitting EngineResultsEvent engineRunResults = await this.applyInlineSuppressionsToEngineResults(engineRunResults); + // Apply bulk suppressions per-engine AFTER inline suppressions, still BEFORE emitting EngineResultsEvent + engineRunResults = this.applyBulkSuppressionsToEngineResults(engineRunResults); + this.emitEvent({ type: EventType.EngineRunProgressEvent, timestamp: this.clock.now(), engineName: engineName, percentComplete: 100 }); @@ -845,6 +929,10 @@ class WorkspaceImpl implements Workspace { return this.delegate.getWorkspaceId(); } + getWorkspaceRoot(): string | null { + return this.delegate.getWorkspaceRoot(); + } + getRawFilesAndFolders(): string[] { return this.delegate.getRawFilesAndFolders(); } diff --git a/packages/code-analyzer-core/src/config.ts b/packages/code-analyzer-core/src/config.ts index f67cb814..082f942a 100644 --- a/packages/code-analyzer-core/src/config.ts +++ b/packages/code-analyzer-core/src/config.ts @@ -50,11 +50,21 @@ export type Ignores = { files: string[] } +/** + * Rule for bulk suppression of violations + */ +export type BulkSuppressionRule = { + rule_selector: string; + max_suppressed_violations?: number | null; + reason?: string; +}; + /** * Object containing the user specified suppressions configuration */ export type Suppressions = { - disable_suppressions: boolean + disable_suppressions: boolean; + bulk_suppressions: Record; } type TopLevelConfig = { @@ -75,7 +85,7 @@ export const DEFAULT_CONFIG: TopLevelConfig = { config_root: process.cwd(), log_folder: os.tmpdir(), log_level: LogLevel.Debug, - suppressions: { disable_suppressions: false }, // Suppressions enabled by default + suppressions: { disable_suppressions: false, bulk_suppressions: {} }, // Suppressions enabled by default rules: {}, engines: {}, ignores: { files: [] }, @@ -273,6 +283,13 @@ export class CodeAnalyzerConfig { return this.config.suppressions; } + /** + * Returns the bulk suppressions configuration (file paths mapped to suppression rules). + */ + public getBulkSuppressions(): Record { + return this.config.suppressions.bulk_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. @@ -306,6 +323,15 @@ export class CodeAnalyzerConfig { return this.config.root_working_folder; } + /** + * Returns the names of engines that have at least one rule override in the configuration. + * Used when writing config output to preserve rule overrides (e.g. disabled rules) for engines + * that may have no selected rules. + */ + public getEngineNamesWithRuleOverrides(): string[] { + return Object.keys(this.config.rules); + } + /** * Returns a {@link RuleOverrides} instance containing the user specified overrides for all rules associated with the specified engine * @param engineName name of the engine @@ -394,9 +420,61 @@ function extractIgnoresValue(configExtractor: engApi.ConfigValueExtractor): Igno function extractSuppressionsValue(configExtractor: engApi.ConfigValueExtractor): Suppressions { const suppressionsExtractor: engApi.ConfigValueExtractor = configExtractor.extractObjectAsExtractor(FIELDS.SUPPRESSIONS, DEFAULT_CONFIG.suppressions); - suppressionsExtractor.validateContainsOnlySpecifiedKeys([FIELDS.DISABLE_SUPPRESSIONS]); + const disable_suppressions: boolean = suppressionsExtractor.extractBoolean(FIELDS.DISABLE_SUPPRESSIONS, DEFAULT_CONFIG.suppressions.disable_suppressions) || false; - return { disable_suppressions }; + + // Extract bulk suppressions - all keys except 'disable_suppressions' are file/folder paths + const bulk_suppressions: Record = {}; + const suppressionKeys = suppressionsExtractor.getKeys(); + + for (const key of suppressionKeys) { + if (key === FIELDS.DISABLE_SUPPRESSIONS) { + continue; // Skip the disable_suppressions flag + } + + // key is a file/folder path, value should be an array of suppression rules + const rulesArray = suppressionsExtractor.extractArray( + key, + (value, fieldPath) => validateBulkSuppressionRule(value, fieldPath) + ); + + if (rulesArray && rulesArray.length > 0) { + bulk_suppressions[key] = rulesArray; + } + } + + return { disable_suppressions, bulk_suppressions }; +} + +function validateBulkSuppressionRule(value: unknown, fieldPath: string): BulkSuppressionRule { + if (typeof value !== 'object' || value === null || Array.isArray(value)) { + throw new Error(getMessage('InvalidBulkSuppressionRule', fieldPath, 'Expected an object')); + } + + const rule = value as Record; + + // rule_selector is required + if (!rule.rule_selector || typeof rule.rule_selector !== 'string') { + throw new Error(getMessage('InvalidBulkSuppressionRule', fieldPath, 'rule_selector is required and must be a string')); + } + + // max_suppressed_violations is optional, can be number or null + if (rule.max_suppressed_violations !== undefined && + rule.max_suppressed_violations !== null && + typeof rule.max_suppressed_violations !== 'number') { + throw new Error(getMessage('InvalidBulkSuppressionRule', fieldPath, 'max_suppressed_violations must be a number or null')); + } + + // reason is optional + if (rule.reason !== undefined && typeof rule.reason !== 'string') { + throw new Error(getMessage('InvalidBulkSuppressionRule', fieldPath, 'reason must be a string')); + } + + return { + rule_selector: rule.rule_selector as string, + max_suppressed_violations: rule.max_suppressed_violations as number | null | undefined, + reason: rule.reason as string | undefined + }; } /** diff --git a/packages/code-analyzer-core/src/messages.ts b/packages/code-analyzer-core/src/messages.ts index dc97ca57..baeccf6d 100644 --- a/packages/code-analyzer-core/src/messages.ts +++ b/packages/code-analyzer-core/src/messages.ts @@ -152,6 +152,9 @@ const MESSAGE_CATALOG : MessageCatalog = { InvalidGlobPattern: `The configuration field '%s' contains an invalid glob pattern '%s': %s`, + InvalidBulkSuppressionRule: + `The bulk suppression rule at '%s' is invalid: %s`, + RulePropertyOverridden: `The %s value of rule '%s' of engine '%s' was overridden according to the specified configuration. The old value '%s' was replaced with the new value '%s'.`, @@ -240,7 +243,13 @@ const MESSAGE_CATALOG : MessageCatalog = { `Since the engine '%s' emitted an error, the following temporary working folder will not be removed: %s`, SuppressedViolationsCount: - `%d violation(s) were suppressed by inline suppression markers.` + `%d violation(s) were suppressed by inline suppression markers.`, + + InlineSuppressedViolationsCount: + `%d violation(s) were suppressed by inline suppression markers.`, + + BulkSuppressedViolationsCount: + `%d violation(s) were suppressed by bulk suppressions.` } /** diff --git a/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts b/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts new file mode 100644 index 00000000..066f51a2 --- /dev/null +++ b/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts @@ -0,0 +1,247 @@ +import {Violation} from "../results"; +import {BulkSuppressionRule} from "../config"; +import {toSelector, Selector} from "../selectors"; +import {SeverityLevel} from "../rules"; +import * as path from 'node:path'; +import {LoggerCallback} from "./suppression-processor"; + +/** + * Tracks the number of suppressions applied per config path and rule selector combination + * Key format: "configPath|ruleSelector" - shared across all files matching the config path + */ +export type BulkSuppressionQuotas = Map; // key: "configPath|ruleSelector" + +/** + * Bulk suppression rule paired with its config path (for quota tracking) + */ +type RuleWithConfigPath = { + rule: BulkSuppressionRule; + configPath: string; // The path from config that matched (e.g., "src/" or "src/file.js") + ruleIndex: number; // Index in the rules array for unique quota tracking +}; + +/** + * Result of applying bulk suppressions to a list of violations + */ +export type BulkSuppressionResult = { + unsuppressedViolations: Violation[]; + suppressedCount: number; +}; + +/** + * Applies bulk suppressions to violations based on config, updating quota tracking + * @param violations List of violations to process + * @param bulkConfig Bulk suppression configuration from YAML + * @param quotas Shared quota tracker (mutated by this function) + * @param workspaceRoot Root directory for resolving relative paths + * @param logger Optional logger callback for debug/info messages + * @returns Object containing unsuppressed violations and count of suppressions applied + */ +export function applyBulkSuppressions( + violations: Violation[], + bulkConfig: Record, + quotas: BulkSuppressionQuotas, + workspaceRoot: string, + logger?: LoggerCallback +): BulkSuppressionResult { + const log = (msg: string) => logger?.('debug', msg); + + log(`Bulk suppressions: processing ${violations.length} violation(s) with workspace root ${workspaceRoot}`); + + if (Object.keys(bulkConfig).length === 0) { + log('No bulk suppressions configured'); + return { unsuppressedViolations: violations, suppressedCount: 0 }; + } + + log(`Bulk suppression config paths: ${Object.keys(bulkConfig).join(', ')}`); + + // Sort violations for deterministic processing within this engine + const sortedViolations = [...violations].sort((a, b) => { + const fileA = a.getPrimaryLocation().getFile() || ''; + const fileB = b.getPrimaryLocation().getFile() || ''; + if (fileA !== fileB) { + return fileA.localeCompare(fileB); + } + + const lineA = a.getPrimaryLocation().getStartLine() || 0; + const lineB = b.getPrimaryLocation().getStartLine() || 0; + return lineA - lineB; + }); + + const unsuppressedViolations: Violation[] = []; + let suppressedCount = 0; + + for (const violation of sortedViolations) { + const violationFile = violation.getPrimaryLocation().getFile(); + if (!violationFile) { + // Skip violations id no file path is present + unsuppressedViolations.push(violation); + continue; + } + + // Find matching suppression rules for this violation's file + const matchingRules = findMatchingBulkSuppressionRules(violationFile, bulkConfig, workspaceRoot); + + let suppressed = false; + for (const ruleWithPath of matchingRules) { + if (shouldSuppressViolation(violation, ruleWithPath.rule, quotas, ruleWithPath.configPath, ruleWithPath.ruleIndex, logger)) { + suppressed = true; + suppressedCount++; + break; // Violation suppressed, move to next + } + } + + if (!suppressed) { + unsuppressedViolations.push(violation); + } + } + + log(`Bulk suppressions: ${suppressedCount} suppressed, ${unsuppressedViolations.length} unsuppressed`); + + return { unsuppressedViolations, suppressedCount }; +} + +/** + * Finds bulk suppression rules that match the given file path + * Returns rules paired with their config paths for shared quota tracking + * @param violationFile Absolute file path from violation + * @param bulkConfig Bulk suppression configuration + * @param workspaceRoot Root directory for resolving relative paths + * @returns Array of rules with their matching config paths + */ +function findMatchingBulkSuppressionRules( + violationFile: string, + bulkConfig: Record, + workspaceRoot: string +): RuleWithConfigPath[] { + const matchingRules: RuleWithConfigPath[] = []; + + for (const [configPath, rules] of Object.entries(bulkConfig)) { + const matches = doesFileMatchConfigPath(violationFile, configPath, workspaceRoot); + + if (matches) { + // Add each rule with its config path and index for unique quota tracking + for (let i = 0; i < rules.length; i++) { + matchingRules.push({ rule: rules[i], configPath, ruleIndex: i }); + } + } + } + + return matchingRules; +} + +/** + * Checks if a file path matches a config path (file or folder) + * @param violationFile Absolute file path from violation + * @param configPath Relative path from config (file or folder) + * @param workspaceRoot Root directory for resolving relative paths + * @returns true if the file matches + */ +function doesFileMatchConfigPath( + violationFile: string, + configPath: string, + workspaceRoot: string +): boolean { + // Convert config path to absolute + const absoluteConfigPath = path.isAbsolute(configPath) + ? configPath + : path.resolve(workspaceRoot, configPath); + + // Normalize paths for comparison + const normalizedViolationFile = path.normalize(violationFile); + const normalizedConfigPath = path.normalize(absoluteConfigPath); + + // Check if it's an exact file match + if (normalizedViolationFile === normalizedConfigPath) { + return true; + } + + // Check if violation file is within config folder + // Config path is a folder if it matches the start of the file path + const configPathWithSep = normalizedConfigPath.endsWith(path.sep) + ? normalizedConfigPath + : normalizedConfigPath + path.sep; + + return normalizedViolationFile.startsWith(configPathWithSep); +} + +/** + * Determines if a violation should be suppressed based on a bulk suppression rule + * Updates the quota tracker if suppression is applied + * Uses shared quota across all files matching the config path, but each array entry gets independent quota + * @param violation The violation to check + * @param rule The bulk suppression rule to apply + * @param quotas Quota tracker (mutated if suppressed) - shared across config path + * @param configPath Config path from YAML (e.g., "src/" or "src/file.js") for quota tracking + * @param ruleIndex Index of rule in the array (for unique quota when duplicates exist) + * @returns true if violation should be suppressed + */ +function shouldSuppressViolation( + violation: Violation, + rule: BulkSuppressionRule, + quotas: BulkSuppressionQuotas, + configPath: string, + ruleIndex: number, + logger?: LoggerCallback +): boolean { + // Check if the rule selector matches this violation + if (!ruleMatches(violation, rule.rule_selector, logger)) { + return false; + } + + // Quota key includes rule index to give each array entry independent quota + // This allows duplicate rules to each have their own quota allocation + const quotaKey = `${configPath}|${ruleIndex}|${rule.rule_selector}`; + const currentCount = quotas.get(quotaKey) || 0; + const maxLimit = rule.max_suppressed_violations; + + // If maxLimit is null/undefined, suppress all matching violations + if (maxLimit === null || maxLimit === undefined) { + quotas.set(quotaKey, currentCount + 1); + return true; + } + + // Check if we've reached the quota limit (shared across all files in config path) + if (currentCount < maxLimit) { + quotas.set(quotaKey, currentCount + 1); + return true; + } + + // Quota exceeded, don't suppress + return false; +} + +/** + * Checks if a rule selector matches a violation + * Uses the same rule selector matching logic as rule selection + * @param violation The violation to check + * @param ruleSelector The rule selector string (e.g., "pmd:UnusedMethod", "3,4", etc.) + * @param logger Optional logger callback for debug messages + * @returns true if the violation matches the selector + */ +function ruleMatches(violation: Violation, ruleSelector: string, logger?: LoggerCallback): boolean { + const log = (msg: string) => logger?.('debug', msg); + + try { + const selector: Selector = toSelector(ruleSelector); + const rule = violation.getRule(); + + // Build selectables array from rule (same as Rule.matchesRuleSelector) + 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 selector.matchesSelectables(selectables); + } catch (error) { + // If selector is invalid, don't match + log(`Invalid rule selector "${ruleSelector}": ${error instanceof Error ? error.message : String(error)}`); + return false; + } +} diff --git a/packages/code-analyzer-core/src/suppressions/index.ts b/packages/code-analyzer-core/src/suppressions/index.ts index fee3ef4f..5560b985 100644 --- a/packages/code-analyzer-core/src/suppressions/index.ts +++ b/packages/code-analyzer-core/src/suppressions/index.ts @@ -23,3 +23,12 @@ export { } from './suppression-processor'; export type { LoggerCallback } from './suppression-processor'; + +export { + applyBulkSuppressions +} from './bulk-suppression-processor'; + +export type { + BulkSuppressionQuotas, + BulkSuppressionResult +} from './bulk-suppression-processor'; diff --git a/packages/code-analyzer-core/test/bulk-suppressions-e2e.test.ts b/packages/code-analyzer-core/test/bulk-suppressions-e2e.test.ts new file mode 100644 index 00000000..73cd2806 --- /dev/null +++ b/packages/code-analyzer-core/test/bulk-suppressions-e2e.test.ts @@ -0,0 +1,290 @@ +/** + * End-to-end integration tests for bulk suppressions with actual engine execution + * These tests run real stub engines to verify bulk suppressions work in the full flow + * with YAML config files and test workspaces + */ + +import * as path from 'node:path'; +import { CodeAnalyzer } from '../src/code-analyzer'; +import { CodeAnalyzerConfig } from '../src/config'; +import * as stubs from './stubs'; +import { changeWorkingDirectoryToPackageRoot, FakeFileSystem } from './test-helpers'; + +changeWorkingDirectoryToPackageRoot(); + +describe('Bulk Suppressions E2E Tests (with YAML config and test workspace)', () => { + const testWorkspaceDir = path.resolve(__dirname, 'test-data', 'bulk-suppressions-workspace'); + const configFile = path.join(testWorkspaceDir, 'code-analyzer.yml'); + + describe('file-level bulk suppressions', () => { + it('should suppress violations up to quota limit for specific file', async () => { + // Config has: file1.js with max_suppressed_violations: 2 + // We'll create 3 violations in file1.js + // Expected: 2 suppressed, 1 unsuppressed + + const config = await CodeAnalyzerConfig.fromFile(configFile); + const codeAnalyzer = new CodeAnalyzer(config, new FakeFileSystem()); + + const plugin = new stubs.StubEnginePlugin(); + await codeAnalyzer.addEnginePlugin(plugin); + + const file1Path = path.join(testWorkspaceDir, 'file1.js'); + const workspace = await codeAnalyzer.createWorkspace([file1Path]); + const ruleSelection = await codeAnalyzer.selectRules(['stubEngine1'], { workspace }); + + // Create 3 violations in file1.js + const stubEngine1 = plugin.getCreatedEngine('stubEngine1') as stubs.StubEngine1; + stubEngine1.resultsToReturn = { + violations: [ + { + ruleName: 'stub1RuleA', + message: 'Violation 1', + codeLocations: [{ file: file1Path, startLine: 3, startColumn: 1 }], + primaryLocationIndex: 0 + }, + { + ruleName: 'stub1RuleA', + message: 'Violation 2', + codeLocations: [{ file: file1Path, startLine: 6, startColumn: 1 }], + primaryLocationIndex: 0 + }, + { + ruleName: 'stub1RuleA', + message: 'Violation 3', + codeLocations: [{ file: file1Path, startLine: 10, startColumn: 1 }], + primaryLocationIndex: 0 + } + ] + }; + + const results = await codeAnalyzer.run(ruleSelection, { workspace }); + const violations = results.getViolations(); + + // Only 1 violation should remain (quota was 2) + expect(violations.length).toBe(1); + expect(violations[0].getPrimaryLocation().getStartLine()).toBe(10); + }); + + it('should suppress all violations with null quota limit', async () => { + // Config has: file2.js with max_suppressed_violations: null (unlimited) + // Expected: all violations suppressed + + const config = await CodeAnalyzerConfig.fromFile(configFile); + const codeAnalyzer = new CodeAnalyzer(config, new FakeFileSystem()); + + const plugin = new stubs.StubEnginePlugin(); + await codeAnalyzer.addEnginePlugin(plugin); + + const file2Path = path.join(testWorkspaceDir, 'file2.js'); + const workspace = await codeAnalyzer.createWorkspace([file2Path]); + const ruleSelection = await codeAnalyzer.selectRules(['stubEngine1'], { workspace }); + + const stubEngine1 = plugin.getCreatedEngine('stubEngine1') as stubs.StubEngine1; + stubEngine1.resultsToReturn = { + violations: [ + { + ruleName: 'stub1RuleA', + message: 'Violation 1', + codeLocations: [{ file: file2Path, startLine: 3, startColumn: 1 }], + primaryLocationIndex: 0 + } + ] + }; + + const results = await codeAnalyzer.run(ruleSelection, { workspace }); + const violations = results.getViolations(); + + // All violations should be suppressed + expect(violations.length).toBe(0); + }); + }); + + describe('folder-level bulk suppressions with shared quota', () => { + it('should share quota across all files in folder', async () => { + // Config has: src/controllers/ with max_suppressed_violations: 2 (shared) + // We have controller1.js with 2 violations and controller2.js with 1 violation + // Expected: First 2 violations suppressed (across files), 1 remains + + const config = await CodeAnalyzerConfig.fromFile(configFile); + const codeAnalyzer = new CodeAnalyzer(config, new FakeFileSystem()); + + const plugin = new stubs.StubEnginePlugin(); + await codeAnalyzer.addEnginePlugin(plugin); + + const controller1Path = path.join(testWorkspaceDir, 'src', 'controllers', 'controller1.js'); + const controller2Path = path.join(testWorkspaceDir, 'src', 'controllers', 'controller2.js'); + // Pass testWorkspaceDir as workspace folder to set correct workspace root + const workspace = await codeAnalyzer.createWorkspace([testWorkspaceDir], [controller1Path, controller2Path]); + const ruleSelection = await codeAnalyzer.selectRules(['stubEngine1'], { workspace }); + + const stubEngine1 = plugin.getCreatedEngine('stubEngine1') as stubs.StubEngine1; + stubEngine1.resultsToReturn = { + violations: [ + { + ruleName: 'stub1RuleA', + message: 'Violation in controller1 line 3', + codeLocations: [{ file: controller1Path, startLine: 3, startColumn: 1 }], + primaryLocationIndex: 0 + }, + { + ruleName: 'stub1RuleA', + message: 'Violation in controller1 line 6', + codeLocations: [{ file: controller1Path, startLine: 6, startColumn: 1 }], + primaryLocationIndex: 0 + }, + { + ruleName: 'stub1RuleA', + message: 'Violation in controller2 line 3', + codeLocations: [{ file: controller2Path, startLine: 3, startColumn: 1 }], + primaryLocationIndex: 0 + } + ] + }; + + const results = await codeAnalyzer.run(ruleSelection, { workspace }); + const violations = results.getViolations(); + + // Only 1 violation should remain (quota of 2 shared across folder) + expect(violations.length).toBe(1); + expect(violations[0].getPrimaryLocation().getFile()).toBe(controller2Path); + expect(violations[0].getPrimaryLocation().getStartLine()).toBe(3); + }); + }); + + describe('multiple rules with duplicate selectors', () => { + it('should give each array entry independent quota', async () => { + // Create a custom config with duplicate selectors + const customConfig = CodeAnalyzerConfig.fromObject({ + suppressions: { + disable_suppressions: false, + 'file1.js': [ + { rule_selector: 'all', max_suppressed_violations: 2 }, + { rule_selector: 'all', max_suppressed_violations: 1 } // Duplicate + ] + }, + log_level: 'error' + }); + + const codeAnalyzer = new CodeAnalyzer(customConfig, new FakeFileSystem()); + const plugin = new stubs.StubEnginePlugin(); + await codeAnalyzer.addEnginePlugin(plugin); + + const file1Path = path.join(testWorkspaceDir, 'file1.js'); + const workspace = await codeAnalyzer.createWorkspace([file1Path]); + const ruleSelection = await codeAnalyzer.selectRules(['stubEngine1'], { workspace }); + + // Create 4 violations + const stubEngine1 = plugin.getCreatedEngine('stubEngine1') as stubs.StubEngine1; + stubEngine1.resultsToReturn = { + violations: [ + { + ruleName: 'stub1RuleA', + message: 'Violation 1', + codeLocations: [{ file: file1Path, startLine: 3, startColumn: 1 }], + primaryLocationIndex: 0 + }, + { + ruleName: 'stub1RuleA', + message: 'Violation 2', + codeLocations: [{ file: file1Path, startLine: 6, startColumn: 1 }], + primaryLocationIndex: 0 + }, + { + ruleName: 'stub1RuleA', + message: 'Violation 3', + codeLocations: [{ file: file1Path, startLine: 10, startColumn: 1 }], + primaryLocationIndex: 0 + }, + { + ruleName: 'stub1RuleA', + message: 'Violation 4', + codeLocations: [{ file: file1Path, startLine: 11, startColumn: 1 }], + primaryLocationIndex: 0 + } + ] + }; + + const results = await codeAnalyzer.run(ruleSelection, { workspace }); + const violations = results.getViolations(); + + // Should suppress 3 total (2 + 1), leaving 1 unsuppressed + expect(violations.length).toBe(1); + expect(violations[0].getPrimaryLocation().getStartLine()).toBe(11); + }); + }); + + describe('workspace root path resolution', () => { + it('should resolve paths relative to workspace root, not config file location', async () => { + // This test verifies Bug #2 fix: paths in config are relative to workspace root + // Config is in testWorkspaceDir but paths should be relative to workspace root + + const config = await CodeAnalyzerConfig.fromFile(configFile); + const codeAnalyzer = new CodeAnalyzer(config, new FakeFileSystem()); + + const plugin = new stubs.StubEnginePlugin(); + await codeAnalyzer.addEnginePlugin(plugin); + + const file1Path = path.join(testWorkspaceDir, 'file1.js'); + const workspace = await codeAnalyzer.createWorkspace([file1Path]); + const ruleSelection = await codeAnalyzer.selectRules(['stubEngine1'], { workspace }); + + const stubEngine1 = plugin.getCreatedEngine('stubEngine1') as stubs.StubEngine1; + stubEngine1.resultsToReturn = { + violations: [ + { + ruleName: 'stub1RuleA', + message: 'Violation', + codeLocations: [{ file: file1Path, startLine: 3, startColumn: 1 }], + primaryLocationIndex: 0 + } + ] + }; + + const results = await codeAnalyzer.run(ruleSelection, { workspace }); + const violations = results.getViolations(); + + // Should be suppressed because config path "file1.js" is relative to workspace root + expect(violations.length).toBe(0); + }); + }); + + describe('with suppressions disabled', () => { + it('should not suppress any violations when disable_suppressions is true', async () => { + const config = CodeAnalyzerConfig.fromObject({ + suppressions: { + disable_suppressions: true, + 'file1.js': [ + { rule_selector: 'all', max_suppressed_violations: null } + ] + }, + log_level: 'error' + }); + + const codeAnalyzer = new CodeAnalyzer(config, new FakeFileSystem()); + const plugin = new stubs.StubEnginePlugin(); + await codeAnalyzer.addEnginePlugin(plugin); + + const file1Path = path.join(testWorkspaceDir, 'file1.js'); + const workspace = await codeAnalyzer.createWorkspace([file1Path]); + const ruleSelection = await codeAnalyzer.selectRules(['stubEngine1'], { workspace }); + + const stubEngine1 = plugin.getCreatedEngine('stubEngine1') as stubs.StubEngine1; + stubEngine1.resultsToReturn = { + violations: [ + { + ruleName: 'stub1RuleA', + message: 'Violation', + codeLocations: [{ file: file1Path, startLine: 3, startColumn: 1 }], + primaryLocationIndex: 0 + } + ] + }; + + const results = await codeAnalyzer.run(ruleSelection, { workspace }); + const violations = results.getViolations(); + + // Should NOT be suppressed (suppressions disabled) + expect(violations.length).toBe(1); + }); + }); +}); diff --git a/packages/code-analyzer-core/test/config.test.ts b/packages/code-analyzer-core/test/config.test.ts index cee2ba8e..646c69e9 100644 --- a/packages/code-analyzer-core/test/config.test.ts +++ b/packages/code-analyzer-core/test/config.test.ts @@ -293,6 +293,16 @@ describe("Tests for creating and accessing configuration values", () => { }); }); + it("When config has rule overrides, getEngineNamesWithRuleOverrides returns engine names that have at least one rule override", () => { + const conf: CodeAnalyzerConfig = CodeAnalyzerConfig.fromFile(path.join(TEST_DATA_DIR, 'sample-config-with-disabled-rule.yaml')); + expect(conf.getEngineNamesWithRuleOverrides()).toEqual(['stubEngine1', 'stubEngine2']); + }); + + it("When config has no rule overrides, getEngineNamesWithRuleOverrides returns empty array", () => { + const conf: CodeAnalyzerConfig = CodeAnalyzerConfig.withDefaults(); + expect(conf.getEngineNamesWithRuleOverrides()).toEqual([]); + }); + it("When log_folder does not exist, then throw an error", () => { const nonExistingFolder: string = path.resolve(__dirname, "doesNotExist"); expect(() => CodeAnalyzerConfig.fromObject({log_folder: nonExistingFolder})).toThrow( diff --git a/packages/code-analyzer-core/test/stubs.ts b/packages/code-analyzer-core/test/stubs.ts index 0dee059c..52e0296f 100644 --- a/packages/code-analyzer-core/test/stubs.ts +++ b/packages/code-analyzer-core/test/stubs.ts @@ -13,6 +13,10 @@ export class StubWorkspace implements Workspace { return "dummyId"; } + getWorkspaceRoot(): string | null { + return SAMPLE_WORKSPACE_FOLDER; + } + getRawFilesAndFolders(): string[] { return [path.join(SAMPLE_WORKSPACE_FOLDER)]; } diff --git a/packages/code-analyzer-core/test/suppressions-e2e.test.ts b/packages/code-analyzer-core/test/suppressions-e2e.test.ts new file mode 100644 index 00000000..50bdbc82 --- /dev/null +++ b/packages/code-analyzer-core/test/suppressions-e2e.test.ts @@ -0,0 +1,495 @@ +/** + * End-to-end integration tests for suppression markers with actual engine execution + * These tests run real engines (stub engines) to verify suppressions work in the full flow + */ + +import * as path from 'node:path'; +import { CodeAnalyzer } from '../src/code-analyzer'; +import { CodeAnalyzerConfig } from '../src/config'; +import * as stubs from './stubs'; +import { SeverityLevel } from '@salesforce/code-analyzer-engine-api'; +import { changeWorkingDirectoryToPackageRoot, FakeFileSystem } from './test-helpers'; + +changeWorkingDirectoryToPackageRoot(); + +describe('Suppression Markers E2E Tests (with actual engine execution)', () => { + const testDataDir = path.resolve(__dirname, 'test-data', 'suppression-markers'); + + describe('with suppressions enabled', () => { + it('should suppress all violations with suppress(all) marker', async () => { + // Create CodeAnalyzer with suppressions enabled + const config = CodeAnalyzerConfig.fromObject({ + suppressions: { disable_suppressions: false }, + log_level: 'error' + }); + const codeAnalyzer = new CodeAnalyzer(config, new FakeFileSystem()); + + // Add stub engine plugin + const plugin = new stubs.StubEnginePlugin(); + await codeAnalyzer.addEnginePlugin(plugin); + + // Create workspace with test file + const filePath = path.join(testDataDir, 'file-with-suppress-all.js'); + const workspace = await codeAnalyzer.createWorkspace([filePath]); + + // Select rules + const ruleSelection = await codeAnalyzer.selectRules(['stubEngine1'], { workspace }); + + // Configure engine to return violations + 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 + }, + { + ruleName: 'stub1RuleC', + message: 'Violation on line 9', + codeLocations: [{ file: filePath, startLine: 9, startColumn: 1 }], + primaryLocationIndex: 0 + } + ] + }; + + // Run the analyzer (this actually runs the engine) + const results = await codeAnalyzer.run(ruleSelection, { workspace }); + + // All violations should be suppressed (suppress(all) on line 6) + const violations = results.getViolations(); + expect(violations.length).toBe(0); + }); + + it('should suppress only engine-specific violations with suppress(engine)', async () => { + 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, 'e2e-file-with-suppress-engine.js'); + const workspace = await codeAnalyzer.createWorkspace([filePath]); + const ruleSelection = await codeAnalyzer.selectRules(['stubEngine1'], { workspace }); + + const stubEngine1 = plugin.getCreatedEngine('stubEngine1') as stubs.StubEngine1; + stubEngine1.resultsToReturn = { + violations: [ + { + ruleName: 'stub1RuleA', + message: 'Violation on line 6 (before marker)', + codeLocations: [{ file: filePath, startLine: 6, startColumn: 1 }], + primaryLocationIndex: 0 + }, + { + ruleName: 'stub1RuleB', + message: 'Violation on line 9 (after suppress(stubEngine1))', + codeLocations: [{ file: filePath, startLine: 9, startColumn: 1 }], + primaryLocationIndex: 0 + } + ] + }; + + const results = await codeAnalyzer.run(ruleSelection, { workspace }); + + // Only violation before marker should remain + const violations = results.getViolations(); + expect(violations.length).toBe(1); + expect(violations[0].getPrimaryLocation().getStartLine()).toBe(6); + }); + + it('should suppress only specific rule violations with suppress(engine:rule)', async () => { + 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); + + // File has: suppress(stubEngine1:stub1RuleA) on line 8 + const filePath = path.join(testDataDir, 'e2e-file-with-suppress-specific-rule.js'); + const workspace = await codeAnalyzer.createWorkspace([filePath]); + const ruleSelection = await codeAnalyzer.selectRules(['stubEngine1'], { workspace }); + + const stubEngine1 = plugin.getCreatedEngine('stubEngine1') as stubs.StubEngine1; + stubEngine1.resultsToReturn = { + violations: [ + { + ruleName: 'stub1RuleA', + message: 'Rule A on line 6 (before marker)', + codeLocations: [{ file: filePath, startLine: 6, startColumn: 1 }], + primaryLocationIndex: 0 + }, + { + ruleName: 'stub1RuleA', + message: 'Rule A on line 9 (should be suppressed)', + codeLocations: [{ file: filePath, startLine: 9, startColumn: 1 }], + primaryLocationIndex: 0 + }, + { + ruleName: 'stub1RuleB', + message: 'Rule B on line 10 (different rule, not suppressed)', + codeLocations: [{ file: filePath, startLine: 10, startColumn: 1 }], + primaryLocationIndex: 0 + }, + { + ruleName: 'stub1RuleA', + message: 'Rule A on line 11 (should be suppressed)', + codeLocations: [{ file: filePath, startLine: 11, startColumn: 1 }], + primaryLocationIndex: 0 + } + ] + }; + + const results = await codeAnalyzer.run(ruleSelection, { workspace }); + + // Should have: line 6 (before marker) and line 10 (different rule) + const violations = results.getViolations(); + expect(violations.length).toBe(2); + expect(violations[0].getPrimaryLocation().getStartLine()).toBe(6); + expect(violations[1].getPrimaryLocation().getStartLine()).toBe(10); + }); + + it('should re-enable violations with unsuppress marker', async () => { + 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); + + // File has: suppress(stubEngine1:stub1RuleA) on line 6, unsuppress on line 9 + const filePath = path.join(testDataDir, 'e2e-file-with-unsuppress.js'); + const workspace = await codeAnalyzer.createWorkspace([filePath]); + const ruleSelection = await codeAnalyzer.selectRules(['stubEngine1'], { workspace }); + + const stubEngine1 = plugin.getCreatedEngine('stubEngine1') as stubs.StubEngine1; + stubEngine1.resultsToReturn = { + violations: [ + { + ruleName: 'stub1RuleA', + message: 'Line 7 (after suppress, before unsuppress)', + codeLocations: [{ file: filePath, startLine: 7, startColumn: 1 }], + primaryLocationIndex: 0 + }, + { + ruleName: 'stub1RuleA', + message: 'Line 10 (after unsuppress)', + codeLocations: [{ file: filePath, startLine: 10, startColumn: 1 }], + primaryLocationIndex: 0 + } + ] + }; + + const results = await codeAnalyzer.run(ruleSelection, { workspace }); + + // Line 7 suppressed, line 10 NOT suppressed + const violations = results.getViolations(); + expect(violations.length).toBe(1); + expect(violations[0].getPrimaryLocation().getStartLine()).toBe(10); + }); + + it('should suppress violations with specified severities', async () => { + 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); + + // File has: suppress(stubEngine1:(3,4)) - suppresses Moderate and Low + const filePath = path.join(testDataDir, 'e2e-file-with-suppress-severity.js'); + const workspace = await codeAnalyzer.createWorkspace([filePath]); + const ruleSelection = await codeAnalyzer.selectRules(['stubEngine1'], { workspace }); + + const stubEngine1 = plugin.getCreatedEngine('stubEngine1') as stubs.StubEngine1; + stubEngine1.resultsToReturn = { + violations: [ + { + ruleName: 'stub1RuleA', // Severity 4 (Low) - should be suppressed + message: 'Low severity on line 8', + codeLocations: [{ file: filePath, startLine: 8, startColumn: 1 }], + primaryLocationIndex: 0 + }, + { + ruleName: 'stub1RuleB', // Severity 2 (High) - should NOT be suppressed + message: 'High severity on line 9', + codeLocations: [{ file: filePath, startLine: 9, startColumn: 1 }], + primaryLocationIndex: 0 + } + ] + }; + + const results = await codeAnalyzer.run(ruleSelection, { workspace }); + + // Only High severity (line 9) should remain + const violations = results.getViolations(); + expect(violations.length).toBe(1); + expect(violations[0].getRule().getSeverityLevel()).toBe(SeverityLevel.High); + }); + + it('should apply hierarchical specificity rules correctly', async () => { + 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); + + // File has: suppress(all) on line 6, unsuppress(stubEngine1:stub1RuleA) on line 9 + const filePath = path.join(testDataDir, 'e2e-file-with-hierarchical-suppressions.js'); + const workspace = await codeAnalyzer.createWorkspace([filePath]); + const ruleSelection = await codeAnalyzer.selectRules(['stubEngine1'], { workspace }); + + const stubEngine1 = plugin.getCreatedEngine('stubEngine1') as stubs.StubEngine1; + stubEngine1.resultsToReturn = { + violations: [ + { + ruleName: 'stub1RuleA', + message: 'Line 7 (suppressed by all)', + codeLocations: [{ file: filePath, startLine: 7, startColumn: 1 }], + primaryLocationIndex: 0 + }, + { + ruleName: 'stub1RuleA', + message: 'Line 10 (unsuppressed - higher specificity)', + codeLocations: [{ file: filePath, startLine: 10, startColumn: 1 }], + primaryLocationIndex: 0 + }, + { + ruleName: 'stub1RuleB', + message: 'Line 12 (still suppressed by all)', + codeLocations: [{ file: filePath, startLine: 12, startColumn: 1 }], + primaryLocationIndex: 0 + } + ] + }; + + const results = await codeAnalyzer.run(ruleSelection, { workspace }); + + // Only line 10 should remain (unsuppress with higher specificity) + const violations = results.getViolations(); + expect(violations.length).toBe(1); + expect(violations[0].getPrimaryLocation().getStartLine()).toBe(10); + }); + + it('should have no effect when only unsuppress markers are present', async () => { + 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); + + // File has only unsuppress markers, no suppress markers + const filePath = path.join(testDataDir, 'e2e-file-with-only-unsuppress.js'); + const workspace = await codeAnalyzer.createWorkspace([filePath]); + const ruleSelection = await codeAnalyzer.selectRules(['stubEngine1'], { workspace }); + + const stubEngine1 = plugin.getCreatedEngine('stubEngine1') as stubs.StubEngine1; + stubEngine1.resultsToReturn = { + violations: [ + { + ruleName: 'stub1RuleA', + message: 'Line 7 (before unsuppress)', + codeLocations: [{ file: filePath, startLine: 7, startColumn: 1 }], + primaryLocationIndex: 0 + }, + { + ruleName: 'stub1RuleA', + message: 'Line 10 (after unsuppress - no effect)', + codeLocations: [{ file: filePath, startLine: 10, startColumn: 1 }], + primaryLocationIndex: 0 + }, + { + ruleName: 'stub1RuleB', + message: 'Line 13 (after unsuppress(all) - no effect)', + codeLocations: [{ file: filePath, startLine: 13, startColumn: 1 }], + primaryLocationIndex: 0 + } + ] + }; + + const results = await codeAnalyzer.run(ruleSelection, { workspace }); + + // All violations should remain - unsuppress without suppress has no effect + const violations = results.getViolations(); + expect(violations.length).toBe(3); + expect(violations[0].getPrimaryLocation().getStartLine()).toBe(7); + expect(violations[1].getPrimaryLocation().getStartLine()).toBe(10); + expect(violations[2].getPrimaryLocation().getStartLine()).toBe(13); + }); + + it('should create exception range with nested suppress/unsuppress against broader suppress', async () => { + // Regression test: suppress(all) -> suppress(specific) -> unsuppress(specific) + // After unsuppress, specific rule should NOT be suppressed by broader suppress(all) + 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, 'e2e-file-with-nested-suppress-unsuppress.js'); + const workspace = await codeAnalyzer.createWorkspace([filePath]); + const ruleSelection = await codeAnalyzer.selectRules(['stubEngine1'], { workspace }); + + const stubEngine1 = plugin.getCreatedEngine('stubEngine1') as stubs.StubEngine1; + stubEngine1.resultsToReturn = { + violations: [ + { + ruleName: 'stub1RuleA', + message: 'Line 10 (suppressed by all)', + codeLocations: [{ file: filePath, startLine: 10, startColumn: 1 }], + primaryLocationIndex: 0 + }, + { + ruleName: 'stub1RuleA', + message: 'Line 13 (suppressed by specific)', + codeLocations: [{ file: filePath, startLine: 13, startColumn: 1 }], + primaryLocationIndex: 0 + }, + { + ruleName: 'stub1RuleA', + message: 'Line 16 (after unsuppress - should NOT be suppressed)', + codeLocations: [{ file: filePath, startLine: 16, startColumn: 1 }], + primaryLocationIndex: 0 + }, + { + ruleName: 'stub1RuleB', + message: 'Line 19 (different rule - still suppressed by all)', + codeLocations: [{ file: filePath, startLine: 19, startColumn: 1 }], + primaryLocationIndex: 0 + } + ] + }; + + const results = await codeAnalyzer.run(ruleSelection, { workspace }); + + // Critical: Line 16 should NOT be suppressed + // unsuppress(stubEngine1:stub1RuleA) creates exception range against suppress(all) + const violations = results.getViolations(); + expect(violations.length).toBe(1); + expect(violations[0].getPrimaryLocation().getStartLine()).toBe(16); + expect(violations[0].getRule().getName()).toBe('stub1RuleA'); + }); + }); + + describe('with suppressions disabled', () => { + it('should report all violations when suppressions are disabled', async () => { + // Create CodeAnalyzer with suppressions DISABLED + const config = CodeAnalyzerConfig.fromObject({ + suppressions: { disable_suppressions: true }, // Explicitly disabled + 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]); + const ruleSelection = await codeAnalyzer.selectRules(['stubEngine1'], { workspace }); + + 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 + } + ] + }; + + const results = await codeAnalyzer.run(ruleSelection, { workspace }); + + // Should have all violations even though file has suppress(all) + const violations = results.getViolations(); + expect(violations.length).toBe(2); + }); + }); + + describe('comparing with and without suppressions', () => { + it('should have fewer violations with suppressions than without', async () => { + // Create two analyzers - one with, one without suppressions + const configWith = CodeAnalyzerConfig.fromObject({ + suppressions: { disable_suppressions: false }, + log_level: 'error' + }); + const configWithout = CodeAnalyzerConfig.fromObject({ + suppressions: { disable_suppressions: true }, + log_level: 'error' + }); + + const analyzerWith = new CodeAnalyzer(configWith, new FakeFileSystem()); + const analyzerWithout = new CodeAnalyzer(configWithout, new FakeFileSystem()); + + // Add plugins to both + const pluginWith = new stubs.StubEnginePlugin(); + const pluginWithout = new stubs.StubEnginePlugin(); + await analyzerWith.addEnginePlugin(pluginWith); + await analyzerWithout.addEnginePlugin(pluginWithout); + + const filePath = path.join(testDataDir, 'file-with-suppress-all.js'); + + // Run with suppressions + const workspaceWith = await analyzerWith.createWorkspace([filePath]); + const ruleSelectionWith = await analyzerWith.selectRules(['stubEngine1'], { workspace: workspaceWith }); + const stubEngine1With = pluginWith.getCreatedEngine('stubEngine1') as stubs.StubEngine1; + stubEngine1With.resultsToReturn = { + violations: [ + { + ruleName: 'stub1RuleA', + message: 'Violation', + codeLocations: [{ file: filePath, startLine: 7, startColumn: 1 }], + primaryLocationIndex: 0 + } + ] + }; + const resultsWith = await analyzerWith.run(ruleSelectionWith, { workspace: workspaceWith }); + + // Run without suppressions + const workspaceWithout = await analyzerWithout.createWorkspace([filePath]); + const ruleSelectionWithout = await analyzerWithout.selectRules(['stubEngine1'], { workspace: workspaceWithout }); + const stubEngine1Without = pluginWithout.getCreatedEngine('stubEngine1') as stubs.StubEngine1; + stubEngine1Without.resultsToReturn = { + violations: [ + { + ruleName: 'stub1RuleA', + message: 'Violation', + codeLocations: [{ file: filePath, startLine: 7, startColumn: 1 }], + primaryLocationIndex: 0 + } + ] + }; + const resultsWithout = await analyzerWithout.run(ruleSelectionWithout, { workspace: workspaceWithout }); + + // With suppressions: 0 violations + // Without suppressions: 1 violation + expect(resultsWith.getViolations().length).toBe(0); + expect(resultsWithout.getViolations().length).toBe(1); + }); + }); +}); diff --git a/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts b/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts new file mode 100644 index 00000000..750b68da --- /dev/null +++ b/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts @@ -0,0 +1,791 @@ +/** + * Unit tests for bulk-suppression-processor.ts + */ + +import { describe, it, expect } from '@jest/globals'; +import { + applyBulkSuppressions, + BulkSuppressionQuotas +} from '../../src/suppressions/bulk-suppression-processor'; +import { BulkSuppressionRule } from '../../src/config'; +import { Violation, CodeLocation } from '../../src/results'; +import { Rule, SeverityLevel } from '../../src/rules'; + +// Mock implementations for testing +class MockCodeLocation implements CodeLocation { + constructor( + private file?: string, + private startLine?: number, + private startColumn?: number, + private endLine?: number, + private endColumn?: number + ) {} + + getFile(): string | undefined { + return this.file; + } + + getStartLine(): number | undefined { + return this.startLine; + } + + getStartColumn(): number | undefined { + return this.startColumn; + } + + getEndLine(): number | undefined { + return this.endLine; + } + + getEndColumn(): number | undefined { + return this.endColumn; + } + + getComment(): string | undefined { + return undefined; + } +} + +class MockRule implements Rule { + constructor( + private engineName: string, + private name: string, + private severityLevel: SeverityLevel = SeverityLevel.High, + private tags: string[] = [] + ) {} + + getEngineName(): string { + return this.engineName; + } + + getName(): string { + return this.name; + } + + getSeverityLevel(): SeverityLevel { + return this.severityLevel; + } + + getTags(): string[] { + return this.tags; + } + + getDescription(): string { + return 'Mock rule'; + } + + getResourceUrls(): string[] { + return []; + } +} + +class MockViolation implements Violation { + constructor( + private rule: Rule, + private message: string, + private primaryLocation: CodeLocation + ) {} + + getRule(): Rule { + return this.rule; + } + + getMessage(): string { + return this.message; + } + + getCodeLocations(): CodeLocation[] { + return [this.primaryLocation]; + } + + getPrimaryLocation(): CodeLocation { + return this.primaryLocation; + } + + getPrimaryLocationIndex(): number { + return 0; + } + + getResourceUrls(): string[] { + return []; + } + + getFixes(): any[] { + return []; + } + + getSuggestions(): any[] { + return []; + } +} + +describe('applyBulkSuppressions', () => { + const workspaceRoot = '/workspace'; + + describe('Basic suppression scenarios', () => { + it('should suppress violations matching rule selector with no quota limit', () => { + const violations = [ + new MockViolation( + new MockRule('pmd', 'UnusedMethod'), + 'Unused method', + new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + ), + new MockViolation( + new MockRule('pmd', 'UnusedMethod'), + 'Unused method', + new MockCodeLocation('/workspace/src/file1.apex', 20, 1, 20, 20) + ) + ]; + + const bulkConfig = { + 'src/file1.apex': [ + { + rule_selector: 'pmd:UnusedMethod', + max_suppressed_violations: null + } + ] + }; + + const quotas: BulkSuppressionQuotas = new Map(); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + + expect(result.unsuppressedViolations).toHaveLength(0); + expect(result.suppressedCount).toBe(2); + // Quota key uses config path, not file path + expect(quotas.get('src/file1.apex|pmd:UnusedMethod')).toBe(2); + }); + + it('should suppress violations up to quota limit', () => { + const violations = [ + new MockViolation( + new MockRule('pmd', 'UnusedMethod'), + 'Unused method', + new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + ), + new MockViolation( + new MockRule('pmd', 'UnusedMethod'), + 'Unused method', + new MockCodeLocation('/workspace/src/file1.apex', 20, 1, 20, 20) + ), + new MockViolation( + new MockRule('pmd', 'UnusedMethod'), + 'Unused method', + new MockCodeLocation('/workspace/src/file1.apex', 30, 1, 30, 20) + ) + ]; + + const bulkConfig = { + 'src/file1.apex': [ + { + rule_selector: 'pmd:UnusedMethod', + max_suppressed_violations: 2 + } + ] + }; + + const quotas: BulkSuppressionQuotas = new Map(); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + + expect(result.unsuppressedViolations).toHaveLength(1); + expect(result.suppressedCount).toBe(2); + expect(result.unsuppressedViolations[0].getPrimaryLocation().getStartLine()).toBe(30); + // Quota key uses config path, not file path + expect(quotas.get('src/file1.apex|pmd:UnusedMethod')).toBe(2); + }); + + it('should not suppress violations when quota is already exhausted', () => { + const violations = [ + new MockViolation( + new MockRule('pmd', 'UnusedMethod'), + 'Unused method', + new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + ) + ]; + + const bulkConfig = { + 'src/file1.apex': [ + { + rule_selector: 'pmd:UnusedMethod', + max_suppressed_violations: 1 + } + ] + }; + + const quotas: BulkSuppressionQuotas = new Map(); + // Quota key uses config path, not file path + quotas.set('src/file1.apex|pmd:UnusedMethod', 1); // Quota already used + + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + + expect(result.unsuppressedViolations).toHaveLength(1); + expect(result.suppressedCount).toBe(0); + expect(quotas.get('src/file1.apex|pmd:UnusedMethod')).toBe(1); + }); + + it('should not suppress violations that do not match rule selector', () => { + const violations = [ + new MockViolation( + new MockRule('pmd', 'UnusedMethod'), + 'Unused method', + new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + ), + new MockViolation( + new MockRule('eslint', 'no-unused-vars'), + 'Unused variable', + new MockCodeLocation('/workspace/src/file1.apex', 20, 1, 20, 20) + ) + ]; + + const bulkConfig = { + 'src/file1.apex': [ + { + rule_selector: 'pmd:UnusedMethod', + max_suppressed_violations: null + } + ] + }; + + const quotas: BulkSuppressionQuotas = new Map(); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + + expect(result.unsuppressedViolations).toHaveLength(1); + expect(result.suppressedCount).toBe(1); + expect(result.unsuppressedViolations[0].getRule().getEngineName()).toBe('eslint'); + }); + }); + + describe('File path matching', () => { + it('should match violations in exact file path', () => { + const violations = [ + new MockViolation( + new MockRule('pmd', 'UnusedMethod'), + 'Unused method', + new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + ) + ]; + + const bulkConfig = { + 'src/file1.apex': [ + { + rule_selector: 'pmd', + max_suppressed_violations: null + } + ] + }; + + const quotas: BulkSuppressionQuotas = new Map(); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + + expect(result.suppressedCount).toBe(1); + }); + + it('should match violations in folder path', () => { + const violations = [ + new MockViolation( + new MockRule('pmd', 'UnusedMethod'), + 'Unused method', + new MockCodeLocation('/workspace/src/folder/file1.apex', 10, 1, 10, 20) + ), + new MockViolation( + new MockRule('pmd', 'UnusedMethod'), + 'Unused method', + new MockCodeLocation('/workspace/src/folder/file2.apex', 20, 1, 20, 20) + ) + ]; + + const bulkConfig = { + 'src/folder': [ + { + rule_selector: 'pmd', + max_suppressed_violations: null + } + ] + }; + + const quotas: BulkSuppressionQuotas = new Map(); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + + expect(result.suppressedCount).toBe(2); + }); + + it('should not match violations outside the configured path', () => { + const violations = [ + new MockViolation( + new MockRule('pmd', 'UnusedMethod'), + 'Unused method', + new MockCodeLocation('/workspace/other/file1.apex', 10, 1, 10, 20) + ) + ]; + + const bulkConfig = { + 'src/folder': [ + { + rule_selector: 'pmd', + max_suppressed_violations: null + } + ] + }; + + const quotas: BulkSuppressionQuotas = new Map(); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + + expect(result.suppressedCount).toBe(0); + expect(result.unsuppressedViolations).toHaveLength(1); + }); + + it('should handle absolute paths in config', () => { + const violations = [ + new MockViolation( + new MockRule('pmd', 'UnusedMethod'), + 'Unused method', + new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + ) + ]; + + const bulkConfig = { + '/workspace/src/file1.apex': [ + { + rule_selector: 'pmd', + max_suppressed_violations: null + } + ] + }; + + const quotas: BulkSuppressionQuotas = new Map(); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + + expect(result.suppressedCount).toBe(1); + }); + }); + + describe('Rule selector matching', () => { + it('should match violations by engine name', () => { + const violations = [ + new MockViolation( + new MockRule('pmd', 'UnusedMethod'), + 'Unused method', + new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + ), + new MockViolation( + new MockRule('eslint', 'no-unused-vars'), + 'Unused variable', + new MockCodeLocation('/workspace/src/file1.apex', 20, 1, 20, 20) + ) + ]; + + const bulkConfig = { + 'src/file1.apex': [ + { + rule_selector: 'pmd', + max_suppressed_violations: null + } + ] + }; + + const quotas: BulkSuppressionQuotas = new Map(); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + + expect(result.suppressedCount).toBe(1); + expect(result.unsuppressedViolations).toHaveLength(1); + expect(result.unsuppressedViolations[0].getRule().getEngineName()).toBe('eslint'); + }); + + it('should match violations by engine:rule', () => { + const violations = [ + new MockViolation( + new MockRule('pmd', 'UnusedMethod'), + 'Unused method', + new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + ), + new MockViolation( + new MockRule('pmd', 'UnusedVariable'), + 'Unused variable', + new MockCodeLocation('/workspace/src/file1.apex', 20, 1, 20, 20) + ) + ]; + + const bulkConfig = { + 'src/file1.apex': [ + { + rule_selector: 'pmd:UnusedMethod', + max_suppressed_violations: null + } + ] + }; + + const quotas: BulkSuppressionQuotas = new Map(); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + + expect(result.suppressedCount).toBe(1); + expect(result.unsuppressedViolations).toHaveLength(1); + expect(result.unsuppressedViolations[0].getRule().getName()).toBe('UnusedVariable'); + }); + + it('should match violations by severity level', () => { + const violations = [ + new MockViolation( + new MockRule('pmd', 'Rule1', SeverityLevel.High), + 'High severity', + new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + ), + new MockViolation( + new MockRule('pmd', 'Rule2', SeverityLevel.Moderate), + 'Moderate severity', + new MockCodeLocation('/workspace/src/file1.apex', 20, 1, 20, 20) + ), + new MockViolation( + new MockRule('pmd', 'Rule3', SeverityLevel.Low), + 'Low severity', + new MockCodeLocation('/workspace/src/file1.apex', 30, 1, 30, 20) + ) + ]; + + const bulkConfig = { + 'src/file1.apex': [ + { + rule_selector: '3,4', + max_suppressed_violations: null + } + ] + }; + + const quotas: BulkSuppressionQuotas = new Map(); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + + expect(result.suppressedCount).toBe(2); + expect(result.unsuppressedViolations).toHaveLength(1); + expect(result.unsuppressedViolations[0].getRule().getSeverityLevel()).toBe(SeverityLevel.High); + }); + + it('should match violations by tag', () => { + const violations = [ + new MockViolation( + new MockRule('pmd', 'Rule1', SeverityLevel.High, ['Recommended']), + 'Tagged violation', + new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + ), + new MockViolation( + new MockRule('pmd', 'Rule2', SeverityLevel.High, ['Security']), + 'Security violation', + new MockCodeLocation('/workspace/src/file1.apex', 20, 1, 20, 20) + ) + ]; + + const bulkConfig = { + 'src/file1.apex': [ + { + rule_selector: 'Recommended', + max_suppressed_violations: null + } + ] + }; + + const quotas: BulkSuppressionQuotas = new Map(); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + + expect(result.suppressedCount).toBe(1); + expect(result.unsuppressedViolations).toHaveLength(1); + expect(result.unsuppressedViolations[0].getRule().getTags()).toContain('Security'); + }); + }); + + describe('Multiple rules per file', () => { + it('should apply multiple rules to the same file', () => { + const violations = [ + new MockViolation( + new MockRule('pmd', 'UnusedMethod'), + 'Unused method', + new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + ), + new MockViolation( + new MockRule('eslint', 'no-unused-vars'), + 'Unused variable', + new MockCodeLocation('/workspace/src/file1.apex', 20, 1, 20, 20) + ) + ]; + + const bulkConfig = { + 'src/file1.apex': [ + { + rule_selector: 'pmd', + max_suppressed_violations: null + }, + { + rule_selector: 'eslint', + max_suppressed_violations: null + } + ] + }; + + const quotas: BulkSuppressionQuotas = new Map(); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + + expect(result.suppressedCount).toBe(2); + expect(result.unsuppressedViolations).toHaveLength(0); + }); + + it('should apply first matching rule when multiple rules match', () => { + const violations = [ + new MockViolation( + new MockRule('pmd', 'UnusedMethod'), + 'Unused method', + new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + ) + ]; + + const bulkConfig = { + 'src/file1.apex': [ + { + rule_selector: 'pmd', + max_suppressed_violations: 1 + }, + { + rule_selector: 'pmd:UnusedMethod', + max_suppressed_violations: 1 + } + ] + }; + + const quotas: BulkSuppressionQuotas = new Map(); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + + expect(result.suppressedCount).toBe(1); + // Quota key uses config path, not file path + expect(quotas.get('src/file1.apex|pmd')).toBe(1); + expect(quotas.get('src/file1.apex|pmd:UnusedMethod')).toBeUndefined(); + }); + }); + + describe('Deterministic ordering', () => { + it('should process violations in deterministic order (file, then line)', () => { + const violations = [ + new MockViolation( + new MockRule('pmd', 'UnusedMethod'), + 'Unused method', + new MockCodeLocation('/workspace/src/file2.apex', 10, 1, 10, 20) + ), + new MockViolation( + new MockRule('pmd', 'UnusedMethod'), + 'Unused method', + new MockCodeLocation('/workspace/src/file1.apex', 20, 1, 20, 20) + ), + new MockViolation( + new MockRule('pmd', 'UnusedMethod'), + 'Unused method', + new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + ) + ]; + + const bulkConfig = { + 'src': [ + { + rule_selector: 'pmd', + max_suppressed_violations: 2 + } + ] + }; + + const quotas: BulkSuppressionQuotas = new Map(); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + + expect(result.suppressedCount).toBe(2); + expect(result.unsuppressedViolations).toHaveLength(1); + // Should suppress file1:10 and file1:20 (sorted by file, then line) + expect(result.unsuppressedViolations[0].getPrimaryLocation().getFile()).toBe('/workspace/src/file2.apex'); + }); + }); + + describe('Quota tracking across engines', () => { + it('should share quota across multiple files in same folder', () => { + // Critical test: Folder-level quota should be shared across all files in folder + const violations = [ + new MockViolation( + new MockRule('pmd', 'UnusedMethod'), + 'Unused method in file1', + new MockCodeLocation('/workspace/src/controllers/file1.apex', 10, 1, 10, 20) + ), + new MockViolation( + new MockRule('pmd', 'UnusedMethod'), + 'Unused method in file2', + new MockCodeLocation('/workspace/src/controllers/file2.apex', 20, 1, 20, 20) + ), + new MockViolation( + new MockRule('pmd', 'UnusedMethod'), + 'Unused method in file3', + new MockCodeLocation('/workspace/src/controllers/file3.apex', 30, 1, 30, 20) + ) + ]; + + const bulkConfig = { + 'src/controllers': [ // Folder-level suppression + { + rule_selector: 'pmd:UnusedMethod', + max_suppressed_violations: 2 // Total across ALL files in folder + } + ] + }; + + const quotas: BulkSuppressionQuotas = new Map(); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + + // Should suppress first 2 violations (across different files), reject 3rd + expect(result.suppressedCount).toBe(2); + expect(result.unsuppressedViolations).toHaveLength(1); + expect(result.unsuppressedViolations[0].getPrimaryLocation().getFile()).toBe('/workspace/src/controllers/file3.apex'); + + // Quota is shared at folder level, not per-file + expect(quotas.get('src/controllers|pmd:UnusedMethod')).toBe(2); + }); + + it('should maintain quota state across multiple calls', () => { + const violations1 = [ + new MockViolation( + new MockRule('pmd', 'UnusedMethod'), + 'Unused method', + new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + ) + ]; + + const violations2 = [ + new MockViolation( + new MockRule('pmd', 'UnusedMethod'), + 'Unused method', + new MockCodeLocation('/workspace/src/file1.apex', 20, 1, 20, 20) + ) + ]; + + const bulkConfig = { + 'src/file1.apex': [ + { + rule_selector: 'pmd:UnusedMethod', + max_suppressed_violations: 1 + } + ] + }; + + const quotas: BulkSuppressionQuotas = new Map(); + + // First call - should suppress + const result1 = applyBulkSuppressions(violations1, bulkConfig, quotas, workspaceRoot); + expect(result1.suppressedCount).toBe(1); + // Quota key uses config path, not file path + expect(quotas.get('src/file1.apex|pmd:UnusedMethod')).toBe(1); + + // Second call - quota exhausted, should not suppress + const result2 = applyBulkSuppressions(violations2, bulkConfig, quotas, workspaceRoot); + expect(result2.suppressedCount).toBe(0); + expect(result2.unsuppressedViolations).toHaveLength(1); + }); + }); + + describe('Edge cases', () => { + it('should return all violations when bulkConfig is empty', () => { + const violations = [ + new MockViolation( + new MockRule('pmd', 'UnusedMethod'), + 'Unused method', + new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + ) + ]; + + const quotas: BulkSuppressionQuotas = new Map(); + const result = applyBulkSuppressions(violations, {}, quotas, workspaceRoot); + + expect(result.unsuppressedViolations).toHaveLength(1); + expect(result.suppressedCount).toBe(0); + }); + + it('should return early when violations array is empty', () => { + const bulkConfig = { + 'src/file1.apex': [ + { + rule_selector: 'pmd', + max_suppressed_violations: null + } + ] + }; + + const quotas: BulkSuppressionQuotas = new Map(); + const result = applyBulkSuppressions([], bulkConfig, quotas, workspaceRoot); + + expect(result.unsuppressedViolations).toHaveLength(0); + expect(result.suppressedCount).toBe(0); + }); + + it('should skip violations without file path', () => { + const violations = [ + new MockViolation( + new MockRule('pmd', 'UnusedMethod'), + 'Unused method', + new MockCodeLocation(undefined, 10, 1, 10, 20) + ) + ]; + + const bulkConfig = { + 'src/file1.apex': [ + { + rule_selector: 'pmd', + max_suppressed_violations: null + } + ] + }; + + const quotas: BulkSuppressionQuotas = new Map(); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + + expect(result.unsuppressedViolations).toHaveLength(1); + expect(result.suppressedCount).toBe(0); + }); + + it('should handle invalid rule selector gracefully', () => { + const violations = [ + new MockViolation( + new MockRule('pmd', 'UnusedMethod'), + 'Unused method', + new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + ) + ]; + + const bulkConfig = { + 'src/file1.apex': [ + { + rule_selector: 'invalid:::selector', + max_suppressed_violations: null + } + ] + }; + + const quotas: BulkSuppressionQuotas = new Map(); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + + // Should not suppress due to invalid selector + expect(result.unsuppressedViolations).toHaveLength(1); + expect(result.suppressedCount).toBe(0); + }); + + it('should handle max_suppressed_violations of 0', () => { + const violations = [ + new MockViolation( + new MockRule('pmd', 'UnusedMethod'), + 'Unused method', + new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + ) + ]; + + const bulkConfig = { + 'src/file1.apex': [ + { + rule_selector: 'pmd', + max_suppressed_violations: 0 + } + ] + }; + + const quotas: BulkSuppressionQuotas = new Map(); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + + expect(result.unsuppressedViolations).toHaveLength(1); + expect(result.suppressedCount).toBe(0); + }); + }); +}); diff --git a/packages/code-analyzer-core/test/suppressions/bulk-suppression-workspace-root.test.ts b/packages/code-analyzer-core/test/suppressions/bulk-suppression-workspace-root.test.ts new file mode 100644 index 00000000..e351f56b --- /dev/null +++ b/packages/code-analyzer-core/test/suppressions/bulk-suppression-workspace-root.test.ts @@ -0,0 +1,356 @@ +import {applyBulkSuppressions, BulkSuppressionQuotas} from "../../src/suppressions/bulk-suppression-processor"; +import {BulkSuppressionRule} from "../../src/config"; +import {Violation, CodeLocation} from "../../src/results"; +import {RuleImpl} from "../../src/rules"; + +/** + * Tests specifically for Bug #2: Workspace Root vs Config Root inconsistency + * + * BUG: Bulk suppressions were using config.getConfigRoot() while ignores feature + * uses workspace.getWorkspaceRoot(). This caused path resolution inconsistency. + * + * SCENARIO: User runs from /workspace/dreamhouse with config at + * /workspace/dreamhouse/force-app/main/default/react-components/code-analyzer.yml + * + * EXPECTED: Paths in config should be relative to workspace root (consistent with ignores) + * ACTUAL (Bug): Paths were relative to config root (where config file lives) + */ + +// Mock violation and code location for testing +class MockCodeLocation implements CodeLocation { + constructor( + private file: string, + private startLine: number, + private startColumn: number, + private endLine: number, + private endColumn: number + ) {} + + getFile(): string { return this.file; } + getStartLine(): number { return this.startLine; } + getStartColumn(): number { return this.startColumn; } + getEndLine(): number { return this.endLine; } + getEndColumn(): number { return this.endColumn; } + getComment(): string | undefined { return undefined; } +} + +class MockViolation implements Violation { + constructor( + private rule: RuleImpl, + private message: string, + private primaryLocation: CodeLocation, + private otherLocations: CodeLocation[] = [] + ) {} + + getRule(): RuleImpl { return this.rule; } + getMessage(): string { return this.message; } + getPrimaryLocation(): CodeLocation { return this.primaryLocation; } + getCodeLocations(): CodeLocation[] { + return [this.primaryLocation, ...this.otherLocations]; + } + getPrimaryLocationIndex(): number { return 0; } + getResourceUrls(): string[] { return []; } + getFixes(): any[] { return []; } + getSuggestions(): any[] { return []; } +} + +// Mock rule for testing +const mockRule = { + getName: () => 'no-console', + getEngineName: () => 'eslint', + getSeverityLevel: () => 3, + getTags: () => ['Recommended'], + getDescription: () => 'Mock rule', + getResourceUrls: () => [], + matchesRuleSelector: () => true +} as unknown as RuleImpl; + +describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { + /** + * BUG #2 TEST: Config in subdirectory, workspace root at parent level + * + * Setup mirrors real-world scenario: + * - Workspace root: /Users/user/workspace/dreamhouse + * - Config file at: /Users/user/workspace/dreamhouse/force-app/main/default/react-components/code-analyzer.yml + * - Violation file: /Users/user/workspace/dreamhouse/force-app/main/default/react-components/utils.js + * + * Config path should be: "force-app/main/default/react-components/utils.js" (relative to workspace root) + * NOT: "utils.js" (relative to config root) + */ + it('BUG #2: Paths in config are resolved relative to WORKSPACE ROOT, not config file location', () => { + // Workspace root (where -w flag points) + const workspaceRoot = '/Users/user/workspace/dreamhouse'; + + // Violation file path (absolute) + const violationFilePath = '/Users/user/workspace/dreamhouse/force-app/main/default/react-components/utils.js'; + + // Create violation + const violation = new MockViolation( + mockRule, + 'Unexpected console statement', + new MockCodeLocation(violationFilePath, 10, 1, 10, 20) + ); + + // Bulk config with path relative to WORKSPACE ROOT (consistent with ignores) + // This is what user should write in config file + const bulkConfig: Record = { + 'force-app/main/default/react-components/utils.js': [ + { + rule_selector: 'eslint:all', + max_suppressed_violations: 3 + } + ] + }; + + const quotas: BulkSuppressionQuotas = new Map(); + + // Apply suppressions using workspace root (Bug #2 fix) + const result = applyBulkSuppressions([violation], bulkConfig, quotas, workspaceRoot); + + // Violation should be suppressed + expect(result.unsuppressedViolations).toHaveLength(0); + expect(result.suppressedCount).toBe(1); + }); + + it('BUG #2: Same directory level - workspace root equals config directory (worked before bug fix)', () => { + // This case worked even with Bug #2 because workspace root === config root + const workspaceRoot = '/Users/user/workspace/project'; + const violationFilePath = '/Users/user/workspace/project/utils.js'; + + const violation = new MockViolation( + mockRule, + 'Unexpected console statement', + new MockCodeLocation(violationFilePath, 10, 1, 10, 20) + ); + + const bulkConfig: Record = { + 'utils.js': [ + { + rule_selector: 'eslint:all', + max_suppressed_violations: 3 + } + ] + }; + + const quotas: BulkSuppressionQuotas = new Map(); + const result = applyBulkSuppressions([violation], bulkConfig, quotas, workspaceRoot); + + // Should work regardless of bug + expect(result.unsuppressedViolations).toHaveLength(0); + expect(result.suppressedCount).toBe(1); + }); + + it('BUG #2: Config in subdirectory with WRONG path (relative to config root) does NOT suppress', () => { + // This demonstrates the bug - if user writes path relative to config file location + const workspaceRoot = '/Users/user/workspace/dreamhouse'; + const violationFilePath = '/Users/user/workspace/dreamhouse/force-app/main/default/react-components/utils.js'; + + const violation = new MockViolation( + mockRule, + 'Unexpected console statement', + new MockCodeLocation(violationFilePath, 10, 1, 10, 20) + ); + + // WRONG: Path relative to config file location (where old bug would require) + const bulkConfig: Record = { + 'utils.js': [ // This is relative to config dir, not workspace root + { + rule_selector: 'eslint:all', + max_suppressed_violations: 3 + } + ] + }; + + const quotas: BulkSuppressionQuotas = new Map(); + const result = applyBulkSuppressions([violation], bulkConfig, quotas, workspaceRoot); + + // Should NOT suppress because path doesn't match + expect(result.unsuppressedViolations).toHaveLength(1); + expect(result.suppressedCount).toBe(0); + }); + + it('BUG #2: Multiple files at different levels all resolve correctly from workspace root', () => { + const workspaceRoot = '/workspace/project'; + + // Violations at different directory levels + const violation1 = new MockViolation( + mockRule, + 'Console in nested file', + new MockCodeLocation('/workspace/project/src/utils.js', 10, 1, 10, 20) + ); + + const violation2 = new MockViolation( + mockRule, + 'Console in deeper nested file', + new MockCodeLocation('/workspace/project/force-app/main/default/lwc/component.js', 20, 1, 20, 20) + ); + + const violation3 = new MockViolation( + mockRule, + 'Console in root file', + new MockCodeLocation('/workspace/project/index.js', 30, 1, 30, 20) + ); + + // All paths relative to workspace root + const bulkConfig: Record = { + 'src/utils.js': [ + { rule_selector: 'eslint:all', max_suppressed_violations: 1 } + ], + 'force-app/main/default/lwc/component.js': [ + { rule_selector: 'eslint:all', max_suppressed_violations: 1 } + ], + 'index.js': [ + { rule_selector: 'eslint:all', max_suppressed_violations: 1 } + ] + }; + + const quotas: BulkSuppressionQuotas = new Map(); + const result = applyBulkSuppressions( + [violation1, violation2, violation3], + bulkConfig, + quotas, + workspaceRoot + ); + + // All should be suppressed + expect(result.unsuppressedViolations).toHaveLength(0); + expect(result.suppressedCount).toBe(3); + }); + + it('BUG #2: Folder-level suppression from workspace root matches nested files', () => { + const workspaceRoot = '/workspace/project'; + + // Violations in nested folder + const violation1 = new MockViolation( + mockRule, + 'Console', + new MockCodeLocation('/workspace/project/src/utils/helper.js', 10, 1, 10, 20) + ); + + const violation2 = new MockViolation( + mockRule, + 'Console', + new MockCodeLocation('/workspace/project/src/utils/formatter.js', 20, 1, 20, 20) + ); + + // Suppress entire folder (path relative to workspace root) + const bulkConfig: Record = { + 'src/utils/': [ + { rule_selector: 'eslint:all', max_suppressed_violations: 10 } + ] + }; + + const quotas: BulkSuppressionQuotas = new Map(); + const result = applyBulkSuppressions( + [violation1, violation2], + bulkConfig, + quotas, + workspaceRoot + ); + + // Both files in folder should be suppressed + expect(result.unsuppressedViolations).toHaveLength(0); + expect(result.suppressedCount).toBe(2); + }); + + it('BUG #2: Absolute paths in config still work (edge case)', () => { + // Users shouldn't use absolute paths, but they should still work + const workspaceRoot = '/workspace/project'; + const violationFilePath = '/workspace/project/src/file.js'; + + const violation = new MockViolation( + mockRule, + 'Console', + new MockCodeLocation(violationFilePath, 10, 1, 10, 20) + ); + + // Absolute path in config (not recommended but should work) + const bulkConfig: Record = { + '/workspace/project/src/file.js': [ + { rule_selector: 'eslint:all', max_suppressed_violations: 1 } + ] + }; + + const quotas: BulkSuppressionQuotas = new Map(); + const result = applyBulkSuppressions([violation], bulkConfig, quotas, workspaceRoot); + + // Should suppress + expect(result.unsuppressedViolations).toHaveLength(0); + expect(result.suppressedCount).toBe(1); + }); + + it('BUG #2: Path resolution consistent with ignores feature behavior', () => { + /** + * This test documents that bulk suppressions now behave the same as ignores: + * - Both use workspace.getWorkspaceRoot() for path resolution + * - Both resolve paths relative to workspace root + * - Both fall back to absolute paths if workspace root is null + */ + const workspaceRoot = '/Users/user/dreamhouse'; + + // Imagine ignores config: ["force-app/test/**"] + // Imagine bulk suppressions: "force-app/main/default/utils.js" + // Both should resolve relative to /Users/user/dreamhouse + + const violation = new MockViolation( + mockRule, + 'Console', + new MockCodeLocation('/Users/user/dreamhouse/force-app/main/default/utils.js', 10, 1, 10, 20) + ); + + const bulkConfig: Record = { + 'force-app/main/default/utils.js': [ + { rule_selector: 'eslint:all', max_suppressed_violations: 1 } + ] + }; + + const quotas: BulkSuppressionQuotas = new Map(); + const result = applyBulkSuppressions([violation], bulkConfig, quotas, workspaceRoot); + + expect(result.unsuppressedViolations).toHaveLength(0); + expect(result.suppressedCount).toBe(1); + }); + + it('Duplicate rule selectors have independent quotas', () => { + /** + * When config has duplicate rule_selector entries, each should get its own quota. + * This allows users to specify multiple quota allocations for the same selector. + * + * Example use case: + * suppressions: + * force-app/utils.js: + * - rule_selector: no-magic-numbers,pmd + * max_suppressed_violations: 3 + * - rule_selector: no-magic-numbers,pmd + * max_suppressed_violations: 2 + * + * Expected: 5 total violations suppressed (3 + 2), not 3 with shared quota + */ + const workspaceRoot = '/Users/user/workspace'; + const filePath = '/Users/user/workspace/force-app/utils.js'; + + // Create 6 violations that would match the duplicate selectors + const violations: Violation[] = []; + for (let i = 1; i <= 6; i++) { + violations.push(new MockViolation( + mockRule, + `Violation ${i}`, + new MockCodeLocation(filePath, i, 1, i, 10) + )); + } + + const bulkConfig: Record = { + 'force-app/utils.js': [ + { rule_selector: 'eslint:all', max_suppressed_violations: 3 }, + { rule_selector: 'eslint:all', max_suppressed_violations: 2 } // Duplicate selector + ] + }; + + const quotas: BulkSuppressionQuotas = new Map(); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + + // First rule suppresses 3, second rule suppresses 2 = 5 total + expect(result.suppressedCount).toBe(5); + expect(result.unsuppressedViolations).toHaveLength(1); + }); +}); 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 f0864fec..78511bff 100644 --- a/packages/code-analyzer-core/test/suppressions/suppression-parser.test.ts +++ b/packages/code-analyzer-core/test/suppressions/suppression-parser.test.ts @@ -306,6 +306,41 @@ describe('buildSuppressionRanges', () => { }); }); + it('should create unsuppression range when closing broader suppress (exception behavior)', () => { + const markers: SuppressionMarker[] = [ + createMarker('suppress', 'all', 1), + createMarker('suppress', 'pmd:UnusedMethod', 5), + createMarker('unsuppress', 'pmd:UnusedMethod', 10) + ]; + + const ranges = buildSuppressionRanges(markers, '/test/file.apex'); + + // Critical: unsuppress must create a range to act as exception against broader suppress(all) + expect(ranges).toHaveLength(3); + + // Sort ranges by startLine for deterministic test assertions + const sortedRanges = ranges.sort((a, b) => a.startLine - b.startLine); + + expect(sortedRanges[0]).toMatchObject({ + startLine: 1, + endLine: undefined, + ruleSelectorString: 'all', + isSuppressed: true + }); + expect(sortedRanges[1]).toMatchObject({ + startLine: 5, + endLine: 9, + ruleSelectorString: 'pmd:UnusedMethod', + isSuppressed: true + }); + expect(sortedRanges[2]).toMatchObject({ + startLine: 10, + endLine: undefined, + ruleSelectorString: 'pmd:UnusedMethod', + isSuppressed: false + }); + }); + it('should handle empty markers array', () => { const markers: SuppressionMarker[] = []; diff --git a/packages/code-analyzer-core/test/test-data/bulk-suppressions-workspace/code-analyzer.yml b/packages/code-analyzer-core/test/test-data/bulk-suppressions-workspace/code-analyzer.yml new file mode 100644 index 00000000..61bceb8f --- /dev/null +++ b/packages/code-analyzer-core/test/test-data/bulk-suppressions-workspace/code-analyzer.yml @@ -0,0 +1,17 @@ +suppressions: + disable_suppressions: false + # Suppress 2 violations in file1.js + file1.js: + - rule_selector: all + max_suppressed_violations: 2 + reason: Test file-level suppression with quota + # Suppress all violations in file2.js + file2.js: + - rule_selector: all + max_suppressed_violations: null + reason: Test unlimited suppression + # Suppress 2 violations across all files in src/controllers folder + src/controllers/: + - rule_selector: all + max_suppressed_violations: 2 + reason: Test folder-level shared quota diff --git a/packages/code-analyzer-core/test/test-data/bulk-suppressions-workspace/file1.js b/packages/code-analyzer-core/test/test-data/bulk-suppressions-workspace/file1.js new file mode 100644 index 00000000..6d0024f4 --- /dev/null +++ b/packages/code-analyzer-core/test/test-data/bulk-suppressions-workspace/file1.js @@ -0,0 +1,12 @@ +// Test file 1 with multiple violations +function unusedFunction1() { + console.log('violation on line 3'); +} + +function unusedFunction2() { + console.log('violation on line 6'); +} + +function unusedFunction3() { + console.log('violation on line 10'); +} diff --git a/packages/code-analyzer-core/test/test-data/bulk-suppressions-workspace/file2.js b/packages/code-analyzer-core/test/test-data/bulk-suppressions-workspace/file2.js new file mode 100644 index 00000000..49c609ab --- /dev/null +++ b/packages/code-analyzer-core/test/test-data/bulk-suppressions-workspace/file2.js @@ -0,0 +1,4 @@ +// Test file 2 with violations +function anotherUnusedFunction() { + console.log('violation on line 3'); +} diff --git a/packages/code-analyzer-core/test/test-data/bulk-suppressions-workspace/src/controllers/controller1.js b/packages/code-analyzer-core/test/test-data/bulk-suppressions-workspace/src/controllers/controller1.js new file mode 100644 index 00000000..f6f4a9a0 --- /dev/null +++ b/packages/code-analyzer-core/test/test-data/bulk-suppressions-workspace/src/controllers/controller1.js @@ -0,0 +1,8 @@ +// Controller 1 +function controllerMethod1() { + console.log('violation on line 3'); +} + +function controllerMethod2() { + console.log('violation on line 6'); +} diff --git a/packages/code-analyzer-core/test/test-data/bulk-suppressions-workspace/src/controllers/controller2.js b/packages/code-analyzer-core/test/test-data/bulk-suppressions-workspace/src/controllers/controller2.js new file mode 100644 index 00000000..0805e923 --- /dev/null +++ b/packages/code-analyzer-core/test/test-data/bulk-suppressions-workspace/src/controllers/controller2.js @@ -0,0 +1,4 @@ +// Controller 2 +function controllerMethod3() { + console.log('violation on line 3'); +} From ef9435ee39c08b1ddb28071ec05216b6909ea93e Mon Sep 17 00:00:00 2001 From: Namrata Gupta Date: Fri, 10 Apr 2026 13:40:03 +0530 Subject: [PATCH 02/16] fixing UTs --- packages/code-analyzer-core/src/config.ts | 8 +++--- .../bulk-suppression-processor.test.ts | 28 +++++++++---------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/packages/code-analyzer-core/src/config.ts b/packages/code-analyzer-core/src/config.ts index 082f942a..931fdb63 100644 --- a/packages/code-analyzer-core/src/config.ts +++ b/packages/code-analyzer-core/src/config.ts @@ -85,7 +85,7 @@ export const DEFAULT_CONFIG: TopLevelConfig = { config_root: process.cwd(), log_folder: os.tmpdir(), log_level: LogLevel.Debug, - suppressions: { disable_suppressions: false, bulk_suppressions: {} }, // Suppressions enabled by default + suppressions: { disable_suppressions: false, bulk_suppressions: {} }, // Suppressions enabled by default, no bulk suppressions by default rules: {}, engines: {}, ignores: { files: [] }, @@ -423,13 +423,13 @@ function extractSuppressionsValue(configExtractor: engApi.ConfigValueExtractor): const disable_suppressions: boolean = suppressionsExtractor.extractBoolean(FIELDS.DISABLE_SUPPRESSIONS, DEFAULT_CONFIG.suppressions.disable_suppressions) || false; - // Extract bulk suppressions - all keys except 'disable_suppressions' are file/folder paths + // Extract bulk suppressions - all keys except 'disable_suppressions' and 'bulk_suppressions' are file/folder paths const bulk_suppressions: Record = {}; const suppressionKeys = suppressionsExtractor.getKeys(); for (const key of suppressionKeys) { - if (key === FIELDS.DISABLE_SUPPRESSIONS) { - continue; // Skip the disable_suppressions flag + if (key === FIELDS.DISABLE_SUPPRESSIONS || key === 'bulk_suppressions') { + continue; // Skip the disable_suppressions flag and bulk_suppressions placeholder from default config } // key is a file/folder path, value should be an array of suppression rules diff --git a/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts b/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts index 750b68da..ab091b37 100644 --- a/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts +++ b/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts @@ -151,8 +151,8 @@ describe('applyBulkSuppressions', () => { expect(result.unsuppressedViolations).toHaveLength(0); expect(result.suppressedCount).toBe(2); - // Quota key uses config path, not file path - expect(quotas.get('src/file1.apex|pmd:UnusedMethod')).toBe(2); + // Quota key uses config path with rule index + expect(quotas.get('src/file1.apex|0|pmd:UnusedMethod')).toBe(2); }); it('should suppress violations up to quota limit', () => { @@ -189,8 +189,8 @@ describe('applyBulkSuppressions', () => { expect(result.unsuppressedViolations).toHaveLength(1); expect(result.suppressedCount).toBe(2); expect(result.unsuppressedViolations[0].getPrimaryLocation().getStartLine()).toBe(30); - // Quota key uses config path, not file path - expect(quotas.get('src/file1.apex|pmd:UnusedMethod')).toBe(2); + // Quota key uses config path with rule index + expect(quotas.get('src/file1.apex|0|pmd:UnusedMethod')).toBe(2); }); it('should not suppress violations when quota is already exhausted', () => { @@ -212,14 +212,14 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - // Quota key uses config path, not file path - quotas.set('src/file1.apex|pmd:UnusedMethod', 1); // Quota already used + // Quota key uses config path with rule index + quotas.set('src/file1.apex|0|pmd:UnusedMethod', 1); // Quota already used const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); expect(result.unsuppressedViolations).toHaveLength(1); expect(result.suppressedCount).toBe(0); - expect(quotas.get('src/file1.apex|pmd:UnusedMethod')).toBe(1); + expect(quotas.get('src/file1.apex|0|pmd:UnusedMethod')).toBe(1); }); it('should not suppress violations that do not match rule selector', () => { @@ -550,9 +550,9 @@ describe('applyBulkSuppressions', () => { const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); expect(result.suppressedCount).toBe(1); - // Quota key uses config path, not file path - expect(quotas.get('src/file1.apex|pmd')).toBe(1); - expect(quotas.get('src/file1.apex|pmd:UnusedMethod')).toBeUndefined(); + // Quota key uses config path with rule index (first rule at index 0 is used) + expect(quotas.get('src/file1.apex|0|pmd')).toBe(1); + expect(quotas.get('src/file1.apex|1|pmd:UnusedMethod')).toBeUndefined(); }); }); @@ -633,8 +633,8 @@ describe('applyBulkSuppressions', () => { expect(result.unsuppressedViolations).toHaveLength(1); expect(result.unsuppressedViolations[0].getPrimaryLocation().getFile()).toBe('/workspace/src/controllers/file3.apex'); - // Quota is shared at folder level, not per-file - expect(quotas.get('src/controllers|pmd:UnusedMethod')).toBe(2); + // Quota is shared at folder level, not per-file (rule index 0) + expect(quotas.get('src/controllers|0|pmd:UnusedMethod')).toBe(2); }); it('should maintain quota state across multiple calls', () => { @@ -668,8 +668,8 @@ describe('applyBulkSuppressions', () => { // First call - should suppress const result1 = applyBulkSuppressions(violations1, bulkConfig, quotas, workspaceRoot); expect(result1.suppressedCount).toBe(1); - // Quota key uses config path, not file path - expect(quotas.get('src/file1.apex|pmd:UnusedMethod')).toBe(1); + // Quota key uses config path with rule index + expect(quotas.get('src/file1.apex|0|pmd:UnusedMethod')).toBe(1); // Second call - quota exhausted, should not suppress const result2 = applyBulkSuppressions(violations2, bulkConfig, quotas, workspaceRoot); From 260deea3fe231e10cf80baad17c4b6246b623651 Mon Sep 17 00:00:00 2001 From: Namrata Gupta Date: Fri, 10 Apr 2026 13:45:21 +0530 Subject: [PATCH 03/16] Removing unrelated test --- .../test/suppressions-e2e.test.ts | 495 ------------------ 1 file changed, 495 deletions(-) delete mode 100644 packages/code-analyzer-core/test/suppressions-e2e.test.ts diff --git a/packages/code-analyzer-core/test/suppressions-e2e.test.ts b/packages/code-analyzer-core/test/suppressions-e2e.test.ts deleted file mode 100644 index 50bdbc82..00000000 --- a/packages/code-analyzer-core/test/suppressions-e2e.test.ts +++ /dev/null @@ -1,495 +0,0 @@ -/** - * End-to-end integration tests for suppression markers with actual engine execution - * These tests run real engines (stub engines) to verify suppressions work in the full flow - */ - -import * as path from 'node:path'; -import { CodeAnalyzer } from '../src/code-analyzer'; -import { CodeAnalyzerConfig } from '../src/config'; -import * as stubs from './stubs'; -import { SeverityLevel } from '@salesforce/code-analyzer-engine-api'; -import { changeWorkingDirectoryToPackageRoot, FakeFileSystem } from './test-helpers'; - -changeWorkingDirectoryToPackageRoot(); - -describe('Suppression Markers E2E Tests (with actual engine execution)', () => { - const testDataDir = path.resolve(__dirname, 'test-data', 'suppression-markers'); - - describe('with suppressions enabled', () => { - it('should suppress all violations with suppress(all) marker', async () => { - // Create CodeAnalyzer with suppressions enabled - const config = CodeAnalyzerConfig.fromObject({ - suppressions: { disable_suppressions: false }, - log_level: 'error' - }); - const codeAnalyzer = new CodeAnalyzer(config, new FakeFileSystem()); - - // Add stub engine plugin - const plugin = new stubs.StubEnginePlugin(); - await codeAnalyzer.addEnginePlugin(plugin); - - // Create workspace with test file - const filePath = path.join(testDataDir, 'file-with-suppress-all.js'); - const workspace = await codeAnalyzer.createWorkspace([filePath]); - - // Select rules - const ruleSelection = await codeAnalyzer.selectRules(['stubEngine1'], { workspace }); - - // Configure engine to return violations - 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 - }, - { - ruleName: 'stub1RuleC', - message: 'Violation on line 9', - codeLocations: [{ file: filePath, startLine: 9, startColumn: 1 }], - primaryLocationIndex: 0 - } - ] - }; - - // Run the analyzer (this actually runs the engine) - const results = await codeAnalyzer.run(ruleSelection, { workspace }); - - // All violations should be suppressed (suppress(all) on line 6) - const violations = results.getViolations(); - expect(violations.length).toBe(0); - }); - - it('should suppress only engine-specific violations with suppress(engine)', async () => { - 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, 'e2e-file-with-suppress-engine.js'); - const workspace = await codeAnalyzer.createWorkspace([filePath]); - const ruleSelection = await codeAnalyzer.selectRules(['stubEngine1'], { workspace }); - - const stubEngine1 = plugin.getCreatedEngine('stubEngine1') as stubs.StubEngine1; - stubEngine1.resultsToReturn = { - violations: [ - { - ruleName: 'stub1RuleA', - message: 'Violation on line 6 (before marker)', - codeLocations: [{ file: filePath, startLine: 6, startColumn: 1 }], - primaryLocationIndex: 0 - }, - { - ruleName: 'stub1RuleB', - message: 'Violation on line 9 (after suppress(stubEngine1))', - codeLocations: [{ file: filePath, startLine: 9, startColumn: 1 }], - primaryLocationIndex: 0 - } - ] - }; - - const results = await codeAnalyzer.run(ruleSelection, { workspace }); - - // Only violation before marker should remain - const violations = results.getViolations(); - expect(violations.length).toBe(1); - expect(violations[0].getPrimaryLocation().getStartLine()).toBe(6); - }); - - it('should suppress only specific rule violations with suppress(engine:rule)', async () => { - 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); - - // File has: suppress(stubEngine1:stub1RuleA) on line 8 - const filePath = path.join(testDataDir, 'e2e-file-with-suppress-specific-rule.js'); - const workspace = await codeAnalyzer.createWorkspace([filePath]); - const ruleSelection = await codeAnalyzer.selectRules(['stubEngine1'], { workspace }); - - const stubEngine1 = plugin.getCreatedEngine('stubEngine1') as stubs.StubEngine1; - stubEngine1.resultsToReturn = { - violations: [ - { - ruleName: 'stub1RuleA', - message: 'Rule A on line 6 (before marker)', - codeLocations: [{ file: filePath, startLine: 6, startColumn: 1 }], - primaryLocationIndex: 0 - }, - { - ruleName: 'stub1RuleA', - message: 'Rule A on line 9 (should be suppressed)', - codeLocations: [{ file: filePath, startLine: 9, startColumn: 1 }], - primaryLocationIndex: 0 - }, - { - ruleName: 'stub1RuleB', - message: 'Rule B on line 10 (different rule, not suppressed)', - codeLocations: [{ file: filePath, startLine: 10, startColumn: 1 }], - primaryLocationIndex: 0 - }, - { - ruleName: 'stub1RuleA', - message: 'Rule A on line 11 (should be suppressed)', - codeLocations: [{ file: filePath, startLine: 11, startColumn: 1 }], - primaryLocationIndex: 0 - } - ] - }; - - const results = await codeAnalyzer.run(ruleSelection, { workspace }); - - // Should have: line 6 (before marker) and line 10 (different rule) - const violations = results.getViolations(); - expect(violations.length).toBe(2); - expect(violations[0].getPrimaryLocation().getStartLine()).toBe(6); - expect(violations[1].getPrimaryLocation().getStartLine()).toBe(10); - }); - - it('should re-enable violations with unsuppress marker', async () => { - 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); - - // File has: suppress(stubEngine1:stub1RuleA) on line 6, unsuppress on line 9 - const filePath = path.join(testDataDir, 'e2e-file-with-unsuppress.js'); - const workspace = await codeAnalyzer.createWorkspace([filePath]); - const ruleSelection = await codeAnalyzer.selectRules(['stubEngine1'], { workspace }); - - const stubEngine1 = plugin.getCreatedEngine('stubEngine1') as stubs.StubEngine1; - stubEngine1.resultsToReturn = { - violations: [ - { - ruleName: 'stub1RuleA', - message: 'Line 7 (after suppress, before unsuppress)', - codeLocations: [{ file: filePath, startLine: 7, startColumn: 1 }], - primaryLocationIndex: 0 - }, - { - ruleName: 'stub1RuleA', - message: 'Line 10 (after unsuppress)', - codeLocations: [{ file: filePath, startLine: 10, startColumn: 1 }], - primaryLocationIndex: 0 - } - ] - }; - - const results = await codeAnalyzer.run(ruleSelection, { workspace }); - - // Line 7 suppressed, line 10 NOT suppressed - const violations = results.getViolations(); - expect(violations.length).toBe(1); - expect(violations[0].getPrimaryLocation().getStartLine()).toBe(10); - }); - - it('should suppress violations with specified severities', async () => { - 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); - - // File has: suppress(stubEngine1:(3,4)) - suppresses Moderate and Low - const filePath = path.join(testDataDir, 'e2e-file-with-suppress-severity.js'); - const workspace = await codeAnalyzer.createWorkspace([filePath]); - const ruleSelection = await codeAnalyzer.selectRules(['stubEngine1'], { workspace }); - - const stubEngine1 = plugin.getCreatedEngine('stubEngine1') as stubs.StubEngine1; - stubEngine1.resultsToReturn = { - violations: [ - { - ruleName: 'stub1RuleA', // Severity 4 (Low) - should be suppressed - message: 'Low severity on line 8', - codeLocations: [{ file: filePath, startLine: 8, startColumn: 1 }], - primaryLocationIndex: 0 - }, - { - ruleName: 'stub1RuleB', // Severity 2 (High) - should NOT be suppressed - message: 'High severity on line 9', - codeLocations: [{ file: filePath, startLine: 9, startColumn: 1 }], - primaryLocationIndex: 0 - } - ] - }; - - const results = await codeAnalyzer.run(ruleSelection, { workspace }); - - // Only High severity (line 9) should remain - const violations = results.getViolations(); - expect(violations.length).toBe(1); - expect(violations[0].getRule().getSeverityLevel()).toBe(SeverityLevel.High); - }); - - it('should apply hierarchical specificity rules correctly', async () => { - 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); - - // File has: suppress(all) on line 6, unsuppress(stubEngine1:stub1RuleA) on line 9 - const filePath = path.join(testDataDir, 'e2e-file-with-hierarchical-suppressions.js'); - const workspace = await codeAnalyzer.createWorkspace([filePath]); - const ruleSelection = await codeAnalyzer.selectRules(['stubEngine1'], { workspace }); - - const stubEngine1 = plugin.getCreatedEngine('stubEngine1') as stubs.StubEngine1; - stubEngine1.resultsToReturn = { - violations: [ - { - ruleName: 'stub1RuleA', - message: 'Line 7 (suppressed by all)', - codeLocations: [{ file: filePath, startLine: 7, startColumn: 1 }], - primaryLocationIndex: 0 - }, - { - ruleName: 'stub1RuleA', - message: 'Line 10 (unsuppressed - higher specificity)', - codeLocations: [{ file: filePath, startLine: 10, startColumn: 1 }], - primaryLocationIndex: 0 - }, - { - ruleName: 'stub1RuleB', - message: 'Line 12 (still suppressed by all)', - codeLocations: [{ file: filePath, startLine: 12, startColumn: 1 }], - primaryLocationIndex: 0 - } - ] - }; - - const results = await codeAnalyzer.run(ruleSelection, { workspace }); - - // Only line 10 should remain (unsuppress with higher specificity) - const violations = results.getViolations(); - expect(violations.length).toBe(1); - expect(violations[0].getPrimaryLocation().getStartLine()).toBe(10); - }); - - it('should have no effect when only unsuppress markers are present', async () => { - 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); - - // File has only unsuppress markers, no suppress markers - const filePath = path.join(testDataDir, 'e2e-file-with-only-unsuppress.js'); - const workspace = await codeAnalyzer.createWorkspace([filePath]); - const ruleSelection = await codeAnalyzer.selectRules(['stubEngine1'], { workspace }); - - const stubEngine1 = plugin.getCreatedEngine('stubEngine1') as stubs.StubEngine1; - stubEngine1.resultsToReturn = { - violations: [ - { - ruleName: 'stub1RuleA', - message: 'Line 7 (before unsuppress)', - codeLocations: [{ file: filePath, startLine: 7, startColumn: 1 }], - primaryLocationIndex: 0 - }, - { - ruleName: 'stub1RuleA', - message: 'Line 10 (after unsuppress - no effect)', - codeLocations: [{ file: filePath, startLine: 10, startColumn: 1 }], - primaryLocationIndex: 0 - }, - { - ruleName: 'stub1RuleB', - message: 'Line 13 (after unsuppress(all) - no effect)', - codeLocations: [{ file: filePath, startLine: 13, startColumn: 1 }], - primaryLocationIndex: 0 - } - ] - }; - - const results = await codeAnalyzer.run(ruleSelection, { workspace }); - - // All violations should remain - unsuppress without suppress has no effect - const violations = results.getViolations(); - expect(violations.length).toBe(3); - expect(violations[0].getPrimaryLocation().getStartLine()).toBe(7); - expect(violations[1].getPrimaryLocation().getStartLine()).toBe(10); - expect(violations[2].getPrimaryLocation().getStartLine()).toBe(13); - }); - - it('should create exception range with nested suppress/unsuppress against broader suppress', async () => { - // Regression test: suppress(all) -> suppress(specific) -> unsuppress(specific) - // After unsuppress, specific rule should NOT be suppressed by broader suppress(all) - 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, 'e2e-file-with-nested-suppress-unsuppress.js'); - const workspace = await codeAnalyzer.createWorkspace([filePath]); - const ruleSelection = await codeAnalyzer.selectRules(['stubEngine1'], { workspace }); - - const stubEngine1 = plugin.getCreatedEngine('stubEngine1') as stubs.StubEngine1; - stubEngine1.resultsToReturn = { - violations: [ - { - ruleName: 'stub1RuleA', - message: 'Line 10 (suppressed by all)', - codeLocations: [{ file: filePath, startLine: 10, startColumn: 1 }], - primaryLocationIndex: 0 - }, - { - ruleName: 'stub1RuleA', - message: 'Line 13 (suppressed by specific)', - codeLocations: [{ file: filePath, startLine: 13, startColumn: 1 }], - primaryLocationIndex: 0 - }, - { - ruleName: 'stub1RuleA', - message: 'Line 16 (after unsuppress - should NOT be suppressed)', - codeLocations: [{ file: filePath, startLine: 16, startColumn: 1 }], - primaryLocationIndex: 0 - }, - { - ruleName: 'stub1RuleB', - message: 'Line 19 (different rule - still suppressed by all)', - codeLocations: [{ file: filePath, startLine: 19, startColumn: 1 }], - primaryLocationIndex: 0 - } - ] - }; - - const results = await codeAnalyzer.run(ruleSelection, { workspace }); - - // Critical: Line 16 should NOT be suppressed - // unsuppress(stubEngine1:stub1RuleA) creates exception range against suppress(all) - const violations = results.getViolations(); - expect(violations.length).toBe(1); - expect(violations[0].getPrimaryLocation().getStartLine()).toBe(16); - expect(violations[0].getRule().getName()).toBe('stub1RuleA'); - }); - }); - - describe('with suppressions disabled', () => { - it('should report all violations when suppressions are disabled', async () => { - // Create CodeAnalyzer with suppressions DISABLED - const config = CodeAnalyzerConfig.fromObject({ - suppressions: { disable_suppressions: true }, // Explicitly disabled - 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]); - const ruleSelection = await codeAnalyzer.selectRules(['stubEngine1'], { workspace }); - - 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 - } - ] - }; - - const results = await codeAnalyzer.run(ruleSelection, { workspace }); - - // Should have all violations even though file has suppress(all) - const violations = results.getViolations(); - expect(violations.length).toBe(2); - }); - }); - - describe('comparing with and without suppressions', () => { - it('should have fewer violations with suppressions than without', async () => { - // Create two analyzers - one with, one without suppressions - const configWith = CodeAnalyzerConfig.fromObject({ - suppressions: { disable_suppressions: false }, - log_level: 'error' - }); - const configWithout = CodeAnalyzerConfig.fromObject({ - suppressions: { disable_suppressions: true }, - log_level: 'error' - }); - - const analyzerWith = new CodeAnalyzer(configWith, new FakeFileSystem()); - const analyzerWithout = new CodeAnalyzer(configWithout, new FakeFileSystem()); - - // Add plugins to both - const pluginWith = new stubs.StubEnginePlugin(); - const pluginWithout = new stubs.StubEnginePlugin(); - await analyzerWith.addEnginePlugin(pluginWith); - await analyzerWithout.addEnginePlugin(pluginWithout); - - const filePath = path.join(testDataDir, 'file-with-suppress-all.js'); - - // Run with suppressions - const workspaceWith = await analyzerWith.createWorkspace([filePath]); - const ruleSelectionWith = await analyzerWith.selectRules(['stubEngine1'], { workspace: workspaceWith }); - const stubEngine1With = pluginWith.getCreatedEngine('stubEngine1') as stubs.StubEngine1; - stubEngine1With.resultsToReturn = { - violations: [ - { - ruleName: 'stub1RuleA', - message: 'Violation', - codeLocations: [{ file: filePath, startLine: 7, startColumn: 1 }], - primaryLocationIndex: 0 - } - ] - }; - const resultsWith = await analyzerWith.run(ruleSelectionWith, { workspace: workspaceWith }); - - // Run without suppressions - const workspaceWithout = await analyzerWithout.createWorkspace([filePath]); - const ruleSelectionWithout = await analyzerWithout.selectRules(['stubEngine1'], { workspace: workspaceWithout }); - const stubEngine1Without = pluginWithout.getCreatedEngine('stubEngine1') as stubs.StubEngine1; - stubEngine1Without.resultsToReturn = { - violations: [ - { - ruleName: 'stub1RuleA', - message: 'Violation', - codeLocations: [{ file: filePath, startLine: 7, startColumn: 1 }], - primaryLocationIndex: 0 - } - ] - }; - const resultsWithout = await analyzerWithout.run(ruleSelectionWithout, { workspace: workspaceWithout }); - - // With suppressions: 0 violations - // Without suppressions: 1 violation - expect(resultsWith.getViolations().length).toBe(0); - expect(resultsWithout.getViolations().length).toBe(1); - }); - }); -}); From be287f91186eaa3521ef5aed08a88f0587305127 Mon Sep 17 00:00:00 2001 From: Namrata Gupta Date: Fri, 10 Apr 2026 14:20:31 +0530 Subject: [PATCH 04/16] fixing path separator UT for windows --- .../bulk-suppression-processor.ts | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts b/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts index 066f51a2..91648ab7 100644 --- a/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts +++ b/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts @@ -130,6 +130,15 @@ function findMatchingBulkSuppressionRules( return matchingRules; } +/** + * Normalizes path separators to forward slashes for cross-platform comparison + * @param filePath Path to normalize + * @returns Path with forward slashes + */ +function normalizeSeparators(filePath: string): string { + return filePath.split(path.sep).join('/'); +} + /** * Checks if a file path matches a config path (file or folder) * @param violationFile Absolute file path from violation @@ -147,9 +156,9 @@ function doesFileMatchConfigPath( ? configPath : path.resolve(workspaceRoot, configPath); - // Normalize paths for comparison - const normalizedViolationFile = path.normalize(violationFile); - const normalizedConfigPath = path.normalize(absoluteConfigPath); + // Normalize paths for comparison (use forward slashes for cross-platform compatibility) + const normalizedViolationFile = normalizeSeparators(path.normalize(violationFile)); + const normalizedConfigPath = normalizeSeparators(path.normalize(absoluteConfigPath)); // Check if it's an exact file match if (normalizedViolationFile === normalizedConfigPath) { @@ -158,9 +167,9 @@ function doesFileMatchConfigPath( // Check if violation file is within config folder // Config path is a folder if it matches the start of the file path - const configPathWithSep = normalizedConfigPath.endsWith(path.sep) + const configPathWithSep = normalizedConfigPath.endsWith('/') ? normalizedConfigPath - : normalizedConfigPath + path.sep; + : normalizedConfigPath + '/'; return normalizedViolationFile.startsWith(configPathWithSep); } From 65d9832047e0d3042036662042b966f1bd423428 Mon Sep 17 00:00:00 2001 From: Namrata Gupta Date: Fri, 10 Apr 2026 14:32:18 +0530 Subject: [PATCH 05/16] fix normalization for windows --- .../bulk-suppression-processor.ts | 53 +++++++++++-------- 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts b/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts index 91648ab7..2a6a4784 100644 --- a/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts +++ b/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts @@ -130,17 +130,11 @@ function findMatchingBulkSuppressionRules( return matchingRules; } -/** - * Normalizes path separators to forward slashes for cross-platform comparison - * @param filePath Path to normalize - * @returns Path with forward slashes - */ -function normalizeSeparators(filePath: string): string { - return filePath.split(path.sep).join('/'); -} - /** * Checks if a file path matches a config path (file or folder) + * Uses Node's path module for proper path operations, then normalizes separators for comparison. + * This follows the same pattern as the ignores feature in workspace.ts for cross-platform compatibility. + * * @param violationFile Absolute file path from violation * @param configPath Relative path from config (file or folder) * @param workspaceRoot Root directory for resolving relative paths @@ -151,27 +145,40 @@ function doesFileMatchConfigPath( configPath: string, workspaceRoot: string ): boolean { - // Convert config path to absolute + // Step 1: Use path module to properly resolve and normalize paths + // This handles . and .., redundant separators, and makes paths absolute + const normalizedWorkspaceRoot = path.normalize(workspaceRoot); const absoluteConfigPath = path.isAbsolute(configPath) - ? configPath - : path.resolve(workspaceRoot, configPath); + ? path.normalize(configPath) + : path.resolve(normalizedWorkspaceRoot, configPath); - // Normalize paths for comparison (use forward slashes for cross-platform compatibility) - const normalizedViolationFile = normalizeSeparators(path.normalize(violationFile)); - const normalizedConfigPath = normalizeSeparators(path.normalize(absoluteConfigPath)); + const normalizedViolationFile = path.normalize(violationFile); + + // Step 2: Normalize to POSIX separators for cross-platform comparison + // This follows the same pattern as ignores feature (workspace.ts lines 203-206) + // Windows: C:\workspace\src\file.apex -> C:/workspace/src/file.apex + // Unix: /workspace/src/file.apex -> /workspace/src/file.apex (no change) + let comparisonViolationFile = normalizedViolationFile; + let comparisonConfigPath = absoluteConfigPath; + + if (path.sep !== '/') { + comparisonViolationFile = comparisonViolationFile.split(path.sep).join('/'); + comparisonConfigPath = comparisonConfigPath.split(path.sep).join('/'); + } - // Check if it's an exact file match - if (normalizedViolationFile === normalizedConfigPath) { + // Step 3: Check if it's an exact file match + if (comparisonViolationFile === comparisonConfigPath) { return true; } - // Check if violation file is within config folder - // Config path is a folder if it matches the start of the file path - const configPathWithSep = normalizedConfigPath.endsWith('/') - ? normalizedConfigPath - : normalizedConfigPath + '/'; + // Step 4: Check if violation file is within config folder + // Add separator to ensure we match whole directory names + // e.g., "src/utils" should match "src/utils/file.js" but not "src/utils2/file.js" + const configPathWithSep = comparisonConfigPath.endsWith('/') + ? comparisonConfigPath + : comparisonConfigPath + '/'; - return normalizedViolationFile.startsWith(configPathWithSep); + return comparisonViolationFile.startsWith(configPathWithSep); } /** From 2eade722d52a3414013358ed07144adb937f3168 Mon Sep 17 00:00:00 2001 From: Namrata Gupta Date: Fri, 10 Apr 2026 14:48:32 +0530 Subject: [PATCH 06/16] adding changes for testing --- .github/workflows/verify-pr.yml | 2 + .../bulk-suppression-processor.ts | 81 ++++++++++++++----- .../bulk-suppression-processor.test.ts | 56 +++++++------ .../bulk-suppression-workspace-root.test.ts | 28 +++++-- 4 files changed, 116 insertions(+), 51 deletions(-) diff --git a/.github/workflows/verify-pr.yml b/.github/workflows/verify-pr.yml index fe441410..c1bce6ea 100644 --- a/.github/workflows/verify-pr.yml +++ b/.github/workflows/verify-pr.yml @@ -96,3 +96,5 @@ jobs: npm run build `cat workspace-args.txt` npm run lint npm run test `cat workspace-args.txt` + env: + DEBUG_BULK_SUPPRESSIONS: ${{ matrix.os == 'windows-latest' && 'true' || 'false' }} diff --git a/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts b/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts index 2a6a4784..281a6f5a 100644 --- a/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts +++ b/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts @@ -80,7 +80,7 @@ export function applyBulkSuppressions( } // Find matching suppression rules for this violation's file - const matchingRules = findMatchingBulkSuppressionRules(violationFile, bulkConfig, workspaceRoot); + const matchingRules = findMatchingBulkSuppressionRules(violationFile, bulkConfig, workspaceRoot, logger); let suppressed = false; for (const ruleWithPath of matchingRules) { @@ -107,17 +107,19 @@ export function applyBulkSuppressions( * @param violationFile Absolute file path from violation * @param bulkConfig Bulk suppression configuration * @param workspaceRoot Root directory for resolving relative paths + * @param logger Optional logger callback for debug messages * @returns Array of rules with their matching config paths */ function findMatchingBulkSuppressionRules( violationFile: string, bulkConfig: Record, - workspaceRoot: string + workspaceRoot: string, + logger?: LoggerCallback ): RuleWithConfigPath[] { const matchingRules: RuleWithConfigPath[] = []; for (const [configPath, rules] of Object.entries(bulkConfig)) { - const matches = doesFileMatchConfigPath(violationFile, configPath, workspaceRoot); + const matches = doesFileMatchConfigPath(violationFile, configPath, workspaceRoot, logger); if (matches) { // Add each rule with its config path and index for unique quota tracking @@ -143,31 +145,68 @@ function findMatchingBulkSuppressionRules( function doesFileMatchConfigPath( violationFile: string, configPath: string, - workspaceRoot: string + workspaceRoot: string, + logger?: LoggerCallback ): boolean { + const log = (msg: string) => logger?.('debug', msg); + // Step 1: Use path module to properly resolve and normalize paths // This handles . and .., redundant separators, and makes paths absolute - const normalizedWorkspaceRoot = path.normalize(workspaceRoot); - const absoluteConfigPath = path.isAbsolute(configPath) - ? path.normalize(configPath) - : path.resolve(normalizedWorkspaceRoot, configPath); - const normalizedViolationFile = path.normalize(violationFile); + // Helper to check if a path is absolute (cross-platform) + // Handles: Unix (/foo), Windows (C:\foo), UNC (\\server\share) + const isAbsolutePath = (p: string): boolean => { + // Unix absolute path starts with / + if (p.startsWith('/')) return true; + // Windows absolute path: C:\ or C:/ (drive letter + colon) + if (/^[A-Za-z]:[\\/]/.test(p)) return true; + // UNC path: \\server\share + if (p.startsWith('\\\\')) return true; + // Use path.isAbsolute() for platform-native check + if (path.isAbsolute(p)) return true; + return false; + }; + + // Helper to join paths in a cross-platform way + // When joining Windows paths on Unix or vice versa, path.resolve() doesn't work correctly + // So we do simple string concatenation and normalize separators later + const joinPaths = (base: string, relative: string): string => { + // Normalize the base path separator to match its style + const baseEndsWithSep = base.endsWith('/') || base.endsWith('\\'); + const sep = base.includes('\\') ? '\\' : '/'; + return baseEndsWithSep ? base + relative : base + sep + relative; + }; + + // Normalize workspace root - keep as-is if absolute, otherwise normalize with platform path module + const normalizedWorkspaceRoot = isAbsolutePath(workspaceRoot) + ? workspaceRoot + : path.normalize(workspaceRoot); + + // For config path: if absolute, use as-is; otherwise join with workspace root + const absoluteConfigPath = isAbsolutePath(configPath) + ? configPath + : joinPaths(normalizedWorkspaceRoot, configPath); + + // Same for violation file + const normalizedViolationFile = isAbsolutePath(violationFile) + ? violationFile + : joinPaths(normalizedWorkspaceRoot, violationFile); + + log(`Path matching - violationFile: "${violationFile}", configPath: "${configPath}", workspaceRoot: "${workspaceRoot}"`); + log(`After normalization - normalizedViolationFile: "${normalizedViolationFile}", absoluteConfigPath: "${absoluteConfigPath}"`); // Step 2: Normalize to POSIX separators for cross-platform comparison - // This follows the same pattern as ignores feature (workspace.ts lines 203-206) - // Windows: C:\workspace\src\file.apex -> C:/workspace/src/file.apex - // Unix: /workspace/src/file.apex -> /workspace/src/file.apex (no change) - let comparisonViolationFile = normalizedViolationFile; - let comparisonConfigPath = absoluteConfigPath; - - if (path.sep !== '/') { - comparisonViolationFile = comparisonViolationFile.split(path.sep).join('/'); - comparisonConfigPath = comparisonConfigPath.split(path.sep).join('/'); - } + // Convert all backslashes to forward slashes for consistent comparison + // This handles both: Windows paths on Unix (C:\foo -> C:/foo) and Unix paths on Windows + const comparisonViolationFile = normalizedViolationFile.replace(/\\/g, '/'); + const comparisonConfigPath = absoluteConfigPath.replace(/\\/g, '/'); + + log(`After separator normalization - comparisonViolationFile: "${comparisonViolationFile}", comparisonConfigPath: "${comparisonConfigPath}"`); + // Step 3: Check if it's an exact file match if (comparisonViolationFile === comparisonConfigPath) { + log(`Path matched (exact file match)`); return true; } @@ -178,7 +217,9 @@ function doesFileMatchConfigPath( ? comparisonConfigPath : comparisonConfigPath + '/'; - return comparisonViolationFile.startsWith(configPathWithSep); + const matches = comparisonViolationFile.startsWith(configPathWithSep); + log(`Path ${matches ? 'matched' : 'did not match'} (folder prefix: "${configPathWithSep}")`); + return matches; } /** diff --git a/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts b/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts index ab091b37..b52eddae 100644 --- a/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts +++ b/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts @@ -11,6 +11,16 @@ import { BulkSuppressionRule } from '../../src/config'; import { Violation, CodeLocation } from '../../src/results'; import { Rule, SeverityLevel } from '../../src/rules'; +// Helper to enable debug logging in tests via environment variable +const createTestLogger = () => { + if (process.env.DEBUG_BULK_SUPPRESSIONS === 'true') { + return (level: 'error' | 'warn' | 'debug', message: string) => { + console.log(`[${level.toUpperCase()}] ${message}`); + }; + } + return undefined; +}; + // Mock implementations for testing class MockCodeLocation implements CodeLocation { constructor( @@ -147,7 +157,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); expect(result.unsuppressedViolations).toHaveLength(0); expect(result.suppressedCount).toBe(2); @@ -184,7 +194,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); expect(result.unsuppressedViolations).toHaveLength(1); expect(result.suppressedCount).toBe(2); @@ -215,7 +225,7 @@ describe('applyBulkSuppressions', () => { // Quota key uses config path with rule index quotas.set('src/file1.apex|0|pmd:UnusedMethod', 1); // Quota already used - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); expect(result.unsuppressedViolations).toHaveLength(1); expect(result.suppressedCount).toBe(0); @@ -246,7 +256,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); expect(result.unsuppressedViolations).toHaveLength(1); expect(result.suppressedCount).toBe(1); @@ -274,7 +284,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); expect(result.suppressedCount).toBe(1); }); @@ -303,7 +313,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); expect(result.suppressedCount).toBe(2); }); @@ -327,7 +337,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); expect(result.suppressedCount).toBe(0); expect(result.unsuppressedViolations).toHaveLength(1); @@ -352,7 +362,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); expect(result.suppressedCount).toBe(1); }); @@ -383,7 +393,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); expect(result.suppressedCount).toBe(1); expect(result.unsuppressedViolations).toHaveLength(1); @@ -414,7 +424,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); expect(result.suppressedCount).toBe(1); expect(result.unsuppressedViolations).toHaveLength(1); @@ -450,7 +460,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); expect(result.suppressedCount).toBe(2); expect(result.unsuppressedViolations).toHaveLength(1); @@ -481,7 +491,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); expect(result.suppressedCount).toBe(1); expect(result.unsuppressedViolations).toHaveLength(1); @@ -518,7 +528,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); expect(result.suppressedCount).toBe(2); expect(result.unsuppressedViolations).toHaveLength(0); @@ -547,7 +557,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); expect(result.suppressedCount).toBe(1); // Quota key uses config path with rule index (first rule at index 0 is used) @@ -586,7 +596,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); expect(result.suppressedCount).toBe(2); expect(result.unsuppressedViolations).toHaveLength(1); @@ -626,7 +636,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); // Should suppress first 2 violations (across different files), reject 3rd expect(result.suppressedCount).toBe(2); @@ -666,13 +676,13 @@ describe('applyBulkSuppressions', () => { const quotas: BulkSuppressionQuotas = new Map(); // First call - should suppress - const result1 = applyBulkSuppressions(violations1, bulkConfig, quotas, workspaceRoot); + const result1 = applyBulkSuppressions(violations1, bulkConfig, quotas, workspaceRoot, createTestLogger()); expect(result1.suppressedCount).toBe(1); // Quota key uses config path with rule index expect(quotas.get('src/file1.apex|0|pmd:UnusedMethod')).toBe(1); // Second call - quota exhausted, should not suppress - const result2 = applyBulkSuppressions(violations2, bulkConfig, quotas, workspaceRoot); + const result2 = applyBulkSuppressions(violations2, bulkConfig, quotas, workspaceRoot, createTestLogger()); expect(result2.suppressedCount).toBe(0); expect(result2.unsuppressedViolations).toHaveLength(1); }); @@ -689,7 +699,7 @@ describe('applyBulkSuppressions', () => { ]; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, {}, quotas, workspaceRoot); + const result = applyBulkSuppressions(violations, {}, quotas, workspaceRoot, createTestLogger()); expect(result.unsuppressedViolations).toHaveLength(1); expect(result.suppressedCount).toBe(0); @@ -706,7 +716,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions([], bulkConfig, quotas, workspaceRoot); + const result = applyBulkSuppressions([], bulkConfig, quotas, workspaceRoot, createTestLogger()); expect(result.unsuppressedViolations).toHaveLength(0); expect(result.suppressedCount).toBe(0); @@ -731,7 +741,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); expect(result.unsuppressedViolations).toHaveLength(1); expect(result.suppressedCount).toBe(0); @@ -756,7 +766,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); // Should not suppress due to invalid selector expect(result.unsuppressedViolations).toHaveLength(1); @@ -782,7 +792,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); expect(result.unsuppressedViolations).toHaveLength(1); expect(result.suppressedCount).toBe(0); diff --git a/packages/code-analyzer-core/test/suppressions/bulk-suppression-workspace-root.test.ts b/packages/code-analyzer-core/test/suppressions/bulk-suppression-workspace-root.test.ts index e351f56b..a4f1684a 100644 --- a/packages/code-analyzer-core/test/suppressions/bulk-suppression-workspace-root.test.ts +++ b/packages/code-analyzer-core/test/suppressions/bulk-suppression-workspace-root.test.ts @@ -3,6 +3,16 @@ import {BulkSuppressionRule} from "../../src/config"; import {Violation, CodeLocation} from "../../src/results"; import {RuleImpl} from "../../src/rules"; +// Helper to enable debug logging in tests via environment variable +const createTestLogger = () => { + if (process.env.DEBUG_BULK_SUPPRESSIONS === 'true') { + return (level: 'error' | 'warn' | 'debug', message: string) => { + console.log(`[${level.toUpperCase()}] ${message}`); + }; + } + return undefined; +}; + /** * Tests specifically for Bug #2: Workspace Root vs Config Root inconsistency * @@ -105,7 +115,7 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { const quotas: BulkSuppressionQuotas = new Map(); // Apply suppressions using workspace root (Bug #2 fix) - const result = applyBulkSuppressions([violation], bulkConfig, quotas, workspaceRoot); + const result = applyBulkSuppressions([violation], bulkConfig, quotas, workspaceRoot, createTestLogger()); // Violation should be suppressed expect(result.unsuppressedViolations).toHaveLength(0); @@ -133,7 +143,7 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions([violation], bulkConfig, quotas, workspaceRoot); + const result = applyBulkSuppressions([violation], bulkConfig, quotas, workspaceRoot, createTestLogger()); // Should work regardless of bug expect(result.unsuppressedViolations).toHaveLength(0); @@ -162,7 +172,7 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions([violation], bulkConfig, quotas, workspaceRoot); + const result = applyBulkSuppressions([violation], bulkConfig, quotas, workspaceRoot, createTestLogger()); // Should NOT suppress because path doesn't match expect(result.unsuppressedViolations).toHaveLength(1); @@ -209,7 +219,8 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { [violation1, violation2, violation3], bulkConfig, quotas, - workspaceRoot + workspaceRoot, + createTestLogger() ); // All should be suppressed @@ -245,7 +256,8 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { [violation1, violation2], bulkConfig, quotas, - workspaceRoot + workspaceRoot, + createTestLogger() ); // Both files in folder should be suppressed @@ -272,7 +284,7 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions([violation], bulkConfig, quotas, workspaceRoot); + const result = applyBulkSuppressions([violation], bulkConfig, quotas, workspaceRoot, createTestLogger()); // Should suppress expect(result.unsuppressedViolations).toHaveLength(0); @@ -305,7 +317,7 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions([violation], bulkConfig, quotas, workspaceRoot); + const result = applyBulkSuppressions([violation], bulkConfig, quotas, workspaceRoot, createTestLogger()); expect(result.unsuppressedViolations).toHaveLength(0); expect(result.suppressedCount).toBe(1); @@ -347,7 +359,7 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); // First rule suppresses 3, second rule suppresses 2 = 5 total expect(result.suppressedCount).toBe(5); From e1f5b7bbe5fc04891fd84f094246af190d825018 Mon Sep 17 00:00:00 2001 From: Namrata Gupta Date: Fri, 10 Apr 2026 15:09:43 +0530 Subject: [PATCH 07/16] fix absolute path handling --- .github/workflows/verify-pr.yml | 2 - .../code-analyzer-core/src/code-analyzer.ts | 10 +- .../bulk-suppression-processor.ts | 104 ++++-------------- .../bulk-suppression-processor.test.ts | 56 ++++------ .../bulk-suppression-workspace-root.test.ts | 38 +++---- 5 files changed, 60 insertions(+), 150 deletions(-) diff --git a/.github/workflows/verify-pr.yml b/.github/workflows/verify-pr.yml index c1bce6ea..fe441410 100644 --- a/.github/workflows/verify-pr.yml +++ b/.github/workflows/verify-pr.yml @@ -96,5 +96,3 @@ jobs: npm run build `cat workspace-args.txt` npm run lint npm run test `cat workspace-args.txt` - env: - DEBUG_BULK_SUPPRESSIONS: ${{ matrix.os == 'windows-latest' && 'true' || 'false' }} diff --git a/packages/code-analyzer-core/src/code-analyzer.ts b/packages/code-analyzer-core/src/code-analyzer.ts index 6ff954c6..c94593d4 100644 --- a/packages/code-analyzer-core/src/code-analyzer.ts +++ b/packages/code-analyzer-core/src/code-analyzer.ts @@ -552,19 +552,11 @@ export class CodeAnalyzer { // This is set during run() and should never be null at this point const workspaceRoot = this.currentWorkspaceRoot || this.config.getConfigRoot(); - // Create logger for bulk suppression debug output - 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 bulkResult = applyBulkSuppressions( violations, bulkConfig, this.bulkSuppressionQuotas, - workspaceRoot, - logger + workspaceRoot ); const suppressedCount = bulkResult.suppressedCount; diff --git a/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts b/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts index 281a6f5a..94642702 100644 --- a/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts +++ b/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts @@ -3,7 +3,6 @@ import {BulkSuppressionRule} from "../config"; import {toSelector, Selector} from "../selectors"; import {SeverityLevel} from "../rules"; import * as path from 'node:path'; -import {LoggerCallback} from "./suppression-processor"; /** * Tracks the number of suppressions applied per config path and rule selector combination @@ -34,27 +33,18 @@ export type BulkSuppressionResult = { * @param bulkConfig Bulk suppression configuration from YAML * @param quotas Shared quota tracker (mutated by this function) * @param workspaceRoot Root directory for resolving relative paths - * @param logger Optional logger callback for debug/info messages * @returns Object containing unsuppressed violations and count of suppressions applied */ export function applyBulkSuppressions( violations: Violation[], bulkConfig: Record, quotas: BulkSuppressionQuotas, - workspaceRoot: string, - logger?: LoggerCallback + workspaceRoot: string ): BulkSuppressionResult { - const log = (msg: string) => logger?.('debug', msg); - - log(`Bulk suppressions: processing ${violations.length} violation(s) with workspace root ${workspaceRoot}`); - if (Object.keys(bulkConfig).length === 0) { - log('No bulk suppressions configured'); return { unsuppressedViolations: violations, suppressedCount: 0 }; } - log(`Bulk suppression config paths: ${Object.keys(bulkConfig).join(', ')}`); - // Sort violations for deterministic processing within this engine const sortedViolations = [...violations].sort((a, b) => { const fileA = a.getPrimaryLocation().getFile() || ''; @@ -80,11 +70,11 @@ export function applyBulkSuppressions( } // Find matching suppression rules for this violation's file - const matchingRules = findMatchingBulkSuppressionRules(violationFile, bulkConfig, workspaceRoot, logger); + const matchingRules = findMatchingBulkSuppressionRules(violationFile, bulkConfig, workspaceRoot); let suppressed = false; for (const ruleWithPath of matchingRules) { - if (shouldSuppressViolation(violation, ruleWithPath.rule, quotas, ruleWithPath.configPath, ruleWithPath.ruleIndex, logger)) { + if (shouldSuppressViolation(violation, ruleWithPath.rule, quotas, ruleWithPath.configPath, ruleWithPath.ruleIndex)) { suppressed = true; suppressedCount++; break; // Violation suppressed, move to next @@ -96,7 +86,6 @@ export function applyBulkSuppressions( } } - log(`Bulk suppressions: ${suppressedCount} suppressed, ${unsuppressedViolations.length} unsuppressed`); return { unsuppressedViolations, suppressedCount }; } @@ -113,13 +102,12 @@ export function applyBulkSuppressions( function findMatchingBulkSuppressionRules( violationFile: string, bulkConfig: Record, - workspaceRoot: string, - logger?: LoggerCallback + workspaceRoot: string ): RuleWithConfigPath[] { const matchingRules: RuleWithConfigPath[] = []; for (const [configPath, rules] of Object.entries(bulkConfig)) { - const matches = doesFileMatchConfigPath(violationFile, configPath, workspaceRoot, logger); + const matches = doesFileMatchConfigPath(violationFile, configPath, workspaceRoot); if (matches) { // Add each rule with its config path and index for unique quota tracking @@ -145,55 +133,20 @@ function findMatchingBulkSuppressionRules( function doesFileMatchConfigPath( violationFile: string, configPath: string, - workspaceRoot: string, - logger?: LoggerCallback + workspaceRoot: string ): boolean { - const log = (msg: string) => logger?.('debug', msg); - - // Step 1: Use path module to properly resolve and normalize paths - // This handles . and .., redundant separators, and makes paths absolute - - // Helper to check if a path is absolute (cross-platform) - // Handles: Unix (/foo), Windows (C:\foo), UNC (\\server\share) - const isAbsolutePath = (p: string): boolean => { - // Unix absolute path starts with / - if (p.startsWith('/')) return true; - // Windows absolute path: C:\ or C:/ (drive letter + colon) - if (/^[A-Za-z]:[\\/]/.test(p)) return true; - // UNC path: \\server\share - if (p.startsWith('\\\\')) return true; - // Use path.isAbsolute() for platform-native check - if (path.isAbsolute(p)) return true; - return false; - }; - - // Helper to join paths in a cross-platform way - // When joining Windows paths on Unix or vice versa, path.resolve() doesn't work correctly - // So we do simple string concatenation and normalize separators later - const joinPaths = (base: string, relative: string): string => { - // Normalize the base path separator to match its style - const baseEndsWithSep = base.endsWith('/') || base.endsWith('\\'); - const sep = base.includes('\\') ? '\\' : '/'; - return baseEndsWithSep ? base + relative : base + sep + relative; - }; - - // Normalize workspace root - keep as-is if absolute, otherwise normalize with platform path module - const normalizedWorkspaceRoot = isAbsolutePath(workspaceRoot) - ? workspaceRoot - : path.normalize(workspaceRoot); - - // For config path: if absolute, use as-is; otherwise join with workspace root - const absoluteConfigPath = isAbsolutePath(configPath) - ? configPath - : joinPaths(normalizedWorkspaceRoot, configPath); - - // Same for violation file - const normalizedViolationFile = isAbsolutePath(violationFile) - ? violationFile - : joinPaths(normalizedWorkspaceRoot, violationFile); - - log(`Path matching - violationFile: "${violationFile}", configPath: "${configPath}", workspaceRoot: "${workspaceRoot}"`); - log(`After normalization - normalizedViolationFile: "${normalizedViolationFile}", absoluteConfigPath: "${absoluteConfigPath}"`); + // Step 1: Use path module to resolve config paths relative to workspace root + // Config paths should always be relative (like .gitignore patterns) + // Violation file paths are absolute (from the file system) + + const normalizedWorkspaceRoot = path.normalize(workspaceRoot); + + // Config paths are always treated as relative to workspace root + // If user provides absolute path in config, it won't match - that's correct behavior + const absoluteConfigPath = path.resolve(normalizedWorkspaceRoot, configPath); + + // Violation files are always absolute paths from the engine + const normalizedViolationFile = path.normalize(violationFile); // Step 2: Normalize to POSIX separators for cross-platform comparison // Convert all backslashes to forward slashes for consistent comparison @@ -201,12 +154,8 @@ function doesFileMatchConfigPath( const comparisonViolationFile = normalizedViolationFile.replace(/\\/g, '/'); const comparisonConfigPath = absoluteConfigPath.replace(/\\/g, '/'); - log(`After separator normalization - comparisonViolationFile: "${comparisonViolationFile}", comparisonConfigPath: "${comparisonConfigPath}"`); - - // Step 3: Check if it's an exact file match if (comparisonViolationFile === comparisonConfigPath) { - log(`Path matched (exact file match)`); return true; } @@ -217,9 +166,7 @@ function doesFileMatchConfigPath( ? comparisonConfigPath : comparisonConfigPath + '/'; - const matches = comparisonViolationFile.startsWith(configPathWithSep); - log(`Path ${matches ? 'matched' : 'did not match'} (folder prefix: "${configPathWithSep}")`); - return matches; + return comparisonViolationFile.startsWith(configPathWithSep); } /** @@ -238,11 +185,10 @@ function shouldSuppressViolation( rule: BulkSuppressionRule, quotas: BulkSuppressionQuotas, configPath: string, - ruleIndex: number, - logger?: LoggerCallback + ruleIndex: number ): boolean { // Check if the rule selector matches this violation - if (!ruleMatches(violation, rule.rule_selector, logger)) { + if (!ruleMatches(violation, rule.rule_selector)) { return false; } @@ -273,12 +219,9 @@ function shouldSuppressViolation( * Uses the same rule selector matching logic as rule selection * @param violation The violation to check * @param ruleSelector The rule selector string (e.g., "pmd:UnusedMethod", "3,4", etc.) - * @param logger Optional logger callback for debug messages * @returns true if the violation matches the selector */ -function ruleMatches(violation: Violation, ruleSelector: string, logger?: LoggerCallback): boolean { - const log = (msg: string) => logger?.('debug', msg); - +function ruleMatches(violation: Violation, ruleSelector: string): boolean { try { const selector: Selector = toSelector(ruleSelector); const rule = violation.getRule(); @@ -296,9 +239,8 @@ function ruleMatches(violation: Violation, ruleSelector: string, logger?: Logger ]; return selector.matchesSelectables(selectables); - } catch (error) { + } catch (_error) { // If selector is invalid, don't match - log(`Invalid rule selector "${ruleSelector}": ${error instanceof Error ? error.message : String(error)}`); return false; } } diff --git a/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts b/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts index b52eddae..ab091b37 100644 --- a/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts +++ b/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts @@ -11,16 +11,6 @@ import { BulkSuppressionRule } from '../../src/config'; import { Violation, CodeLocation } from '../../src/results'; import { Rule, SeverityLevel } from '../../src/rules'; -// Helper to enable debug logging in tests via environment variable -const createTestLogger = () => { - if (process.env.DEBUG_BULK_SUPPRESSIONS === 'true') { - return (level: 'error' | 'warn' | 'debug', message: string) => { - console.log(`[${level.toUpperCase()}] ${message}`); - }; - } - return undefined; -}; - // Mock implementations for testing class MockCodeLocation implements CodeLocation { constructor( @@ -157,7 +147,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); expect(result.unsuppressedViolations).toHaveLength(0); expect(result.suppressedCount).toBe(2); @@ -194,7 +184,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); expect(result.unsuppressedViolations).toHaveLength(1); expect(result.suppressedCount).toBe(2); @@ -225,7 +215,7 @@ describe('applyBulkSuppressions', () => { // Quota key uses config path with rule index quotas.set('src/file1.apex|0|pmd:UnusedMethod', 1); // Quota already used - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); expect(result.unsuppressedViolations).toHaveLength(1); expect(result.suppressedCount).toBe(0); @@ -256,7 +246,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); expect(result.unsuppressedViolations).toHaveLength(1); expect(result.suppressedCount).toBe(1); @@ -284,7 +274,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); expect(result.suppressedCount).toBe(1); }); @@ -313,7 +303,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); expect(result.suppressedCount).toBe(2); }); @@ -337,7 +327,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); expect(result.suppressedCount).toBe(0); expect(result.unsuppressedViolations).toHaveLength(1); @@ -362,7 +352,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); expect(result.suppressedCount).toBe(1); }); @@ -393,7 +383,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); expect(result.suppressedCount).toBe(1); expect(result.unsuppressedViolations).toHaveLength(1); @@ -424,7 +414,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); expect(result.suppressedCount).toBe(1); expect(result.unsuppressedViolations).toHaveLength(1); @@ -460,7 +450,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); expect(result.suppressedCount).toBe(2); expect(result.unsuppressedViolations).toHaveLength(1); @@ -491,7 +481,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); expect(result.suppressedCount).toBe(1); expect(result.unsuppressedViolations).toHaveLength(1); @@ -528,7 +518,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); expect(result.suppressedCount).toBe(2); expect(result.unsuppressedViolations).toHaveLength(0); @@ -557,7 +547,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); expect(result.suppressedCount).toBe(1); // Quota key uses config path with rule index (first rule at index 0 is used) @@ -596,7 +586,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); expect(result.suppressedCount).toBe(2); expect(result.unsuppressedViolations).toHaveLength(1); @@ -636,7 +626,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); // Should suppress first 2 violations (across different files), reject 3rd expect(result.suppressedCount).toBe(2); @@ -676,13 +666,13 @@ describe('applyBulkSuppressions', () => { const quotas: BulkSuppressionQuotas = new Map(); // First call - should suppress - const result1 = applyBulkSuppressions(violations1, bulkConfig, quotas, workspaceRoot, createTestLogger()); + const result1 = applyBulkSuppressions(violations1, bulkConfig, quotas, workspaceRoot); expect(result1.suppressedCount).toBe(1); // Quota key uses config path with rule index expect(quotas.get('src/file1.apex|0|pmd:UnusedMethod')).toBe(1); // Second call - quota exhausted, should not suppress - const result2 = applyBulkSuppressions(violations2, bulkConfig, quotas, workspaceRoot, createTestLogger()); + const result2 = applyBulkSuppressions(violations2, bulkConfig, quotas, workspaceRoot); expect(result2.suppressedCount).toBe(0); expect(result2.unsuppressedViolations).toHaveLength(1); }); @@ -699,7 +689,7 @@ describe('applyBulkSuppressions', () => { ]; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, {}, quotas, workspaceRoot, createTestLogger()); + const result = applyBulkSuppressions(violations, {}, quotas, workspaceRoot); expect(result.unsuppressedViolations).toHaveLength(1); expect(result.suppressedCount).toBe(0); @@ -716,7 +706,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions([], bulkConfig, quotas, workspaceRoot, createTestLogger()); + const result = applyBulkSuppressions([], bulkConfig, quotas, workspaceRoot); expect(result.unsuppressedViolations).toHaveLength(0); expect(result.suppressedCount).toBe(0); @@ -741,7 +731,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); expect(result.unsuppressedViolations).toHaveLength(1); expect(result.suppressedCount).toBe(0); @@ -766,7 +756,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); // Should not suppress due to invalid selector expect(result.unsuppressedViolations).toHaveLength(1); @@ -792,7 +782,7 @@ describe('applyBulkSuppressions', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); expect(result.unsuppressedViolations).toHaveLength(1); expect(result.suppressedCount).toBe(0); diff --git a/packages/code-analyzer-core/test/suppressions/bulk-suppression-workspace-root.test.ts b/packages/code-analyzer-core/test/suppressions/bulk-suppression-workspace-root.test.ts index a4f1684a..8bb5e4a8 100644 --- a/packages/code-analyzer-core/test/suppressions/bulk-suppression-workspace-root.test.ts +++ b/packages/code-analyzer-core/test/suppressions/bulk-suppression-workspace-root.test.ts @@ -3,16 +3,6 @@ import {BulkSuppressionRule} from "../../src/config"; import {Violation, CodeLocation} from "../../src/results"; import {RuleImpl} from "../../src/rules"; -// Helper to enable debug logging in tests via environment variable -const createTestLogger = () => { - if (process.env.DEBUG_BULK_SUPPRESSIONS === 'true') { - return (level: 'error' | 'warn' | 'debug', message: string) => { - console.log(`[${level.toUpperCase()}] ${message}`); - }; - } - return undefined; -}; - /** * Tests specifically for Bug #2: Workspace Root vs Config Root inconsistency * @@ -115,7 +105,7 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { const quotas: BulkSuppressionQuotas = new Map(); // Apply suppressions using workspace root (Bug #2 fix) - const result = applyBulkSuppressions([violation], bulkConfig, quotas, workspaceRoot, createTestLogger()); + const result = applyBulkSuppressions([violation], bulkConfig, quotas, workspaceRoot); // Violation should be suppressed expect(result.unsuppressedViolations).toHaveLength(0); @@ -143,7 +133,7 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions([violation], bulkConfig, quotas, workspaceRoot, createTestLogger()); + const result = applyBulkSuppressions([violation], bulkConfig, quotas, workspaceRoot); // Should work regardless of bug expect(result.unsuppressedViolations).toHaveLength(0); @@ -172,7 +162,7 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions([violation], bulkConfig, quotas, workspaceRoot, createTestLogger()); + const result = applyBulkSuppressions([violation], bulkConfig, quotas, workspaceRoot); // Should NOT suppress because path doesn't match expect(result.unsuppressedViolations).toHaveLength(1); @@ -219,8 +209,7 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { [violation1, violation2, violation3], bulkConfig, quotas, - workspaceRoot, - createTestLogger() + workspaceRoot ); // All should be suppressed @@ -256,8 +245,7 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { [violation1, violation2], bulkConfig, quotas, - workspaceRoot, - createTestLogger() + workspaceRoot ); // Both files in folder should be suppressed @@ -265,8 +253,8 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { expect(result.suppressedCount).toBe(2); }); - it('BUG #2: Absolute paths in config still work (edge case)', () => { - // Users shouldn't use absolute paths, but they should still work + it('Config paths must be relative to workspace root (not absolute)', () => { + // This test documents the correct usage: paths relative to workspace root const workspaceRoot = '/workspace/project'; const violationFilePath = '/workspace/project/src/file.js'; @@ -276,17 +264,17 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { new MockCodeLocation(violationFilePath, 10, 1, 10, 20) ); - // Absolute path in config (not recommended but should work) + // ✅ CORRECT: Use relative path from workspace root const bulkConfig: Record = { - '/workspace/project/src/file.js': [ + 'src/file.js': [ // Relative to /workspace/project { rule_selector: 'eslint:all', max_suppressed_violations: 1 } ] }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions([violation], bulkConfig, quotas, workspaceRoot, createTestLogger()); + const result = applyBulkSuppressions([violation], bulkConfig, quotas, workspaceRoot); - // Should suppress + // Should suppress because path is correctly specified as relative expect(result.unsuppressedViolations).toHaveLength(0); expect(result.suppressedCount).toBe(1); }); @@ -317,7 +305,7 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions([violation], bulkConfig, quotas, workspaceRoot, createTestLogger()); + const result = applyBulkSuppressions([violation], bulkConfig, quotas, workspaceRoot); expect(result.unsuppressedViolations).toHaveLength(0); expect(result.suppressedCount).toBe(1); @@ -359,7 +347,7 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { }; const quotas: BulkSuppressionQuotas = new Map(); - const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot, createTestLogger()); + const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); // First rule suppresses 3, second rule suppresses 2 = 5 total expect(result.suppressedCount).toBe(5); From 4b6e5b72a083ad439a18d6a240a6957f2e8d21fa Mon Sep 17 00:00:00 2001 From: Namrata Gupta Date: Fri, 10 Apr 2026 15:16:54 +0530 Subject: [PATCH 08/16] fix absolute path handling --- .github/workflows/verify-pr.yml | 2 + .../bulk-suppression-processor.ts | 38 +++++++++++++++---- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/.github/workflows/verify-pr.yml b/.github/workflows/verify-pr.yml index fe441410..c1bce6ea 100644 --- a/.github/workflows/verify-pr.yml +++ b/.github/workflows/verify-pr.yml @@ -96,3 +96,5 @@ jobs: npm run build `cat workspace-args.txt` npm run lint npm run test `cat workspace-args.txt` + env: + DEBUG_BULK_SUPPRESSIONS: ${{ matrix.os == 'windows-latest' && 'true' || 'false' }} diff --git a/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts b/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts index 94642702..b98d91c1 100644 --- a/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts +++ b/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts @@ -135,27 +135,49 @@ function doesFileMatchConfigPath( configPath: string, workspaceRoot: string ): boolean { + const DEBUG = process.env.DEBUG_BULK_SUPPRESSIONS === 'true'; + const log = (msg: string) => DEBUG && console.log(`[DEBUG] ${msg}`); + + log(`Path matching - violationFile: "${violationFile}", configPath: "${configPath}", workspaceRoot: "${workspaceRoot}"`); + // Step 1: Use path module to resolve config paths relative to workspace root // Config paths should always be relative (like .gitignore patterns) // Violation file paths are absolute (from the file system) - const normalizedWorkspaceRoot = path.normalize(workspaceRoot); - - // Config paths are always treated as relative to workspace root - // If user provides absolute path in config, it won't match - that's correct behavior + // CROSS-PLATFORM TEST COMPATIBILITY: + // On Windows, Unix-style paths like /workspace are NOT recognized as absolute by path.isAbsolute() + // because Windows expects drive letters (C:\). However, our tests use Unix-style paths and run + // on all platforms (including Windows CI). In production, all paths are native format, so this + // check is primarily for test compatibility. + const isAbsolutePath = (p: string) => path.isAbsolute(p) || p.startsWith('/'); + + // Keep workspace root as-is if it looks absolute (avoids path.normalize() issues on cross-platform tests) + const normalizedWorkspaceRoot = isAbsolutePath(workspaceRoot) + ? workspaceRoot + : path.normalize(workspaceRoot); + log(` After normalize workspace: "${normalizedWorkspaceRoot}"`); + + // Config paths are ALWAYS treated as relative to workspace root (like .gitignore) + // User should never provide absolute paths in config const absoluteConfigPath = path.resolve(normalizedWorkspaceRoot, configPath); + log(` After resolve config: "${absoluteConfigPath}"`); - // Violation files are always absolute paths from the engine - const normalizedViolationFile = path.normalize(violationFile); + // Violation files: keep as-is if absolute (for cross-platform test compatibility) + const normalizedViolationFile = isAbsolutePath(violationFile) + ? violationFile + : path.normalize(violationFile); + log(` After normalize file: "${normalizedViolationFile}"`); // Step 2: Normalize to POSIX separators for cross-platform comparison // Convert all backslashes to forward slashes for consistent comparison // This handles both: Windows paths on Unix (C:\foo -> C:/foo) and Unix paths on Windows const comparisonViolationFile = normalizedViolationFile.replace(/\\/g, '/'); const comparisonConfigPath = absoluteConfigPath.replace(/\\/g, '/'); + log(` After sep normalize - file: "${comparisonViolationFile}", config: "${comparisonConfigPath}"`); // Step 3: Check if it's an exact file match if (comparisonViolationFile === comparisonConfigPath) { + log(` ✓ Matched (exact)`); return true; } @@ -166,7 +188,9 @@ function doesFileMatchConfigPath( ? comparisonConfigPath : comparisonConfigPath + '/'; - return comparisonViolationFile.startsWith(configPathWithSep); + const matches = comparisonViolationFile.startsWith(configPathWithSep); + log(` ${matches ? '✓' : '✗'} Folder prefix match with "${configPathWithSep}"`); + return matches; } /** From c538853f388e73c3bb9ec2adaf740dac2b79b20c Mon Sep 17 00:00:00 2001 From: Namrata Gupta Date: Fri, 10 Apr 2026 15:23:14 +0530 Subject: [PATCH 09/16] fix absolute path handling --- .../bulk-suppression-processor.ts | 21 +++++++++++++------ .../bulk-suppression-processor.test.ts | 10 ++++++--- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts b/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts index b98d91c1..57322756 100644 --- a/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts +++ b/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts @@ -149,21 +149,30 @@ function doesFileMatchConfigPath( // because Windows expects drive letters (C:\). However, our tests use Unix-style paths and run // on all platforms (including Windows CI). In production, all paths are native format, so this // check is primarily for test compatibility. - const isAbsolutePath = (p: string) => path.isAbsolute(p) || p.startsWith('/'); + const isUnixStylePath = (p: string) => p.startsWith('/'); - // Keep workspace root as-is if it looks absolute (avoids path.normalize() issues on cross-platform tests) - const normalizedWorkspaceRoot = isAbsolutePath(workspaceRoot) + // If workspace root is Unix-style path (starts with /), keep it as-is + const normalizedWorkspaceRoot = isUnixStylePath(workspaceRoot) ? workspaceRoot : path.normalize(workspaceRoot); log(` After normalize workspace: "${normalizedWorkspaceRoot}"`); // Config paths are ALWAYS treated as relative to workspace root (like .gitignore) // User should never provide absolute paths in config - const absoluteConfigPath = path.resolve(normalizedWorkspaceRoot, configPath); + let absoluteConfigPath: string; + if (isUnixStylePath(normalizedWorkspaceRoot)) { + // For Unix-style paths, use simple concatenation (path.resolve doesn't work cross-platform) + // e.g., "/workspace" + "src/file.js" = "/workspace/src/file.js" + const separator = normalizedWorkspaceRoot.endsWith('/') ? '' : '/'; + absoluteConfigPath = normalizedWorkspaceRoot + separator + configPath; + } else { + // For native paths, use path.resolve (handles Windows paths properly) + absoluteConfigPath = path.resolve(normalizedWorkspaceRoot, configPath); + } log(` After resolve config: "${absoluteConfigPath}"`); - // Violation files: keep as-is if absolute (for cross-platform test compatibility) - const normalizedViolationFile = isAbsolutePath(violationFile) + // Violation files: keep as-is if Unix-style (for cross-platform test compatibility) + const normalizedViolationFile = isUnixStylePath(violationFile) ? violationFile : path.normalize(violationFile); log(` After normalize file: "${normalizedViolationFile}"`); diff --git a/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts b/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts index ab091b37..9d4bc47e 100644 --- a/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts +++ b/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts @@ -333,7 +333,9 @@ describe('applyBulkSuppressions', () => { expect(result.unsuppressedViolations).toHaveLength(1); }); - it('should handle absolute paths in config', () => { + it('should NOT support absolute paths in config (relative paths only)', () => { + // Config paths must be relative to workspace root + // Absolute paths in config are not supported (similar to .gitignore) const violations = [ new MockViolation( new MockRule('pmd', 'UnusedMethod'), @@ -343,7 +345,7 @@ describe('applyBulkSuppressions', () => { ]; const bulkConfig = { - '/workspace/src/file1.apex': [ + '/workspace/src/file1.apex': [ // ❌ Absolute path - not supported { rule_selector: 'pmd', max_suppressed_violations: null @@ -354,7 +356,9 @@ describe('applyBulkSuppressions', () => { const quotas: BulkSuppressionQuotas = new Map(); const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); - expect(result.suppressedCount).toBe(1); + // Should NOT suppress - absolute paths in config are not supported + expect(result.suppressedCount).toBe(0); + expect(result.unsuppressedViolations).toHaveLength(1); }); }); From a331abf03e1fa57f06a2785ed5b987ba78e1a548 Mon Sep 17 00:00:00 2001 From: Namrata Gupta Date: Fri, 10 Apr 2026 15:28:35 +0530 Subject: [PATCH 10/16] fix absolute path handling --- .github/workflows/verify-pr.yml | 2 -- .../src/suppressions/bulk-suppression-processor.ts | 14 +------------- .../bulk-suppression-processor.test.ts | 2 +- 3 files changed, 2 insertions(+), 16 deletions(-) diff --git a/.github/workflows/verify-pr.yml b/.github/workflows/verify-pr.yml index c1bce6ea..fe441410 100644 --- a/.github/workflows/verify-pr.yml +++ b/.github/workflows/verify-pr.yml @@ -96,5 +96,3 @@ jobs: npm run build `cat workspace-args.txt` npm run lint npm run test `cat workspace-args.txt` - env: - DEBUG_BULK_SUPPRESSIONS: ${{ matrix.os == 'windows-latest' && 'true' || 'false' }} diff --git a/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts b/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts index 57322756..2d8ccc03 100644 --- a/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts +++ b/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts @@ -135,11 +135,6 @@ function doesFileMatchConfigPath( configPath: string, workspaceRoot: string ): boolean { - const DEBUG = process.env.DEBUG_BULK_SUPPRESSIONS === 'true'; - const log = (msg: string) => DEBUG && console.log(`[DEBUG] ${msg}`); - - log(`Path matching - violationFile: "${violationFile}", configPath: "${configPath}", workspaceRoot: "${workspaceRoot}"`); - // Step 1: Use path module to resolve config paths relative to workspace root // Config paths should always be relative (like .gitignore patterns) // Violation file paths are absolute (from the file system) @@ -155,7 +150,6 @@ function doesFileMatchConfigPath( const normalizedWorkspaceRoot = isUnixStylePath(workspaceRoot) ? workspaceRoot : path.normalize(workspaceRoot); - log(` After normalize workspace: "${normalizedWorkspaceRoot}"`); // Config paths are ALWAYS treated as relative to workspace root (like .gitignore) // User should never provide absolute paths in config @@ -169,24 +163,20 @@ function doesFileMatchConfigPath( // For native paths, use path.resolve (handles Windows paths properly) absoluteConfigPath = path.resolve(normalizedWorkspaceRoot, configPath); } - log(` After resolve config: "${absoluteConfigPath}"`); // Violation files: keep as-is if Unix-style (for cross-platform test compatibility) const normalizedViolationFile = isUnixStylePath(violationFile) ? violationFile : path.normalize(violationFile); - log(` After normalize file: "${normalizedViolationFile}"`); // Step 2: Normalize to POSIX separators for cross-platform comparison // Convert all backslashes to forward slashes for consistent comparison // This handles both: Windows paths on Unix (C:\foo -> C:/foo) and Unix paths on Windows const comparisonViolationFile = normalizedViolationFile.replace(/\\/g, '/'); const comparisonConfigPath = absoluteConfigPath.replace(/\\/g, '/'); - log(` After sep normalize - file: "${comparisonViolationFile}", config: "${comparisonConfigPath}"`); // Step 3: Check if it's an exact file match if (comparisonViolationFile === comparisonConfigPath) { - log(` ✓ Matched (exact)`); return true; } @@ -197,9 +187,7 @@ function doesFileMatchConfigPath( ? comparisonConfigPath : comparisonConfigPath + '/'; - const matches = comparisonViolationFile.startsWith(configPathWithSep); - log(` ${matches ? '✓' : '✗'} Folder prefix match with "${configPathWithSep}"`); - return matches; + return comparisonViolationFile.startsWith(configPathWithSep); } /** diff --git a/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts b/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts index 9d4bc47e..6a9a5beb 100644 --- a/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts +++ b/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts @@ -345,7 +345,7 @@ describe('applyBulkSuppressions', () => { ]; const bulkConfig = { - '/workspace/src/file1.apex': [ // ❌ Absolute path - not supported + '/workspace/src/file1.apex': [ // Absolute path - not supported { rule_selector: 'pmd', max_suppressed_violations: null From a35034e737682a6e47515187171e760c3ff82073 Mon Sep 17 00:00:00 2001 From: Namrata Gupta Date: Fri, 10 Apr 2026 15:45:47 +0530 Subject: [PATCH 11/16] fix absolute path handling --- .github/workflows/verify-pr.yml | 2 + .../bulk-suppression-processor.ts | 84 ++++++++-------- .../bulk-suppression-processor.test.ts | 95 ++++++++++--------- .../bulk-suppression-workspace-root.test.ts | 57 +++++------ 4 files changed, 126 insertions(+), 112 deletions(-) diff --git a/.github/workflows/verify-pr.yml b/.github/workflows/verify-pr.yml index fe441410..c88e1d68 100644 --- a/.github/workflows/verify-pr.yml +++ b/.github/workflows/verify-pr.yml @@ -88,6 +88,8 @@ jobs: python-version: 3.12 - run: npm install - name: Test changes in affected packages + env: + DEBUG_BULK_SUPPRESSIONS: ${{ matrix.os == 'windows-latest' && 'true' || 'false' }} run: | BASE_SHA=${{ github.event.pull_request.base.sha }} HEAD_SHA=${{ github.event.pull_request.head.sha }} diff --git a/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts b/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts index 2d8ccc03..ec59864c 100644 --- a/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts +++ b/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts @@ -126,7 +126,7 @@ function findMatchingBulkSuppressionRules( * This follows the same pattern as the ignores feature in workspace.ts for cross-platform compatibility. * * @param violationFile Absolute file path from violation - * @param configPath Relative path from config (file or folder) + * @param configPath Relative or absolute path from config (file or folder) * @param workspaceRoot Root directory for resolving relative paths * @returns true if the file matches */ @@ -135,59 +135,67 @@ function doesFileMatchConfigPath( configPath: string, workspaceRoot: string ): boolean { - // Step 1: Use path module to resolve config paths relative to workspace root - // Config paths should always be relative (like .gitignore patterns) - // Violation file paths are absolute (from the file system) - - // CROSS-PLATFORM TEST COMPATIBILITY: - // On Windows, Unix-style paths like /workspace are NOT recognized as absolute by path.isAbsolute() - // because Windows expects drive letters (C:\). However, our tests use Unix-style paths and run - // on all platforms (including Windows CI). In production, all paths are native format, so this - // check is primarily for test compatibility. - const isUnixStylePath = (p: string) => p.startsWith('/'); - - // If workspace root is Unix-style path (starts with /), keep it as-is - const normalizedWorkspaceRoot = isUnixStylePath(workspaceRoot) - ? workspaceRoot - : path.normalize(workspaceRoot); - - // Config paths are ALWAYS treated as relative to workspace root (like .gitignore) - // User should never provide absolute paths in config - let absoluteConfigPath: string; - if (isUnixStylePath(normalizedWorkspaceRoot)) { - // For Unix-style paths, use simple concatenation (path.resolve doesn't work cross-platform) - // e.g., "/workspace" + "src/file.js" = "/workspace/src/file.js" - const separator = normalizedWorkspaceRoot.endsWith('/') ? '' : '/'; - absoluteConfigPath = normalizedWorkspaceRoot + separator + configPath; - } else { - // For native paths, use path.resolve (handles Windows paths properly) - absoluteConfigPath = path.resolve(normalizedWorkspaceRoot, configPath); + const debugEnabled = process.env.DEBUG_BULK_SUPPRESSIONS === 'true'; + + if (debugEnabled) { + console.log('[DEBUG] doesFileMatchConfigPath called:'); + console.log(' violationFile:', violationFile); + console.log(' configPath:', configPath); + console.log(' workspaceRoot:', workspaceRoot); + console.log(' path.isAbsolute(configPath):', path.isAbsolute(configPath)); } - // Violation files: keep as-is if Unix-style (for cross-platform test compatibility) - const normalizedViolationFile = isUnixStylePath(violationFile) - ? violationFile - : path.normalize(violationFile); + // Config paths can be relative (recommended, like .gitignore) or absolute + // If relative, resolve against workspace root + // If absolute, use as-is + const absoluteConfigPath = path.resolve(workspaceRoot, configPath); - // Step 2: Normalize to POSIX separators for cross-platform comparison + if (debugEnabled) { + console.log(' After path.resolve:', absoluteConfigPath); + } + + // Normalize paths for consistent comparison + const normalizedViolationFile = path.normalize(violationFile); + const normalizedConfigPath = path.normalize(absoluteConfigPath); + + if (debugEnabled) { + console.log(' After normalize - violation:', normalizedViolationFile); + console.log(' After normalize - config:', normalizedConfigPath); + } + + // Normalize to POSIX separators for cross-platform comparison // Convert all backslashes to forward slashes for consistent comparison - // This handles both: Windows paths on Unix (C:\foo -> C:/foo) and Unix paths on Windows const comparisonViolationFile = normalizedViolationFile.replace(/\\/g, '/'); - const comparisonConfigPath = absoluteConfigPath.replace(/\\/g, '/'); + const comparisonConfigPath = normalizedConfigPath.replace(/\\/g, '/'); - // Step 3: Check if it's an exact file match + if (debugEnabled) { + console.log(' After POSIX normalize - violation:', comparisonViolationFile); + console.log(' After POSIX normalize - config:', comparisonConfigPath); + } + + // Check if it's an exact file match if (comparisonViolationFile === comparisonConfigPath) { + if (debugEnabled) { + console.log(' Result: EXACT MATCH'); + } return true; } - // Step 4: Check if violation file is within config folder + // Check if violation file is within config folder // Add separator to ensure we match whole directory names // e.g., "src/utils" should match "src/utils/file.js" but not "src/utils2/file.js" const configPathWithSep = comparisonConfigPath.endsWith('/') ? comparisonConfigPath : comparisonConfigPath + '/'; - return comparisonViolationFile.startsWith(configPathWithSep); + const result = comparisonViolationFile.startsWith(configPathWithSep); + + if (debugEnabled) { + console.log(' Folder match check - configPathWithSep:', configPathWithSep); + console.log(' Result:', result ? 'FOLDER MATCH' : 'NO MATCH'); + } + + return result; } /** diff --git a/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts b/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts index 6a9a5beb..39a4d422 100644 --- a/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts +++ b/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts @@ -8,6 +8,7 @@ import { BulkSuppressionQuotas } from '../../src/suppressions/bulk-suppression-processor'; import { BulkSuppressionRule } from '../../src/config'; +import * as path from 'node:path'; import { Violation, CodeLocation } from '../../src/results'; import { Rule, SeverityLevel } from '../../src/rules'; @@ -120,7 +121,7 @@ class MockViolation implements Violation { } describe('applyBulkSuppressions', () => { - const workspaceRoot = '/workspace'; + const workspaceRoot = path.join(path.sep, 'workspace'); describe('Basic suppression scenarios', () => { it('should suppress violations matching rule selector with no quota limit', () => { @@ -128,12 +129,12 @@ describe('applyBulkSuppressions', () => { new MockViolation( new MockRule('pmd', 'UnusedMethod'), 'Unused method', - new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 10, 1, 10, 20) ), new MockViolation( new MockRule('pmd', 'UnusedMethod'), 'Unused method', - new MockCodeLocation('/workspace/src/file1.apex', 20, 1, 20, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 20, 1, 20, 20) ) ]; @@ -160,17 +161,17 @@ describe('applyBulkSuppressions', () => { new MockViolation( new MockRule('pmd', 'UnusedMethod'), 'Unused method', - new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 10, 1, 10, 20) ), new MockViolation( new MockRule('pmd', 'UnusedMethod'), 'Unused method', - new MockCodeLocation('/workspace/src/file1.apex', 20, 1, 20, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 20, 1, 20, 20) ), new MockViolation( new MockRule('pmd', 'UnusedMethod'), 'Unused method', - new MockCodeLocation('/workspace/src/file1.apex', 30, 1, 30, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 30, 1, 30, 20) ) ]; @@ -198,7 +199,7 @@ describe('applyBulkSuppressions', () => { new MockViolation( new MockRule('pmd', 'UnusedMethod'), 'Unused method', - new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 10, 1, 10, 20) ) ]; @@ -227,12 +228,12 @@ describe('applyBulkSuppressions', () => { new MockViolation( new MockRule('pmd', 'UnusedMethod'), 'Unused method', - new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 10, 1, 10, 20) ), new MockViolation( new MockRule('eslint', 'no-unused-vars'), 'Unused variable', - new MockCodeLocation('/workspace/src/file1.apex', 20, 1, 20, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 20, 1, 20, 20) ) ]; @@ -260,7 +261,7 @@ describe('applyBulkSuppressions', () => { new MockViolation( new MockRule('pmd', 'UnusedMethod'), 'Unused method', - new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 10, 1, 10, 20) ) ]; @@ -284,12 +285,12 @@ describe('applyBulkSuppressions', () => { new MockViolation( new MockRule('pmd', 'UnusedMethod'), 'Unused method', - new MockCodeLocation('/workspace/src/folder/file1.apex', 10, 1, 10, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'folder', 'file1.apex'), 10, 1, 10, 20) ), new MockViolation( new MockRule('pmd', 'UnusedMethod'), 'Unused method', - new MockCodeLocation('/workspace/src/folder/file2.apex', 20, 1, 20, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'folder', 'file2.apex'), 20, 1, 20, 20) ) ]; @@ -313,7 +314,7 @@ describe('applyBulkSuppressions', () => { new MockViolation( new MockRule('pmd', 'UnusedMethod'), 'Unused method', - new MockCodeLocation('/workspace/other/file1.apex', 10, 1, 10, 20) + new MockCodeLocation(path.join(workspaceRoot, 'other', 'file1.apex'), 10, 1, 10, 20) ) ]; @@ -333,19 +334,21 @@ describe('applyBulkSuppressions', () => { expect(result.unsuppressedViolations).toHaveLength(1); }); - it('should NOT support absolute paths in config (relative paths only)', () => { - // Config paths must be relative to workspace root - // Absolute paths in config are not supported (similar to .gitignore) + it('should support absolute paths in config', () => { + // Config paths can be absolute (in addition to relative paths) + // This provides flexibility for users who need to specify exact file locations const violations = [ new MockViolation( new MockRule('pmd', 'UnusedMethod'), 'Unused method', - new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 10, 1, 10, 20) ) ]; + // Create config with absolute path key + const absolutePath = path.join(workspaceRoot, 'src', 'file1.apex'); const bulkConfig = { - '/workspace/src/file1.apex': [ // Absolute path - not supported + [absolutePath]: [ { rule_selector: 'pmd', max_suppressed_violations: null @@ -356,9 +359,9 @@ describe('applyBulkSuppressions', () => { const quotas: BulkSuppressionQuotas = new Map(); const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot); - // Should NOT suppress - absolute paths in config are not supported - expect(result.suppressedCount).toBe(0); - expect(result.unsuppressedViolations).toHaveLength(1); + // Should suppress - absolute paths are supported + expect(result.suppressedCount).toBe(1); + expect(result.unsuppressedViolations).toHaveLength(0); }); }); @@ -368,12 +371,12 @@ describe('applyBulkSuppressions', () => { new MockViolation( new MockRule('pmd', 'UnusedMethod'), 'Unused method', - new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 10, 1, 10, 20) ), new MockViolation( new MockRule('eslint', 'no-unused-vars'), 'Unused variable', - new MockCodeLocation('/workspace/src/file1.apex', 20, 1, 20, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 20, 1, 20, 20) ) ]; @@ -399,12 +402,12 @@ describe('applyBulkSuppressions', () => { new MockViolation( new MockRule('pmd', 'UnusedMethod'), 'Unused method', - new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 10, 1, 10, 20) ), new MockViolation( new MockRule('pmd', 'UnusedVariable'), 'Unused variable', - new MockCodeLocation('/workspace/src/file1.apex', 20, 1, 20, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 20, 1, 20, 20) ) ]; @@ -430,17 +433,17 @@ describe('applyBulkSuppressions', () => { new MockViolation( new MockRule('pmd', 'Rule1', SeverityLevel.High), 'High severity', - new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 10, 1, 10, 20) ), new MockViolation( new MockRule('pmd', 'Rule2', SeverityLevel.Moderate), 'Moderate severity', - new MockCodeLocation('/workspace/src/file1.apex', 20, 1, 20, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 20, 1, 20, 20) ), new MockViolation( new MockRule('pmd', 'Rule3', SeverityLevel.Low), 'Low severity', - new MockCodeLocation('/workspace/src/file1.apex', 30, 1, 30, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 30, 1, 30, 20) ) ]; @@ -466,12 +469,12 @@ describe('applyBulkSuppressions', () => { new MockViolation( new MockRule('pmd', 'Rule1', SeverityLevel.High, ['Recommended']), 'Tagged violation', - new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 10, 1, 10, 20) ), new MockViolation( new MockRule('pmd', 'Rule2', SeverityLevel.High, ['Security']), 'Security violation', - new MockCodeLocation('/workspace/src/file1.apex', 20, 1, 20, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 20, 1, 20, 20) ) ]; @@ -499,12 +502,12 @@ describe('applyBulkSuppressions', () => { new MockViolation( new MockRule('pmd', 'UnusedMethod'), 'Unused method', - new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 10, 1, 10, 20) ), new MockViolation( new MockRule('eslint', 'no-unused-vars'), 'Unused variable', - new MockCodeLocation('/workspace/src/file1.apex', 20, 1, 20, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 20, 1, 20, 20) ) ]; @@ -533,7 +536,7 @@ describe('applyBulkSuppressions', () => { new MockViolation( new MockRule('pmd', 'UnusedMethod'), 'Unused method', - new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 10, 1, 10, 20) ) ]; @@ -566,17 +569,17 @@ describe('applyBulkSuppressions', () => { new MockViolation( new MockRule('pmd', 'UnusedMethod'), 'Unused method', - new MockCodeLocation('/workspace/src/file2.apex', 10, 1, 10, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'file2.apex'), 10, 1, 10, 20) ), new MockViolation( new MockRule('pmd', 'UnusedMethod'), 'Unused method', - new MockCodeLocation('/workspace/src/file1.apex', 20, 1, 20, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 20, 1, 20, 20) ), new MockViolation( new MockRule('pmd', 'UnusedMethod'), 'Unused method', - new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 10, 1, 10, 20) ) ]; @@ -595,7 +598,7 @@ describe('applyBulkSuppressions', () => { expect(result.suppressedCount).toBe(2); expect(result.unsuppressedViolations).toHaveLength(1); // Should suppress file1:10 and file1:20 (sorted by file, then line) - expect(result.unsuppressedViolations[0].getPrimaryLocation().getFile()).toBe('/workspace/src/file2.apex'); + expect(result.unsuppressedViolations[0].getPrimaryLocation().getFile()).toBe(path.join(workspaceRoot, 'src', 'file2.apex')); }); }); @@ -606,17 +609,17 @@ describe('applyBulkSuppressions', () => { new MockViolation( new MockRule('pmd', 'UnusedMethod'), 'Unused method in file1', - new MockCodeLocation('/workspace/src/controllers/file1.apex', 10, 1, 10, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'controllers', 'file1.apex'), 10, 1, 10, 20) ), new MockViolation( new MockRule('pmd', 'UnusedMethod'), 'Unused method in file2', - new MockCodeLocation('/workspace/src/controllers/file2.apex', 20, 1, 20, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'controllers', 'file2.apex'), 20, 1, 20, 20) ), new MockViolation( new MockRule('pmd', 'UnusedMethod'), 'Unused method in file3', - new MockCodeLocation('/workspace/src/controllers/file3.apex', 30, 1, 30, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'controllers', 'file3.apex'), 30, 1, 30, 20) ) ]; @@ -635,7 +638,7 @@ describe('applyBulkSuppressions', () => { // Should suppress first 2 violations (across different files), reject 3rd expect(result.suppressedCount).toBe(2); expect(result.unsuppressedViolations).toHaveLength(1); - expect(result.unsuppressedViolations[0].getPrimaryLocation().getFile()).toBe('/workspace/src/controllers/file3.apex'); + expect(result.unsuppressedViolations[0].getPrimaryLocation().getFile()).toBe(path.join(workspaceRoot, 'src', 'controllers', 'file3.apex')); // Quota is shared at folder level, not per-file (rule index 0) expect(quotas.get('src/controllers|0|pmd:UnusedMethod')).toBe(2); @@ -646,7 +649,7 @@ describe('applyBulkSuppressions', () => { new MockViolation( new MockRule('pmd', 'UnusedMethod'), 'Unused method', - new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 10, 1, 10, 20) ) ]; @@ -654,7 +657,7 @@ describe('applyBulkSuppressions', () => { new MockViolation( new MockRule('pmd', 'UnusedMethod'), 'Unused method', - new MockCodeLocation('/workspace/src/file1.apex', 20, 1, 20, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 20, 1, 20, 20) ) ]; @@ -688,7 +691,7 @@ describe('applyBulkSuppressions', () => { new MockViolation( new MockRule('pmd', 'UnusedMethod'), 'Unused method', - new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 10, 1, 10, 20) ) ]; @@ -746,7 +749,7 @@ describe('applyBulkSuppressions', () => { new MockViolation( new MockRule('pmd', 'UnusedMethod'), 'Unused method', - new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 10, 1, 10, 20) ) ]; @@ -772,7 +775,7 @@ describe('applyBulkSuppressions', () => { new MockViolation( new MockRule('pmd', 'UnusedMethod'), 'Unused method', - new MockCodeLocation('/workspace/src/file1.apex', 10, 1, 10, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 10, 1, 10, 20) ) ]; diff --git a/packages/code-analyzer-core/test/suppressions/bulk-suppression-workspace-root.test.ts b/packages/code-analyzer-core/test/suppressions/bulk-suppression-workspace-root.test.ts index 8bb5e4a8..83bf1614 100644 --- a/packages/code-analyzer-core/test/suppressions/bulk-suppression-workspace-root.test.ts +++ b/packages/code-analyzer-core/test/suppressions/bulk-suppression-workspace-root.test.ts @@ -2,6 +2,7 @@ import {applyBulkSuppressions, BulkSuppressionQuotas} from "../../src/suppressio import {BulkSuppressionRule} from "../../src/config"; import {Violation, CodeLocation} from "../../src/results"; import {RuleImpl} from "../../src/rules"; +import * as path from 'node:path'; /** * Tests specifically for Bug #2: Workspace Root vs Config Root inconsistency @@ -78,11 +79,11 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { * NOT: "utils.js" (relative to config root) */ it('BUG #2: Paths in config are resolved relative to WORKSPACE ROOT, not config file location', () => { - // Workspace root (where -w flag points) - const workspaceRoot = '/Users/user/workspace/dreamhouse'; + // Workspace root (where -w flag points) - use platform-appropriate paths + const workspaceRoot = path.join(path.sep, 'Users', 'user', 'workspace', 'dreamhouse'); - // Violation file path (absolute) - const violationFilePath = '/Users/user/workspace/dreamhouse/force-app/main/default/react-components/utils.js'; + // Violation file path (absolute) - use platform-appropriate paths + const violationFilePath = path.join(workspaceRoot, 'force-app', 'main', 'default', 'react-components', 'utils.js'); // Create violation const violation = new MockViolation( @@ -92,7 +93,7 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { ); // Bulk config with path relative to WORKSPACE ROOT (consistent with ignores) - // This is what user should write in config file + // This is what user should write in config file (always use forward slashes) const bulkConfig: Record = { 'force-app/main/default/react-components/utils.js': [ { @@ -114,8 +115,8 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { it('BUG #2: Same directory level - workspace root equals config directory (worked before bug fix)', () => { // This case worked even with Bug #2 because workspace root === config root - const workspaceRoot = '/Users/user/workspace/project'; - const violationFilePath = '/Users/user/workspace/project/utils.js'; + const workspaceRoot = path.join(path.sep, 'Users', 'user', 'workspace', 'project'); + const violationFilePath = path.join(workspaceRoot, 'utils.js'); const violation = new MockViolation( mockRule, @@ -142,8 +143,8 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { it('BUG #2: Config in subdirectory with WRONG path (relative to config root) does NOT suppress', () => { // This demonstrates the bug - if user writes path relative to config file location - const workspaceRoot = '/Users/user/workspace/dreamhouse'; - const violationFilePath = '/Users/user/workspace/dreamhouse/force-app/main/default/react-components/utils.js'; + const workspaceRoot = path.join(path.sep, 'Users', 'user', 'workspace', 'dreamhouse'); + const violationFilePath = path.join(workspaceRoot, 'force-app', 'main', 'default', 'react-components', 'utils.js'); const violation = new MockViolation( mockRule, @@ -170,25 +171,25 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { }); it('BUG #2: Multiple files at different levels all resolve correctly from workspace root', () => { - const workspaceRoot = '/workspace/project'; + const workspaceRoot = path.join(path.sep, 'workspace', 'project'); // Violations at different directory levels const violation1 = new MockViolation( mockRule, 'Console in nested file', - new MockCodeLocation('/workspace/project/src/utils.js', 10, 1, 10, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'utils.js'), 10, 1, 10, 20) ); const violation2 = new MockViolation( mockRule, 'Console in deeper nested file', - new MockCodeLocation('/workspace/project/force-app/main/default/lwc/component.js', 20, 1, 20, 20) + new MockCodeLocation(path.join(workspaceRoot, 'force-app', 'main', 'default', 'lwc', 'component.js'), 20, 1, 20, 20) ); const violation3 = new MockViolation( mockRule, 'Console in root file', - new MockCodeLocation('/workspace/project/index.js', 30, 1, 30, 20) + new MockCodeLocation(path.join(workspaceRoot, 'index.js'), 30, 1, 30, 20) ); // All paths relative to workspace root @@ -218,19 +219,19 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { }); it('BUG #2: Folder-level suppression from workspace root matches nested files', () => { - const workspaceRoot = '/workspace/project'; + const workspaceRoot = path.join(path.sep, 'workspace', 'project'); // Violations in nested folder const violation1 = new MockViolation( mockRule, 'Console', - new MockCodeLocation('/workspace/project/src/utils/helper.js', 10, 1, 10, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'utils', 'helper.js'), 10, 1, 10, 20) ); const violation2 = new MockViolation( mockRule, 'Console', - new MockCodeLocation('/workspace/project/src/utils/formatter.js', 20, 1, 20, 20) + new MockCodeLocation(path.join(workspaceRoot, 'src', 'utils', 'formatter.js'), 20, 1, 20, 20) ); // Suppress entire folder (path relative to workspace root) @@ -253,10 +254,10 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { expect(result.suppressedCount).toBe(2); }); - it('Config paths must be relative to workspace root (not absolute)', () => { - // This test documents the correct usage: paths relative to workspace root - const workspaceRoot = '/workspace/project'; - const violationFilePath = '/workspace/project/src/file.js'; + it('Config paths can be relative to workspace root (recommended)', () => { + // This test documents the recommended usage: paths relative to workspace root + const workspaceRoot = path.join(path.sep, 'workspace', 'project'); + const violationFilePath = path.join(workspaceRoot, 'src', 'file.js'); const violation = new MockViolation( mockRule, @@ -264,9 +265,9 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { new MockCodeLocation(violationFilePath, 10, 1, 10, 20) ); - // ✅ CORRECT: Use relative path from workspace root + // RECOMMENDED: Use relative path from workspace root const bulkConfig: Record = { - 'src/file.js': [ // Relative to /workspace/project + 'src/file.js': [ // Relative to workspace root { rule_selector: 'eslint:all', max_suppressed_violations: 1 } ] }; @@ -274,7 +275,7 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { const quotas: BulkSuppressionQuotas = new Map(); const result = applyBulkSuppressions([violation], bulkConfig, quotas, workspaceRoot); - // Should suppress because path is correctly specified as relative + // Should suppress because path is correctly specified expect(result.unsuppressedViolations).toHaveLength(0); expect(result.suppressedCount).toBe(1); }); @@ -286,16 +287,16 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { * - Both resolve paths relative to workspace root * - Both fall back to absolute paths if workspace root is null */ - const workspaceRoot = '/Users/user/dreamhouse'; + const workspaceRoot = path.join(path.sep, 'Users', 'user', 'dreamhouse'); // Imagine ignores config: ["force-app/test/**"] // Imagine bulk suppressions: "force-app/main/default/utils.js" - // Both should resolve relative to /Users/user/dreamhouse + // Both should resolve relative to workspace root const violation = new MockViolation( mockRule, 'Console', - new MockCodeLocation('/Users/user/dreamhouse/force-app/main/default/utils.js', 10, 1, 10, 20) + new MockCodeLocation(path.join(workspaceRoot, 'force-app', 'main', 'default', 'utils.js'), 10, 1, 10, 20) ); const bulkConfig: Record = { @@ -326,8 +327,8 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { * * Expected: 5 total violations suppressed (3 + 2), not 3 with shared quota */ - const workspaceRoot = '/Users/user/workspace'; - const filePath = '/Users/user/workspace/force-app/utils.js'; + const workspaceRoot = path.join(path.sep, 'Users', 'user', 'workspace'); + const filePath = path.join(workspaceRoot, 'force-app', 'utils.js'); // Create 6 violations that would match the duplicate selectors const violations: Violation[] = []; From 5ecdf6931648b55eaba4fdcdf1e3f2733ec6532d Mon Sep 17 00:00:00 2001 From: Namrata Gupta Date: Fri, 10 Apr 2026 15:53:47 +0530 Subject: [PATCH 12/16] fix absolute path handling --- .../bulk-suppression-processor.test.ts | 18 +++++++++++- .../bulk-suppression-workspace-root.test.ts | 28 +++++++++++++++---- 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts b/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts index 39a4d422..7464f108 100644 --- a/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts +++ b/packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts @@ -9,6 +9,22 @@ import { } from '../../src/suppressions/bulk-suppression-processor'; import { BulkSuppressionRule } from '../../src/config'; import * as path from 'node:path'; +import * as os from 'node:os'; + +/** + * Helper to create platform-appropriate absolute paths for testing + * On Windows: C:\workspace\... + * On Unix: /workspace/... + */ +function createTestAbsolutePath(...segments: string[]): string { + if (os.platform() === 'win32') { + // Windows: start with C:\ drive + return path.join('C:', path.sep, ...segments); + } else { + // Unix: start with / + return path.join(path.sep, ...segments); + } +} import { Violation, CodeLocation } from '../../src/results'; import { Rule, SeverityLevel } from '../../src/rules'; @@ -121,7 +137,7 @@ class MockViolation implements Violation { } describe('applyBulkSuppressions', () => { - const workspaceRoot = path.join(path.sep, 'workspace'); + const workspaceRoot = createTestAbsolutePath('workspace'); describe('Basic suppression scenarios', () => { it('should suppress violations matching rule selector with no quota limit', () => { diff --git a/packages/code-analyzer-core/test/suppressions/bulk-suppression-workspace-root.test.ts b/packages/code-analyzer-core/test/suppressions/bulk-suppression-workspace-root.test.ts index 83bf1614..a2b27ce9 100644 --- a/packages/code-analyzer-core/test/suppressions/bulk-suppression-workspace-root.test.ts +++ b/packages/code-analyzer-core/test/suppressions/bulk-suppression-workspace-root.test.ts @@ -3,6 +3,22 @@ import {BulkSuppressionRule} from "../../src/config"; import {Violation, CodeLocation} from "../../src/results"; import {RuleImpl} from "../../src/rules"; import * as path from 'node:path'; +import * as os from 'node:os'; + +/** + * Helper to create platform-appropriate absolute paths for testing + * On Windows: C:\Users\... + * On Unix: /Users/... + */ +function createTestAbsolutePath(...segments: string[]): string { + if (os.platform() === 'win32') { + // Windows: start with C:\ drive + return path.join('C:', path.sep, ...segments); + } else { + // Unix: start with / + return path.join(path.sep, ...segments); + } +} /** * Tests specifically for Bug #2: Workspace Root vs Config Root inconsistency @@ -80,7 +96,7 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { */ it('BUG #2: Paths in config are resolved relative to WORKSPACE ROOT, not config file location', () => { // Workspace root (where -w flag points) - use platform-appropriate paths - const workspaceRoot = path.join(path.sep, 'Users', 'user', 'workspace', 'dreamhouse'); + const workspaceRoot = createTestAbsolutePath('Users', 'user', 'workspace', 'dreamhouse'); // Violation file path (absolute) - use platform-appropriate paths const violationFilePath = path.join(workspaceRoot, 'force-app', 'main', 'default', 'react-components', 'utils.js'); @@ -115,7 +131,7 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { it('BUG #2: Same directory level - workspace root equals config directory (worked before bug fix)', () => { // This case worked even with Bug #2 because workspace root === config root - const workspaceRoot = path.join(path.sep, 'Users', 'user', 'workspace', 'project'); + const workspaceRoot = createTestAbsolutePath('Users', 'user', 'workspace', 'project'); const violationFilePath = path.join(workspaceRoot, 'utils.js'); const violation = new MockViolation( @@ -171,7 +187,7 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { }); it('BUG #2: Multiple files at different levels all resolve correctly from workspace root', () => { - const workspaceRoot = path.join(path.sep, 'workspace', 'project'); + const workspaceRoot = createTestAbsolutePath('workspace', 'project'); // Violations at different directory levels const violation1 = new MockViolation( @@ -219,7 +235,7 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { }); it('BUG #2: Folder-level suppression from workspace root matches nested files', () => { - const workspaceRoot = path.join(path.sep, 'workspace', 'project'); + const workspaceRoot = createTestAbsolutePath('workspace', 'project'); // Violations in nested folder const violation1 = new MockViolation( @@ -256,7 +272,7 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { it('Config paths can be relative to workspace root (recommended)', () => { // This test documents the recommended usage: paths relative to workspace root - const workspaceRoot = path.join(path.sep, 'workspace', 'project'); + const workspaceRoot = createTestAbsolutePath('workspace', 'project'); const violationFilePath = path.join(workspaceRoot, 'src', 'file.js'); const violation = new MockViolation( @@ -287,7 +303,7 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { * - Both resolve paths relative to workspace root * - Both fall back to absolute paths if workspace root is null */ - const workspaceRoot = path.join(path.sep, 'Users', 'user', 'dreamhouse'); + const workspaceRoot = createTestAbsolutePath('Users', 'user', 'dreamhouse'); // Imagine ignores config: ["force-app/test/**"] // Imagine bulk suppressions: "force-app/main/default/utils.js" From d755ab1917b7916b9e99ce1b94b4a8dd77af7892 Mon Sep 17 00:00:00 2001 From: Namrata Gupta Date: Fri, 10 Apr 2026 16:08:35 +0530 Subject: [PATCH 13/16] fix absolute path handling --- .../test/suppressions/bulk-suppression-workspace-root.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/code-analyzer-core/test/suppressions/bulk-suppression-workspace-root.test.ts b/packages/code-analyzer-core/test/suppressions/bulk-suppression-workspace-root.test.ts index a2b27ce9..a96d4094 100644 --- a/packages/code-analyzer-core/test/suppressions/bulk-suppression-workspace-root.test.ts +++ b/packages/code-analyzer-core/test/suppressions/bulk-suppression-workspace-root.test.ts @@ -343,7 +343,7 @@ describe('Bulk Suppressions - Workspace Root Path Resolution (Bug #2)', () => { * * Expected: 5 total violations suppressed (3 + 2), not 3 with shared quota */ - const workspaceRoot = path.join(path.sep, 'Users', 'user', 'workspace'); + const workspaceRoot = createTestAbsolutePath('Users', 'user', 'workspace'); const filePath = path.join(workspaceRoot, 'force-app', 'utils.js'); // Create 6 violations that would match the duplicate selectors From 269aae91b824227b5f32d055ddf5aef9d2c7d820 Mon Sep 17 00:00:00 2001 From: Namrata Gupta Date: Fri, 10 Apr 2026 16:11:31 +0530 Subject: [PATCH 14/16] removing debug logs --- .github/workflows/verify-pr.yml | 2 -- .../bulk-suppression-processor.ts | 36 +------------------ 2 files changed, 1 insertion(+), 37 deletions(-) diff --git a/.github/workflows/verify-pr.yml b/.github/workflows/verify-pr.yml index c88e1d68..fe441410 100644 --- a/.github/workflows/verify-pr.yml +++ b/.github/workflows/verify-pr.yml @@ -88,8 +88,6 @@ jobs: python-version: 3.12 - run: npm install - name: Test changes in affected packages - env: - DEBUG_BULK_SUPPRESSIONS: ${{ matrix.os == 'windows-latest' && 'true' || 'false' }} run: | BASE_SHA=${{ github.event.pull_request.base.sha }} HEAD_SHA=${{ github.event.pull_request.head.sha }} diff --git a/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts b/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts index ec59864c..d4b13cf4 100644 --- a/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts +++ b/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts @@ -135,49 +135,22 @@ function doesFileMatchConfigPath( configPath: string, workspaceRoot: string ): boolean { - const debugEnabled = process.env.DEBUG_BULK_SUPPRESSIONS === 'true'; - - if (debugEnabled) { - console.log('[DEBUG] doesFileMatchConfigPath called:'); - console.log(' violationFile:', violationFile); - console.log(' configPath:', configPath); - console.log(' workspaceRoot:', workspaceRoot); - console.log(' path.isAbsolute(configPath):', path.isAbsolute(configPath)); - } - // Config paths can be relative (recommended, like .gitignore) or absolute // If relative, resolve against workspace root // If absolute, use as-is const absoluteConfigPath = path.resolve(workspaceRoot, configPath); - if (debugEnabled) { - console.log(' After path.resolve:', absoluteConfigPath); - } - // Normalize paths for consistent comparison const normalizedViolationFile = path.normalize(violationFile); const normalizedConfigPath = path.normalize(absoluteConfigPath); - if (debugEnabled) { - console.log(' After normalize - violation:', normalizedViolationFile); - console.log(' After normalize - config:', normalizedConfigPath); - } - // Normalize to POSIX separators for cross-platform comparison // Convert all backslashes to forward slashes for consistent comparison const comparisonViolationFile = normalizedViolationFile.replace(/\\/g, '/'); const comparisonConfigPath = normalizedConfigPath.replace(/\\/g, '/'); - if (debugEnabled) { - console.log(' After POSIX normalize - violation:', comparisonViolationFile); - console.log(' After POSIX normalize - config:', comparisonConfigPath); - } - // Check if it's an exact file match if (comparisonViolationFile === comparisonConfigPath) { - if (debugEnabled) { - console.log(' Result: EXACT MATCH'); - } return true; } @@ -188,14 +161,7 @@ function doesFileMatchConfigPath( ? comparisonConfigPath : comparisonConfigPath + '/'; - const result = comparisonViolationFile.startsWith(configPathWithSep); - - if (debugEnabled) { - console.log(' Folder match check - configPathWithSep:', configPathWithSep); - console.log(' Result:', result ? 'FOLDER MATCH' : 'NO MATCH'); - } - - return result; + return comparisonViolationFile.startsWith(configPathWithSep); } /** From 65ba700df397e058717899d2b05bb1c4fbe067c5 Mon Sep 17 00:00:00 2001 From: Namrata Gupta Date: Mon, 13 Apr 2026 14:21:19 +0530 Subject: [PATCH 15/16] Adding changes for -ve values --- packages/code-analyzer-core/src/config.ts | 11 +- .../code-analyzer-core/test/config.test.ts | 137 ++++++++++++++++++ 2 files changed, 145 insertions(+), 3 deletions(-) diff --git a/packages/code-analyzer-core/src/config.ts b/packages/code-analyzer-core/src/config.ts index 931fdb63..fc6eb5e3 100644 --- a/packages/code-analyzer-core/src/config.ts +++ b/packages/code-analyzer-core/src/config.ts @@ -64,7 +64,7 @@ export type BulkSuppressionRule = { */ export type Suppressions = { disable_suppressions: boolean; - bulk_suppressions: Record; + bulk_suppressions?: Record; } type TopLevelConfig = { @@ -85,7 +85,7 @@ export const DEFAULT_CONFIG: TopLevelConfig = { config_root: process.cwd(), log_folder: os.tmpdir(), log_level: LogLevel.Debug, - suppressions: { disable_suppressions: false, bulk_suppressions: {} }, // Suppressions enabled by default, no bulk suppressions by default + suppressions: { disable_suppressions: false }, // Suppressions enabled by default, no bulk suppressions by default rules: {}, engines: {}, ignores: { files: [] }, @@ -287,7 +287,7 @@ export class CodeAnalyzerConfig { * Returns the bulk suppressions configuration (file paths mapped to suppression rules). */ public getBulkSuppressions(): Record { - return this.config.suppressions.bulk_suppressions; + return this.config.suppressions.bulk_suppressions || {}; } /** @@ -465,6 +465,11 @@ function validateBulkSuppressionRule(value: unknown, fieldPath: string): BulkSup throw new Error(getMessage('InvalidBulkSuppressionRule', fieldPath, 'max_suppressed_violations must be a number or null')); } + // max_suppressed_violations must be non-negative if specified + if (typeof rule.max_suppressed_violations === 'number' && rule.max_suppressed_violations < 0) { + throw new Error(getMessage('InvalidBulkSuppressionRule', fieldPath, 'max_suppressed_violations must be a non-negative number')); + } + // reason is optional if (rule.reason !== undefined && typeof rule.reason !== 'string') { throw new Error(getMessage('InvalidBulkSuppressionRule', fieldPath, 'reason must be a string')); diff --git a/packages/code-analyzer-core/test/config.test.ts b/packages/code-analyzer-core/test/config.test.ts index 646c69e9..3dfa8044 100644 --- a/packages/code-analyzer-core/test/config.test.ts +++ b/packages/code-analyzer-core/test/config.test.ts @@ -642,3 +642,140 @@ describe("Tests for glob pattern validation in ignores", () => { })).toThrow(getMessage('InvalidGlobPattern', 'ignores.files[2]', '**/invalid{pattern', 'unclosed brace {')); }); }); + +describe("Tests for bulk suppressions configuration", () => { + it("When suppressions.disable_suppressions is set correctly, config is valid", () => { + const conf: CodeAnalyzerConfig = CodeAnalyzerConfig.fromYamlString(` +suppressions: + disable_suppressions: true +`); + expect(conf.getSuppressions().disable_suppressions).toEqual(true); + }); + + it("When bulk suppressions are configured with valid values, config is valid", () => { + const conf: CodeAnalyzerConfig = CodeAnalyzerConfig.fromYamlString(` +suppressions: + disable_suppressions: false + "src/file.js": + - rule_selector: "eslint:no-console" + max_suppressed_violations: 5 + reason: "Legacy code" +`); + expect(conf.getBulkSuppressions()).toEqual({ + "src/file.js": [{ + rule_selector: "eslint:no-console", + max_suppressed_violations: 5, + reason: "Legacy code" + }] + }); + }); + + it("When max_suppressed_violations is null, config is valid", () => { + const conf: CodeAnalyzerConfig = CodeAnalyzerConfig.fromYamlString(` +suppressions: + "src/file.js": + - rule_selector: "eslint:no-console" + max_suppressed_violations: null +`); + expect(conf.getBulkSuppressions()["src/file.js"][0].max_suppressed_violations).toBeNull(); + }); + + it("When max_suppressed_violations is 0, config is valid", () => { + const conf: CodeAnalyzerConfig = CodeAnalyzerConfig.fromYamlString(` +suppressions: + "src/file.js": + - rule_selector: "eslint:no-console" + max_suppressed_violations: 0 +`); + expect(conf.getBulkSuppressions()["src/file.js"][0].max_suppressed_violations).toEqual(0); + }); + + it("When max_suppressed_violations is negative, then we throw an error", () => { + expect(() => CodeAnalyzerConfig.fromYamlString(` +suppressions: + "src/file.js": + - rule_selector: "eslint:no-console" + max_suppressed_violations: -1 +`)).toThrow('max_suppressed_violations must be a non-negative number'); + }); + + it("When max_suppressed_violations is negative decimal, then we throw an error", () => { + expect(() => CodeAnalyzerConfig.fromYamlString(` +suppressions: + "src/utils/": + - rule_selector: "pmd:UnusedMethod" + max_suppressed_violations: -5.5 +`)).toThrow('max_suppressed_violations must be a non-negative number'); + }); + + it("When max_suppressed_violations is a large negative number, then we throw an error", () => { + expect(() => CodeAnalyzerConfig.fromObject({ + suppressions: { + "file.js": [{ + rule_selector: "all", + max_suppressed_violations: -9999 + }] + } + })).toThrow('max_suppressed_violations must be a non-negative number'); + }); + + it("When rule_selector is missing, then we throw an error", () => { + expect(() => CodeAnalyzerConfig.fromYamlString(` +suppressions: + "src/file.js": + - max_suppressed_violations: 5 +`)).toThrow('rule_selector is required and must be a string'); + }); + + it("When rule_selector is not a string, then we throw an error", () => { + expect(() => CodeAnalyzerConfig.fromObject({ + suppressions: { + "file.js": [{ + rule_selector: 123, + max_suppressed_violations: 5 + }] + } + })).toThrow('rule_selector is required and must be a string'); + }); + + it("When max_suppressed_violations is not a number or null, then we throw an error", () => { + expect(() => CodeAnalyzerConfig.fromObject({ + suppressions: { + "file.js": [{ + rule_selector: "all", + max_suppressed_violations: "invalid" + }] + } + })).toThrow('max_suppressed_violations must be a number or null'); + }); + + it("When reason is not a string, then we throw an error", () => { + expect(() => CodeAnalyzerConfig.fromObject({ + suppressions: { + "file.js": [{ + rule_selector: "all", + max_suppressed_violations: 5, + reason: 123 + }] + } + })).toThrow('reason must be a string'); + }); + + it("When bulk suppression rule is not an object, then we throw an error", () => { + expect(() => CodeAnalyzerConfig.fromObject({ + suppressions: { + "file.js": ["invalid"] + } + })).toThrow('Expected an object'); + }); + + it("When bulk suppression value for file is not an array, then we throw an error", () => { + expect(() => CodeAnalyzerConfig.fromObject({ + suppressions: { + "file.js": { + rule_selector: "all" + } + } + })).toThrow("must be of type 'array'"); + }); +}); From 0bdc8cf98dbf4aa47c9ad941fe1a9cacc597e5ab Mon Sep 17 00:00:00 2001 From: Namrata Gupta Date: Thu, 16 Apr 2026 14:10:25 +0530 Subject: [PATCH 16/16] updating doc --- .../src/suppressions/bulk-suppression-processor.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts b/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts index d4b13cf4..eaacb60c 100644 --- a/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts +++ b/packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts @@ -29,9 +29,14 @@ export type BulkSuppressionResult = { /** * Applies bulk suppressions to violations based on config, updating quota tracking + * + * IMPORTANT: This function mutates the quotas Map as a side effect. The Map tracks + * suppression counts across multiple calls and should be shared across all engines + * in a single analysis run to maintain accurate quota limits. + * * @param violations List of violations to process * @param bulkConfig Bulk suppression configuration from YAML - * @param quotas Shared quota tracker (mutated by this function) + * @param quotas Shared quota tracker (MUTATED by this function - maintains state across calls) * @param workspaceRoot Root directory for resolving relative paths * @returns Object containing unsuppressed violations and count of suppressions applied */