Fix new Error Prone 2.4.0 detected issues. #403
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,9 +16,9 @@ | |
| package com.uber.nullaway.jarinfer; | ||
|
|
||
| import com.google.common.base.Preconditions; | ||
| import com.google.common.collect.ImmutableList; | ||
| import java.io.IOException; | ||
| import java.io.InputStream; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.jar.JarEntry; | ||
|
|
@@ -50,14 +50,11 @@ public class AnnotationChecker { | |
| * 'Expect*' annotations are present. | ||
| * @throws IOException if an error happens when reading the AAR file. | ||
| */ | ||
| public static boolean checkMethodAnnotationsInAar( | ||
| String aarFile, Map<String, String> expectedToActualAnnotations) throws IOException { | ||
| Preconditions.checkArgument(aarFile.endsWith(".aar"), "invalid aar file: " + aarFile); | ||
| ZipFile zip = new ZipFile(aarFile); | ||
| for (ZipEntry zipEntry : zip.stream().collect(ImmutableList.toImmutableList())) { | ||
| if (zipEntry.getName().equals("classes.jar")) { | ||
| JarInputStream jarIS = new JarInputStream(zip.getInputStream(zipEntry)); | ||
| JarEntry jarEntry = jarIS.getNextJarEntry(); | ||
|
|
@@ -86,14 +83,11 @@ public static boolean checkMethodAnnotationsInAar( | |
| * 'Expect*' annotations are present. | ||
| * @throws IOException if an error happens when reading the jar file. | ||
| */ | ||
| public static boolean checkMethodAnnotationsInJar( | ||
| String jarFile, Map<String, String> expectedToActualAnnotations) throws IOException { | ||
| Preconditions.checkArgument(jarFile.endsWith(".jar"), "invalid jar file: " + jarFile); | ||
| JarFile jar = new JarFile(jarFile); | ||
| for (JarEntry entry : jar.stream().collect(ImmutableList.toImmutableList())) { | ||
| if (entry.getName().endsWith(".class") | ||
| && !checkMethodAnnotationsInClass( | ||
| jar.getInputStream(entry), expectedToActualAnnotations)) { | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,10 +16,10 @@ | |
| package com.uber.nullaway.jarinfer; | ||
|
|
||
| import com.google.common.base.Preconditions; | ||
| import com.google.common.collect.ImmutableSet; | ||
| import java.io.BufferedReader; | ||
| import java.io.IOException; | ||
| import java.io.InputStreamReader; | ||
| import java.util.HashSet; | ||
| import java.util.Set; | ||
| import java.util.jar.JarEntry; | ||
|
|
@@ -40,20 +40,15 @@ public class EntriesComparator { | |
| * @return True iff the entries present in the two jar files are the same. | ||
| * @throws IOException if an error happens when reading jar files. | ||
| */ | ||
| public static boolean compareEntriesInJars(String jarFile1, String jarFile2) throws IOException { | ||
| Preconditions.checkArgument(jarFile1.endsWith(".jar"), "invalid jar file: " + jarFile1); | ||
| Preconditions.checkArgument(jarFile2.endsWith(".jar"), "invalid jar file: " + jarFile2); | ||
| JarFile jar1 = new JarFile(jarFile1); | ||
| JarFile jar2 = new JarFile(jarFile2); | ||
| Set<String> jar1Entries = | ||
| jar1.stream().map(ZipEntry::getName).collect(ImmutableSet.toImmutableSet()); | ||
| Set<String> jar2Entries = | ||
| jar2.stream().map(ZipEntry::getName).collect(ImmutableSet.toImmutableSet()); | ||
| return jar1Entries.equals(jar2Entries); | ||
| } | ||
|
|
||
|
|
@@ -67,20 +62,15 @@ public static boolean compareEntriesInJars(String jarFile1, String jarFile2) thr | |
| * "classes.jar" in the two aar files are the same. | ||
| * @throws IOException if an error happens when reading aar files. | ||
| */ | ||
| public static boolean compareEntriesInAars(String aarFile1, String aarFile2) throws IOException { | ||
| Preconditions.checkArgument(aarFile1.endsWith(".aar"), "invalid aar file: " + aarFile1); | ||
| Preconditions.checkArgument(aarFile2.endsWith(".aar"), "invalid aar file: " + aarFile2); | ||
| ZipFile zip1 = new ZipFile(aarFile1); | ||
| ZipFile zip2 = new ZipFile(aarFile2); | ||
| Set<String> zip1Entries = | ||
| zip1.stream().map(ZipEntry::getName).collect(ImmutableSet.toImmutableSet()); | ||
| Set<String> zip2Entries = | ||
| zip2.stream().map(ZipEntry::getName).collect(ImmutableSet.toImmutableSet()); | ||
| if (!zip1Entries.equals(zip2Entries)) { | ||
| return false; | ||
| } | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -174,9 +174,7 @@ public class NullAway extends BugChecker | |
| static final String INITIALIZATION_CHECK_NAME = "NullAway.Init"; | ||
| static final String OPTIONAL_CHECK_NAME = "NullAway.Optional"; | ||
|
|
||
| private static final Matcher<ExpressionTree> THIS_MATCHER = NullAway::isThisIdentifierMatcher; | ||
|
|
||
| private final Predicate<MethodInvocationNode> nonAnnotatedMethod; | ||
|
|
||
|
|
@@ -233,15 +231,15 @@ public class NullAway extends BugChecker | |
| public NullAway() { | ||
| config = new DummyOptionsConfig(); | ||
| handler = Handlers.buildEmpty(); | ||
| nonAnnotatedMethod = this::isMethodUnannotated; | ||
| customSuppressionAnnotations = ImmutableSet.of(); | ||
| errorBuilder = new ErrorBuilder(config, "", ImmutableSet.of()); | ||
| } | ||
|
|
||
| public NullAway(ErrorProneFlags flags) { | ||
| config = new ErrorProneCLIFlagsConfig(flags); | ||
| handler = Handlers.buildDefault(config); | ||
| nonAnnotatedMethod = this::isMethodUnannotated; | ||
| customSuppressionAnnotations = initCustomSuppressions(); | ||
| errorBuilder = new ErrorBuilder(config, canonicalName(), allNames()); | ||
| // workaround for Checker Framework static state bug; | ||
|
|
@@ -263,12 +261,9 @@ private ImmutableSet<Class<? extends Annotation>> initCustomSuppressions() { | |
| return builder.build(); | ||
| } | ||
|
|
||
| private boolean isMethodUnannotated(MethodInvocationNode invocationNode) { | ||
| return invocationNode == null | ||
| || NullabilityUtil.isUnannotated(ASTHelpers.getSymbol(invocationNode.getTree()), config); | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -2102,6 +2097,11 @@ private static boolean isThisIdentifier(ExpressionTree expressionTree) { | |
| && ((IdentifierTree) expressionTree).getName().toString().equals("this"); | ||
| } | ||
|
|
||
| private static boolean isThisIdentifierMatcher( | ||
| ExpressionTree expressionTree, VisitorState state) { | ||
| return isThisIdentifier(expressionTree); | ||
| } | ||
|
|
||
| public ErrorBuilder getErrorBuilder() { | ||
| return errorBuilder; | ||
| } | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In these cases can we do a
forEach()on the stream rather than collecting in a list and looping over that? That seems a bit cleaner and avoids copying to another list.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, we can't. The code bellow can throw
IOException. If that happens within a lambda, we need to catch it there, if it happens within an iteration we can propagate it up.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a code comment to that effect? Or not if you think it's obvious, up to you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also done 😄 (before I saw your comment, but not sure if before you made it...)