Visitar URL original
Do not pull Supertype-Cast to instanceof by dnl50 · Pull Request #577 · openrewrite/rewrite-static-analysis · GitHub
Skip to content

Do not pull Supertype-Cast to instanceof - #577

Merged
sambsnyd merged 1 commit into
openrewrite:mainfrom
dnl50:GH-572
May 29, 2025
Merged

sambsnyd merged 1 commit into
openrewrite:mainfrom
dnl50:GH-572

Conversation

@dnl50

@dnl50 dnl50 commented May 29, 2025

Copy link
Copy Markdown
Contributor

When the cast type expression is a supertype of the instanceof type, the recipe should back off. The code semantics could change otherwise, because more instances can potentially pass the check. Fixes GH-572.

What's changed?

Type casts after instanceof checks which use a different type than the one used in the instanceof check itself will now be ignored since changing the instanceof type might alter the code semantics.

What's your motivation?

Issue #572

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

The isCheckedCastCompatible might be extended even further to take generic type variables into account. Cases like these could also be migrated, since the type variables are the same:

// java 11
class A<T> {
   void test(List<T> o) {
       if (o instanceof ArrayList<?>) {
           ((ArrayList<T>) o).trimToSize();
       }
   }
}

// java 16+ where the reifiable type-restriction was lifted
class A<T> {
   void test(List<T> o) {
       if (o instanceof ArrayList<T> list) {
           list.trimToSize();
       }
   }
}

The isCheckedCastCompatible might be a duplicate of existing code, but I didn't find any publicly accessible methods which perform such checks ☹️

Have you considered any alternatives or workarounds?

In the first try I just flipped the type TypeUtils#isAssignableTo parameters in the registerTypeCast method, but then subtype casts were pulled up to the instanceof check, resulting in the same problem of changed semantics.

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, since the formatting differs a bit from the existing code when using the IntelliJ formatter

When the cast type expression is a supertype of the instanceof type,
the recipe should back off. The code semantics could change
otherwise, because more instances can potentially pass the check.
Fixes openrewriteGH-572.
@github-project-automation github-project-automation Bot moved this to In Progress in OpenRewrite May 29, 2025

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some suggestions could not be made:

  • src/main/java/org/openrewrite/staticanalysis/CustomImportOrder.java
    • lines 24-24

@sambsnyd
sambsnyd merged commit 3ec7c4a into openrewrite:main May 29, 2025
@github-project-automation github-project-automation Bot moved this from In Progress to Done in OpenRewrite May 29, 2025
@sambsnyd

Copy link
Copy Markdown
Member

Thanks @dnl50 !

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.

InstanceofPatternMatching recipe prefers type of cast instead of instanceof check

2 participants