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

FinalClass should exempt private-only constructor classes that are extended by another class in the same compilation unit #10859

Closed
JnRouvignac opened this issue Oct 12, 2021 · 8 comments · Fixed by #10968
Milestone

Comments

@JnRouvignac
Copy link

JnRouvignac commented Oct 12, 2021

$ cat src/main/java/Main.java
public final class Main {
    private class SuperClass {
        private SuperClass() {
        }
    }

    private final class SubClass extends SuperClass {
    }
}
$ cat checkstyle.xml 
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
        "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
        "https://checkstyle.org/dtds/configuration_1_3.dtd">
<module name="Checker">
    <module name="TreeWalker">
        <module name="FinalClass"/>
    </module>
</module>
$ cat pom.xml
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
  <modelVersion>4.0.0</modelVersion>
  <groupId>test</groupId>
  <artifactId>test</artifactId>
  <version>1.0-SNAPSHOT</version>
  <name>test checkstyle</name>
  <properties>
    <maven.compiler.source>11</maven.compiler.source>
    <maven.compiler.target>11</maven.compiler.target>
  </properties>
  <build>
    <plugins>
     <plugin>
       <groupId>org.apache.maven.plugins</groupId>
       <artifactId>maven-checkstyle-plugin</artifactId>
       <version>3.1.2</version>
          <dependencies>
            <dependency>
              <groupId>com.puppycrawl.tools</groupId>
              <artifactId>checkstyle</artifactId>
              <version>9.0.1</version>
            </dependency>
          </dependencies>
       <configuration>
         <configLocation>checkstyle.xml</configLocation>
         <encoding>UTF-8</encoding>
         <consoleOutput>true</consoleOutput>
         <failsOnError>true</failsOnError>
         <linkXRef>false</linkXRef>
       </configuration>
       <executions>
         <execution>
           <id>validate</id>
           <phase>validate</phase>
           <goals>
             <goal>check</goal>
           </goals>
         </execution>
       </executions>
     </plugin>
    </plugins>
  </build>
</project>
$ mvn clean install 
[INFO] Scanning for projects...
[INFO] 
[INFO] -----------------------------< test:test >------------------------------
[INFO] Building test checkstyle 1.0-SNAPSHOT
[INFO] --------------------------------[ jar ]---------------------------------
[INFO] 
[INFO] --- maven-clean-plugin:2.5:clean (default-clean) @ test ---
[INFO] Deleting /home/jean-noel/git/checkstyle/target
[INFO] 
[INFO] --- maven-checkstyle-plugin:3.1.2:check (validate) @ test ---
[INFO] Starting audit...
[ERROR] /home/jean-noel/git/checkstyle/src/main/java/Main.java:2:5: Class SuperClass
should be declared as final. [FinalClass]
Audit done.
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  0.809 s
[INFO] Finished at: 2021-10-12T12:08:27+02:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.1.2:check
(validate) on project test: Failed during checkstyle execution: There is 1 error reported
by Checkstyle 9.0.1 with checkstyle.xml ruleset. -> [Help 1]
...

Similarly to #9357, the FinalClass module is doing it's job correctly in that it's detecting a non-final class with only private constructor(s), but it's not smart enough to realize that in this situation, there is another class that extends it in the same java file.
This is allowed because these classes are nestmates.

Following checkstyle advice would result in compilation failure.

@Vyom-Yadav
Copy link
Member

Vyom-Yadav commented Oct 16, 2021

@strkkk @rnveach @pbludov Why isn't this approved yet? I will work on it once approved

@pbludov
Copy link
Member

pbludov commented Oct 16, 2021

I will work on it once approved

Done. I'm able to reproduce this issue with JDK 8 and JDK17 on my PC.

Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Oct 21, 2021
…or classes that are extended by another class in the same compilation unit
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Oct 21, 2021
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Oct 21, 2021
…or classes that are extended by another class in the same compilation unit
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Oct 21, 2021
…or classes that are extended by another class in the same compilation unit
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Oct 21, 2021
…or classes that are extended by another class in the same compilation unit
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Oct 24, 2021
…or classes that are extended by another class in the same compilation unit
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Oct 24, 2021
…or classes that are extended by another class in the same compilation unit
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Oct 24, 2021
…or classes that are extended by another class in the same compilation unit
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Oct 24, 2021
…or classes that are extended by another class in the same compilation unit
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Oct 24, 2021
…or classes that are extended by another class in the same compilation unit
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Oct 24, 2021
…or classes that are extended by another class in the same compilation unit
@romani
Copy link
Member

romani commented Oct 24, 2021

@JnRouvignac , please next time report issue as we request it, without any plugin, just our CLI.

@JnRouvignac
Copy link
Author

Sorry I was a bit in a hurry. I will next time.

Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Oct 26, 2021
…or classes that are extended by another class in the same compilation unit
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Oct 26, 2021
…or classes that are extended by another class in the same compilation unit
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Oct 26, 2021
…or classes that are extended by another class in the same compilation unit
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Oct 26, 2021
…or classes that are extended by another class in the same compilation unit
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Oct 26, 2021
…or classes that are extended by another class in the same compilation unit
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Oct 26, 2021
…or classes that are extended by another class in the same compilation unit
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Oct 26, 2021
…or classes that are extended by another class in the same compilation unit
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Oct 26, 2021
…or classes that are extended by another class in the same compilation unit
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Oct 27, 2021
…or classes that are extended by another class in the same compilation unit
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Oct 28, 2021
…or classes that are extended by another class in the same compilation unit
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Oct 28, 2021
…or classes that are extended by another class in the same compilation unit
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Oct 28, 2021
…or classes that are extended by another class in the same compilation unit
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Oct 28, 2021
…or classes that are extended by another class in the same compilation unit
pbludov added a commit to pbludov/checkstyle that referenced this issue Dec 21, 2021
pbludov added a commit to pbludov/checkstyle that referenced this issue Dec 21, 2021
…ses that are extended by another class in the same CU
pbludov added a commit to pbludov/checkstyle that referenced this issue Dec 21, 2021
pbludov added a commit to pbludov/checkstyle that referenced this issue Dec 21, 2021
…ses that are extended by another class in the same CU
pbludov added a commit to pbludov/checkstyle that referenced this issue Dec 22, 2021
pbludov added a commit to pbludov/checkstyle that referenced this issue Dec 22, 2021
…ses that are extended by another class in the same CU
pbludov added a commit to pbludov/checkstyle that referenced this issue Dec 22, 2021
…ses that are extended by another class in the same CU
github-actions bot pushed a commit to pbludov/checkstyle that referenced this issue Dec 26, 2021
github-actions bot pushed a commit to pbludov/checkstyle that referenced this issue Dec 26, 2021
…ses that are extended by another class in the same CU
pbludov added a commit to pbludov/checkstyle that referenced this issue Dec 30, 2021
pbludov added a commit to pbludov/checkstyle that referenced this issue Dec 30, 2021
…ses that are extended by another class in the same CU
pbludov added a commit to pbludov/checkstyle that referenced this issue Jan 1, 2022
pbludov added a commit to pbludov/checkstyle that referenced this issue Jan 1, 2022
…ses that are extended by another class in the same CU
romani pushed a commit to pbludov/checkstyle that referenced this issue Feb 5, 2022
romani pushed a commit to pbludov/checkstyle that referenced this issue Feb 5, 2022
…ses that are extended by another class in the same CU
pbludov added a commit to pbludov/checkstyle that referenced this issue Feb 7, 2022
pbludov added a commit to pbludov/checkstyle that referenced this issue Feb 7, 2022
…ses that are extended by another class in the same CU
pbludov added a commit to pbludov/checkstyle that referenced this issue Feb 7, 2022
…ses that are extended by another class in the same CU
pbludov added a commit to pbludov/checkstyle that referenced this issue Feb 23, 2022
pbludov added a commit to pbludov/checkstyle that referenced this issue Feb 23, 2022
…ses that are extended by another class in the same CU
pbludov added a commit to pbludov/checkstyle that referenced this issue Feb 26, 2022
pbludov added a commit to pbludov/checkstyle that referenced this issue Feb 26, 2022
…ses that are extended by another class in the same CU
rnveach pushed a commit that referenced this issue Feb 26, 2022
@rnveach
Copy link
Member

rnveach commented Feb 26, 2022

Fix is merged

@rnveach rnveach added the bug label Feb 26, 2022
@rnveach rnveach added this to the 10.0 milestone Feb 26, 2022
@JnRouvignac
Copy link
Author

Thank you very much to all involved!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment