Repository navigation
Drop forced maybeAddImport(.., false) where templates already attribute the added types - #1234
Conversation
|
I checked whether dropping What happens to the types. A context-free template compiles
Scratch tests. Customer types came from a separate source file (
The test classes for the recipes changed here, plus Remaining risk. An import would only go missing if a template produced the call with no declaring type at all, for example a template missing Two comments still describe the forced import (in |
maybeAddImport(fqn, false)adds the import even when nothing in the result references the type. It hides templates that produce untyped code, and it can leave an unused import behind when the recipe ends up not inserting the code. Most of these calls are no longer needed: the templates already have a classpath, or the recipe sets the types itself (AssertThrowsOnLastStatement,JMockitBlockRewriter). The plainmaybeAddImportkeeps every import the tests expect.This drops the
falseargument from 47 calls in 27 files. That covers the AssertJJUnit*ToAssertThatrecipes andJUnitFailToAssertJFail, the TestNG-to-AssertJ recipes, the JMockit rewriters,AssertThrowsOnLastStatement,UpdateTestAnnotation,ExpectedExceptionToAssertThrows,TruthAssertWithMessageToAssertJandAnyStringToNullable.MockitoWhenOnStaticToMockStaticneeded two more changes for theMockedStaticimport. The template that adds theMockedStatic<A>field now has themockito-core-5classpath. The recipe also requests the import after it adds the field, because the earlierAddImportran before the field existed.Skipped:
MockitoWhenOnStaticToMockStatickeeps the forced import of the stubbed class. That class is usually a project type, so the template parser can't resolve it, andmockStatic(A.class)stays untyped. Setting the identifier types by hand only exposes the missingmockStaticmethod type. This is the same gap the test'sidentifiers(false)relaxation covers.AssertToAssertionskeeps its forced wildcardAssertions.*import. It swaps an existing wildcard import, andmissingTypeInfo()checks that this still happens when the code doesn't type-check.WhiteboxToReflectionVisitoris unchanged, as its forced imports have a documented reason.Verified with
./gradlew teston theassertj,testng,jmockit,truth,mockito,junit5,junit6,junitandhamcresttest packages. Those packages hold the touched recipes' tests and the JUnit-to-AssertJ, TestNG-to-AssertJ, JMockit-to-Mockito, Mockito and JUnit 4-to-5 migration tests. The first run (1589 tests) showed 23 failures, all inMockitoWhenOnStaticToMockStaticTestandAssertToAssertionsTest. Those are the spots fixed or kept above, and both classes now pass along with the other packages (769 tests, 0 failures).Fixes item N2 of #1211.