refactor: Common static analysis issues

Use this link to re-run the recipe: https://app.moderne.io/recipes/org.openrewrite.staticanalysis.CommonStaticAnalysis?organizationId=T3BlblJld3JpdGU%3D Co-authored-by: Moderne <[email protected]>
openrewrite · Oct 21, 2023 · 5f2b9f3 · 5f2b9f3
1 parent b7a3112
commit 5f2b9f3
Show file tree

Hide file tree

Showing 23 changed files with 110 additions and 107 deletions.
diff --git a/src/main/java/org/openrewrite/analysis/InvocationMatcher.java b/src/main/java/org/openrewrite/analysis/InvocationMatcher.java
@@ -107,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)
             );
         }
 

diff --git a/src/main/java/org/openrewrite/analysis/controlflow/ControlFlow.java b/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();
                 }

diff --git a/src/main/java/org/openrewrite/analysis/controlflow/ControlFlowIllegalStateException.java b/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<>();

diff --git a/src/main/java/org/openrewrite/analysis/controlflow/ControlFlowJavaPrinter.java b/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);

diff --git a/src/main/java/org/openrewrite/analysis/controlflow/ControlFlowNode.java b/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() {

diff --git a/src/main/java/org/openrewrite/analysis/controlflow/ControlFlowSummaryDotVisualizer.java b/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;

diff --git a/src/main/java/org/openrewrite/analysis/controlflow/ControlFlowVisualizationVisitor.java b/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

diff --git a/src/main/java/org/openrewrite/analysis/controlflow/Guard.java b/src/main/java/org/openrewrite/analysis/controlflow/Guard.java
@@ -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);
             }
         }

diff --git a/src/main/java/org/openrewrite/analysis/dataflow/ExternalFlowModels.java b/src/main/java/org/openrewrite/analysis/dataflow/ExternalFlowModels.java
@@ -101,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;
 
@@ -243,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)

diff --git a/src/main/java/org/openrewrite/analysis/search/FindFlowBetweenMethods.java b/src/main/java/org/openrewrite/analysis/search/FindFlowBetweenMethods.java
@@ -96,7 +96,7 @@ public GlobalDataFlow.Accumulator getInitialValue(ExecutionContext ctx) {
 
 
         String flow = this.flow == null ? "Data" : this.flow;
-        if (flow.equals("Taint")) {
+        if ("Taint".equals(flow)) {
             return GlobalDataFlow.accumulator(new TaintFlowSpec() {
 
                 @Override

diff --git a/src/main/java/org/openrewrite/analysis/trait/Top.java b/src/main/java/org/openrewrite/analysis/trait/Top.java
@@ -30,8 +30,12 @@ public interface Top {
     UUID getId();
 
     static boolean equals(Top e1, @Nullable Object other) {
-        if (e1 == other) return true;
-        if (other == null || e1.getClass() != other.getClass()) return false;
+        if (e1 == other) {
+            return true;
+        }
+        if (other == null || e1.getClass() != other.getClass()) {
+            return false;
+        }
         Top e2 = (Top) other;
         return e1.getId().equals(e2.getId());
     }

diff --git a/src/main/java/org/openrewrite/analysis/trait/member/StaticInitializerMethod.java b/src/main/java/org/openrewrite/analysis/trait/member/StaticInitializerMethod.java
@@ -23,7 +23,7 @@
  * A static initializer is a method that contains all static
  * field initializations and static initializer blocks.
  */
-public class StaticInitializerMethod extends InitializerMethodBase {
+public final class StaticInitializerMethod extends InitializerMethodBase {
     private StaticInitializerMethod(Cursor cursor) {
         super(cursor);
     }

diff --git a/src/test/java/org/openrewrite/analysis/dataflow/FindLocalFlowPathsStringTest.java b/src/test/java/org/openrewrite/analysis/dataflow/FindLocalFlowPathsStringTest.java
@@ -57,8 +57,8 @@ public boolean isSource(DataFlowNode srcNode) {
                   .map("42"::equals)
                   .orSome(() -> srcNode
                     .asExpr(MethodAccess.class)
-                    .map(ma -> ma.getSimpleName()
-                      .equals("source"))
+                    .map(ma -> "source"
+                      .equals(ma.getSimpleName()))
                     .orSome(false));
             }
 
@@ -70,7 +70,7 @@ public boolean isSink(DataFlowNode sinkNode) {
             @Override
             public boolean isBarrierGuard(Guard guard, boolean branch) {
                 if (guard.getExpression() instanceof J.MethodInvocation) {
-                    return branch && ((J.MethodInvocation) guard.getExpression()).getName().getSimpleName().equals("guard");
+                    return branch && "guard".equals(((J.MethodInvocation) guard.getExpression()).getName().getSimpleName());
                 }
                 return super.isBarrierGuard(guard, branch);
             }

diff --git a/src/test/java/org/openrewrite/analysis/dataflow/ParameterDataflowTest.java b/src/test/java/org/openrewrite/analysis/dataflow/ParameterDataflowTest.java
@@ -30,7 +30,7 @@ public void defaults(RecipeSpec spec) {
             public boolean isSource(DataFlowNode srcNode) {
                 return srcNode
                   .asParameter()
-                  .map(p -> p.getName().equals("source"))
+                  .map(p -> "source".equals(p.getName()))
                   .orSome(false);
             }
 

diff --git a/src/test/resources/dataflow-functional-tests/ArchiveAnalyzer.java b/src/test/resources/dataflow-functional-tests/ArchiveAnalyzer.java
@@ -84,7 +84,7 @@ public class ArchiveAnalyzer extends AbstractFileTypeAnalyzer {
     /**
      * The parent directory for the individual directories per archive.
      */
-    private File tempFileLocation = null;
+    private File tempFileLocation;
     /**
      * The max scan depth that the analyzer will recursively extract nested
      * archives.
@@ -93,7 +93,7 @@ public class ArchiveAnalyzer extends AbstractFileTypeAnalyzer {
     /**
      * The file filter used to filter supported files.
      */
-    private FileFilter fileFilter = null;
+    private FileFilter fileFilter;
     /**
      * The set of things we can handle with Zip methods
      */
@@ -315,7 +315,7 @@ private void extractAndAnalyze(Dependency dependency, Engine engine, int scanDep
                         extractAndAnalyze(d, engine, scanDepth + 1);
                     }
                 } else {
-                    dependencySet.stream().filter((sub) -> sub.getFilePath().startsWith(tmpDir.getAbsolutePath())).forEach((sub) -> {
+                    dependencySet.stream().filter(sub -> sub.getFilePath().startsWith(tmpDir.getAbsolutePath())).forEach(sub -> {
                         final String displayPath = String.format("%s%s",
                                 dependency.getFilePath(),
                                 sub.getActualFilePath().substring(tmpDir.getAbsolutePath().length()));
@@ -361,13 +361,13 @@ private void addDisguisedJarsToDependencies(Dependency dependency, Engine engine
                 org.apache.commons.io.FileUtils.copyFile(dependency.getActualFile(), tmpLoc);
                 final List<Dependency> dependencySet = findMoreDependencies(engine, tmpLoc);
                 if (dependencySet != null && !dependencySet.isEmpty()) {
-                    dependencySet.forEach((d) -> {
+                    dependencySet.forEach(d -> {
                         //fix the dependency's display name and path
                         if (d.getActualFile().equals(tmpLoc)) {
                             d.setFilePath(dependency.getFilePath());
                             d.setDisplayFileName(dependency.getFileName());
                         } else {
-                            d.getRelatedDependencies().stream().filter((rel) -> rel.getActualFile().equals(tmpLoc)).forEach((rel) -> {
+                            d.getRelatedDependencies().stream().filter(rel -> rel.getActualFile().equals(tmpLoc)).forEach(rel -> {
                                 rel.setFilePath(dependency.getFilePath());
                                 rel.setDisplayFileName(dependency.getFileName());
                             });

diff --git a/src/test/resources/dataflow-functional-tests/FileUtils.java b/src/test/resources/dataflow-functional-tests/FileUtils.java
@@ -256,13 +256,13 @@ public static String byteCountToDisplaySize(long size) {
         String displaySize;
 
         if (size / ONE_GB > 0) {
-            displaySize = String.valueOf(size / ONE_GB) + " GB";
+            displaySize = size / ONE_GB + " GB";
         } else if (size / ONE_MB > 0) {
-            displaySize = String.valueOf(size / ONE_MB) + " MB";
+            displaySize = size / ONE_MB + " MB";
         } else if (size / ONE_KB > 0) {
-            displaySize = String.valueOf(size / ONE_KB) + " KB";
+            displaySize = size / ONE_KB + " KB";
         } else {
-            displaySize = String.valueOf(size) + " bytes";
+            displaySize = size + " bytes";
         }
         return displaySize;
     }
@@ -376,7 +376,7 @@ public static Collection<File> listFiles(
         }
 
         //Find files
-        Collection<File> files = new java.util.LinkedList<File>();
+        Collection<File> files = new java.util.LinkedList<>();
         innerListFiles(files, directory,
                 FileFilterUtils.or(effFileFilter, effDirFilter));
         return files;
@@ -615,7 +615,7 @@ public static File[] toFiles(URL[] urls) {
         for (int i = 0; i < urls.length; i++) {
             URL url = urls[i];
             if (url != null) {
-                if (url.getProtocol().equals("file") == false) {
+                if ("file".equals(url.getProtocol()) == false) {
                     throw new IllegalArgumentException(
                             "URL could not be converted to a File: " + url);
                 }
@@ -1046,7 +1046,7 @@ public static void copyDirectory(File srcDir, File destDir,
         if (canonicalDestDir.startsWith(srcDir.getCanonicalPath())) {
             File[] srcFiles = filter == null ? srcDir.listFiles() : srcDir.listFiles(filter);
             if (srcFiles != null && srcFiles.length > 0) {
-                exclusionList = new ArrayList<String>(srcFiles.length);
+                exclusionList = new ArrayList<>(srcFiles.length);
                 for (File srcFile : srcFiles) {
                     File copiedFile = new File(destDir, srcFile.getName());
                     exclusionList.add(copiedFile.getCanonicalPath());
@@ -2206,10 +2206,6 @@ public static boolean isSymlink(File file) throws IOException {
             fileInCanonicalDir = new File(canonicalDir, file.getName());
         }
 
-        if (fileInCanonicalDir.getCanonicalFile().equals(fileInCanonicalDir.getAbsoluteFile())) {
-            return false;
-        } else {
-            return true;
-        }
+        return !(fileInCanonicalDir.getCanonicalFile().equals(fileInCanonicalDir.getAbsoluteFile()));
     }
 }