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

MethodCountCheck: class counts include anonymous methods #4539

Closed
rnveach opened this Issue Jun 27, 2017 · 4 comments

Comments

Projects
None yet
2 participants
@rnveach
Member

rnveach commented Jun 27, 2017

Identified in regression at #4535 :

$ cat TestClass.java
public class TestClass {
    void method() {
        Runnable r = (new Runnable() {
                public void run() {
                }
            });
    }
}

$ cat TestConfig.xml
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
          "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
          "http://www.puppycrawl.com/dtds/configuration_1_3.dtd">

<module name="Checker">
    <property name="charset" value="UTF-8"/>

    <module name="TreeWalker">
<module name="MethodCountCheck">
  <property name="maxTotal" value="1" />
</module>
    </module>
</module>

$ java -jar checkstyle-7.8.2-all.jar -c TestConfig.xml TestClass.java
Starting audit...
[ERROR] TestClass.java:1: Total number of methods is 2 (max allowed is 1). [MethodCount]
Audit done.
Checkstyle ends with 1 errors.

Method count is set to allow a max of 1 method per class.
Checkstyle incorrectly reports that the class TestClass has 2 methods. It is incorrectly including the anonymous method in the counting of TestClass. I don't think it should be included, as it is a separate class from TestClass.
I am expecting no violations on this code.

Attention: after fix, counts are expected to be lowered, so we should change our checkstyle_checks.xml

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Jul 19, 2017

Member

after fix, counts are expected to be lowered, so we should change our checkstyle_checks.xml

With fix, the lowest we can go without violations (or suppressions) is 34. Current value is 35.
Here are violations with 28:

Starting audit...
[ERROR] \checkstyle\src\main\java\com\puppycrawl\tools\checkstyle\Checker.java:66: Total number of methods is 32 (max allowed is 28). [MethodCount]
[ERROR] \checkstyle\src\main\java\com\puppycrawl\tools\checkstyle\checks\coding\FinalLocalVariableCheck.java:77: Total number of methods is 34 (max allowed is 28). [MethodCount]
[ERROR] \checkstyle\src\main\java\com\puppycrawl\tools\checkstyle\checks\indentation\AbstractExpressionHandler.java:33: Total number of methods is 31 (max allowed is 28). [MethodCount]
[ERROR] \checkstyle\src\main\java\com\puppycrawl\tools\checkstyle\TreeWalker.java:62: Total number of methods is 29 (max allowed is 28). [MethodCount]
Audit done.
Checkstyle ends with 4 errors.

Member

rnveach commented Jul 19, 2017

after fix, counts are expected to be lowered, so we should change our checkstyle_checks.xml

With fix, the lowest we can go without violations (or suppressions) is 34. Current value is 35.
Here are violations with 28:

Starting audit...
[ERROR] \checkstyle\src\main\java\com\puppycrawl\tools\checkstyle\Checker.java:66: Total number of methods is 32 (max allowed is 28). [MethodCount]
[ERROR] \checkstyle\src\main\java\com\puppycrawl\tools\checkstyle\checks\coding\FinalLocalVariableCheck.java:77: Total number of methods is 34 (max allowed is 28). [MethodCount]
[ERROR] \checkstyle\src\main\java\com\puppycrawl\tools\checkstyle\checks\indentation\AbstractExpressionHandler.java:33: Total number of methods is 31 (max allowed is 28). [MethodCount]
[ERROR] \checkstyle\src\main\java\com\puppycrawl\tools\checkstyle\TreeWalker.java:62: Total number of methods is 29 (max allowed is 28). [MethodCount]
Audit done.
Checkstyle ends with 4 errors.

rnveach added a commit to rnveach/checkstyle that referenced this issue Jul 19, 2017

rnveach added a commit to rnveach/checkstyle that referenced this issue Jul 29, 2017

rnveach added a commit to rnveach/checkstyle that referenced this issue Jul 29, 2017

rnveach added a commit to rnveach/checkstyle that referenced this issue Jul 29, 2017

romani added a commit that referenced this issue Jul 30, 2017

@romani romani added the bug label Jul 30, 2017

@romani romani added this to the 8.2 milestone Jul 30, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jul 30, 2017

Member

fix is merged

Member

romani commented Jul 30, 2017

fix is merged

@romani romani closed this Jul 30, 2017

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Jul 30, 2017

Member

@romani You made mention in this issue you wanted to change our numbers for the MethodCountCheck configuration. You no longer want to do this?

Member

rnveach commented Jul 30, 2017

@romani You made mention in this issue you wanted to change our numbers for the MethodCountCheck configuration. You no longer want to do this?

romani added a commit that referenced this issue Aug 1, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Aug 1, 2017

Member

You no longer want to do this?

thanks a lot for pinning me, I missed this.
In my local:

execute:
     [echo] Checkstyle started (checkstyle_checks.xml): 01/08/2017 04:01:13 PM
[checkstyle] Running Checkstyle 8.2-SNAPSHOT on 938 files
[checkstyle] [ERROR] .../checks/coding/FinalLocalVariableCheck.java:77: 
Total number of methods is 34 (max allowed is 32). [MethodCount]

fixed in 29e131f

Member

romani commented Aug 1, 2017

You no longer want to do this?

thanks a lot for pinning me, I missed this.
In my local:

execute:
     [echo] Checkstyle started (checkstyle_checks.xml): 01/08/2017 04:01:13 PM
[checkstyle] Running Checkstyle 8.2-SNAPSHOT on 938 files
[checkstyle] [ERROR] .../checks/coding/FinalLocalVariableCheck.java:77: 
Total number of methods is 34 (max allowed is 32). [MethodCount]

fixed in 29e131f

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