Skip to content

feat(xfs): improve IsSamePath with abs path and refactor util.go to use it #367

Open
wenchy wants to merge 1 commit intomasterfrom
feat/xfs-is-same-path
Open

feat(xfs): improve IsSamePath with abs path and refactor util.go to use it #367
wenchy wants to merge 1 commit intomasterfrom
feat/xfs-is-same-path

Conversation

@wenchy
Copy link
Copy Markdown
Member

@wenchy wenchy commented Mar 17, 2026

Summary

xfs.IsSamePath improvement

  • Resolve both paths to absolute paths before comparison, so mixed
    relative/absolute or dot-segment paths pointing to the same location
    are correctly considered equal
  • Fall back to CleanSlashPath comparison if filepath.Abs fails

Test coverage

  • Add comprehensive test cases in internal/x/xfs/path_test.go covering:
    • Absolute vs relative paths pointing to the same location
    • Dot-segment paths (./foo/../bar)
    • Trailing slash normalization
    • Cross-platform slash normalization
    • Clearly different paths

prepareOutdir refactor

  • Replace manual filepath.Abs + xfs.CleanSlashPath map-based approach
    with xfs.IsSamePath via an isImported closure
  • Removes two filepath.Abs calls and their associated error handling,
    making the logic simpler and more readable

…se it

- IsSamePath now resolves both paths to absolute paths before comparison,
  so mixed relative/absolute or dot-segment paths pointing to the same
  location are correctly considered equal
- Add comprehensive test cases for IsSamePath covering abs/rel, dot-segments,
  symlink-free equivalence, and cross-platform slash normalization
- Refactor prepareOutdir in util.go to use IsSamePath instead of manually
  calling filepath.Abs + xfs.CleanSlashPath, simplifying the logic
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 17, 2026

Codecov Report

❌ Patch coverage is 50.00000% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.90%. Comparing base (bc70718) to head (f8739c9).

Files with missing lines Patch % Lines
internal/protogen/util.go 55.55% 1 Missing and 3 partials ⚠️
internal/x/xfs/path.go 42.85% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #367      +/-   ##
==========================================
- Coverage   73.91%   73.90%   -0.01%     
==========================================
  Files          87       87              
  Lines        8664     8673       +9     
==========================================
+ Hits         6404     6410       +6     
  Misses       1700     1700              
- Partials      560      563       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

// remove all *.proto file but not Imports
imports := make(map[string]bool)
// Collect all glob-expanded import paths.
var importPaths []string
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using map[string]bool makes sense here because one importPath may match mutiple patterns.

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.

2 participants