solven-eu / cleanthat

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

Add all mutators #748

Closed katzuv closed 9 months ago

katzuv commented 9 months ago

I'm using cleanthat with Spotless in a Gradle project. Is there a way to include all mutators? I don't want to add each one manually.

blacelle commented 9 months ago

This can typically be achieved with pre-existing composite mutators:

cleanthat()
    [...]
    .addMutator('SafeAndConsensual')
    .addMutator('SafeButNotConsensual')
    .addMutator('SafeButControversial')

SafeButControversial holds more advanced but not fitting all projects mutators than SafeButNotConsensual.

If you're adventurous enough, you can include rules not considered safe:

cleanthat()
    [...]
    .addMutator('AllIncludingDraftSingleMutators')
    .includeDraft(false) 

If you're very adventurous, you can include draft rules, and we'll be happy to work on the issues you would encounter:

cleanthat()
    [...]
    .addMutator('AllIncludingDraftSingleMutators')
    .excludeMutator('UseCollectionIsEmpty')
    .includeDraft(true) 

Anyway, you can always exclude specific mutators (would they not fit your needs, or lead to issues over your codebase):

cleanthat()
    [...]
    .addMutator('AllIncludingDraftSingleMutators')
    .excludeMutator('SomeRuleId')
    .includeDraft(true) 

The list of mutators is available at https://github.com/solven-eu/cleanthat/blob/master/MUTATORS.generated.MD#composite-mutators

katzuv commented 9 months ago

Thanks, that's great! Could you explain how addMutator('AllIncludingDraftSingleMutators') includes drafts but then you add includeDraft(false)? Does "draft" mean different things in these two functions?

blacelle commented 9 months ago

For some maintenance argument, we introduced an isDraft filter. It is false by default to automatically exclude any draft mutator. If turned to true, draft mutators are included. Hence, isDraft=true is the way to go include draft mutators.

AllIncludingDraftSingleMutators is a composite mutator, named this way to underline draft mutators are also included. But it does not superseed isDraft toggle. With AllIncludingDraftSingleMutators, draft mutators are all candiates, but the isDraft toggle will later decide to filter them or not.

We may have introduced AllIncludingDraftSingleMutators and AllNotIncludingDraftSingleMutators, but isDraft toggle would still be useful to handle inclusion/exclusion from other composite mutators (e.g. PMDMutator: we would not want to manage PMDIncludingDraftMutators and PMDExcludingDraftMutators).

Hope it makes such design choices a bit clearer.

katzuv commented 9 months ago

Yes, that's clearer now. So if we want all mutators, including "aggressive" ones, but excluding drafts, we should use AllNotIncludingDraftSingleMutators?

blacelle commented 9 months ago

Yes:

cleanthat()
    [...]
    .addMutator('AllIncludingDraftSingleMutators')
katzuv commented 9 months ago

Thank you very much! Perhaps this is something that should be documented in the README file