Skip to content

Fix Hamcrest not() unwrap infinite loop and support CoreMatchers#1054

Merged
timtebeek merged 2 commits into
mainfrom
hamcrest-not-corematchers
Jul 20, 2026
Merged

Fix Hamcrest not() unwrap infinite loop and support CoreMatchers#1054
timtebeek merged 2 commits into
mainfrom
hamcrest-not-corematchers

Conversation

@Jammy-Louie

@Jammy-Louie Jammy-Louie commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

What's changed

Two commits:

  1. Fix an infinite loop in HamcrestMatcherToJUnit5 / HamcrestInstanceOfToJUnit5 on a not(...) matcher that isn't org.hamcrest.Matchers.not.
  2. Migrate CoreMatchers variants of not/instanceOf/isA instead of skipping them.

The hang

The unwrap loop continues on a name check but only makes progress on a type check, so they disagree:

while ("not".equals(matcherInvocation.getSimpleName())) {          // any not
    matcherInvocation = new RemoveNotMatcherVisitor().visit(...);  // only strips org.hamcrest.Matchers.not
}

A CoreMatchers.not (or a not with unresolved type) never gets stripped, the name stays "not", and the loop spins forever. Repro:

import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.CoreMatchers.equalTo;
assertThat(a, not(equalTo(b))); // hangs

Fix: loop on the matcher that decides strippability, so continuation and progress can't disagree:

while (RemoveNotMatcherVisitor.NOT_MATCHER.matches(matcherInvocation)) {

No change to existing migrations.

CoreMatchers support

Broadens the three matchers from org.hamcrest.Matchers to org.hamcrest.*Matchers (matches exactly Matchers and CoreMatchers, identical methods), so CoreMatchers.not/instanceOf/isA migrate too.

Tests

notFromCoreMatchers (both recipes, asserting the real migration, @Timeout-guarded so a regressed hang fails fast) and instanceOfAndIsAFromCoreMatchers. Full suite passes.

@github-project-automation github-project-automation Bot moved this to In Progress in OpenRewrite Jul 17, 2026
@Jammy-Louie
Jammy-Louie marked this pull request as ready for review July 20, 2026 13:19
@Jammy-Louie
Jammy-Louie requested a review from timtebeek July 20, 2026 13:19

@timtebeek timtebeek left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect, thanks a lot! This explains why I was seeing timeouts with AssertJ indeed.

@github-project-automation github-project-automation Bot moved this from In Progress to Ready to Review in OpenRewrite Jul 20, 2026
@timtebeek
timtebeek merged commit f0f55ab into main Jul 20, 2026
1 check passed
@timtebeek
timtebeek deleted the hamcrest-not-corematchers branch July 20, 2026 13:37
@github-project-automation github-project-automation Bot moved this from Ready to Review to Done in OpenRewrite Jul 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants