Skip to content

UnnecessaryExplicitTypeArguments: retain load-bearing witness in argument position on instance methods too - #946

Merged
timtebeek merged 1 commit into
openrewrite:mainfrom
neil-mushell:neil-mushell/fix-unnecessary-explicit-type-arguments-argument-position
Jul 21, 2026
Merged

timtebeek merged 1 commit into
openrewrite:mainfrom
neil-mushell:neil-mushell/fix-unnecessary-explicit-type-arguments-argument-position

Conversation

@neil-mushell

@neil-mushell neil-mushell commented Jul 21, 2026 •

Copy link
Copy Markdown
Contributor

What's changed?

Generalizes UnnecessaryExplicitTypeArguments's argument-position guard to retain a type witness on instance methods too, not just static ones, when the witness's type variables aren't inferable from the call's own arguments. Removes the now-redundant shouldRetainOnStaticMethod helper in favor of applying canInferTypeArgumentsFromArguments uniformly — the same check already used for the sibling select-position branch.

What's your motivation?

The argument-position branch only guarded static methods (via shouldRetainOnStaticMethod) against removing a witness whose type variables aren't inferable from the call's own arguments. Instance methods used as arguments skipped that guard entirely, so a witness like t.<STATE_VALUE>source() passed to a generic static factory Transition.of(...) got stripped even though STATE_VALUE has no argument to infer it from and the factory itself provides no target type, leaving STATE_VALUE unresolved.

We hit this piloting the recipe against a state-machine interpreter with recursive/self-bounded generics (StateMachineInterpreter<STATE_VALUE extends Enum<STATE_VALUE>, ...>), where it silently broke compilation.

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

Whether folding the argument-position and select-position branches into one shared check is worth a larger refactor — they both now reduce to "retain unless inferable from the call's own arguments" and only diverge afterward on the overload-ambiguity check, which is argument-position-only.

Anyone you would like to review specifically?

No specific request — open to whoever's around.

Have you considered any alternatives or workarounds?

Considered keeping shouldRetainOnStaticMethod's name/shape and just widening its condition (adding || !isStatic or similar). Removed it instead since "static-only" naming became misleading once the check applies to both static and instance methods, and both branches already share canInferTypeArgumentsFromArguments.

Any additional context

