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

Fix 🦶🔫: Validate visitNonNull arguments #4016

Merged

Conversation

JLLeitschuh
Copy link
Collaborator

I often find myself shooting myself in the foot passing the wrong Cursor to visitNonNull.

This is a very easy mistake to make and often leads to quite a bit of confusion.

This fixes that by validating that a parent cursor is being passed, not the current cursor.

Signed-off-by: Jonathan Leitschuh [email protected]

What's changed?

Adds a bit of sanity validation to visitNonNull(Tree tree, P p, Cursor parent)

AFAIK this method is really only called when composing one visitor of another visitor, and
isn't called in the hot-path for normal LST tree traversal.

What's your motivation?

Reduce how often I and other's new to OpenRewrite make the mistake of passing the Cursor for the tree, instead of passing the parent cursor.

Anything in particular you'd like reviewers to focus on?

Anyone you would like to review specifically?

Have you considered any alternatives or workarounds?

Any additional context

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

I often find myself shooting myself in the foot passing the wrong `Cursor` to `visitNonNull`.

This is a very easy mistake to make and often leads to quite a bit of confusion.

This fixes that by validating that a parent cursor is being passed, not the current cursor.

Signed-off-by: Jonathan Leitschuh <[email protected]>
@JLLeitschuh
Copy link
Collaborator Author

@sambsnyd want to take a look at this too?

@knutwannheden knutwannheden merged commit b077208 into openrewrite:main Feb 19, 2024
1 check passed
@JLLeitschuh JLLeitschuh deleted the feat/JLL/verify-visitNonNull-args branch February 19, 2024 14:46
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