Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
traceyyoshima opened this issue Nov 13, 2023 · 6 comments
Labels
bug Something isn't working

Comments

@traceyyoshima
Copy link
Contributor

traceyyoshima commented Nov 13, 2023

  • Duplicate key J.Block on src/main/java/com/amazonaws/kinesisvideo/internal/service/BlockingAckConsumer.java
  • No current node! on JS files
  • Could not parse as Java on sdk1/src/test/java/com/amazonaws/services/dynamodbv2/mapper/integration/DynamoDBTestBase.java
  • Unable to create new J.VariableDeclarations with name enumSymbolIterator on avro-kafkaconnect-converter/src/main/java/com/amazonaws/services/schemaregistry/kafkaconnect/avrodata/AvroData.java
  • Condition Node is not a guard! on src/main/webapp/config.js [JS file]
@traceyyoshima traceyyoshima moved this to Backlog in OpenRewrite Nov 13, 2023
@traceyyoshima traceyyoshima added the bug Something isn't working label Nov 13, 2023
@JLLeitschuh
Copy link
Collaborator

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
Copy link

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
Copy link
Collaborator

JLLeitschuh commented Jan 22, 2024

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
Copy link

josetesan commented Jan 22, 2024

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

@JLLeitschuh
Copy link
Collaborator

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
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Backlog
Development

No branches or pull requests

3 participants