refactor: Common static analysis issues

gradle/wrapper/gradle-wrapper.properties

@@ -1,6 +1,6 @@
 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
-distributionUrl=https\://services.gradle.org/distributions/gradle-7.6-bin.zip
-distributionSha256Sum=7ba68c54029790ab444b39d7e293d3236b2632631fb5f2e012bb28b4ff669e4b
+distributionUrl=https\://services.gradle.org/distributions/gradle-8.3-bin.zip
+distributionSha256Sum=591855b517fc635b9e04de1d05d5e76ada3f89f5fc76f87978d1b245b4f69225
 zipStoreBase=GRADLE_USER_HOME
 zipStorePath=wrapper/dists

gradlew

@@ -55,7 +55,7 @@
 #       Darwin, MinGW, and NonStop.
 #
 #   (3) This script is generated from the Groovy template
-#       https://github.com/gradle/gradle/blob/master/subprojects/plugins/src/main/resources/org/gradle/api/internal/plugins/unixStartScript.txt
+#       https://github.com/gradle/gradle/blob/HEAD/subprojects/plugins/src/main/resources/org/gradle/api/internal/plugins/unixStartScript.txt
 #       within the Gradle project.
 #
 #       You can find Gradle at https://github.com/gradle/gradle/.
@@ -80,13 +80,11 @@ do
     esac
 done
 
-APP_HOME=$( cd "${APP_HOME:-./}" && pwd -P ) || exit
-
-APP_NAME="Gradle"
+# This is normally unused
+# shellcheck disable=SC2034
 APP_BASE_NAME=${0##*/}
-
-# Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to pass JVM options to this script.
-DEFAULT_JVM_OPTS='"-Xmx64m" "-Xms64m"'
+# Discard cd standard output in case $CDPATH is set (https://github.com/gradle/gradle/issues/25036)
+APP_HOME=$( cd "${APP_HOME:-./}" > /dev/null && pwd -P ) || exit
 
 # Use the maximum available, or set MAX_FD != -1 to use that value.
 MAX_FD=maximum
@@ -133,22 +131,29 @@ location of your Java installation."
     fi
 else
     JAVACMD=java
-    which java >/dev/null 2>&1 || die "ERROR: JAVA_HOME is not set and no 'java' command could be found in your PATH.
+    if ! command -v java >/dev/null 2>&1
+    then
+        die "ERROR: JAVA_HOME is not set and no 'java' command could be found in your PATH.
 
 Please set the JAVA_HOME variable in your environment to match the
 location of your Java installation."
+    fi
 fi
 
 # Increase the maximum file descriptors if we can.
 if ! "$cygwin" && ! "$darwin" && ! "$nonstop" ; then
     case $MAX_FD in #(
       max*)
+        # In POSIX sh, ulimit -H is undefined. That's why the result is checked to see if it worked.
+        # shellcheck disable=SC3045
         MAX_FD=$( ulimit -H -n ) ||
             warn "Could not query maximum file descriptor limit"
     esac
     case $MAX_FD in  #(
       '' | soft) :;; #(
       *)
+        # In POSIX sh, ulimit -n is undefined. That's why the result is checked to see if it worked.
+        # shellcheck disable=SC3045
         ulimit -n "$MAX_FD" ||
             warn "Could not set maximum file descriptor limit to $MAX_FD"
     esac
@@ -193,6 +198,10 @@ if "$cygwin" || "$msys" ; then
     done
 fi
 
+
+# Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to pass JVM options to this script.
+DEFAULT_JVM_OPTS='"-Xmx64m" "-Xms64m"'
+
 # Collect all arguments for the java command;
 #   * $DEFAULT_JVM_OPTS, $JAVA_OPTS, and $GRADLE_OPTS can contain fragments of
 #     shell script including quotes and variable substitutions, so put them in

gradlew.bat

@@ -26,6 +26,7 @@ if "%OS%"=="Windows_NT" setlocal
 
 set DIRNAME=%~dp0
 if "%DIRNAME%"=="" set DIRNAME=.
