Unexpected warning during spotlessApply: Cannot be 'abstract' and also 'private'.

[shrugalic]

Setup

We recently added cleanthat to our Gradle build of a Java 17 project, via spotless (v6.25.0):

cleanthat()
    .version("2.19")
    .sourceCompatibility("17")
    .addMutator("SafeButNotConsensual")
    .excludeMutator("UseUnderscoresInNumericLiterals")
    .excludeMutator("AvoidInlineConditionals")

Issue

During a ./gradlew spotlessApply we get the following warning:

> Task :server:spotlessJava
eu.solven.cleanthat.engine.java.refactorer.mutators.UnnecessaryModifier@61dd5e74 generated invalid code over cleanthat/path_is_not_available
Issue parsing some source. 1 problems. First problem: (line 60,col 3) Cannot be 'abstract' and also 'private'.
Not able to parse path='cleanthat/path_is_not_available' with com.github.javaparser.JavaParser@4a8d2db4

BUILD SUCCESSFUL in 37s

Unfortunately, I was unable to reproduce the issue. I did find a few private.*abstract things in our code base, and removed the private part in those, but it didn't change anything: this message still appears during a build.

[shrugalic]

Possibly related: Originally, the warning was preceded by two other lines:

eu.solven.cleanthat.engine.java.refactorer.mutators.UseIndexOfChar@57f566f8 generated invalid code over cleanthat/path_is_not_available
eu.solven.cleanthat.engine.java.refactorer.mutators.UnnecessaryModifier@7edec166 generated invalid code over cleanthat/path_is_not_available

I did find some unresolved instances of someString.indexOf("c") which I manually replaced with someString.indexOf('c'), after which the UseIndexOfChar line disappeared.

Similarly, I found some instances of static inner interfaces, from which I removed the unnecessary implicit static key word. I did not find any public static final methods or constants in interfaces. Regardless of my manual changes, the UnnecessaryModifier line remains in the output, so it seems as if cleanThat thinks there is more to be found.

[blacelle]

Thanks for the report. Do you get additional details with debug logs?

[shrugalic]

Do you get additional details with debug logs?

I just searched for how to enable debug logs in cleanThat or spotless, but was unable to find any info. Do you happen to have any pointers as to how to enable them?

[blacelle]

I would activate standard gradle debug logs.

[shrugalic]

According to the Gradle Logging documentation adding --debug to the ./gradlew spotlessApply comment logs a lot more info (other less verbose logging levels are also available).

I'll have to spend some time filtering the 600K+ Log lines. ;)

[blacelle]

I'll try to improve theses cases of cleanthat/path_is_not_available, which is a placeholder when the original path is not easily available.

[blacelle]

Unfortunately, I was unable to reproduce the issue. I did find a few private.*abstract things in our code base, and removed the private part in those, but it didn't change anything: this message still appears during a build.

I wonder if cleanthat may sometimes produce such invalid code, but it is not persisted due to the fact the output code would be invalid,

[blacelle]

In normal logs, do you see files modified by UnnecessaryModifier ?

[shrugalic]

Hmm, it appears I'm somewhat consistently able to reproduce the issue. Edit: Only in my very large repo with thousands of files where an issue could be lurking.

In my case I created this example of unnecessary public static final modifiers:

public class Sample {
    public static interface TestInterface {
        public static final String TEST_CONSTANT = "test";
    }
}

With this, I got this message:

> Task :server:spotlessJava
eu.solven.cleanthat.engine.java.refactorer.mutators.UnnecessaryModifier@4f71e78d generated invalid code over cleanthat/path_is_not_available
Issue parsing some source. 1 problems. First problem: (line 60,col 3) Cannot be 'abstract' and also 'private'.
Not able to parse path='cleanthat/path_is_not_available' with com.github.javaparser.JavaParser@276ae05c

Despite the error message, the code is cleaned up, and the next time the message will not re-appear. Edit: Correction, it will re-appear if a file changes, my bad.

[shrugalic]

I wonder if cleanthat may sometimes produce such invalid code, but it is not persisted due to the fact the output code would be invalid

That seems very plausible indeed.

In normal logs, do you see files modified by UnnecessaryModifier?

Yes. Like you wrote, I assume there are still issues it would want to fix, but can't, and does not persist anything.

Issues it can fix are fixed, such as the sample in my previous comment.

[ergon-veenj]

Hi, I happened to invest some more time to analyse this. The problem comes from the upstream parser used by cleanthat and they have fixed it already, see this issue. I think upgrading the parser to >= 3.25.10 will fix this for cleanthat as well.

For reference: I came up with this minimal interface reproducing the issue (strange formatting to tease our reformatter):

public interface A {
    private void test()
    {}
}

This triggers the !parsed.isSuccessful() branch in eu.solven.cleanthat.engine.java.refactorer.JavaRefactorer.parseSourceCode() with error Parsing failed: (line 2,col 2) Cannot be 'abstract' and also 'private'. on a properly formatted version of the input file (local var sourceCode):

public interface A {
    private void test() {
    }
}

[blacelle]

Thanks for the details. I'll make a new release very soon.

[blacelle]

This is fixed in Cleanthat 2.20. Thanks a lot for the report and the investigation.

[shrugalic]

Thanks. It seems v2.20 was released, but without listing the changes.

[shrugalic]

With the new version, I get this message:

> Task :server:spotlessJava
eu.solven.cleanthat.engine.java.refactorer.mutators.UnnecessaryModifier@655a1146 generated invalid code over cleanthat/path_is_not_available

where previously I got this:

