From dca7e796d60d333a521b27aa26c58442f0498925 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Mon, 28 Jun 2021 15:04:34 -0700 Subject: [PATCH] Fix `package-info.java` identification on Windows 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=https://github.com/google/error-prone/pull/2404 from PicnicSupermarket:bugfix/windows-package-info-detection e928daed4b28dc3dbf95b262d03533fed5f5183b PiperOrigin-RevId: 381954227 --- .../java/com/google/errorprone/bugpatterns/PackageInfo.java | 3 ++- .../errorprone/bugpatterns/UnnecessarilyFullyQualified.java | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/PackageInfo.java b/core/src/main/java/com/google/errorprone/bugpatterns/PackageInfo.java index 03d5cd50e89..829ec3bb560 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/PackageInfo.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/PackageInfo.java @@ -23,6 +23,7 @@ import com.google.errorprone.VisitorState; import com.google.errorprone.bugpatterns.BugChecker.CompilationUnitTreeMatcher; import com.google.errorprone.matchers.Description; +import com.google.errorprone.util.ASTHelpers; import com.sun.source.tree.CompilationUnitTree; /** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */ @@ -36,7 +37,7 @@ public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState s if (tree.getSourceFile() == null) { return NO_MATCH; } - String name = tree.getSourceFile().getName(); + String name = ASTHelpers.getFileName(tree); int idx = name.lastIndexOf('/'); if (idx != -1) { name = name.substring(idx + 1); diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/UnnecessarilyFullyQualified.java b/core/src/main/java/com/google/errorprone/bugpatterns/UnnecessarilyFullyQualified.java index ff07de98ab1..79d5ef4609f 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/UnnecessarilyFullyQualified.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/UnnecessarilyFullyQualified.java @@ -32,6 +32,7 @@ import com.google.errorprone.bugpatterns.BugChecker.CompilationUnitTreeMatcher; import com.google.errorprone.fixes.SuggestedFix; import com.google.errorprone.matchers.Description; +import com.google.errorprone.util.ASTHelpers; import com.sun.source.tree.CompilationUnitTree; import com.sun.source.tree.IdentifierTree; import com.sun.source.tree.ImportTree; @@ -188,7 +189,7 @@ public Void visitIdentifier(IdentifierTree identifierTree, Void aVoid) { } private static boolean isPackageInfo(CompilationUnitTree tree) { - String name = tree.getSourceFile().getName(); + String name = ASTHelpers.getFileName(tree); int idx = name.lastIndexOf('/'); if (idx != -1) { name = name.substring(idx + 1);