Skip to content

Commit

Permalink
SONARJAVA-3970 Fix FP on Try.run with onFailure (#4880)
Browse files Browse the repository at this point in the history
  • Loading branch information
irina-batinic-sonarsource authored Sep 25, 2024
1 parent fa31182 commit e3a2872
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 0 deletions.
5 changes: 5 additions & 0 deletions java-checks-test-sources/default/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -976,6 +976,11 @@
<version>2.1.1</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>io.vavr</groupId>
<artifactId>vavr</artifactId>
<version>0.10.4</version>
</dependency>
</dependencies>

<profiles>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,64 @@
package checks;

import play.Logger;

import java.io.IOException;
import java.net.InetAddress;
import javax.naming.NamingException;
import javax.servlet.ServletException;
import javax.servlet.annotation.WebServlet;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.util.function.Consumer;
import io.vavr.control.Try;

// http://localhost:9090/securityapp/s1989/noncompliantvavr
@WebServlet(urlPatterns = "/s1989/noncompliantvavr")
class S1989noncompliantvavr extends HttpServlet {

@Override
protected final void doGet(HttpServletRequest request, HttpServletResponse response) {

Try.run(() -> { // Noncompliant
// let's try to raise an exception
});

Try.run(() -> { // Noncompliant
// let's try to raise an exception
}).isEmpty();

Try.runRunnable(() -> { // Noncompliant
// let's try to raise an exception
});

Try.runRunnable(() -> { // Noncompliant
// let's try to raise an exception
}).isEmpty();
}
}

// http://localhost:9090/securityapp/s1989/compliantvavr
@WebServlet(urlPatterns = "/s1989/compliantvavr")
class S1989compliantvavr extends HttpServlet {

@Override
protected final void doGet(HttpServletRequest request, HttpServletResponse response) {
final Consumer<Throwable> logError = x -> Logger.error("hey"+x.getMessage(), x);
Try.run(() -> { // Compliant
// let's try to raise an exception
}).onFailure(logError);

Try.runRunnable(() -> { // Compliant
// let's try to raise an exception
}).onFailure(logError);

Try.of(() -> { // Compliant
// let's try to raise an exception
return null;
});
}
}

class ServletMethodsExceptionsThrownCheckSample extends HttpServlet {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
import org.sonar.plugins.java.api.semantic.Symbol;
import org.sonar.plugins.java.api.semantic.Type;
import org.sonar.plugins.java.api.tree.CatchTree;
import org.sonar.plugins.java.api.tree.IdentifierTree;
import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree;
import org.sonar.plugins.java.api.tree.MethodInvocationTree;
import org.sonar.plugins.java.api.tree.MethodTree;
import org.sonar.plugins.java.api.tree.ThrowStatementTree;
Expand Down Expand Up @@ -95,6 +97,15 @@ private static List<Type> getCaughtExceptions(List<CatchTree> catches) {
}

private void checkMethodInvocation(MethodInvocationTree node) {
if (node.methodSelect() instanceof MemberSelectExpressionTree memberSelect && isRunMethod(memberSelect)) {
Tree parent = ExpressionUtils.getParentOfType(memberSelect, Tree.Kind.MEMBER_SELECT);
var parentMemberSelect = (MemberSelectExpressionTree) parent;
if (parentMemberSelect == null || !"onFailure".equals(parentMemberSelect.identifier().name())) {
reportIssue(memberSelect, "Handle the exception thrown by this method call.");
return;
}
}

Symbol.MethodSymbol symbol = node.methodSymbol();
if (!symbol.isUnknown()) {
List<Type> types = symbol.thrownTypes();
Expand All @@ -104,6 +115,11 @@ private void checkMethodInvocation(MethodInvocationTree node) {
}
}

private static boolean isRunMethod(MemberSelectExpressionTree memberSelect) {
return memberSelect.expression() instanceof IdentifierTree identifier && "Try".equals(identifier.name())
&& ("run".equals(memberSelect.identifier().name()) || "runRunnable".equals(memberSelect.identifier().name()));
}

private void addIssueIfNotCaught(Type thrown, Tree node) {
if (isNotCaught(thrown)) {
String message = "Handle the " + "\"" + thrown.name() + "\"" + " thrown here in a \"try/catch\" block.";
Expand Down

0 comments on commit e3a2872

Please sign in to comment.