-
Notifications
You must be signed in to change notification settings - Fork 750
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix package-info.java
identification on Windows
#2404
Fix package-info.java
identification on Windows
#2404
Conversation
Okay, so this change breaks the "JDK 16 on windows-latest" build:
And:
This might be due to the use of literal forward slashes in these tests (a, b). That could perhaps be fixed by using Alternatively we could rely on |
My colleague confirms that the (initial) fix works. ✔️ However, I had another look with a fresh head and noticed that other checks use |
Seems this would fix #2164. |
@PhilippWendler indeed! Seems I forgot about that issue; tnx for linking :) |
My colleague confirmed that the latest version of this PR also resolves the issue ✔️. With that I think this PR is ready for review. |
It seems the issue on Windows was fixed by google/error-prone#2404.
A colleague using Windows reported the issue discussed in #1652, even though we use (a fork of) Error Prone 2.7.1, which contains a fix for said issue.
The problem appears to be that
UnnecessarilyFullyQualified
looks for a forward slash incompilationUnitTree.getSourceFile().getName()
, thoughFileObject#getName()
says:I found a similar bit of code in the
PackageInfo
check. This change replaces the forward slash withFile.separatorChar
. An alternative (presumably slightly less performant) fix is to rely onFileObject#toUri
instead. Let me know if you prefer that approach.I'll update our fork with this change and ask my colleague to test the fix.