Skip to content

Commit

Permalink
Fix package-info.java identification on Windows
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Stephan202 authored and Error Prone Team committed Jun 28, 2021
1 parent 1a56270 commit dca7e79
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 2 deletions.
Expand Up @@ -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. */
Expand All @@ -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);
Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit dca7e79

Please sign in to comment.