Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Custom path to Optional class for Optional emptiness handler #288

Merged
merged 5 commits into from Mar 22, 2019
Merged

Custom path to Optional class for Optional emptiness handler #288

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c366ede
Custom path to Optional class for Optional emptiness handler
shubhamugare Mar 14, 2019
ba417ed
Addressed comments
shubhamugare Mar 18, 2019
89a416b
Addressed comments
shubhamugare Mar 21, 2019
ebb6870
Addressed comments
shubhamugare Mar 21, 2019
eddf6ae
minor nit
shubhamugare Mar 22, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 17 additions & 19 deletions nullaway/src/main/java/com/uber/nullaway/handlers/OptionalEmptinessHandler.java
View file
Open in desktop
Expand Up @@ -21,6 +21,7 @@
*/
package com.uber.nullaway.handlers;

import com.google.common.collect.ImmutableSet;
import com.google.errorprone.VisitorState;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ClassTree;
Expand All @@ -36,8 +37,7 @@
import com.uber.nullaway.Nullness;
import com.uber.nullaway.dataflow.AccessPath;
import com.uber.nullaway.dataflow.AccessPathNullnessPropagation;
import java.util.Optional;
import java.util.Set;
import java.util.Objects;
import java.util.stream.Collectors;
import javax.annotation.Nullable;
import javax.lang.model.element.Element;
Expand All @@ -51,7 +51,7 @@
*/
public class OptionalEmptinessHandler extends BaseNoOpHandler {

@Nullable private Set<Optional<Type>> optionalTypes;
@Nullable private ImmutableSet<Type> optionalTypes;
private final Config config;

OptionalEmptinessHandler(Config config) {
Expand All @@ -72,14 +72,14 @@ && optionalIsGetCall((Symbol.MethodSymbol) ASTHelpers.getSymbol(expr), state.get
public void onMatchTopLevelClass(
NullAway analysis, ClassTree tree, VisitorState state, Symbol.ClassSymbol classSymbol) {
optionalTypes =
config
.getOptionalClassPaths()
.stream()
.map(
type ->
Optional.ofNullable(state.getTypeFromString(type))
.map(state.getTypes()::erasure))
.collect(Collectors.toSet());
ImmutableSet.copyOf(
config
.getOptionalClassPaths()
.stream()
.map(state::getTypeFromString)
.filter(Objects::nonNull)
.map(state.getTypes()::erasure)
.collect(Collectors.toSet()));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guava 22 has collectors: https://google.github.io/guava/releases/22.0/api/docs/com/google/common/collect/ImmutableSet.html#toImmutableSet--

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So no need to build a Java set and call copyOf(...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Done 👍

}

@Override
Expand Down Expand Up @@ -150,21 +150,19 @@ private void updateNonNullAPsForElement(
}

private boolean optionalIsPresentCall(Symbol.MethodSymbol symbol, Types types) {
for (Optional<Type> optionalType : optionalTypes) {
if (optionalType.isPresent()
&& symbol.getSimpleName().toString().equals("isPresent")
for (Type optionalType : optionalTypes) {
if (symbol.getSimpleName().toString().equals("isPresent")
&& symbol.getParameters().length() == 0
&& types.isSubtype(symbol.owner.type, optionalType.get())) return true;
&& types.isSubtype(symbol.owner.type, optionalType)) return true;
}
return false;
}

private boolean optionalIsGetCall(Symbol.MethodSymbol symbol, Types types) {
for (Optional<Type> optionalType : optionalTypes) {
if (optionalType.isPresent()
&& symbol.getSimpleName().toString().equals("get")
for (Type optionalType : optionalTypes) {
if (symbol.getSimpleName().toString().equals("get")
&& symbol.getParameters().length() == 0
&& types.isSubtype(symbol.owner.type, optionalType.get())) return true;
&& types.isSubtype(symbol.owner.type, optionalType)) return true;
}
return false;
}
Expand Down