eclipse-jdt / eclipse.jdt.core

Eclipse Public License 2.0
160 stars 129 forks source link

var and method reference - unnecessary cast warning (missing?) #2762

Closed carstenartur closed 2 months ago

carstenartur commented 2 months ago

eclipse.buildId=4.33.0.20240711-1200 java.version=22.0.1 java.vendor=Eclipse Adoptium BootLoader constants: OS=macosx, ARCH=x86_64, WS=cocoa, NL=de_DE

Working on a bug in a jdt.ui cleanup to make use of "var" I experienced some strange behavior. See https://github.com/eclipse-jdt/eclipse.jdt.ui/issues/1524

    public interface MyIntf  {
        void bla(Object... o);
    }
    /**
     * @param o
     */
    private  void bla(Object... o) {

    }

    private void blubb() {
        new ArrayList<Object>().forEach((o) -> bla(o));
        var myin=(MyIntf)this::bla;
    }

When I mark the right side of the line with "var" and extract as lokal variable I get this:

        MyIntf myIntf= (MyIntf)this::bla;
        var myin=myIntf;

In the resulting line the cast is not marked as unneeded. I can remove it without getting an error:

        MyIntf myIntf= this::bla;
        var myin=myIntf;

The question is: is this to be expected?

stephan-herrmann commented 2 months ago

[...]

      MyIntf myIntf= (MyIntf)this::bla;
      var myin=myIntf;

In the resulting line the cast is not marked as unneeded. I can remove it without getting an error:

      MyIntf myIntf= this::bla;
      var myin=myIntf;

The question is: is this to be expected?

I'm afraid, yes. This situation is not covered by normal detection of unnecessary casts, because:

If deemed relevant, someone would have to implement an additional analysis, that detects when cast type and the expected type at its location are the same type.

carstenartur commented 2 months ago

Don't know if such additional analysis is necessary. As the cleanup that exchanges explicit type by var causes a "Method reference needs an explicit target-type" error we exclude var from methods references now in the cleanup. That is the right thing to do, isn't it?

carstenartur commented 2 months ago

Please advise, should I close this issue?

stephan-herrmann commented 2 months ago

As the cleanup that exchanges explicit type by var causes a "Method reference needs an explicit target-type" error we exclude var from methods references now in the cleanup.

"we" == the cleanup implementation in jdt.ui?

That is the right thing to do, isn't it?

Sure, anything that removes the target type of a method reference or lambda breaks compilation.

As you're not interested in additional cast-analysis, I'm closing this issue.

carstenartur commented 2 months ago

As the cleanup that exchanges explicit type by var causes a "Method reference needs an explicit target-type" error we exclude var from methods references now in the cleanup.

"we" == the cleanup implementation in jdt.ui?

Yes, the proposed fix from Jeff

https://github.com/eclipse-jdt/eclipse.jdt.ui/pull/1541

That is the right thing to do, isn't it?

Sure, anything that removes the target type of a method reference or lambda breaks compilation.

Ok

As you're not interested in additional cast-analysis, I'm closing this issue.

Hope you do not get it wrong. I had a concrete issue in the cleanup that I would like to see fixed and doubt that I can do the job to implement the additional analysis.

Thanks for sharing your thoughts. That is really helpful.