+@rem This is normally unused
 set APP_BASE_NAME=%~n0
 set APP_HOME=%DIRNAME%
 

src/main/java/org/openrewrite/analysis/InvocationMatcher.java

@@ -19,6 +19,7 @@
 import lombok.AllArgsConstructor;
 import org.openrewrite.Cursor;
 import org.openrewrite.Incubating;
+import org.openrewrite.Tree;
 import org.openrewrite.internal.lang.Nullable;
 import org.openrewrite.java.MethodMatcher;
 import org.openrewrite.java.tree.*;
@@ -106,10 +107,8 @@ public boolean isSelect(Cursor cursor) {
         public boolean isAnyArgument(Cursor cursor) {
             return asExpression(
                     cursor,
-                    expression -> nearestMethodCall(cursor).map(call -> {
-                        return call.getArguments().contains(expression)
-                               && matcher.matches(call); // Do the matcher.matches(...) last as this can be expensive
-                    }).orElse(false)
+                    expression -> nearestMethodCall(cursor).map(call -> call.getArguments().contains(expression)
+                               && matcher.matches(call)).orElse(false)
             );
         }
 
@@ -155,7 +154,7 @@ private static Optional<JavaType.Method> getType(MethodCall expression) {
         }
 
         private static Optional<MethodCall> nearestMethodCall(Cursor cursor) {
-            J closestJ = cursor.getParentTreeCursor().getValue();
+            Tree closestJ = cursor.getParentTreeCursor().getValue();
             if (closestJ instanceof MethodCall) {
                 return Optional.of((MethodCall) closestJ);
             }

src/main/java/org/openrewrite/analysis/controlflow/ControlFlow.java

@@ -234,11 +234,11 @@ public J.Block visitBlock(J.Block block, P p) {
         }
 
         @Override
-        public J.Synchronized visitSynchronized(J.Synchronized _sync, P p) {
+        public J.Synchronized visitSynchronized(J.Synchronized sync, P p) {
             addCursorToBasicBlock();
-            visit(_sync.getLock(), p);
-            visit(_sync.getBody(), p);
-            return _sync;
+            visit(sync.getLock(), p);
+            visit(sync.getBody(), p);
+            return sync;
         }
 
         @Override
@@ -353,20 +353,17 @@ public J.Case visitCase(J.Case _case, P p) {
                             ControlFlowNode.ConditionNode conditionNode = currentAsBasicBlock().addConditionNodeTruthFirst();
                             current = Stream.concat(Stream.of(conditionNode), caseFlow.stream()).collect(Collectors.toSet());
                             caseFlow.clear();
-                            switch (_case.getType()) {
-                                case Statement:
-                                    if (_case.getStatements().isEmpty()) {
-                                        // Visit a fake empty statement to ensure that the ConditionNode has a true successor
-                                        visit(new J.Empty(randomId(), Space.EMPTY, Markers.EMPTY), p);
-                                    } else {
-                                        visitStatementList(_case.getStatements(), p);
-                                    }
-                                    break;
-                                case Rule:
-                                    visit(_case.getBody(), p);
-                                    breakFlow.add(currentAsBasicBlock());
-                                    current = Collections.emptySet();
-                                    break;
+                            if (_case.getType() == J.Case.Type.Statement) {
+                                if (_case.getStatements().isEmpty()) {
+                                    // Visit a fake empty statement to ensure that the ConditionNode has a true successor
+                                    visit(new J.Empty(randomId(), Space.EMPTY, Markers.EMPTY), p);
+                                } else {
+                                    visitStatementList(_case.getStatements(), p);
+                                }
+                            } else if (_case.getType() == J.Case.Type.Rule) {
+                                visit(_case.getBody(), p);
+                                breakFlow.add(currentAsBasicBlock());
+                                current = Collections.emptySet();
                             }
                             caseFlow.addAll(current);
                             current = Collections.singleton(conditionNode);
@@ -393,7 +390,7 @@ public J.Case visitCase(J.Case _case, P p) {
         private static Set<ControlFlowNode.ConditionNode> allAsConditionNodesMissingTruthFirst(Set<? extends ControlFlowNode> nodes) {
             return nodes.stream().map(controlFlowNode -> {
                 if (controlFlowNode instanceof ControlFlowNode.ConditionNode) {
-                    return ((ControlFlowNode.ConditionNode) controlFlowNode);
+                    return (ControlFlowNode.ConditionNode) controlFlowNode;
                 } else {
                     return controlFlowNode.addConditionNodeTruthFirst();
                 }
@@ -403,7 +400,7 @@ private static Set<ControlFlowNode.ConditionNode> allAsConditionNodesMissingTrut
         private static Set<ControlFlowNode.ConditionNode> allAsConditionNodesMissingFalseFirst(Set<? extends ControlFlowNode> nodes) {
             return nodes.stream().map(controlFlowNode -> {
                 if (controlFlowNode instanceof ControlFlowNode.ConditionNode) {
-                    return ((ControlFlowNode.ConditionNode) controlFlowNode);
+                    return (ControlFlowNode.ConditionNode) controlFlowNode;
                 } else {
                     return controlFlowNode.addConditionNodeFalseFirst();
                 }

src/main/java/org/openrewrite/analysis/controlflow/ControlFlowIllegalStateException.java

@@ -43,7 +43,7 @@ static class Message {
         Set<ControlFlowNode> predecessors;
         Cursor cursor;
 
-        static class MessageBuilder {
+        static final class MessageBuilder {
             private final String message;
             // LinkedHashMap to preserve order of insertion1
             private final LinkedHashMap<String, ControlFlowNode> nodes = new LinkedHashMap<>();

src/main/java/org/openrewrite/analysis/controlflow/ControlFlowJavaPrinter.java

@@ -325,7 +325,7 @@ public J visitEmpty(J.Empty empty, PrintOutputCapture<P> pPrintOutputCapture) {
     }
 
     static class ControlFlowPrintOutputCapture<P> extends PrintOutputCapture<P> {
-        boolean enabled = false;
+        boolean enabled;
 
         public ControlFlowPrintOutputCapture(P p) {
             super(p);

src/main/java/org/openrewrite/analysis/controlflow/ControlFlowNode.java

@@ -242,11 +242,10 @@ String toVisualizerString() {
                 // Self loathing
                 J.Case caseStatement = (J.Case) condition;
                 String caseString = caseStatement.toString();
-                switch (caseStatement.getType()) {
-                    case Statement:
-                        return caseString.substring(0, caseString.indexOf(":") + 1);
-                    case Rule:
-                        return caseString.substring(0, caseString.indexOf("->") + 2);
+                if (caseStatement.getType() == J.Case.Type.Statement) {
+                    return caseString.substring(0, caseString.indexOf(":") + 1);
+                } else if (caseStatement.getType() == J.Case.Type.Rule) {
+                    return caseString.substring(0, caseString.indexOf("->") + 2);
                 }
             }
             return condition.toString();
@@ -424,7 +423,7 @@ static final class Start extends ControlFlowNode implements GraphTerminator {
         @Getter
         private final GraphType graphType;
 
-        private ControlFlowNode successor = null;
+        private ControlFlowNode successor;
 
         @Override
         protected void _addSuccessorInternal(ControlFlowNode successor) {
@@ -470,7 +469,7 @@ public String toString() {
     static final class End extends ControlFlowNode implements GraphTerminator {
         @Getter
         private final GraphType graphType;
-        private ControlFlowNode successor = null;
+        private ControlFlowNode successor;
 
         @Override
         Set<ControlFlowNode> getSuccessors() {

src/main/java/org/openrewrite/analysis/controlflow/ControlFlowSummaryDotVisualizer.java

@@ -44,7 +44,8 @@ public String visualizeAsDotfile(String name, boolean darkMode, ControlFlowSumma
                         .map(NodeToNodeText::new)
                         .sorted()
                         .collect(Collectors.toList());
-        int vizSrc = -1, vizSink = -1;
+        int vizSrc = -1;
+        int vizSink = -1;
         for (int i = 0; i < nodeToNodeText.size(); i++) {
             NodeToNodeText toNodeText = nodeToNodeText.get(i);
             ControlFlowNode node = toNodeText.node;

src/main/java/org/openrewrite/analysis/controlflow/ControlFlowVisualizationVisitor.java

@@ -98,7 +98,7 @@ public J.Block visitBlock(J.Block block, P p) {
     private static class ControlFlowMarkingVisitor<P> extends JavaIsoVisitor<P> {
         private final String label;
         private final Map<J, ? extends ControlFlowNode> nodeToBlock;
-        private int nodeNumber = 0;
+        private int nodeNumber;
         private final Map<ControlFlowNode, Integer> nodeNumbers = new HashMap<>();
 
         @Override
@@ -128,7 +128,9 @@ public Statement visitStatement(Statement statement, P p) {
                 } else {
                     return SearchResult.found(statement, "" + number + label);
                 }
-            } else return statement;
+            } else {
+                return statement;
+            }
         }
 
         @Override

src/main/java/org/openrewrite/analysis/controlflow/Guard.java

@@ -89,9 +89,9 @@ private static Optional<J.ControlParentheses<?>> getControlParenthesesFromParent
         J.ControlParentheses<?> parentControlParentheses;
         if (parent instanceof J.If) {
             parentControlParentheses = ((J.If) parent).getIfCondition();
-        } else if (parent instanceof J.WhileLoop){
+        } else if (parent instanceof J.WhileLoop) {
             parentControlParentheses = ((J.WhileLoop) parent).getCondition();
-        } else if (parent instanceof J.DoWhileLoop){
+        } else if (parent instanceof J.DoWhileLoop) {
             parentControlParentheses = ((J.DoWhileLoop) parent).getWhileCondition();
         } else {
             parentControlParentheses = null;
@@ -161,7 +161,7 @@ private static Optional<JavaType> getTypeSafe(Cursor c, Expression e) {
             }
         } else if (e instanceof J.MethodInvocation) {
             J.MethodInvocation methodInvocation = (J.MethodInvocation) e;
-            if (methodInvocation.getSimpleName().equals("equals")) {
+            if ("equals".equals(methodInvocation.getSimpleName())) {
                 return Optional.of(JavaType.Primitive.Boolean);
             }
         }

src/main/java/org/openrewrite/analysis/dataflow/DataFlowNode.java

@@ -65,7 +65,7 @@ public static Option<DataFlowNode> of(Cursor cursor) {
                     .toOption()
                     .map(expr -> new ExpressionDataFlowNode(cursor, expr));
         } else if (cursor.getValue() instanceof J.VariableDeclarations.NamedVariable) {
-            return Parameter.viewOf(cursor).map(parameter -> (DataFlowNode) new ParameterDataFlowNode(cursor, parameter)).toOption();
+            return Parameter.viewOf(cursor).map(DataFlowNode.class::cast).toOption();
         } else {
             return Option.none();
         }

src/main/java/org/openrewrite/analysis/dataflow/ExternalFlowModels.java

@@ -26,6 +26,7 @@
 import org.openrewrite.java.MethodMatcher;
 import org.openrewrite.java.internal.TypesInUse;
 import org.openrewrite.java.tree.J;
+import org.openrewrite.java.tree.JavaSourceFile;
 import org.openrewrite.java.tree.JavaType;
 
 import java.lang.ref.WeakReference;
@@ -68,10 +69,10 @@ private OptimizedFlowModels getOptimizedFlowModelsForTypesInUse(TypesInUse types
     }
 
     private OptimizedFlowModels getOrComputeOptimizedFlowModels(Cursor cursor) {
-        Cursor cuCursor = cursor.dropParentUntil(J.CompilationUnit.class::isInstance);
+        Cursor cuCursor = cursor.dropParentUntil(JavaSourceFile.class::isInstance);
         return cuCursor.computeMessageIfAbsent(
                 CURSOR_MESSAGE_KEY,
-                __ -> getOptimizedFlowModelsForTypesInUse(cuCursor.<J.CompilationUnit>getValue().getTypesInUse())
+                __ -> getOptimizedFlowModelsForTypesInUse(cuCursor.<JavaSourceFile>getValue().getTypesInUse())
         );
     }
 
@@ -100,7 +101,7 @@ boolean isAdditionalTaintStep(
     }
 
     @AllArgsConstructor
-    final static class OptimizedFlowModels {
+    static final class OptimizedFlowModels {
         private final Map<AdditionalFlowStepPredicate, Set<FlowModel>> value;
         private final Map<AdditionalFlowStepPredicate, Set<FlowModel>> taint;
 
@@ -242,32 +243,26 @@ private Map<AdditionalFlowStepPredicate, Set<FlowModel>> optimize(Collection<Flo
                     flowFromArgumentIndexToReturn
                             .entrySet()
                             .stream()
-                            .map(entry -> {
-                                return new PredicateToFlowModels(forFlowFromArgumentIndexToReturn(
+                            .map(entry -> new PredicateToFlowModels(forFlowFromArgumentIndexToReturn(
                                         entry.getKey(),
                                         entry.getValue()
-                                ), entry.getValue());
-                            });
+                                ), entry.getValue()));
             Stream<PredicateToFlowModels> flowFromArgumentIndexToQualifierStream =
                     flowFromArgumentIndexToQualifier
                             .entrySet()
                             .stream()
-                            .map(entry -> {
-                                return new PredicateToFlowModels(forFlowFromArgumentIndexToQualifier(
+                            .map(entry -> new PredicateToFlowModels(forFlowFromArgumentIndexToQualifier(
                                         entry.getKey(),
                                         entry.getValue()
-                                ), entry.getValue());
-                            });
+                                ), entry.getValue()));
             Stream<PredicateToFlowModels> flowFromArgumentIndexToArgumentIndexStream =
                     flowFromArgumentIndexToArgumentIndex
                             .entrySet()
                             .stream()
-                            .map(entry -> {
-                                return new PredicateToFlowModels(forFlowFromArgumentIndexToArgumentIndex(
+                            .map(entry -> new PredicateToFlowModels(forFlowFromArgumentIndexToArgumentIndex(
                                         entry.getKey(),
                                         entry.getValue()
-                                ), entry.getValue());
-                            });
+                                ), entry.getValue()));
 
             Stream<PredicateToFlowModels> s1 = Stream.concat(flowFromArgumentIndexToReturnStream, flowFromArgumentIndexToQualifierStream);
             return Stream.concat(s1, flowFromArgumentIndexToArgumentIndexStream)

src/main/java/org/openrewrite/analysis/dataflow/ExternalSinkModels.java

@@ -25,6 +25,7 @@
 import org.openrewrite.analysis.trait.expr.Call;
 import org.openrewrite.java.internal.TypesInUse;
 import org.openrewrite.java.tree.J;
+import org.openrewrite.java.tree.JavaSourceFile;
 import org.openrewrite.java.tree.JavaType;
 
 import java.lang.ref.WeakReference;
@@ -72,10 +73,10 @@ private OptimizedSinkModels getOptimizedSinkModelsForTypesInUse(TypesInUse types
     }
 
     private OptimizedSinkModels getOrComputeOptimizedSinkModels(Cursor cursor) {
-        Cursor cuCursor = cursor.dropParentUntil(J.CompilationUnit.class::isInstance);
+        Cursor cuCursor = cursor.dropParentUntil(JavaSourceFile.class::isInstance);
         return cuCursor.computeMessageIfAbsent(
                 CURSOR_MESSAGE_KEY,
-                __ -> getOptimizedSinkModelsForTypesInUse(cuCursor.<J.CompilationUnit>getValue().getTypesInUse())
+                __ -> getOptimizedSinkModelsForTypesInUse(cuCursor.<JavaSourceFile>getValue().getTypesInUse())
         );
     }