Skip to content

org.openrewrite.staticanalysis.RemoveRedundantTypeCast fails to consider generics correctly #353

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
blipper opened this issue Oct 3, 2024 · 5 comments
Labels
bug Something isn't working test provided

Comments

@blipper
Copy link
Contributor

blipper commented Oct 3, 2024

What is the smallest, simplest way to reproduce the problem?

class A {
    void foo() {
        var a = new HashMap<String, String>();
         Optional.of((Map<String, Object>)a)
                .filter(Predicate.not(Map::isEmpty));
    }
}

What did you expect to see?

class A {
    void foo() {
        var a = new HashMap<String, String>();
         Optional.of((Map<String, Object>)a)
                .filter(Predicate.not(Map::isEmpty));
    }
}

What did you see instead?

class A {
    void foo() {
        var a = new HashMap<String, String>();
         Optional.of(a)
                .filter(Predicate.not(Map::isEmpty));
    }
}

This fails to compile because generics require an exact map on type so Map != HashMap.

@blipper blipper added the bug Something isn't working label Oct 3, 2024
@timtebeek timtebeek changed the title rg.openrewrite.staticanalysis.RemoveRedundantTypeCast fails to consider generics correctly org.openrewrite.staticanalysis.RemoveRedundantTypeCast fails to consider generics correctly Oct 6, 2024
@timtebeek timtebeek moved this to Backlog in OpenRewrite Oct 6, 2024
@protocol7
Copy link
Contributor

We've also run into this, here's a test case that replicates what we're seeing.

  • org.openrewrite:rewrite-core:8.48.1
  • org.openrewrite.recipe:rewrite-static-analysis:2.4.0
  @Test
  void removeRedundantCast() {
    rewriteRun(
        spec -> spec.recipe(new RemoveRedundantTypeCast()),
        // language=java
        java(
            """
            package com.helloworld;
            
            import java.util.Optional;

            public class Foo {
                public interface Bar {}

                public static class BarImpl implements Bar {}

                private Bar getBar() {
                    return new BarImpl();
                }

                private BarImpl getBarImpl() {
                    return new BarImpl();
                }

                public Bar baz() {
                 return Optional.of((Bar) getBarImpl()).orElse(getBar());
               }
            }
            """));
  }
}

@timtebeek
Copy link
Member

Thanks for the runnable report! I think this might be a slightly different case as there's no generics involved, right?

Did you already explore a potential fix by running the recipe and seeing where it decides to remove the cast?

@protocol7
Copy link
Contributor

Ah yes, you are right, my example does not concern generic. Let me know if you prefer that I open a separate ticket. I haven't debugged the recipe further.

@timtebeek
Copy link
Member

If you're up for it a draft PR that adds that test would be most helpful; no expectation that you'll work on a fix, but it makes it so we can easily checkout the PR locally, run and debug. 🙏🏻

@protocol7
Copy link
Contributor

Sorry for the slow reply, but here's a PR with the failing test case: #495

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working test provided
Projects
Status: Backlog
Development

No branches or pull requests

3 participants