rewrite-static-analysis icon indicating copy to clipboard operation
rewrite-static-analysis copied to clipboard

`org.openrewrite.staticanalysis.RemoveRedundantTypeCast` fails to consider generics correctly

Open blipper opened this issue 1 year ago • 5 comments

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 avatar Oct 03 '24 02:10 blipper

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());
               }
            }
            """));
  }
}

protocol7 avatar Mar 25 '25 16:03 protocol7

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?

timtebeek avatar Mar 26 '25 12:03 timtebeek

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.

protocol7 avatar Mar 26 '25 21:03 protocol7

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. 🙏🏻

timtebeek avatar Mar 26 '25 21:03 timtebeek

Sorry for the slow reply, but here's a PR with the failing test case: https://github.com/openrewrite/rewrite-static-analysis/pull/495

protocol7 avatar Apr 06 '25 11:04 protocol7

  • Fixed in https://github.com/openrewrite/rewrite-static-analysis/pull/495

timtebeek avatar Jul 21 '25 22:07 timtebeek