solven-eu / cleanthat

Github App opening automatically cleaning PR
56 stars 16 forks source link

The RemoveExplicitCallToSuper mutator removes important this(); calls #804

Closed shrugalic closed 6 months ago

shrugalic commented 7 months ago

Setup

We just added cleanthat to our Gradle build of a Java 17 project, via spotless (v6.25.0), including the SafeButNotConsensual mutator:

cleanthat()
    .version("2.18")
    .sourceCompatibility("17")
    .addMutator("SafeButNotConsensual")

Issue / How to reproduce

The RemoveExplicitCallToSuper mutator produces erroneous code that does not compile. Here's an example:

public class Sample {

    private String stringField;

    private final int intField;

    public Sample() {
        stringField = "default";
        intField = 42;
    }

    public Sample(String otherValue) {
        this(); // This will be removed by RemoveExplicitCallToSuper and not compile afterwards
        stringField = otherValue;
    }
}

Expecation

The this(); call is important and must remain.

Issue Localization

When adding .excludeMutator("RemoveExplicitCallToSuper") to the configuration, it does not happen.

Superficial look

I'm not at all familiar with this code base, and haven't looked into the details, but from a superficial look it seems removing just any old firstStatement in a constructor seems a bit optimistic?

blacelle commented 7 months ago

Thanks for the report.

but from a superficial look it seems removing just any old firstStatement in a constructor seems a bit optimistic?

I confirm this statement, and this issue. It has been detected some time ago, and it seems it is not fixed yet.

blacelle commented 6 months ago

This is fixed in cleanthat 2.19. I dropped constructor reference to this() as it looks like a plain misbehavior.

shrugalic commented 6 months ago

I successfully verified that the issue was fixed, thanks a lot. 🙏