Skip to content

Drop contextSensitive() from self-contained JavaTemplates - #1232

Merged
timtebeek merged 3 commits into
mainfrom
tim/drop-unneeded-context-sensitive
Oct 25, 2026
Merged

timtebeek merged 3 commits into
mainfrom
tim/drop-unneeded-context-sensitive

Conversation

@timtebeek

@timtebeek timtebeek commented Oct 25, 2026 •

Copy link
Copy Markdown
Member

contextSensitive() makes a JavaTemplate compile inside a stub of the surrounding source file. That is slow and defeats the template cache. It is only needed when the snippet refers to code it cannot get as a typed parameter, or when a placeholder's type is declared in the file being changed. This drops it from 17 files whose templates are self-contained.

Dropped from:

  • Cleanup: all 12 assertion templates in AssertEqualsIntegralDeltaToAssertEquals, AssertFalseEqualsToAssertNotEquals, AssertFalseNegationToAssertTrue, AssertFalseNullToAssertNotNull and AssertTrueEqualsToAssertEquals.
  • Hamcrest: HamcrestIsMatcherToAssertJ, HamcrestMatcherToAssertJ (2) and HamcrestNotMatcherToAssertJ (2). These already turn any type outside java.* into #{any(java.lang.Object)}, so the template never names a type from the project.
  • AssertJ: AdoptAssertJDurationAssertions, whose actual is always a java.time.Duration, and SimplifyAssertJInstanceOfAssertion, whose isInstanceOf is declared on AbstractAssert.
  • JUnit 5: RemoveTryCatchFailBlocks and the statement template in UpdateMockWebServer.
  • Mockito: the @InjectMocks and @Disabled annotation templates in PowerMockWhiteboxSetInternalStateToInjectMocks and DisableUnsupportedPowerMockTests, the closeOnDemand() statements in ScopedMocks, the try-with-resources in MockConstructionToTryWithResources, and the try/catch wrapper in WhiteboxToReflectionVisitor.

