SONARJAVA-2473 Ignore exception messages when computing flows for S20…

…95, S2222 and S3546 (#1686)
SonarSource · Sep 19, 2017 · 2ef62ab · 2ef62ab
1 parent 8a173eb
commit 2ef62ab
Show file tree

Hide file tree

Showing 6 changed files with 83 additions and 34 deletions.
diff --git a/java-frontend/src/main/java/org/sonar/java/se/FlowComputation.java b/java-frontend/src/main/java/org/sonar/java/se/FlowComputation.java
@@ -85,13 +85,15 @@ public class FlowComputation {
   private final Predicate<Constraint> terminateTraversal;
   private final Set<SymbolicValue> symbolicValues;
   private final List<Class<? extends Constraint>> domains;
+  private final boolean skipExceptionMessages;
 
   private FlowComputation(Set<SymbolicValue> symbolicValues, Predicate<Constraint> addToFlow,
-                          Predicate<Constraint> terminateTraversal, List<Class<? extends Constraint>> domains) {
+                          Predicate<Constraint> terminateTraversal, List<Class<? extends Constraint>> domains, boolean skipExceptionMessages) {
     this.addToFlow = addToFlow;
     this.terminateTraversal = terminateTraversal;
     this.symbolicValues = symbolicValues;
     this.domains = domains;
+    this.skipExceptionMessages = skipExceptionMessages;
   }
 
   private static Set<SymbolicValue> computedFrom(@Nullable SymbolicValue symbolicValue) {
@@ -106,8 +108,40 @@ private static Set<SymbolicValue> computedFrom(@Nullable SymbolicValue symbolicV
 
   // FIXME It is assumed that this will always return set with at least one element, which could be empty, because result is consumed in other places and messages are
   // added to returned flows. This design should be improved.
-  public static Set<Flow> flow(ExplodedGraph.Node currentNode, Set<SymbolicValue> symbolicValues, Predicate<Constraint> addToFlow,
-                                                                Predicate<Constraint> terminateTraversal, List<Class<? extends Constraint>> domains, Set<Symbol> symbols) {
+  public static Set<Flow> flow(ExplodedGraph.Node currentNode, Set<SymbolicValue> symbolicValues, Predicate<Constraint> addToFlow, Predicate<Constraint> terminateTraversal,
+    List<Class<? extends Constraint>> domains, Set<Symbol> symbols) {
+    return flow(currentNode, symbolicValues, addToFlow, terminateTraversal, domains, symbols, false);
+  }
+
+  public static Set<Flow> flow(ExplodedGraph.Node currentNode, @Nullable SymbolicValue currentVal, List<Class<? extends Constraint>> domains) {
+    return flow(currentNode, setFromNullable(currentVal), constraint -> true, c -> false, domains, Collections.emptySet(), false);
+  }
+
+  public static Set<Flow> flow(ExplodedGraph.Node currentNode, @Nullable SymbolicValue currentVal, List<Class<? extends Constraint>> domains, @Nullable Symbol trackSymbol) {
+    return flow(currentNode, setFromNullable(currentVal), c -> true, c -> false, domains, setFromNullable(trackSymbol), false);
+  }
+
+  public static Set<Flow> flow(ExplodedGraph.Node currentNode, @Nullable SymbolicValue currentVal, Predicate<Constraint> addToFlow, List<Class<? extends Constraint>> domains) {
+    return flow(currentNode, setFromNullable(currentVal), addToFlow, c -> false, domains, Collections.emptySet(), false);
+  }
+
+  public static Set<Flow> flow(ExplodedGraph.Node currentNode, @Nullable SymbolicValue currentVal, Predicate<Constraint> addToFlow, Predicate<Constraint> terminateTraversal,
+    List<Class<? extends Constraint>> domains) {
+    return flow(currentNode, setFromNullable(currentVal), addToFlow, terminateTraversal, domains, Collections.emptySet(), false);
+  }
+
+  public static Set<Flow> flowWithoutExceptions(ExplodedGraph.Node currentNode, @Nullable SymbolicValue currentVal, Predicate<Constraint> addToFlow,
+    List<Class<? extends Constraint>> domains) {
+    return flow(currentNode, setFromNullable(currentVal), addToFlow, c -> false, domains, Collections.emptySet(), true);
+  }
+
+  public static Set<Flow> flowWithoutExceptions(ExplodedGraph.Node currentNode, @Nullable SymbolicValue currentVal, Predicate<Constraint> addToFlow,
+    Predicate<Constraint> terminateTraversal, List<Class<? extends Constraint>> domains) {
+    return flow(currentNode, setFromNullable(currentVal), addToFlow, terminateTraversal, domains, Collections.emptySet(), true);
+  }
+
+  private static Set<Flow> flow(ExplodedGraph.Node currentNode, Set<SymbolicValue> symbolicValues, Predicate<Constraint> addToFlow,
+    Predicate<Constraint> terminateTraversal, List<Class<? extends Constraint>> domains, Set<Symbol> symbols, boolean skipExceptionMessages) {
     Set<SymbolicValue> allSymbolicValues = symbolicValues.stream()
       .map(FlowComputation::computedFrom)
       .flatMap(Set::stream)
@@ -125,29 +159,10 @@ public static Set<Flow> flow(ExplodedGraph.Node currentNode, Set<SymbolicValue>
         }
       }
     }
-    FlowComputation flowComputation = new FlowComputation(allSymbolicValues, addToFlow, terminateTraversal, domains);
+    FlowComputation flowComputation = new FlowComputation(allSymbolicValues, addToFlow, terminateTraversal, domains, skipExceptionMessages);
     return flowComputation.run(currentNode, trackedSymbols);
   }
 
-  public static Set<Flow> flow(ExplodedGraph.Node currentNode, @Nullable SymbolicValue currentVal, List<Class<? extends Constraint>> domains) {
-    return flow(currentNode, currentVal, constraint -> true, domains);
-  }
-
-  public static Set<Flow> flow(ExplodedGraph.Node currentNode, @Nullable SymbolicValue currentVal, List<Class<? extends Constraint>> domains,
-                                                                @Nullable Symbol trackSymbol) {
-    return flow(currentNode, setFromNullable(currentVal), c -> true, c -> false, domains, setFromNullable(trackSymbol));
-  }
-
-  public static Set<Flow> flow(ExplodedGraph.Node currentNode, @Nullable SymbolicValue currentVal, Predicate<Constraint> addToFlow,
-                                                                List<Class<? extends Constraint>> domains) {
-    return flow(currentNode, currentVal, addToFlow, c -> false, domains);
-  }
-
-  public static Set<Flow> flow(ExplodedGraph.Node currentNode, @Nullable SymbolicValue currentVal, Predicate<Constraint> addToFlow,
-                                                                Predicate<Constraint> terminateTraversal, List<Class<? extends Constraint>> domains) {
-    return flow(currentNode, setFromNullable(currentVal), addToFlow, terminateTraversal, domains, Collections.emptySet());
-  }
-
   private static <T> Set<T> setFromNullable(@Nullable T val) {
     return val == null ? ImmutableSet.of() : ImmutableSet.of(val);
   }
@@ -296,14 +311,16 @@ Stream<ExecutionPath> addEdge(ExplodedGraph.Edge edge) {
       PSet<Symbol> newTrackSymbols = newTrackedSymbols(edge);
       SameConstraints newSameConstraints = newTrackSymbols == trackedSymbols ? sameConstraints : new SameConstraints(sameConstraints, newTrackSymbols);
 
-      flowFromThrownException(edge).ifPresent(loc -> {
-        flowBuilder.setAsExceptional();
-        flowBuilder.add(loc);
-      });
-      flowFromCaughtException(edge).ifPresent(loc -> {
-        flowBuilder.setAsExceptional();
-        flowBuilder.add(loc);
-      });
+      if (!skipExceptionMessages) {
+        flowFromThrownException(edge).ifPresent(loc -> {
+          flowBuilder.setAsExceptional();
+          flowBuilder.add(loc);
+        });
+        flowFromCaughtException(edge).ifPresent(loc -> {
+          flowBuilder.setAsExceptional();
+          flowBuilder.add(loc);
+        });
+      }
 
       Set<LearnedConstraint> learnedConstraints = learnedConstraints(edge);
       Flow lcFlow = flowFromLearnedConstraints(edge, filterRedundantObjectDomain(learnedConstraints));

diff --git a/java-frontend/src/main/java/org/sonar/java/se/checks/CustomUnclosedResourcesCheck.java b/java-frontend/src/main/java/org/sonar/java/se/checks/CustomUnclosedResourcesCheck.java
@@ -119,7 +119,8 @@ public void checkEndOfExecutionPath(CheckerContext context, ConstraintManager co
 
   private void processUnclosedSymbolicValue(ExplodedGraph.Node node, SymbolicValue sv) {
     List<Class<? extends Constraint>> domains = Lists.newArrayList(CustomResourceConstraint.class);
-    FlowComputation.flow(node, sv, OPENED::equals, domains).stream().flatMap(Flow::stream)
+    FlowComputation.flowWithoutExceptions(node, sv, OPENED::equals, domains).stream()
+      .flatMap(Flow::firstFlowLocation)
       .filter(location -> location.syntaxNode.is(Tree.Kind.NEW_CLASS, Tree.Kind.METHOD_INVOCATION))
       .forEach(this::reportIssue);
   }

diff --git a/java-frontend/src/main/java/org/sonar/java/se/checks/LocksNotUnlockedCheck.java b/java-frontend/src/main/java/org/sonar/java/se/checks/LocksNotUnlockedCheck.java
@@ -184,7 +184,7 @@ public ProgramState checkPostStatement(CheckerContext context, Tree syntaxNode)
   public void checkEndOfExecutionPath(CheckerContext context, ConstraintManager constraintManager) {
     ExplodedGraph.Node node = context.getNode();
     context.getState().getValuesWithConstraints(LockConstraint.LOCKED).stream()
-      .flatMap(lockedSv -> FlowComputation.flow(node, lockedSv, LockConstraint.LOCKED::equals, LockConstraint.UNLOCKED::equals, LOCK_CONSTRAINT_DOMAIN).stream())
+      .flatMap(lockedSv -> FlowComputation.flowWithoutExceptions(node, lockedSv, LockConstraint.LOCKED::equals, LockConstraint.UNLOCKED::equals, LOCK_CONSTRAINT_DOMAIN).stream())
       .flatMap(Flow::firstFlowLocation)
       .forEach(this::reportIssue);
   }

diff --git a/java-frontend/src/main/java/org/sonar/java/se/checks/UnclosedResourcesCheck.java b/java-frontend/src/main/java/org/sonar/java/se/checks/UnclosedResourcesCheck.java
@@ -165,7 +165,7 @@ private static Set<SymbolicValue> symbolicValuesToReport(CheckerContext context)
   }
 
   private void processUnclosedSymbolicValue(ExplodedGraph.Node node, SymbolicValue sv) {
-    FlowComputation.flow(node, sv, OPEN::equals, RESOURCE_CONSTRAINT_DOMAIN).stream()
+    FlowComputation.flowWithoutExceptions(node, sv, OPEN::equals, RESOURCE_CONSTRAINT_DOMAIN).stream()
       .flatMap(Flow::firstFlowLocation)
       .filter(location -> location.syntaxNode.is(Tree.Kind.NEW_CLASS, Tree.Kind.METHOD_INVOCATION))
       .forEach(this::reportIssue);

diff --git a/java-frontend/src/test/files/se/UnclosedResourcesCheckWithoutExceptionMessages.java b/java-frontend/src/test/files/se/UnclosedResourcesCheckWithoutExceptionMessages.java
@@ -0,0 +1,26 @@
+package org.foo;
+
+import java.io.FileWriter;
+import java.io.Writer;
+import java.util.List;
+
+abstract class A {
+  void foo(List<A> elements) {
+    for (A element : elements) {
+      try {
+        String packageName = substring(element.name(), ".");
+
+        Writer writer = new FileWriter(""); // Noncompliant
+
+        put("packageName", packageName);
+
+        writer.close();
+      } catch (Exception e) {
+      }
+    }
+  }
+
+  abstract String substring(String str, String separator);
+  abstract String name();
+  abstract void put(String s, Object o);
+}
diff --git a/java-frontend/src/test/java/org/sonar/java/se/checks/UnclosedResourcesCheckTest.java b/java-frontend/src/test/java/org/sonar/java/se/checks/UnclosedResourcesCheckTest.java
@@ -68,4 +68,9 @@ public void test_value_as_string_for_open_resource_constraints() throws Exceptio
   public void test_streams() throws Exception {
     JavaCheckVerifier.verify("src/test/files/se/UnclosedResourcesCheckStreams.java", new UnclosedResourcesCheck());
   }
+
+  @Test
+  public void skip_exception_messages() throws Exception {
+    JavaCheckVerifier.verify("src/test/files/se/UnclosedResourcesCheckWithoutExceptionMessages.java", new UnclosedResourcesCheck());
+  }
 }