Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions recipe-writing-lessons.md
Original file line number Diff line number Diff line change
Expand Up @@ -319,3 +319,37 @@ mark escape. Key learnings:
the method type still declares the legacy type. With every reference re-typed, `maybeRemoveImport`
removes the import with no downstream cleanup needed. `ReplaceSynchronizedType` is the shared base that
implements this for `Hashtable`/`Vector`/`StringBuffer`.

## Data flow / taint tracking (rewrite-analysis)

### `Dataflow.findSinks` cannot track sources inside `catch` blocks
`org.openrewrite.analysis.dataflow.Dataflow#findSinks` gates its results on control-flow
*reachability*: it computes the control-flow graph of the enclosing method and prunes any
flow whose nodes are not reachable on the normal control-flow path. A `catch` block is only
entered via an exceptional edge, which the control-flow graph does not model, so every
expression inside a `catch` is considered unreachable and the flow is pruned to empty —
`findSinks` returns `Option.none()` even though `DataFlowNode.of(...)` and `spec.isSource(...)`
both succeed.

To taint-track a source that lives inside a `catch` block (e.g. connecting a caught exception
to a newly thrown one), drive the flow engine directly and skip the reachability filter:
```java
DataFlowNode.of(cursor).forEach(node -> {
FlowGraph graph = ForwardFlow.findAllFlows(node, spec, FlowGraph.Factory.defaultFactory());
// BFS/DFS over graph.getEdges(); collect each node.getCursor().getValue() that is an Expression
});
```
This yields the full forward taint graph (every expression the source flows into) without the
control-flow reachability gate. See `FindNewExceptionWithoutCause`.

### Matching a reference to a specific local/caught variable
`JavaType.Variable.equals` is structural (compares `name` + `owner`), so a reference's
`J.Identifier#getFieldType()` reliably equals the declaration's `NamedVariable#getVariableType()`.
Fall back to simple-name comparison only when type attribution is missing.

### `@Nullable` on a nested type is a type-use annotation
Write `JavaType.@Nullable Variable`, not `@Nullable JavaType.Variable`. The latter annotates the
scoping construct `JavaType` and fails to compile ("scoping construct cannot be annotated with
type-use annotation"), which crashes the whole Lombok annotation-processing round and produces a
misleading cascade of "does not override abstract method getDescription()" errors across every
Lombok-annotated recipe.
Original file line number Diff line number Diff line change
@@ -0,0 +1,251 @@
/*
* Copyright 2026 the original author or authors.
* <p>
* Licensed under the Moderne Source Available License (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* <p>
* https://docs.moderne.io/licensing/moderne-source-available-license
* <p>
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.openrewrite.staticanalysis;

import lombok.EqualsAndHashCode;
import lombok.Value;
import org.jspecify.annotations.Nullable;
import org.openrewrite.Cursor;
import org.openrewrite.ExecutionContext;
import org.openrewrite.Preconditions;
import org.openrewrite.Recipe;
import org.openrewrite.TreeVisitor;
import org.openrewrite.analysis.dataflow.DataFlowNode;
import org.openrewrite.analysis.dataflow.TaintFlowSpec;
import org.openrewrite.analysis.dataflow.analysis.FlowGraph;
import org.openrewrite.analysis.dataflow.analysis.ForwardFlow;
import org.openrewrite.java.JavaIsoVisitor;
import org.openrewrite.java.tree.Expression;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.JavaSourceFile;
import org.openrewrite.java.tree.JavaType;
import org.openrewrite.marker.SearchResult;
import org.openrewrite.staticanalysis.groovy.GroovyFileChecker;
import org.openrewrite.staticanalysis.java.JavaFileChecker;
import org.openrewrite.staticanalysis.kotlin.KotlinFileChecker;
import org.openrewrite.staticanalysis.table.ExceptionsWithoutCause;

import java.util.ArrayDeque;
import java.util.Deque;
import java.util.HashSet;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;

@Value
@EqualsAndHashCode(callSuper = false)
public class FindNewExceptionWithoutCause extends Recipe {

private static final String TAINTED_KEY = "caughtExceptionTaintedExpressions";
private static final String CAUGHT_KEY = "caughtExceptionVariable";

transient ExceptionsWithoutCause report = new ExceptionsWithoutCause(this);

@Override
public String getDisplayName() {
return "Find new exceptions thrown without the caught exception";
}

@Override
public String getDescription() {
return "Finds `catch` blocks that throw a newly created exception without referencing the caught exception, " +
"which discards the original exception's stack trace and message. Data flow (taint) tracking is used " +
"to establish whether the caught exception—or any value derived from it—reaches the thrown exception, " +
"so indirect references through local variables and string concatenation are not falsely reported. " +
"This mirrors PMD's `PreserveStackTrace` rule.";
}

@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
return Preconditions.check(Preconditions.or(
new JavaFileChecker<>(),
new GroovyFileChecker<>(),
new KotlinFileChecker<>()
), new JavaIsoVisitor<ExecutionContext>() {

@Override
public J.Try.Catch visitCatch(J.Try.Catch aCatch, ExecutionContext ctx) {
J.VariableDeclarations.NamedVariable caughtVar = aCatch.getParameter().getTree().getVariables().get(0);
JavaType.Variable caughtType = caughtVar.getVariableType();
String caughtName = caughtVar.getSimpleName();

// Seed a taint analysis from every reference to the caught exception (and every value read off of it,
// e.g. `e.getMessage()`) and collect all expressions the caught exception flows into.
Set<Expression> tainted = new HashSet<>();
ExceptionTaintSpec spec = new ExceptionTaintSpec(caughtType, caughtName);
new JavaIsoVisitor<Set<Expression>>() {
@Override
public J.Identifier visitIdentifier(J.Identifier identifier, Set<Expression> set) {
if (referencesCaught(identifier, caughtType, caughtName)) {
seedFlow(getCursor(), spec, set);
}
return identifier;
}

@Override
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Set<Expression> set) {
if (rootReferencesCaught(method.getSelect(), caughtType, caughtName)) {
seedFlow(getCursor(), spec, set);
}
return super.visitMethodInvocation(method, set);
}
}.visit(aCatch.getBody(), tainted, getCursor());

getCursor().putMessage(TAINTED_KEY, tainted);
getCursor().putMessage(CAUGHT_KEY, caughtVar);
return super.visitCatch(aCatch, ctx);
}

@Override
public J.Throw visitThrow(J.Throw thrown, ExecutionContext ctx) {
J.Throw t = super.visitThrow(thrown, ctx);
if (!(t.getException() instanceof J.NewClass)) {
return t;
}
J.NewClass newException = (J.NewClass) t.getException();

// Find the `catch` clause that directly governs this `throw`, bailing out if a `try` body, lambda,
// or other execution boundary sits between them.
Cursor governing = null;
for (Cursor c = getCursor().getParent(); c != null; c = c.getParent()) {
Object v = c.getValue();
if (v instanceof J.Try.Catch) {
governing = c;
break;
}
if (v instanceof J.Try || v instanceof J.Lambda || v instanceof J.MethodDeclaration ||
v instanceof J.ClassDeclaration ||
(v instanceof J.NewClass && ((J.NewClass) v).getBody() != null)) {
break;
}
}
if (governing == null) {
return t;
}

J.VariableDeclarations.NamedVariable caughtVar = governing.getMessage(CAUGHT_KEY);
Set<Expression> tainted = governing.getMessage(TAINTED_KEY);
if (caughtVar == null || tainted == null) {
return t;
}

JavaType.Variable caughtType = caughtVar.getVariableType();
String caughtName = caughtVar.getSimpleName();
if (referencesCaughtException(newException, caughtType, caughtName, tainted)) {
return t;
}

JavaSourceFile sourceFile = getCursor().firstEnclosing(JavaSourceFile.class);
report.insertRow(ctx, new ExceptionsWithoutCause.Row(
sourceFile == null ? "" : sourceFile.getSourcePath().toString(),
String.valueOf(caughtType == null ? caughtVar.getType() : caughtType.getType()),
String.valueOf(newException.getType())
));
return t.withException(SearchResult.found(newException));
}

private void seedFlow(Cursor cursor, ExceptionTaintSpec spec, Set<Expression> tainted) {
// Dataflow#findSinks gates on control-flow reachability, but a `catch` block is only reached via
// an exceptional edge that the control-flow graph does not model, so its expressions are considered
// unreachable and pruned. Drive ForwardFlow directly to obtain the taint graph without that gate.
DataFlowNode.of(cursor).forEach(node -> {
FlowGraph graph = ForwardFlow.findAllFlows(node, spec, FlowGraph.Factory.defaultFactory());
Deque<FlowGraph> worklist = new ArrayDeque<>();
worklist.add(graph);
while (!worklist.isEmpty()) {
FlowGraph current = worklist.poll();
Object value = current.getNode().getCursor().getValue();
if (value instanceof Expression) {
tainted.add((Expression) value);
}
worklist.addAll(current.getEdges());
}
});
}

private boolean referencesCaughtException(J newException, JavaType.@Nullable Variable caughtType,
String caughtName, Set<Expression> tainted) {
AtomicBoolean referenced = new AtomicBoolean(false);
new JavaIsoVisitor<AtomicBoolean>() {
@Override
public Expression visitExpression(Expression expression, AtomicBoolean found) {
if (found.get()) {
return expression;
}
if (tainted.contains(expression) ||
(expression instanceof J.Identifier &&
referencesCaught((J.Identifier) expression, caughtType, caughtName))) {
found.set(true);
return expression;
}
return super.visitExpression(expression, found);
}
}.visit(newException, referenced);
return referenced.get();
}
});
}

private static boolean referencesCaught(J.Identifier identifier, JavaType.@Nullable Variable caughtType,
String caughtName) {
JavaType.Variable fieldType = identifier.getFieldType();
if (caughtType != null && fieldType != null) {
return caughtType.equals(fieldType);
}
return caughtName.equals(identifier.getSimpleName());
}

private static boolean rootReferencesCaught(@Nullable Expression select, JavaType.@Nullable Variable caughtType,
String caughtName) {
Expression e = select;
while (e != null) {
if (e instanceof J.Identifier) {
return referencesCaught((J.Identifier) e, caughtType, caughtName);
}
if (e instanceof J.MethodInvocation) {
e = ((J.MethodInvocation) e).getSelect();
} else if (e instanceof J.FieldAccess) {
e = ((J.FieldAccess) e).getTarget();
} else {
return false;
}
}
return false;
}

@Value
@EqualsAndHashCode(callSuper = false)
private static class ExceptionTaintSpec extends TaintFlowSpec {
JavaType.@Nullable Variable caughtType;
String caughtName;

@Override
public boolean isSource(DataFlowNode srcNode) {
Object v = srcNode.getCursor().getValue();
if (v instanceof J.Identifier) {
return referencesCaught((J.Identifier) v, caughtType, caughtName);
}
if (v instanceof J.MethodInvocation) {
return rootReferencesCaught(((J.MethodInvocation) v).getSelect(), caughtType, caughtName);
}
return false;
}

@Override
public boolean isSink(DataFlowNode sinkNode) {
return true;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* Copyright 2026 the original author or authors.
* <p>
* Licensed under the Moderne Source Available License (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* <p>
* https://docs.moderne.io/licensing/moderne-source-available-license
* <p>
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.openrewrite.staticanalysis.table;

import com.fasterxml.jackson.annotation.JsonIgnoreType;
import lombok.Value;
import org.openrewrite.Column;
import org.openrewrite.DataTable;
import org.openrewrite.Recipe;

@JsonIgnoreType
public class ExceptionsWithoutCause extends DataTable<ExceptionsWithoutCause.Row> {

public ExceptionsWithoutCause(Recipe recipe) {
super(recipe,
"Exceptions thrown without the caught cause",
"New exceptions thrown from a `catch` block that do not reference the caught exception.");
}

@Value
public static class Row {
@Column(displayName = "Source path",
description = "The path to the source file containing the offending `throw`.")
String sourcePath;

@Column(displayName = "Caught exception type",
description = "The declared type of the exception caught by the enclosing `catch` clause.")
String caughtType;

@Column(displayName = "Thrown exception type",
description = "The type of the new exception thrown without referencing the caught exception.")
String thrownType;
}
}
Loading
Loading