[martinfrancois]

Suggested review order: 17 of 35 (Score: 3)
Review first: openrewrite/rewrite-static-analysis#972

What's changed?

Adds 2 known-failing tests to JSpecifyBestPracticesTest that reproduce a gap in org.openrewrite.java.jspecify.JSpecifyBestPractices: on a project with no nullness library at all, the recipe inserts JSpecify annotations but does not add the org.jspecify:jspecify dependency. No recipe code changes. The tests are marked @ExpectedToFail so the suite stays green; removing the mark shows the failure. On main the class has 6 tests, all passing. With my additions it has 8; the 2 new ones report as skipped, 0 failures.

What's your motivation?

Recipe: the JSpecify migration composites that generate org.jspecify.annotations references.

Before

The project has a plain pom with no dependencies and this source:

public class Test {

    public String getString() {
        return null;
    }
}

Actual after the recipe

Using current main, the Java file gains import org.jspecify.annotations.Nullable; and the method becomes public @Nullable String getString(), but the pom stays byte identical to its input:

<project>
    <modelVersion>4.0.0</modelVersion>
    <groupId>com.example.foobar</groupId>
    <artifactId>foobar-core</artifactId>
    <version>1.0.0</version>
</project>

Expected after the recipe

The pom MUST gain org.jspecify:jspecify:1.0.0 so the generated org.jspecify.annotations.* references resolve. It does not, in any cycle. Reading jspecify.yml on main shows why: every AddDependency guard uses onlyIfUsing on javax.annotation, jakarta.annotation, org.jetbrains.annotations, org.springframework.lang or io.micronaut.core.annotation patterns, so none can match a project without a prior nullness library, and the annotator steps such as AnnotateNullableMethods carry no AddDependency of their own. Test addJspecifyDependencyWithoutPriorNullnessAnnotations covers this.

The second test, addDependencyForAnnotationsInsertedInSameRun, shows the constraint that makes the obvious fix awkward. A composite of AnnotateNullableMethods followed by AddDependency with onlyIfUsing: org.jspecify.annotations.* does add the dependency, but one cycle late: the onlyIfUsing scan runs against the original sources and does not see annotations inserted earlier in the same cycle. The test harness fails the run for needing an extra cycle; production runs with their extra cycles do converge. #1192 discloses the same limit, which is why its new step guards on the pre-migration annotation instead.

I found this while preparing #1192, which adds a related migration to this recipe. The gap persists on that branch too; the tests here target main.

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

I think the first case is a genuine bug: the recipe writes annotations whose artifact is not on the project's classpath, so the output references a type the build cannot resolve. If you agree this should change, I would gladly prepare the fix. If this behavior is intended, feel free to close this and I know it is settled.

Have you considered any alternatives or workarounds?

Guarding on the pre-migration annotation, as #1192 does for org.springframework.lang, sidesteps the same-cycle blindness, but there is no pre-migration annotation to guard on when the project starts with no nullness library. Accepting the one-cycle-late addition would work in production runs but fails the single-cycle test expectation.

Any additional context

Pre-existing tests changed: None.

The same-cycle blindness itself is framework level, in the scanning phase of openrewrite/rewrite, where no issue exists for it yet. openrewrite/rewrite#6822 is adjacent but distinct: it concerns onlyIfUsing being ignored in declarative recipes, not the visibility of same-run insertions. This reproduction was prepared with AI assistance (Claude Code). I reviewed the tests and this description.

The added reproduction tests and the existing suite together cover changed and unchanged behavior. The known-failing tests remain disabled until implementation. The formatter run was calibrated per file; untouched lines were not reformatted.

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 run ./gradlew build locally, and committed any resulting changes to recipes.csv
• I've formatted the lines I changed, without reformatting code I didn't touch