I am waiting for you to make the other changes before committing your work.

I am waiting for you to make the other changes before committing your work.

Thank you very much for your guidance!
I’ve updated all the changes you suggested.
Could you please let me know if there’s anything else that needs improvement?

Can you squash your commits to make it more readable? Thank you.

Can you squash your commits to make it more readable? Thank you.

Thanks for the reminder! I’ve already squashed all the commits into one commit.

Now what happens if parsing fails? The issue mainly concerned cases where parsing failed, because in the other case the path is already available via cu.getStorage().getPath().

Now what happens if parsing fails? The issue mainly concerned cases where parsing failed, because in the other case the path is already available via cu.getStorage().getPath().

In my first attempt I added a test resource whose content was a single backslash-u ("\u"). The goal was to deterministically trigger a scanner failure so that ParseResult would be unsuccessful and result() would be empty (as issue #4786 ). This let me verify that getSourcePath is present on failed parses.
The figure below shows the case when parsing fails. We can see that the path

Based on your feedback, I removed the custom “\u” fixture and now use an existing Java file under the utils test resources to trigger the failure.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.333%. Comparing base (d1de37b) to head (9852c2b).
⚠️ Report is 6 commits behind head on master.

@@               Coverage Diff               @@
##              master     #4874       +/-   ##
===============================================
+ Coverage     58.330%   58.333%   +0.003%     
  Complexity      2535      2535               
===============================================
  Files            677       677               
  Lines          39038     39041        +3     
  Branches        7088      7089        +1     
===============================================
+ Hits           22771     22774        +3     
  Misses         13368     13368               
  Partials        2899      2899               

I added sourcePathToStringTest to cover ParseResult.toString() with and without sourcePath, improving branch coverage and guarding against regressions. And I ran Spotless locally.

On second thought, I wonder if it wouldn't be better to set the source path in the JavaParser class at the point where methods take a file or path as a parameter.

On second thought, I wonder if it wouldn't be better to set the source path in the JavaParser class at the point where methods take a file or path as a parameter.

Thanks for the suggestion. To confirm:
Do you mean that we should still generate a result even if parsing fails? (In this case, the result is null, and I would create a new one.) I'm not sure if that could have any side effects. If that’s what you intend, please let me know — I’ll create a new branch to handle it.

On second thought, I wonder if it wouldn't be better to set the source path in the JavaParser class at the point where methods take a file or path as a parameter.

I thought about it again — or do you mean setting the sourcePath here, so that only the files that failed to parse would have their sourcePath set? Please let me know if that’s what you mean, and I’ll give it a try.

On second thought, I wonder if it wouldn't be better to set the source path in the JavaParser class at the point where methods take a file or path as a parameter.

Thanks for the suggestion. To confirm: Do you mean that we should still generate a result even if parsing fails? (In this case, the result is null, and I would create a new one.) I'm not sure if that could have any side effects. If that’s what you intend, please let me know — I’ll create a new branch to handle it.

In practice, ParseResult is instantiated even in the event of a parsing error.

On second thought, I wonder if it wouldn't be better to set the source path in the JavaParser class at the point where methods take a file or path as a parameter.

I thought about it again — or do you mean setting the sourcePath here, so that only the files that failed to parse would have their sourcePath set? Please let me know if that’s what you mean, and I’ll give it a try.

If the parsing runs correctly, the compilation unit already knows the path of the parsed file (storage). However, in the event of an error, there is no information because the compilation unit is not instantiated. So in this case, the path should be stored in the ParseResult that is returned.
Furthermore, JavaParser declares parsing methods for which the provider is instantiated. It is therefore preferable to use these methods in the SourceRoot class.

On second thought, I wonder if it wouldn't be better to set the source path in the JavaParser class at the point where methods take a file or path as a parameter.

I thought about it again — or do you mean setting the sourcePath here, so that only the files that failed to parse would have their sourcePath set? Please let me know if that’s what you mean, and I’ll give it a try.

If the parsing runs correctly, the compilation unit already knows the path of the parsed file (storage). However, in the event of an error, there is no information because the compilation unit is not instantiated. So in this case, the path should be stored in the ParseResult that is returned. Furthermore, JavaParser declares parsing methods for which the provider is instantiated. It is therefore preferable to use these methods in the SourceRoot class.

Thank you for your guidance; I’ll give it a try.

Why did you close this PR? Just make sure to squash your commits and isolate improvements to other classes in a specific commit.

On second thought, I wonder if it wouldn't be better to set the source path in the JavaParser class at the point where methods take a file or path as a parameter.

Sorry to bother you again, but I’m a bit confused about this.
You suggested setting the source path in JavaParser.
However, SourceRoot.tryToParse() internally uses the Provider entry point:

in JavaParser.
Since the Provider doesn’t expose the file path, it seems there’s no way to retrieve it inside JavaParser.parse().
Please correct me if I’m missing something — thank you!

Here is my attempt to move setSourcePath into JavaParser. Since SourceRoot.tryToParse() calls the overload that also instantiates the provider, we cannot retrieve the path at the JavaParser layer. That means we would need to switch the parse() overload used inside SourceRoot.tryToParse(), but this does not seem reliable.

Here is SourceRoot.tryToParse() . I’m a bit worried that replacing the .parse() call with another method might cause some unintended side effects.

Here is what I changed to setSourcePath in JavaParser, but this method is Deprecated.

The method that defines character encoding is deprecated because the input character encoding must be a configuration option so you can use the method that does not define the encoding.

Thank you for your contribution.

Thank you for your contribution.

Thank you for your guidance. I’ve learned a lot from this task.