Skip to content

Commit 8a88b48

Browse files
authored
exception-without-cause (#950)
* Add recipe for finding exceptions which are thrown from catch blocks that do not reference the caught exception * Add recipe for finding exceptions which are thrown from catch blocks that do not reference the caught exception * Update recipes.csv
1 parent 9188c13 commit 8a88b48

5 files changed

Lines changed: 698 additions & 0 deletions

File tree

recipe-writing-lessons.md

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,3 +319,37 @@ mark escape. Key learnings:
319319
the method type still declares the legacy type. With every reference re-typed, `maybeRemoveImport`
320320
removes the import with no downstream cleanup needed. `ReplaceSynchronizedType` is the shared base that
321321
implements this for `Hashtable`/`Vector`/`StringBuffer`.
322+
323+
## Data flow / taint tracking (rewrite-analysis)
324+
325+
### `Dataflow.findSinks` cannot track sources inside `catch` blocks
326+
`org.openrewrite.analysis.dataflow.Dataflow#findSinks` gates its results on control-flow
327+
*reachability*: it computes the control-flow graph of the enclosing method and prunes any
328+
flow whose nodes are not reachable on the normal control-flow path. A `catch` block is only
329+
entered via an exceptional edge, which the control-flow graph does not model, so every
330+
expression inside a `catch` is considered unreachable and the flow is pruned to empty —
331+
`findSinks` returns `Option.none()` even though `DataFlowNode.of(...)` and `spec.isSource(...)`
332+
both succeed.
333+
334+
To taint-track a source that lives inside a `catch` block (e.g. connecting a caught exception
335+
to a newly thrown one), drive the flow engine directly and skip the reachability filter:
336+
```java
337+
DataFlowNode.of(cursor).forEach(node -> {
338+
FlowGraph graph = ForwardFlow.findAllFlows(node, spec, FlowGraph.Factory.defaultFactory());
339+
// BFS/DFS over graph.getEdges(); collect each node.getCursor().getValue() that is an Expression
340+
});
341+
```
342+
This yields the full forward taint graph (every expression the source flows into) without the
343+
control-flow reachability gate. See `FindNewExceptionWithoutCause`.
344+
345+
### Matching a reference to a specific local/caught variable
346+
`JavaType.Variable.equals` is structural (compares `name` + `owner`), so a reference's
347+
`J.Identifier#getFieldType()` reliably equals the declaration's `NamedVariable#getVariableType()`.
348+
Fall back to simple-name comparison only when type attribution is missing.
349+
350+
### `@Nullable` on a nested type is a type-use annotation
351+
Write `JavaType.@Nullable Variable`, not `@Nullable JavaType.Variable`. The latter annotates the
352+
scoping construct `JavaType` and fails to compile ("scoping construct cannot be annotated with
353+
type-use annotation"), which crashes the whole Lombok annotation-processing round and produces a
354+
misleading cascade of "does not override abstract method getDescription()" errors across every
355+
Lombok-annotated recipe.
Lines changed: 251 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,251 @@
1+
/*
2+
* Copyright 2026 the original author or authors.
3+
* <p>
4+
* Licensed under the Moderne Source Available License (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
* <p>
8+
* https://docs.moderne.io/licensing/moderne-source-available-license
9+
* <p>
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.openrewrite.staticanalysis;
17+
18+
import lombok.EqualsAndHashCode;
19+
import lombok.Value;
20+
import org.jspecify.annotations.Nullable;
21+
import org.openrewrite.Cursor;
22+
import org.openrewrite.ExecutionContext;
23+
import org.openrewrite.Preconditions;
24+
import org.openrewrite.Recipe;
25+
import org.openrewrite.TreeVisitor;
26+
import org.openrewrite.analysis.dataflow.DataFlowNode;
27+
import org.openrewrite.analysis.dataflow.TaintFlowSpec;
28+
import org.openrewrite.analysis.dataflow.analysis.FlowGraph;
29+
import org.openrewrite.analysis.dataflow.analysis.ForwardFlow;
30+
import org.openrewrite.java.JavaIsoVisitor;
31+
import org.openrewrite.java.tree.Expression;
32+
import org.openrewrite.java.tree.J;
33+
import org.openrewrite.java.tree.JavaSourceFile;
34+
import org.openrewrite.java.tree.JavaType;
35+
import org.openrewrite.marker.SearchResult;
36+
import org.openrewrite.staticanalysis.groovy.GroovyFileChecker;
37+
import org.openrewrite.staticanalysis.java.JavaFileChecker;
38+
import org.openrewrite.staticanalysis.kotlin.KotlinFileChecker;
39+
import org.openrewrite.staticanalysis.table.ExceptionsWithoutCause;
40+
41+
import java.util.ArrayDeque;
42+
import java.util.Deque;
43+
import java.util.HashSet;
44+
import java.util.Set;
45+
import java.util.concurrent.atomic.AtomicBoolean;
46+
47+
@Value
48+
@EqualsAndHashCode(callSuper = false)
49+
public class FindNewExceptionWithoutCause extends Recipe {
50+
51+
private static final String TAINTED_KEY = "caughtExceptionTaintedExpressions";
52+
private static final String CAUGHT_KEY = "caughtExceptionVariable";
53+
54+
transient ExceptionsWithoutCause report = new ExceptionsWithoutCause(this);
55+
56+
@Override
57+
public String getDisplayName() {
58+
return "Find new exceptions thrown without the caught exception";
59+
}
60+
61+
@Override
62+
public String getDescription() {
63+
return "Finds `catch` blocks that throw a newly created exception without referencing the caught exception, " +
64+
"which discards the original exception's stack trace and message. Data flow (taint) tracking is used " +
65+
"to establish whether the caught exception—or any value derived from it—reaches the thrown exception, " +
66+
"so indirect references through local variables and string concatenation are not falsely reported. " +
67+
"This mirrors PMD's `PreserveStackTrace` rule.";
68+
}
69+
70+
@Override
71+
public TreeVisitor<?, ExecutionContext> getVisitor() {
72+
return Preconditions.check(Preconditions.or(
73+
new JavaFileChecker<>(),
74+
new GroovyFileChecker<>(),
75+
new KotlinFileChecker<>()
76+
), new JavaIsoVisitor<ExecutionContext>() {
77+
78+
@Override
79+
public J.Try.Catch visitCatch(J.Try.Catch aCatch, ExecutionContext ctx) {
80+
J.VariableDeclarations.NamedVariable caughtVar = aCatch.getParameter().getTree().getVariables().get(0);
81+
JavaType.Variable caughtType = caughtVar.getVariableType();
82+
String caughtName = caughtVar.getSimpleName();
83+
84+
// Seed a taint analysis from every reference to the caught exception (and every value read off of it,
85+
// e.g. `e.getMessage()`) and collect all expressions the caught exception flows into.
86+
Set<Expression> tainted = new HashSet<>();
87+
ExceptionTaintSpec spec = new ExceptionTaintSpec(caughtType, caughtName);
88+
new JavaIsoVisitor<Set<Expression>>() {
89+
@Override
90+
public J.Identifier visitIdentifier(J.Identifier identifier, Set<Expression> set) {
91+
if (referencesCaught(identifier, caughtType, caughtName)) {
92+
seedFlow(getCursor(), spec, set);
93+
}
94+
return identifier;
95+
}
96+
97+
@Override
98+
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Set<Expression> set) {
99+
if (rootReferencesCaught(method.getSelect(), caughtType, caughtName)) {
100+
seedFlow(getCursor(), spec, set);
101+
}
102+
return super.visitMethodInvocation(method, set);
103+
}
104+
}.visit(aCatch.getBody(), tainted, getCursor());
105+
106+
getCursor().putMessage(TAINTED_KEY, tainted);
107+
getCursor().putMessage(CAUGHT_KEY, caughtVar);
108+
return super.visitCatch(aCatch, ctx);
109+
}
110+
111+
@Override
112+
public J.Throw visitThrow(J.Throw thrown, ExecutionContext ctx) {
113+
J.Throw t = super.visitThrow(thrown, ctx);
114+
if (!(t.getException() instanceof J.NewClass)) {
115+
return t;
116+
}
117+
J.NewClass newException = (J.NewClass) t.getException();
118+
119+
// Find the `catch` clause that directly governs this `throw`, bailing out if a `try` body, lambda,
120+
// or other execution boundary sits between them.
121+
Cursor governing = null;
122+
for (Cursor c = getCursor().getParent(); c != null; c = c.getParent()) {
123+
Object v = c.getValue();
124+
if (v instanceof J.Try.Catch) {
125+
governing = c;
126+
break;
127+
}
128+
if (v instanceof J.Try || v instanceof J.Lambda || v instanceof J.MethodDeclaration ||
129+
v instanceof J.ClassDeclaration ||
130+
(v instanceof J.NewClass && ((J.NewClass) v).getBody() != null)) {
131+
break;
132+
}
133+
}
134+
if (governing == null) {
135+
return t;
136+
}
137+
138+
J.VariableDeclarations.NamedVariable caughtVar = governing.getMessage(CAUGHT_KEY);
139+
Set<Expression> tainted = governing.getMessage(TAINTED_KEY);
140+
if (caughtVar == null || tainted == null) {
141+
return t;
142+
}
143+
144+
JavaType.Variable caughtType = caughtVar.getVariableType();
145+
String caughtName = caughtVar.getSimpleName();
146+
if (referencesCaughtException(newException, caughtType, caughtName, tainted)) {
147+
return t;
148+
}
149+
150+
JavaSourceFile sourceFile = getCursor().firstEnclosing(JavaSourceFile.class);
151+
report.insertRow(ctx, new ExceptionsWithoutCause.Row(
152+
sourceFile == null ? "" : sourceFile.getSourcePath().toString(),
153+
String.valueOf(caughtType == null ? caughtVar.getType() : caughtType.getType()),
154+
String.valueOf(newException.getType())
155+
));
156+
return t.withException(SearchResult.found(newException));
157+
}
158+
159+
private void seedFlow(Cursor cursor, ExceptionTaintSpec spec, Set<Expression> tainted) {
160+
// Dataflow#findSinks gates on control-flow reachability, but a `catch` block is only reached via
161+
// an exceptional edge that the control-flow graph does not model, so its expressions are considered
162+
// unreachable and pruned. Drive ForwardFlow directly to obtain the taint graph without that gate.
163+
DataFlowNode.of(cursor).forEach(node -> {
164+
FlowGraph graph = ForwardFlow.findAllFlows(node, spec, FlowGraph.Factory.defaultFactory());
165+
Deque<FlowGraph> worklist = new ArrayDeque<>();
166+
worklist.add(graph);
167+
while (!worklist.isEmpty()) {
168+
FlowGraph current = worklist.poll();
169+
Object value = current.getNode().getCursor().getValue();
170+
if (value instanceof Expression) {
171+
tainted.add((Expression) value);
172+
}
173+
worklist.addAll(current.getEdges());
174+
}
175+
});
176+
}
177+
178+
private boolean referencesCaughtException(J newException, JavaType.@Nullable Variable caughtType,
179+
String caughtName, Set<Expression> tainted) {
180+
AtomicBoolean referenced = new AtomicBoolean(false);
181+
new JavaIsoVisitor<AtomicBoolean>() {
182+
@Override
183+
public Expression visitExpression(Expression expression, AtomicBoolean found) {
184+
if (found.get()) {
185+
return expression;
186+
}
187+
if (tainted.contains(expression) ||
188+
(expression instanceof J.Identifier &&
189+
referencesCaught((J.Identifier) expression, caughtType, caughtName))) {
190+
found.set(true);
191+
return expression;
192+
}
193+
return super.visitExpression(expression, found);
194+
}
195+
}.visit(newException, referenced);
196+
return referenced.get();
197+
}
198+
});
199+
}
200+
201+
private static boolean referencesCaught(J.Identifier identifier, JavaType.@Nullable Variable caughtType,
202+
String caughtName) {
203+
JavaType.Variable fieldType = identifier.getFieldType();
204+
if (caughtType != null && fieldType != null) {
205+
return caughtType.equals(fieldType);
206+
}
207+
return caughtName.equals(identifier.getSimpleName());
208+
}
209+
210+
private static boolean rootReferencesCaught(@Nullable Expression select, JavaType.@Nullable Variable caughtType,
211+
String caughtName) {
212+
Expression e = select;
213+
while (e != null) {
214+
if (e instanceof J.Identifier) {
215+
return referencesCaught((J.Identifier) e, caughtType, caughtName);
216+
}
217+
if (e instanceof J.MethodInvocation) {
218+
e = ((J.MethodInvocation) e).getSelect();
219+
} else if (e instanceof J.FieldAccess) {
220+
e = ((J.FieldAccess) e).getTarget();
221+
} else {
222+
return false;
223+
}
224+
}
225+
return false;
226+
}
227+
228+
@Value
229+
@EqualsAndHashCode(callSuper = false)
230+
private static class ExceptionTaintSpec extends TaintFlowSpec {
231+
JavaType.@Nullable Variable caughtType;
232+
String caughtName;
233+
234+
@Override
235+
public boolean isSource(DataFlowNode srcNode) {
236+
Object v = srcNode.getCursor().getValue();
237+
if (v instanceof J.Identifier) {
238+
return referencesCaught((J.Identifier) v, caughtType, caughtName);
239+
}
240+
if (v instanceof J.MethodInvocation) {
241+
return rootReferencesCaught(((J.MethodInvocation) v).getSelect(), caughtType, caughtName);
242+
}
243+
return false;
244+
}
245+
246+
@Override
247+
public boolean isSink(DataFlowNode sinkNode) {
248+
return true;
249+
}
250+
}
251+
}
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
/*
2+
* Copyright 2026 the original author or authors.
3+
* <p>
4+
* Licensed under the Moderne Source Available License (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
* <p>
8+
* https://docs.moderne.io/licensing/moderne-source-available-license
9+
* <p>
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.openrewrite.staticanalysis.table;
17+
18+
import com.fasterxml.jackson.annotation.JsonIgnoreType;
19+
import lombok.Value;
20+
import org.openrewrite.Column;
21+
import org.openrewrite.DataTable;
22+
import org.openrewrite.Recipe;
23+
24+
@JsonIgnoreType
25+
public class ExceptionsWithoutCause extends DataTable<ExceptionsWithoutCause.Row> {
26+
27+
public ExceptionsWithoutCause(Recipe recipe) {
28+
super(recipe,
29+
"Exceptions thrown without the caught cause",
30+
"New exceptions thrown from a `catch` block that do not reference the caught exception.");
31+
}
32+
33+
@Value
34+
public static class Row {
35+
@Column(displayName = "Source path",
36+
description = "The path to the source file containing the offending `throw`.")
37+
String sourcePath;
38+
39+
@Column(displayName = "Caught exception type",
40+
description = "The declared type of the exception caught by the enclosing `catch` clause.")
41+
String caughtType;
42+
43+
@Column(displayName = "Thrown exception type",
44+
description = "The type of the new exception thrown without referencing the caught exception.")
45+
String thrownType;
46+
}
47+
}

0 commit comments

Comments
 (0)