solven-eu / cleanthat

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

UnnecessaryModifier mutator misses several opportunities to remove redundant modifiers #846

Open mches opened 1 week ago

mches commented 1 week ago

To my knowledge, these cases are not covered

blacelle commented 1 week ago

Many of these are covered in https://github.com/solven-eu/cleanthat/pull/848

static and final could be removed from a nested record

Done in #848. These are applied even if the record is not nested.


static could be removed from a nested enum

It is already covered. (right?)


static and abstract could be removed from a nested interface or annotation

I do not understand the case.

public enum Person {
    public static class SomeClass {

    }
}

does not compile.


static and abstract could be removed from a nested interface or annotation

I'm adding:

public record SomeRecord() {
    public static abstract interface SomeClass {

    }

    public static abstract @interface SomeAnnotation {

    }
}
mches commented 1 week ago

I do not understand the case.

public enum Person {
  public static class SomeClass {

  }
}

does not compile.

Try


  public enum Person {
    ;

    public static class SomeClass {}
  }
mches commented 1 week ago

static could be removed from a nested enum

It is already covered. (right?)

I don't think it was covered for a parent other than interface or annotation

mches commented 1 week ago

I updated the description to cover class as a parent, in addition to enum and record.

Put another way

blacelle commented 1 week ago

I don't think it was covered for a parent other than interface or annotation

I indeed reworked UnnecessaryModifier to handle various cases where the grandParentNode is irrelevant (e.g. an enum is always static).

static can be removed from an interface, annotation, enum, or record, regardless of parent

The reworked implementation indeed follows this wording.


static can be removed from an interface, annotation, enum, or record, regardless of parent

Done

abstract can be removed from an interface or annotation, regardless of parent

Done

final can be removed from a record, regardless of parent

Done

mches commented 1 week ago

I had a chance to review. Looks both thorough and simplified. I expanded your new test case in PR #850, and identified one more opportunity to remove the redundant final keyword from private methods.