Kept:

  • assertThat(#{any()}) templates that chain an assertion specific to one kind of type: SimplifyArrayLengthAssertion, SimplifyChainedAssertJAssertion, SimplifyHasSizeFromIsEqualToAssertion, HamcrestEveryItemToAssertJ, HamcrestHasItemMatcherToAssertJ and HamcrestHasPropertyToAssertJ. An untyped #{any()} compiles as __P__.<fqn>p(), using the argument's type. Without context, a type declared in the test file itself, such as static class Item {}, is unknown to the template parser. assertThat then can't be resolved, and hasSize, isEmpty or hasFieldOrProperty loses its method type. Each of these recipes now has a test with such a class, which fails if contextSensitive() is removed. Types from other source files lose their type with or without context; that was already the case before this PR.
  • The allSatisfy/anySatisfy(arg -> assertThat(arg, ..)) templates in HamcrestEveryItemToAssertJ and HamcrestHasItemMatcherToAssertJ, and HamcrestOfMatchersToAssertJ. Without context, the inner assertThat in the lambda loses its method type.
  • Templates that add fields, methods or parameters to a class: the MockedStatic/MockedConstruction fields in CloseUnclosedStaticMocks, ScopedMocks, MockitoWhenOnStaticToMockStatic and JMockitMockUpToMockito, the lifecycle methods from MockitoUtils, ReplaceMockitoTestExecutionListener, UpdateMockWebServer and TemporaryFolderToTempDir, and the parameter in TestRuleToTestInfo. A context-free template compiles these inside a synthetic class Template, so the new member's owner becomes Template and source-path type arguments are left undefined.
  • Templates that refer to local variables, fields or source-path types and methods by name: the EasyMock verify(mock).method(..), the JMockit mock setup templates, the varName.close()/closeOnDemand() calls in ReplaceInitMockToOpenMock, CloseUnclosedStaticMocks and MockitoWhenOnStaticToMockStatic, the static stubbing in PowerMockitoMockStaticToMockito, the reflection call sites in WhiteboxToReflectionVisitor, the lambda body in JUnitTryFailToAssertThatThrownBy, and the this.field and helper-method references in ParameterizedRunnerToParameterized, TestRuleToTestInfo and TemporaryFolderToTempDir.

Verified locally:

  • A scratch test (not committed) ran 28 cases against both versions. Each case uses domain types in assertions, declared either in another source file or as a nested class in the test. Every recipe that drops contextSensitive() gave the same result as before.
  • Each of the six new test-local class tests passes, and fails on missing type information when run against the context-free version of its recipe.
  • ./gradlew test --tests for hamcrest.*, assertj.* and cleanup.*, with --no-build-cache --rerun: 845 tests, 0 failures.
  • Earlier runs of mockito.*, easymock.*, jmockit.*, junit5.* and testng.* passed. Those files haven't changed since.

Fixes item N3 of #1211.

…types

Without context, a type declared in the test file itself, such as a nested
fixture class, is unknown to the template parser. The type-specific assertion
chained onto assertThat then loses its method type.
Each recipe that still needs contextSensitive() gets a test whose actual has a
type declared in the test file; it fails on missing type information once the
template is made context-free. HamcrestMatcherToAssertJ and
HamcrestNotMatcherToAssertJ go back to context-free templates: they pin
non-java.* types to Object, so no such test can fail for them.
@timtebeek
timtebeek marked this pull request as ready for review October 25, 2026 13:52
@timtebeek

Copy link
Copy Markdown
Member Author

Extra safety checks, since our tests mostly assert on JDK types while customer code asserts on its own domain classes.

How placeholders are typed. An untyped #{any()} compiles as __P__.<fqn>p(), using the argument's type. The template parser only sees the template's own classpathFromResources. contextSensitive() doesn't add project classes to that classpath; it only wraps the snippet in a copy of the file being changed.

Scratch test with domain types. I ran 28 cases against both the context-free and the context-sensitive versions. Each case puts domain types in the actual or expected argument, declared either in another source file or as a nested class in the test.

  • Types from another source file: SimplifyArrayLengthAssertion, SimplifyChainedAssertJAssertion, HamcrestEveryItemToAssertJ and HamcrestHasPropertyToAssertJ lose the method type of the converted assertThat(..) chain in both modes. The template can't see those classes either way, so this limitation was already there before this PR.
  • Types declared in the test file: six recipes regressed without context. These were SimplifyArrayLengthAssertion, SimplifyChainedAssertJAssertion, SimplifyHasSizeFromIsEqualToAssertion, HamcrestEveryItemToAssertJ, HamcrestHasItemMatcherToAssertJ and HamcrestHasPropertyToAssertJ. Each of them follows assertThat(..) with an assertion that exists only for some types (hasSize, isEmpty, hasFieldOrProperty, ...). These six keep contextSensitive(). Each test class now has a *DeclaredInTestFile case that I confirmed fails on missing type information once its template is made context-free.
  • Everything else still dropping contextSensitive() gave the same result in both modes:
    • HamcrestIsMatcherToAssertJ, HamcrestMatcherToAssertJ and HamcrestNotMatcherToAssertJ already turn any type outside java.* into #{any(java.lang.Object)}, so their templates never name a project class. I tried five assertions on test-local classes with no difference between modes.
    • SimplifyAssertJInstanceOfAssertion and the JUnit assertEquals(Object, Object) cleanups worked with domain types in both modes.
    • MockConstructionToTryWithResources: the template only adds the try syntax. The resource is the original variable declaration and the body is spliced in afterwards, so both keep their types. This held with a nested class, a class from another file used through var, and a class declared inside the test method.

Separate existing issue, not addressed here. In HamcrestMatcherToAssertJ, greaterThan on a test-local class that implements Comparable loses its type in both modes, because assertThat(Object) has no isGreaterThan.

After these changes, ./gradlew test for hamcrest.*, assertj.* and cleanup.* with --no-build-cache --rerun gives 845 tests and 0 failures.

@timtebeek
timtebeek merged commit d36a312 into main Oct 25, 2026
2 checks passed
@timtebeek
timtebeek deleted the tim/drop-unneeded-context-sensitive branch October 25, 2026 14:03
@github-project-automation github-project-automation Bot moved this from In Progress to Done in OpenRewrite Oct 25, 2026
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.

1 participant