> Task :server:spotlessJava
eu.solven.cleanthat.engine.java.refactorer.mutators.UnnecessaryModifier@7c656082 generated invalid code over cleanthat/path_is_not_available
Issue parsing some source. 1 problems. First problem: (line 60,col 3) Cannot be 'abstract' and also 'private'.
Not able to parse path='cleanthat/path_is_not_available' with com.github.javaparser.JavaParser@1a24d83f

[blacelle]

@shrugalic Have you been able to identity the faulty file? If yes, could you provide it? Thanks

[shrugalic]

I have not.

[blacelle]

The lack of proper file path in logs was related to Spotless integration. An upcoming Spotless release should make it much easier to get which file is faulty.

[shrugalic]

Current state of affairs AFAICS:

• Spotless 7.0 will propagate better error messages from CleanChat, but is currently in beta and not final yet (CHANGES.md)
• The better error messages are part of CleanThat v2.20 (which seems to be missing from its own CHANGES.md)

I checked again because we have a new issue that I cannot localize without knowing where it's from (too large of a code base):

Task :server:spotlessJava eu.solven.cleanthat.engine.java.refactorer.mutators.UnnecessaryModifier@f0988a2 generated invalid code over cleanthat/path_is_not_available Issue parsing some source. 1 problems. First problem: (line 66,col 3) There is no such thing as a local interface. Pay attention that this feature is supported starting from 'JAVA_16' language level. If you need that feature the language level must be configured in the configuration before parsing the source files. Not able to parse path='cleanthat/path_is_not_available' with com.github.javaparser.JavaParser@18983f95

[blacelle]

Could you have a try with Spotless.7.0 even in BETA, as it may provide enough information to find the underlying cleanthat issue?

[shrugalic]

@blacelle: What version of cleanthat would I enter into my build files? As mentioned there are no versions newer than 2.19 listed in the cleanthat change log. Version 2.20 works anyway, but when I try 2.21 (thinking it should be a newer version than the 2.20 we currently use) like so:

cleanthat()
    .version("2.21")
    .sourceCompatibility(javaVersion.toString())
    .addMutator("SafeButNotConsensual")
    .excludeMutator("UseUnderscoresInNumericLiterals")
    .excludeMutator("AvoidInlineConditionals")
endWithNewline()
indentWithTabs()
trimTrailingWhitespace()
encoding("UTF-8")
toggleOffOn("@formatter:off", "@formatter:on")

it gives a build error:

16: Task failed with an exception.
-----------
* What went wrong:
Execution failed for task ':spotlessJava'.
> You need to add a repository containing the '[io.github.solven-eu.cleanthat:java:2.21]' artifact in 'build.gradle'.
  E.g.: 'repositories { mavenCentral() }'

Update

I just removed the version alltogether, while using version 7.0.0.BETA1 of spotless, that works.

The code that produces this error message:

eu.solven.cleanthat.engine.java.refactorer.mutators.UnnecessaryModifier@30add310 generated invalid code over <path/to/the/following/file>

is this:

import java.net.URI;                                                                                                                                            
import java.util.List;                                                                                                                                          

import jakarta.validation.Valid;                                                                                                                                
import jakarta.validation.constraints.NotNull;                                                                                                                  

import org.openapitools.jackson.nullable.JsonNullable;                                                                                                                                                                                                                             

public interface CustomerDataView {                                                                                                                             
    public JsonNullable<String> getName();                                                                                                                      

    public String getStatus();                                                                                                                                  

    public String getStatusReason();                                                                                                                            

    public @Valid List<AccountRef> getAccount();                                                                                                                

    public @Valid List<AgreementRef> getAgreement();                                                                                                            

    public @Valid List<Characteristic> getCharacteristic();                                                                                                     

    public @Valid List<ContactMedium> getContactMedium();                                                                                                       

    public @Valid List<CreditProfile> getCreditProfile();                                                                                                       

    public @NotNull @Valid RelatedParty getEngagedParty();                                                                                                      

    public @Valid List<PaymentMethod> getPaymentMethod();                                                                                                       

    public @Valid List<RelatedParty> getRelatedParty();                                                                                                         

    public @Valid TimePeriod getValidFor();                                                                                                                     

    public String getAtBaseType();                                                                                                                              

    public @Valid URI getAtSchemaLocation();                                                                                                                    

    public String getAtType();                                                                                                                                  
}

The public modifiers are indeed unnecessary, so I'll remove them manually.

[shrugalic]

Because of the new better error messages I was able to solve the issue causing the problem, so I'll close this issue.

Thanks a lot for all the help!

[blacelle]

Oh nice. Given you reported eu.solven.cleanthat.engine.java.refactorer.mutators.UnnecessaryModifier@30add310 generated invalid code over <path/to/the/following/file>, I thought the actual file path was still not reported.

Thanks for the validation, and sorry for the friction in fixing it.

Regarding the UnnecessaryModifier issue o nyour case, I do not foresee why it happens, but I hope your case will help me understand what's wrong so I can fix it.

[shrugalic]

The file path was indeed reported, I just replaced it with something that didn't reveal information I didn't want it to. ;)

I hope the file I posted helps reproduce the issue. Maybe the unnecessary-modifier-removal-code has issues with the extra annotations in this case.

[blacelle]

Mmm. This was due to some JavaParser issue. Post-modification, the source is like:

public interface Issue807 {                                                                                                                                                                                                                                                                                     
     @ValidList<AccountRef> getAccount();                                                                                                                                                                                                                                           
}

I'll try to workaround it within cleanthat, at it seems related with https://github.com/javaparser/javaparser/issues/3935