Skip to content

Fix ThisAccess to actually match this expressions#95

Merged
knutwannheden merged 1 commit into
mainfrom
fix-thisaccess-typo
Apr 21, 2026
Merged

Fix ThisAccess to actually match this expressions#95
knutwannheden merged 1 commit into
mainfrom
fix-thisaccess-typo

Conversation

@knutwannheden
Copy link
Copy Markdown
Contributor

Motivation

The ThisAccess.Factory.viewOf factory — meant to create a trait view for this identifiers — checked for the string "super" instead of "this". As a result, Expr.viewOf(cursor) never resolved for a this identifier; every this expression fell through the trait factory chain:

  • ThisAccess rejected this (because it was looking for super)
  • SuperAccess also rejected this
  • VarAccess doesn't handle this
  • ExprFallback explicitly rejects J.Identifier

So DataFlowNode.of(cursor) returned none for any this identifier. ForwardFlow.computeVariableAssignment then threw Unable to create DataFlowNode via ofOrThrow whenever the algorithm iterated method-invocation arguments and encountered this. This broke any downstream recipe relying on local dataflow (for example ReplaceStackWithDeque) on code that passed this into a method call along a tracked flow path.

Originally surfaced as a crash in ReplaceStackWithDeque on real-world code in finos/messageml-utils and finos/tracdap where result.add(this) and validator.apply(target, arg, this) appeared in the same method as a Stack variable.

Summary

  • Correct the literal comparison in ThisAccess.Factory.viewOf from "super" to "this"
  • Add DataFlowNodeTest.thisIsADataFlowNode — verifies DataFlowNode.of now resolves for a this identifier
  • Add FindLocalFlowPathsStringTest.flowWhereThisIsAnotherArgument — regression test that crashes before this fix (identical Unable to create DataFlowNode stack trace) and passes after

Test plan

  • ./gradlew test passes with the fix
  • Reverting only the one-line fix causes FindLocalFlowPathsStringTest.flowWhereThisIsAnotherArgument to fail with the original production stack trace (DataFlowNode.ofOrThrowForwardFlow.computeVariableAssignment:467)
  • Verified against a ReplaceStackWithDeque reproducer (rewrite-static-analysis) that previously crashed on result.add(this) — recipe now runs cleanly after publishing this change to Maven Local

`ThisAccess.Factory.viewOf` checked for `"super"` instead of `"this"`,
so `Expr.viewOf(cursor)` never resolved for `this` identifiers. This
caused `DataFlowNode.of` to return `none` and
`ForwardFlow.computeVariableAssignment` to throw `Unable to create
DataFlowNode` whenever `this` appeared as an argument to a method
invocation along a tracked flow path.
@github-project-automation github-project-automation Bot moved this to In Progress in OpenRewrite Apr 21, 2026
@knutwannheden knutwannheden changed the title Fix ThisAccess to actually match this expressions Fix ThisAccess to actually match this expressions Apr 21, 2026
@knutwannheden knutwannheden merged commit 24dc67b into main Apr 21, 2026
1 check passed
@knutwannheden knutwannheden deleted the fix-thisaccess-typo branch April 21, 2026 06:38
@github-project-automation github-project-automation Bot moved this from In Progress to Done in OpenRewrite Apr 21, 2026
knutwannheden added a commit to openrewrite/rewrite-static-analysis that referenced this pull request Apr 21, 2026
…ent (#864)

Covers the dataflow crash fixed in openrewrite/rewrite-analysis#95,
where the recipe threw `Unable to create DataFlowNode` when a `Stack`
variable shared a method with a call that passed `this` as an argument.
@JLLeitschuh
Copy link
Copy Markdown
Contributor

Oops 😆 🙈 Good fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants