Skip to content

Fix duplicate path join in OS filesystem utility#287

Merged
System-Glitch merged 1 commit into
go-goyave:masterfrom
fnoopv:fix_repeat_pathjoin
Jun 3, 2026
Merged

Fix duplicate path join in OS filesystem utility#287
System-Glitch merged 1 commit into
go-goyave:masterfrom
fnoopv:fix_repeat_pathjoin

Conversation

@fnoopv

@fnoopv fnoopv commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

References

Issue(s): N/A
Discussion: N/A

Description

Fix osfs.FS.FileExists and osfs.FS.IsDirectory when a base directory is configured with New(...).

Previously, both methods called path.Join(f.dir, name) before delegating to f.Stat(name). Since Stat already resolves the given path relative to the filesystem base directory, the base path was effectively joined twice. This caused lookups such as New("resources").FileExists("test_file.txt") and New("resources").IsDirectory("lang") to fail unexpectedly.

This PR removes the duplicated path.Join in both methods and adds regression tests covering FileExists and IsDirectory with a configured base directory.

Usage example

fs := osfs.New("resources")

exists := fs.FileExists("test_file.txt") // true
isDir := fs.IsDirectory("lang")          // true

Possible drawbacks

This change slightly alters the behavior of FileExists and IsDirectory for callers relying on the previous incorrect path resolution. However, the new behavior is consistent with the rest of the filesystem abstraction and with how Stat already handles base directories.

@coveralls

coveralls commented Jun 2, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 26882796764

Coverage remained the same at 97.515%

Details

  • Coverage remained the same as the base build.
  • Patch coverage: 2 of 2 lines across 1 file are fully covered (100%).
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 6882
Covered Lines: 6711
Line Coverage: 97.52%
Coverage Strength: 297.67 hits per line

💛 - Coveralls

@System-Glitch System-Glitch self-requested a review June 3, 2026 11:20
@System-Glitch System-Glitch self-assigned this Jun 3, 2026
@System-Glitch

Copy link
Copy Markdown
Member

This change slightly alters the behavior of FileExists and IsDirectory for callers relying on the previous incorrect path resolution. However, the new behavior is consistent with the rest of the filesystem abstraction and with how Stat already handles base directories.

Any patch affecting code relying on buggy behavior should not be considered a breaking change. This is fine.

@System-Glitch System-Glitch force-pushed the fix_repeat_pathjoin branch from 8b82c24 to d508550 Compare June 3, 2026 11:44
@System-Glitch System-Glitch force-pushed the fix_repeat_pathjoin branch from d508550 to f3617b8 Compare June 3, 2026 11:49
@System-Glitch System-Glitch merged commit f3ae5f2 into go-goyave:master Jun 3, 2026
5 checks passed
@System-Glitch

Copy link
Copy Markdown
Member

Merged. Thanks!

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.

3 participants