Skip to content
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

UseVarForObject creates broken code when replacing type variable #550

Open
uhafner opened this issue Sep 12, 2024 · 4 comments
Open

UseVarForObject creates broken code when replacing type variable #550

uhafner opened this issue Sep 12, 2024 · 4 comments
Labels
bug Something isn't working test provided

Comments

@uhafner
Copy link

uhafner commented Sep 12, 2024

What version of OpenRewrite are you using?

I am using

  • <rewrite-maven-plugin.version>5.39.2</rewrite-maven-plugin.version>
  • <rewrite-testing-frameworks.version>2.17.1</rewrite-testing-frameworks.version>
  • <rewrite-static-analysis.version>1.15.0</rewrite-static-analysis.version>
  • <rewrite-migrate-java.version>2.23.0</rewrite-migrate-java.version>
  • <rewrite-recommendations.version>1.8.4</rewrite-recommendations.version>

How are you running OpenRewrite?

I am using the Maven plugin, and my project is a single module project. I did run the command mvn -X clean rewrite:run.

See https://github.com/uhafner/codingstyle for details. The OpenRewrite configuration is visible in the pom.xml.

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

package edu.hm.hafner.util;

import java.io.Serializable;

import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;

public abstract class SerializableTest<T extends Serializable> extends ResourceTest {
    protected abstract T createSerializable();

    @Test
    @DisplayName("should be serializable: instance -> byte array -> instance")
    void shouldBeSerializable() {
        T serializableInstance = createSerializable();
    }
}

What did you expect to see?

Type variable should not be replaced by var.

package edu.hm.hafner.util;

import java.io.Serializable;

import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;

public abstract class SerializableTest<T extends Serializable> extends ResourceTest {
    protected abstract T createSerializable();

    @Test
    @DisplayName("should be serializable: instance -> byte array -> instance")
    void shouldBeSerializable() {
        T serializableInstance = createSerializable();
    }
}

What did you see instead?

package edu.hm.hafner.util;

import java.io.Serializable;

import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;

public abstract class SerializableTest<T extends Serializable> extends ResourceTest {
    protected abstract T createSerializable();

    @Test
    @DisplayName("should be serializable: instance -> byte array -> instance")
    void shouldBeSerializable() {
        var serializableInstance = __P__.<error>;
    }
}

What is the full stack trace of any errors you encountered?

No stack trace produced.

Are you interested in [contributing a fix to OpenRewrite]

Not right now.

@uhafner uhafner added the bug Something isn't working label Sep 12, 2024
@uhafner uhafner changed the title lang.var.UseVarForObject creates broken Java code when replacing with type variable lang.var.UseVarForObject creates broken Java code when replacing type variable Sep 12, 2024
@uhafner uhafner changed the title lang.var.UseVarForObject creates broken Java code when replacing type variable UseVarForObject creates broken Java code when replacing type variable Sep 12, 2024
@uhafner uhafner changed the title UseVarForObject creates broken Java code when replacing type variable UseVarForObject creates broken code when replacing type variable Sep 12, 2024
@MBoegers
Copy link
Contributor

In this case the derivation of the type seems broken. Such change is indeed not correct. The expectation seems right, I'll check what's missing. Thanks for reporting 🙏

@MBoegers
Copy link
Contributor

We can reproduce this with the test below. The issue is caused be the type boundary (extends Serializable) at the generic parameter. Further exploration is needed here.

@Test
    @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/550")
    void genericType() {
        rewriteRun(
          java(
            """
              import java.io.Serializable;
              
              abstract class Outter<T extends Serializable> {
                 abstract T doIt();
                 void trigger() {
                    T x = doIt();
                 }
              }
              """, """
              import java.io.Serializable;
              
              abstract class Outter<T extends Serializable> {
                 abstract T doIt();
                 void trigger() {
                    var x = doIt();
                 }
              }
              """
          )
        );
    }

@MBoegers
Copy link
Contributor

One additional question @uhafner, why are you expecting T instead of var? var is perfectly fine here, from the JLS view.

@uhafner
Copy link
Author

uhafner commented Sep 12, 2024

One additional question @uhafner, why are you expecting T instead of var? var is perfectly fine here, from the JLS view.

You are right, it works with var as well (I never used var for type variables before).

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