Skip to content

Skip declaration-site variance for invariant storage - #1007

Draft
martinfrancois wants to merge 1 commit into
openrewrite:mainfrom
martinfrancois:agent/repro-declaration-site-variance-invariant-field
Draft

Skip declaration-site variance for invariant storage#1007
martinfrancois wants to merge 1 commit into
openrewrite:mainfrom
martinfrancois:agent/repro-declaration-site-variance-invariant-field

Conversation

@martinfrancois

@martinfrancois martinfrancois commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Suggested review order: 11 of 52 (Score: 7.5)
Review first: #990

What's changed?

Leaves a declaration-site type unchanged when the parameter is stored directly in an invariant field or explicitly typed local variable. Parameters remain eligible when their destination already accepts the proposed variance or when an adapter expression is stored instead.

What's your motivation?

Recipe: org.openrewrite.staticanalysis.CommonDeclarationSiteTypeVariances.

I found this by running org.openrewrite.staticanalysis.CommonDeclarationSiteTypeVariances, which configures DeclarationSiteTypeVariance, from org.openrewrite.recipe:rewrite-static-analysis:2.39.0 on ConfigResolver.java in Symphony-Trello at a8013f27. I reproduced the same result with the latest released recipe artifact, org.openrewrite.recipe:rewrite-static-analysis:2.41.0. The recipe changed 18 files, and ./mvnw -DskipTests compile then reported four incompatible-type errors.

Before

private final Function<String, Optional<String>> environmentResolver;

public ConfigResolver(Function<String, Optional<String>> environmentResolver) {
    this.environmentResolver = environmentResolver;
}

Actual after the recipe

private final Function<String, Optional<String>> environmentResolver;

public ConfigResolver(Function<? super String, ? extends Optional<String>> environmentResolver) {
    this.environmentResolver = environmentResolver;
}

Expected after the recipe

(unchanged)

The wildcarded parameter is not assignable to the invariant field. The recipe therefore turns compiling production code into code that does not compile.

Confirmed real-world execution

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

Please review the direct-storage boundary. Tim te Beek preferred this approach: skip variance where direct invariant storage makes it invalid, while retaining valid conversions instead of widening unrelated fields.

Have you considered any alternatives or workarounds?

Widening the destination field would expand the recipe beyond declaration-site parameter variance and change APIs and later uses. Skipping only the invalid parameter conversion keeps the change local.

Any additional context

Pre-existing tests changed: None.

  • Target commit: martinfrancois/symphony-trello@a8013f27
  • Discovery release: org.openrewrite.recipe:rewrite-static-analysis:2.39.0
  • Latest verification release: org.openrewrite.recipe:rewrite-static-analysis:2.41.0
  • Latest-release compile failures: ConfigResolver, TrelloBoardSetup, GitHubConfigurator, and SetupSystemProperties
  • Focused tests: DeclarationSiteTypeVarianceTest

This change was prepared with AI assistance. I reviewed the target execution evidence, implementation, tests, and contribution text.

Checklist

@github-project-automation github-project-automation Bot moved this to In Progress in OpenRewrite Aug 15, 2026
@martinfrancois martinfrancois changed the title Do not add declaration-site variance to parameters stored invariantly Skip declaration-site variance for invariant storage Aug 15, 2026
@martinfrancois
martinfrancois force-pushed the agent/repro-declaration-site-variance-invariant-field branch 2 times, most recently from 293c634 to f5bb241 Compare August 16, 2026 02:53
@martinfrancois
martinfrancois force-pushed the agent/repro-declaration-site-variance-invariant-field branch from f5bb241 to ca9a29f Compare August 16, 2026 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

1 participant