Skip to content

Dedupe the hand-rolled file:// conversion behind FileUri - #391

Merged
Firehed merged 6 commits into
mainfrom
slice/SC.4
Aug 3, 2026
Merged

Dedupe the hand-rolled file:// conversion behind FileUri#391
Firehed merged 6 commits into
mainfrom
slice/SC.4

Conversation

@Firehed

@Firehed Firehed commented Aug 3, 2026

Copy link
Copy Markdown
Owner

Slice SC.4: file:// URI and path conversion was hand-rolled in three live places,
each handling the scheme slightly differently and none of them consistently handling
percent-encoding. One FileUri replaces them.

Pre-existing duplication owned by no plan step, which is what the SC.* block is for. It
surfaced while splitting S3.7, whose locator wanted a fourth copy; S3.7c is gated on this
landing rather than adding one.

The three sites

Site Before After
FilesystemBackend private pathFromUri (decoded) + 'file://' . $path FileUri::toPath / FileUri::fromPath
DefaultClassInfoFactory private uriToPath — stripped the scheme, did not decode FileUri::toPath
Location::fromFileLine str_starts_with(...) ? $file : 'file://' . $filenever encoded normalized through the seam

Two behavior fixes fall out

Both were latent: they only appear when the workspace sits under a path containing a
reserved character, which is ordinary on macOS (~/My Projects/...).

  • DefaultClassInfoFactory did not decode, so a class in a directory with a space
    produced a path that does not exist on disk.
  • Location::fromFileLine did not encode, and its output is what a definition or hover
    response carries to the client — an unencoded space or # makes that an invalid
    URI. Covered by a test committed red before the fix.

Not in scope

WorkspaceIndexer:61 holds a fourth copy and is deliberately untouched: it is dead code
with zero references, and slice SC.1 deletes the file outright.

@codecov

codecov Bot commented Aug 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.57%. Comparing base (dc647dc) to head (a594d54).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #391      +/-   ##
============================================
+ Coverage     98.55%   98.57%   +0.02%     
+ Complexity     1753     1751       -2     
============================================
  Files           115      116       +1     
  Lines          4434     4432       -2     
============================================
- Hits           4370     4369       -1     
+ Misses           64       63       -1     

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

@Firehed
Firehed merged commit f40e791 into main Aug 3, 2026
7 checks passed
@Firehed
Firehed deleted the slice/SC.4 branch August 3, 2026 21:17
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.

1 participant