Fix issues with `FileSupport.GetCanonicalPath()` extension method
- Dominant language
- C#
- Stars
- 2.4k
- Forks
- 658
- Avg merge
- 3d 5h
- Merged PRs (30d)
- 9
Description
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Task description
There are currently several issues with the implementation of `FileSupport.GetCanonicalPath()`, which is mostly a port from Apache Harmony. This method is on a required path for nearly all users, so it would be good to ensure that it is optimized and fully functional.
- [ ] No guard clause for `null` check, which should throw `ArgumentNullException`
- [ ] Logic is allocating 1 extra byte to the temporary array, appending a `0` byte at the end, converting to string, then removing the trailing `\0` character that the `0` byte creates. It would be better just to omit the null character from the buffer.
- [ ] The part that resolves symbolic links was not ported (there is a `LUCENENET TODO` for it in the code).
- [ ] The behavior of the `GetOrAdd()` method already allows racing threads to both run the business logic without blocking. If we move the logic that builds the string into the factory parameter of this method, we can eliminate the extra `TryGetValue()` check (and the unnecessary check for a `null` string in the collection).
- [ ] J2N.Text now contains an extension method for `Encoding` that accepts a `Span` so we can allocate the temporary buffer on the stack on all supported target frameworks.
Contributor guide
Research direction
Start at the FileSupport.GetCanonicalPath() extension method and inspect the existing LUCENENET TODO for symbolic-link resolution. Review the current GetOrAdd() flow and J2N.Text’s Encoding extension for Span. Done means all five checklist items are addressed and the method’s canonical-path behavior remains functional.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- operating-systems
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100