Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Sep 23, 2025

This PR centralizes the file path validation logic that was previously duplicated across multiple issue providers (MsBuild, TAP, and SARIF) into a single, reusable method on the BaseIssueProvider class.

Problem

The ValidateFilePath method existed in three different implementations with similar but slightly different logic:

  • MsBuild: BaseMsBuildLogFileFormat.ValidateFilePath - validated absolute paths only
  • TAP: BaseTapLogFileFormat.ValidateFilePath - handled both relative and absolute paths with auto-detection
  • SARIF: SarifIssuesProvider.ValidateFilePath - took an explicit isAbsolutePath parameter

This duplication made the codebase harder to maintain and could lead to inconsistent behavior across providers.

Solution

Added a centralized BaseIssueProvider.ValidateFilePath static method that:

  • Automatically detects if a path is relative or absolute (no need for manual detection)
  • Validates that absolute paths are within the repository using existing extension methods
  • Converts absolute paths to relative paths using the repository root
  • Preserves relative paths unchanged
  • Returns a consistent tuple format: (bool Valid, string FilePath)

All existing provider implementations now delegate to this centralized method:

protected (bool Valid, string FilePath) ValidateFilePath(string filePath, IRepositorySettings repositorySettings) =>
    BaseIssueProvider.ValidateFilePath(filePath, repositorySettings);

Benefits

  • Eliminates code duplication: Removed ~60 lines of duplicated validation logic
  • Ensures consistency: All issue providers now use identical validation behavior
  • Improves maintainability: Changes to validation logic only need to be made in one place
  • Comprehensive coverage: Added thorough tests for all validation scenarios

Testing

  • All existing tests continue to pass (no regressions)
  • Added comprehensive test coverage for the new centralized method
  • Verified functionality across MsBuild, TAP, and SARIF providers

Fixes #975.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

@Copilot Copilot AI changed the title [WIP] Centralize file path validation for issue providers Centralize file path validation for issue providers Sep 24, 2025
Copilot finished work on behalf of pascalberger September 24, 2025 00:23
@Copilot Copilot AI requested a review from pascalberger September 24, 2025 00:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Centralize file path validation for issue providers
2 participants