|
9 | 9 | UninstantiableEngineRunResults, |
10 | 10 | Violation |
11 | 11 | } from "./results" |
12 | | -import {processSuppressions} from "./suppressions" |
| 12 | +import {processSuppressions, extractSuppressionsFromFiles, SuppressionsMap, LoggerCallback} from "./suppressions" |
13 | 13 | import {SemVer} from 'semver'; |
14 | 14 | import { |
15 | 15 | EngineLogEvent, |
@@ -120,6 +120,11 @@ export class CodeAnalyzer { |
120 | 120 | private readonly engineConfigDescriptions: Map<string, ConfigDescription> = new Map(); |
121 | 121 | private readonly rulesCache: Map<string, RuleImpl[]> = new Map(); |
122 | 122 | private readonly engineRuleDiscoveryProgressAggregator: EngineProgressAggregator = new EngineProgressAggregator(); |
| 123 | + // Caching for per-engine suppression processing to avoid duplicate file processing |
| 124 | + private readonly suppressionsMap: SuppressionsMap = new Map(); |
| 125 | + private readonly fileProcessingPromises: Map<string, Promise<void>> = new Map(); |
| 126 | + // Track total suppressed violations for aggregate logging |
| 127 | + private totalSuppressedViolations: number = 0; |
123 | 128 |
|
124 | 129 | constructor(config: CodeAnalyzerConfig, fileSystem: FileSystem = new RealFileSystem(), nodeVersion: string = process.version) { |
125 | 130 | this.validateEnvironment(nodeVersion); |
@@ -348,6 +353,15 @@ export class CodeAnalyzer { |
348 | 353 | // up a bunch of RunResults promises and then does a Promise.all on them. Otherwise, the progress events may |
349 | 354 | // override each other. |
350 | 355 |
|
| 356 | + // Reset suppression counter for this run |
| 357 | + this.totalSuppressedViolations = 0; |
| 358 | + |
| 359 | + // Clear suppression caches from previous runs to prevent unbounded memory growth |
| 360 | + // Each run typically analyzes a different workspace, so caching across runs provides minimal benefit |
| 361 | + // while keeping stale data in memory. |
| 362 | + this.suppressionsMap.clear(); |
| 363 | + this.fileProcessingPromises.clear(); |
| 364 | + |
351 | 365 | this.emitLogEvent(LogLevel.Debug, getMessage('RunningWithWorkspace', JSON.stringify({ |
352 | 366 | filesAndFolders: runOptions.workspace.getRawFilesAndFolders(), |
353 | 367 | targets: runOptions.workspace.getRawTargets() |
@@ -395,55 +409,143 @@ export class CodeAnalyzer { |
395 | 409 | runResults.addEngineRunResults(new UninstantiableEngineRunResults(uninstantiableEngine, error)); |
396 | 410 | } |
397 | 411 |
|
398 | | - // Process inline suppressions (post-processing step) |
399 | | - // This filters out violations that have been suppressed via inline markers |
400 | | - await this.applyInlineSuppressions(runResults); |
| 412 | + // Note: Inline suppressions are now applied per-engine in runEngineAndValidateResults() before EngineResultsEvent is emitted |
| 413 | + |
| 414 | + // Log aggregate suppression count if any violations were suppressed |
| 415 | + if (this.config.getSuppressionsEnabled() && this.totalSuppressedViolations > 0) { |
| 416 | + this.emitLogEvent(LogLevel.Info, getMessage('SuppressedViolationsCount', this.totalSuppressedViolations)); |
| 417 | + } |
401 | 418 |
|
402 | 419 | return runResults; |
403 | 420 | } |
404 | 421 |
|
405 | 422 | /** |
406 | | - * Applies suppression filtering to the run results |
407 | | - * This processes suppression markers in source files and filters out suppressed violations |
408 | | - * @param runResults The run results to apply suppressions to |
| 423 | + * Applies suppression filtering to a single engine's results |
| 424 | + * This processes suppression markers in source files and returns a filtered version of the engine results |
| 425 | + * This method handles race conditions by caching suppression ranges per file |
| 426 | + * @param engineRunResults The engine run results to apply suppressions to |
| 427 | + * @returns Filtered engine run results with suppressions applied |
409 | 428 | */ |
410 | | - private async applyInlineSuppressions(runResults: RunResultsImpl): Promise<void> { |
| 429 | + private async applyInlineSuppressionsToEngineResults( |
| 430 | + engineRunResults: EngineRunResults |
| 431 | + ): Promise<EngineRunResults> { |
411 | 432 | // Check if suppressions are enabled |
412 | 433 | if (!this.config.getSuppressionsEnabled()) { |
413 | | - return; // Feature disabled, skip processing |
| 434 | + return engineRunResults; // Feature disabled, return original results |
414 | 435 | } |
415 | 436 |
|
416 | | - const allViolations = runResults.getViolations(); |
| 437 | + const violations = engineRunResults.getViolations(); |
| 438 | + |
| 439 | + if (violations.length === 0) { |
| 440 | + return engineRunResults; // No violations to process |
| 441 | + } |
| 442 | + |
| 443 | + // Extract unique file paths from violations for race condition handling |
| 444 | + const filePaths = new Set<string>(); |
| 445 | + for (const violation of violations) { |
| 446 | + const primaryLocation = violation.getPrimaryLocation(); |
| 447 | + const file = primaryLocation.getFile(); |
| 448 | + if (file) { |
| 449 | + filePaths.add(file); |
| 450 | + } |
| 451 | + } |
417 | 452 |
|
418 | | - if (allViolations.length === 0) { |
419 | | - return; // No violations to process |
| 453 | + if (filePaths.size === 0) { |
| 454 | + return engineRunResults; // No files with violations |
420 | 455 | } |
421 | 456 |
|
422 | | - this.emitLogEvent(LogLevel.Debug, getMessage('ProcessingInlineSuppressions', allViolations.length)); |
| 457 | + // Process files with race condition handling to pre-populate the shared map |
| 458 | + await this.processFilesForSuppressions(filePaths); |
423 | 459 |
|
424 | | - // Process suppressions (returns filtered violations) |
425 | | - const logger = (level: 'error' | 'warn' | 'debug', message: string) => { |
| 460 | + // Use processSuppressions with the pre-populated shared map |
| 461 | + // This will skip re-parsing files already in the map and just filter violations |
| 462 | + const logger: LoggerCallback = (level: 'error' | 'warn' | 'debug', message: string) => { |
426 | 463 | const logLevel = level === 'error' ? LogLevel.Error : level === 'warn' ? LogLevel.Warn : LogLevel.Debug; |
427 | 464 | this.emitLogEvent(logLevel, message); |
428 | 465 | }; |
429 | | - const filteredViolations = await processSuppressions(allViolations, logger); |
430 | | - |
431 | | - // Calculate which violations were suppressed |
432 | | - const suppressedViolations = new Set<Violation>(); |
433 | | - const filteredSet = new Set(filteredViolations); |
434 | | - for (const violation of allViolations) { |
435 | | - if (!filteredSet.has(violation)) { |
436 | | - suppressedViolations.add(violation); |
437 | | - } |
| 466 | + const filteredViolations = await processSuppressions(violations, logger, this.suppressionsMap); |
| 467 | + |
| 468 | + // Calculate how many violations were suppressed |
| 469 | + const suppressedCount = violations.length - filteredViolations.length; |
| 470 | + |
| 471 | + // If all violations remain (nothing suppressed), return original results |
| 472 | + if (suppressedCount === 0) { |
| 473 | + return engineRunResults; |
438 | 474 | } |
439 | 475 |
|
440 | | - const suppressedCount = suppressedViolations.size; |
441 | | - if (suppressedCount > 0) { |
442 | | - this.emitLogEvent(LogLevel.Info, getMessage('SuppressedViolationsCount', suppressedCount)); |
443 | | - runResults.applySuppressedViolationsFilter(suppressedViolations); |
444 | | - } else { |
445 | | - this.emitLogEvent(LogLevel.Info, getMessage('NoViolationsSuppressed')); |
| 476 | + // Track suppressed violations for aggregate logging |
| 477 | + this.totalSuppressedViolations += suppressedCount; |
| 478 | + |
| 479 | + // Return filtered results using FilteredEngineRunResults wrapper |
| 480 | + return this.createFilteredEngineRunResults(engineRunResults, filteredViolations); |
| 481 | + } |
| 482 | + |
| 483 | + /** |
| 484 | + * Creates a FilteredEngineRunResults wrapper |
| 485 | + * This is a temporary method until FilteredEngineRunResults is exported from results.ts |
| 486 | + */ |
| 487 | + private createFilteredEngineRunResults( |
| 488 | + originalResults: EngineRunResults, |
| 489 | + filteredViolations: Violation[] |
| 490 | + ): EngineRunResults { |
| 491 | + // We need to create an instance that implements EngineRunResults |
| 492 | + // but filters the violations |
| 493 | + return { |
| 494 | + getEngineName: () => originalResults.getEngineName(), |
| 495 | + getEngineVersion: () => originalResults.getEngineVersion(), |
| 496 | + getViolationCount: () => filteredViolations.length, |
| 497 | + getViolationCountOfSeverity: (severity: number) => |
| 498 | + filteredViolations.filter(v => v.getRule().getSeverityLevel() === severity).length, |
| 499 | + getViolations: () => filteredViolations |
| 500 | + }; |
| 501 | + } |
| 502 | + |
| 503 | + /** |
| 504 | + * Processes files for suppression markers with race condition handling |
| 505 | + * Uses caching to avoid processing the same file multiple times when multiple engines |
| 506 | + * return violations for the same file |
| 507 | + * @param filePaths Set of file paths that need suppression information |
| 508 | + */ |
| 509 | + private async processFilesForSuppressions(filePaths: Set<string>): Promise<void> { |
| 510 | + const logger: LoggerCallback = (level: 'error' | 'warn' | 'debug', message: string) => { |
| 511 | + const logLevel = level === 'error' ? LogLevel.Error : level === 'warn' ? LogLevel.Warn : LogLevel.Debug; |
| 512 | + this.emitLogEvent(logLevel, message); |
| 513 | + }; |
| 514 | + |
| 515 | + const processingPromises: Promise<void>[] = []; |
| 516 | + |
| 517 | + for (const filePath of filePaths) { |
| 518 | + // If already in cache, skip |
| 519 | + if (this.suppressionsMap.has(filePath)) { |
| 520 | + continue; |
| 521 | + } |
| 522 | + |
| 523 | + // If currently being processed, await that promise |
| 524 | + let processingPromise = this.fileProcessingPromises.get(filePath); |
| 525 | + |
| 526 | + if (!processingPromise) { |
| 527 | + // Start new processing - wrap extractSuppressionsFromFiles call |
| 528 | + processingPromise = extractSuppressionsFromFiles( |
| 529 | + new Set([filePath]), |
| 530 | + this.suppressionsMap, |
| 531 | + logger |
| 532 | + ).then(() => { |
| 533 | + // Clean up the promise from tracking map since it's done |
| 534 | + this.fileProcessingPromises.delete(filePath); |
| 535 | + }).catch((err) => { |
| 536 | + // Clean up on error too |
| 537 | + this.fileProcessingPromises.delete(filePath); |
| 538 | + throw err; |
| 539 | + }); |
| 540 | + |
| 541 | + this.fileProcessingPromises.set(filePath, processingPromise); |
| 542 | + } |
| 543 | + |
| 544 | + processingPromises.push(processingPromise); |
446 | 545 | } |
| 546 | + |
| 547 | + // Wait for all file processing to complete |
| 548 | + await Promise.all(processingPromises); |
447 | 549 | } |
448 | 550 |
|
449 | 551 | /** |
@@ -565,7 +667,10 @@ export class CodeAnalyzer { |
565 | 667 | } |
566 | 668 |
|
567 | 669 | validateEngineRunResults(engineName, apiEngineRunResults, ruleSelection); |
568 | | - const engineRunResults: EngineRunResults = new EngineRunResultsImpl(engineName, await engine.getEngineVersion(), apiEngineRunResults, ruleSelection); |
| 670 | + let engineRunResults: EngineRunResults = new EngineRunResultsImpl(engineName, await engine.getEngineVersion(), apiEngineRunResults, ruleSelection); |
| 671 | + |
| 672 | + // Apply inline suppressions per-engine BEFORE emitting EngineResultsEvent |
| 673 | + engineRunResults = await this.applyInlineSuppressionsToEngineResults(engineRunResults); |
569 | 674 |
|
570 | 675 | this.emitEvent<EngineRunProgressEvent>({ |
571 | 676 | type: EventType.EngineRunProgressEvent, timestamp: this.clock.now(), engineName: engineName, percentComplete: 100 |
|
0 commit comments