Full module suite: 2147 tests, 0 regressions from this change (one pre-existing intermittent failure in AllBranchesIdenticalTest.collapseIdenticalBranchesPython, reproduced independent of this diff — a flaky Python-parser round-trip test unrelated to this recipe). Added both a negative-case test (witness retained, matching the bug) and a positive-case test (witness still removed when inference from the call's own arguments is possible) to make sure the fix doesn't over-correct into blanket retention.

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 used the IntelliJ IDEA auto-formatter on affected files

…ment position on instance methods too

The argument-position branch only guarded static methods (via
shouldRetainOnStaticMethod) against removing a witness whose type
variables aren't inferable from the call's own arguments. Instance
methods used as arguments skipped that guard entirely, so a witness
like `t.<STATE_VALUE>source()` passed to a generic static factory
`Transition.of(...)` got stripped even though STATE_VALUE has no
argument to infer it from and the factory itself provides no target
type, leaving STATE_VALUE unresolved.

Generalize the check to apply regardless of static/instance, matching
the reasoning already used for the sibling select-position case.
@neil-mushell
neil-mushell force-pushed the neil-mushell/fix-unnecessary-explicit-type-arguments-argument-position branch from 5977e5c to 03032b8 Compare July 21, 2026 14:41

@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.

Thanks for coming back here with another round of fixes. Good to have this hardened against a code base with heavy usage.

@github-project-automation github-project-automation Bot moved this from In Progress to Ready to Review in OpenRewrite Jul 21, 2026
@timtebeek
timtebeek merged commit 1b15142 into openrewrite:main Jul 21, 2026
1 check passed
@github-project-automation github-project-automation Bot moved this from Ready to Review to Done in OpenRewrite Jul 21, 2026
mergify Bot added a commit to robfrank/linklift that referenced this pull request Aug 4, 2026
… 2.39.0 to 2.40.0 [skip ci]

Bumps [org.openrewrite.recipe:rewrite-static-analysis](https://github.com/openrewrite/rewrite-static-analysis) from 2.39.0 to 2.40.0.
Release notes

*Sourced from [org.openrewrite.recipe:rewrite-static-analysis's releases](https://github.com/openrewrite/rewrite-static-analysis/releases).*

> 2.40.0
> ------
>
> What's Changed
> --------------
>
> * Retain UnnecessaryThrows on overridable methods ([#939](https://redirect.github.com/openrewrite/rewrite-static-analysis/issues/939)) by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-static-analysis#940](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/940)
> * Qualify `this` for outer-class methods in ReplaceLambdaWithMethodReference by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-static-analysis#942](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/942)
> * FixStringFormatExpressions: don't strip args with no matched specifiers by [`@​neil-mushell`](https://github.com/neil-mushell) in [openrewrite/rewrite-static-analysis#943](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/943)
> * UnnecessaryExplicitTypeArguments: retain load-bearing witness in argument position on instance methods too by [`@​neil-mushell`](https://github.com/neil-mushell) in [openrewrite/rewrite-static-analysis#946](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/946)
> * Set Develocity `projectId` to "openrewrite" by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-static-analysis#947](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/947)
> * Add recipe for finding public methods missing javadoc by [`@​sambsnyd`](https://github.com/sambsnyd) in [openrewrite/rewrite-static-analysis#948](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/948)
> * Add recipes for migrating off a variety of legacy collection types by [`@​sambsnyd`](https://github.com/sambsnyd) in [openrewrite/rewrite-static-analysis#949](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/949)
> * exception-without-cause by [`@​sambsnyd`](https://github.com/sambsnyd) in [openrewrite/rewrite-static-analysis#950](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/950)
> * Do not wrap operators onto a new line in Kotlin sources by [`@​vlsi`](https://github.com/vlsi) in [openrewrite/rewrite-static-analysis#936](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/936)
> * Do not remove casts from raw types to their parameterized form by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-static-analysis#954](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/954)
> * Forward Code Genome Project publish credentials to `ci-gradle.yml` by [`@​jkschneider`](https://github.com/jkschneider) in [openrewrite/rewrite-static-analysis#955](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/955)
>
> New Contributors
> ----------------
>
> * [`@​neil-mushell`](https://github.com/neil-mushell) made their first contribution in [openrewrite/rewrite-static-analysis#943](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/943)
> * [`@​vlsi`](https://github.com/vlsi) made their first contribution in [openrewrite/rewrite-static-analysis#936](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/936)
>
> **Full Changelog**: <openrewrite/rewrite-static-analysis@v2.39.0...v2.40.0>


Commits

* [`00641d2`](openrewrite/rewrite-static-analysis@00641d2) Forward Code Genome Project publish credentials to `ci-gradle.yml` ([#955](https://redirect.github.com/openrewrite/rewrite-static-analysis/issues/955))
* [`95224f1`](openrewrite/rewrite-static-analysis@95224f1) Fix test
* [`39215cf`](openrewrite/rewrite-static-analysis@39215cf) Trim data table input
* [`e820f02`](openrewrite/rewrite-static-analysis@e820f02) Do not remove casts from raw types to their parameterized form ([#954](https://redirect.github.com/openrewrite/rewrite-static-analysis/issues/954))
* [`5545efa`](openrewrite/rewrite-static-analysis@5545efa) Do not wrap operators onto a new line in Kotlin sources ([#936](https://redirect.github.com/openrewrite/rewrite-static-analysis/issues/936))
* [`8a88b48`](openrewrite/rewrite-static-analysis@8a88b48) exception-without-cause ([#950](https://redirect.github.com/openrewrite/rewrite-static-analysis/issues/950))
* [`9188c13`](openrewrite/rewrite-static-analysis@9188c13) Add recipes for migrating off a variety of legacy collection types ([#949](https://redirect.github.com/openrewrite/rewrite-static-analysis/issues/949))
* [`bcf245c`](openrewrite/rewrite-static-analysis@bcf245c) Add recipe for finding public methods missing javadoc ([#948](https://redirect.github.com/openrewrite/rewrite-static-analysis/issues/948))
* [`8e4b8c8`](openrewrite/rewrite-static-analysis@8e4b8c8) Set Develocity `projectId` to "openrewrite" ([#947](https://redirect.github.com/openrewrite/rewrite-static-analysis/issues/947))
* [`13da514`](openrewrite/rewrite-static-analysis@13da514) Move Develocity to community.develocity.cloud
* Additional commits viewable in [compare view](openrewrite/rewrite-static-analysis@v2.39.0...v2.40.0)
  
[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility\_score?dependency-name=org.openrewrite.recipe:rewrite-static-analysis&package-manager=maven&previous-version=2.39.0&new-version=2.40.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)
Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.
[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)
---
Dependabot commands and options
  
You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot show  ignore conditions` will show all of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
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.

2 participants