-
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
UnnecessarilyFullyQualified
and package-info.java
#1652
Comments
copybara-service bot
pushed a commit
that referenced
this issue
Jan 8, 2021
Fixes #1652 PiperOrigin-RevId: 350601073
copybara-service bot
pushed a commit
that referenced
this issue
Jan 8, 2021
Fixes #1652 PiperOrigin-RevId: 350601073
stevie400
pushed a commit
to HubSpot/error-prone
that referenced
this issue
Jan 15, 2021
Fixes google#1652 PiperOrigin-RevId: 350823261
copybara-service bot
pushed a commit
that referenced
this issue
Jun 28, 2021
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 in `compilationUnitTree.getSourceFile().getName()`, though `FileObject#getName()` says: ```java /** * Returns a user-friendly name for this file object. The exact * value returned is not specified but implementations should take * care to preserve names as given by the user. For example, if * the user writes the filename {@code "BobsApp\Test.java"} on * the command line, this method should return {@code * "BobsApp\Test.java"} whereas the {@linkplain #toUri toUri} * method might return {@code * file:///C:/Documents%20and%20Settings/UncleBob/BobsApp/Test.java}. * * @return a user-friendly name */ ``` I found a similar bit of code in the `PackageInfo` check. This change replaces the forward slash with `File.separatorChar`. An alternative (presumably slightly less performant) fix is to rely on `FileObject#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. Fixes #2404 COPYBARA_INTEGRATE_REVIEW=#2404 from PicnicSupermarket:bugfix/windows-package-info-detection e928dae PiperOrigin-RevId: 381954227
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Consider the following code:
Applying the
UnnecessarilyFullyQualified
check to this code:wget \ https://repo1.maven.org/maven2/com/google/errorprone/error_prone_core/2.4.0/error_prone_core-2.4.0-with-dependencies.jar wget \ https://repo1.maven.org/maven2/com/google/errorprone/error_prone_annotations/2.4.0/error_prone_annotations-2.4.0.jar javac \ -XDcompilePolicy=simple \ -processorpath error_prone_core-2.4.0-with-dependencies.jar \ '-Xplugin:ErrorProne -XepPatchChecks:UnnecessarilyFullyQualified -XepPatchLocation:/tmp' \ -cp error_prone_annotations-2.4.0.jar \ foo/package-info.java
This yields:
In the case of package-level annotations, single-use fully qualified types arguably result in more straightforward code, as otherwise the type is used before it is imported.
(In case a type is referenced multiple times (perhaps in a nested annotation or Javadoc), I guess importing does make sense. 🤔 )
The text was updated successfully, but these errors were encountered: