search

openrewrite / rewrite-analysis

OpenRewrite recipes for data flow analysis.
Apache License 2.0
8 stars 8 forks source link

Misc. errors thrown while running `org.openrewrite.analysis.controlflow.ControlFlowVisualization` #39

Open traceyyoshima opened 9 months ago

traceyyoshima commented 9 months ago
JLLeitschuh commented 9 months ago

Stack traces would be super useful here.

Also, not surprised for the Javascript cases. Both DataFlow and Control Flow are very dependent upon the specific characteristics of the Java language itself, and the control flow graph, as it builds itself, is self-validating to ensure that it isn't created incorrectly. This was incredibly useful for unit testing.

josetesan commented 7 months ago

I've also found different errors regarding switch expressions handling . For example :

public String getValue(Enum enum) {
  String val = null;
  switch (enum) {
    OK -> val = "OK";
    KO -> val = "KO";
   default -> val= "OK";
  }
  return val;
}

is valid, whilst the concise version 

public String getValue(Enum enum) {
  return switch (enum) {
    case OK -> "OK";
    case KO ->  "KO";
   default -> "OK";
  }
}

Error is displayed as :

[ERROR] Failed to execute goal org.openrewrite.maven:rewrite-maven-plugin:5.20.0:run (default-cli) on project com-acme-cli: Execution default-cli of goal org.openrewrite.maven:rewrite-maven-plugin:5.20.0:run failed: Error while visiting src/main/java/com/acme/cli/context/console/impl/CliTerminalConsoleContext.java: org.openrewrite.analysis.controlflow.ControlFlowIllegalStateException: Case statements should be visited by the switch statement
[ERROR]     AST: case OK -> "OK"
[ERROR]     Cursor: Cursor{Case->Block->SwitchExpression->Return->Block->MethodDeclaration->JRightPadded(element=MethodDeclaration{com.acme.cli.context.console.impl.CliTerminalConsoleContext{name=logColorResolver,return=org.fusesource.jansi.Ansi$Color,parameters=[com.acme.context.console.CliTerminalContext$TerminalLogLevel]}}, after=Space(comments=<0 comments>, whitespace=''))->Block->ClassDeclaration->CompilationUnit->root}
[ERROR]   org.openrewrite.analysis.controlflow.ControlFlow$ControlFlowAnalysis.visitCase(ControlFlow.java:385)
[ERROR]   org.openrewrite.analysis.controlflow.ControlFlow$ControlFlowAnalysis.visitCase(ControlFlow.java:80)
[ERROR]   org.openrewrite.java.tree.J$Case.acceptJava(J.java:1088)
[ERROR]   org.openrewrite.java.tree.J.accept(J.java:59)
[ERROR]   org.openrewrite.TreeVisitor.visit(TreeVisitor.java:278)
[ERROR]   org.openrewrite.TreeVisitor.visit(TreeVisitor.java:184)
[ERROR]   org.openrewrite.analysis.controlflow.ControlFlow$ControlFlowAnalysis.visitRecursive(ControlFlow.java:164)
[ERROR]   org.openrewrite.analysis.controlflow.ControlFlow$ControlFlowAnalysis.visitStatementList(ControlFlow.java:209)
JLLeitschuh commented 7 months ago

Thanks for the heads up. Control Flow Analysis is heavily tied to the behavior of the language, so it's not shocking to me that new language features cause it to break.

I don't know if I have time right now to address this. But thank you for letting me know.

josetesan commented 7 months ago

Thanks . Btw , how is this recipe working ? It just appends a lot of grapviz code to Java files , and then , what ?

JLLeitschuh commented 6 months ago

It just appends a lot of grapviz code to Java files , and then , what ?

It's primarily used for debugging the control flow analysis engine. Control flow itself is more useful in the context of data flow analysis.

Here's an example of how it is used in, probably the most complex security fix recipe, ZipSlip:

https://github.com/openrewrite/rewrite-java-security/blob/008c74ab09a96912b9e2895259a7a42bd5c90c8a/src/main/java/org/openrewrite/java/security/ZipSlip.java#L502-L510

JLLeitschuh commented 6 months ago

If you'd like to learn more, I gave a talk on the subject and how we use control flow and data flow analysis here: https://youtu.be/UgGhEfdUSvQ?si=IByhi4TnQfBX-kPv&t=1582