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

UnnecessaryModifier mutator misses several opportunities to remove redundant modifiers #846

Open
mches opened this issue Oct 9, 2024 · 6 comments

Comments

@mches
Copy link
Contributor

mches commented Oct 9, 2024

To my knowledge, these cases are not covered

  • interface or annotation (as a parent)
    • public, static, and final could be removed from a nested record
  • class, enum, or record (as a parent)
    • static and abstract could be removed from a nested interface or annotation
    • static could be removed from a nested enum
    • static and final could be removed from a nested record
  • final could be removed from a private method
@mches mches changed the title UnnecessaryModifier does not fully cover enums and records UnnecessaryModifier mutator does not fully cover enums and records Oct 9, 2024
@blacelle
Copy link
Member

blacelle commented Oct 9, 2024

Many of these are covered in #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
Copy link
Contributor Author

mches commented Oct 9, 2024

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
Copy link
Contributor Author

mches commented Oct 9, 2024

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 mches changed the title UnnecessaryModifier mutator does not fully cover enums and records UnnecessaryModifier mutator does not fully cover classes, enums, and records Oct 9, 2024
@mches
Copy link
Contributor Author

mches commented Oct 9, 2024

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

Put another way

  • static can be removed from an interface, annotation, enum, or record, regardless of parent
  • abstract can be removed from an interface or annotation, regardless of parent
  • final can be removed from a record, regardless of parent

@blacelle
Copy link
Member

blacelle commented Oct 9, 2024

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 added a commit to markitect-dev/cleanthat that referenced this issue Oct 10, 2024
@mches
Copy link
Contributor Author

mches commented Oct 10, 2024

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.

mches added a commit to markitect-dev/cleanthat that referenced this issue Oct 10, 2024
mches added a commit to markitect-dev/cleanthat that referenced this issue Oct 10, 2024
mches added a commit to markitect-dev/cleanthat that referenced this issue Oct 10, 2024
@mches mches changed the title UnnecessaryModifier mutator does not fully cover classes, enums, and records UnnecessaryModifier mutator misses several opportunities to remove redundant modifiers Oct 13, 2024
blacelle pushed a commit that referenced this issue Oct 15, 2024
* Expand test case for issue #846

* Remove redundant final modifier from private methods

* Fix Java source that would not compile
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants