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

ForbidCertainMethodCheck ignores lambda parameters #721

Closed
skitt opened this issue Sep 19, 2018 · 3 comments

Comments

@skitt
Copy link
Contributor

commented Sep 19, 2018

/var/tmp $ javac Demo.java

[No output.]

/var/tmp $ cat Demo.java

package org.opendaylight.netvirt.bgpmanager.api;

import java.util.function.Function;

public class Demo {
    void main(String[] args) {
        System.out.println(checkArray(new int[] { 1, 2, 3 }, value -> { return value != 2; }));
    }
    
    private boolean checkArray(int[] array, Function<Integer, Boolean> filter) {
        for (int value : array) {
            if (filter.apply(value)) {
                return false;
            }
        }
        return true;
    }
}

/var/tmp $ cat config.xml

<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
        "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
        "http://checkstyle.sourceforge.net/dtds/configuration_1_3.dtd">
<module name="Checker">
    <module name="SuppressWarningsFilter" />

    <module name="TreeWalker">
        <module name="SuppressWarningsHolder" />

        <module name="com.github.sevntu.checkstyle.checks.coding.ForbidCertainMethodCheck">
            <property name="methodName" value="checkArray" />
            <property name="argumentCount" value="1"/>
        </module>
    </module>
</module>

/var/tmp $ java -classpath sevntu-checks-1.32.0.jar:checkstyle-8.12-all.jar com.puppycrawl.tools.checkstyle.Main -c config.xml Demo.java

Starting audit...
[ERROR] /home/skitt/tmp/Demo.java:7:38: Call to 'checkArray' method (matches pattern 'checkArray') with '1' arguments (matches pattern '1') is forbidden. [ForbidCertainMethod]
Audit done.
Checkstyle ends with 1 errors.

I expect the audit to complete without flagging any errors.

skitt added a commit to skitt/sevntu.checkstyle that referenced this issue Sep 19, 2018

Issue sevntu-checkstyle#721: Handle lambdas in ForbidCertainMethodCheck
ForbidCertainMethodCheck allows the number of parameters to be used to
specify forbidden methods more precisely than using only their name;
however it counds those parameters by counting EXPR tokens. Lambdas
appear as LAMBDA tokens, not EXPR tokens, so they aren’t counted
towards the real argument count, resulting in false-positives or
false-negatives depending on the test being run.

This patch changes the argument count to count commas instead, all at
the same level just under the ELIST. This ensures that arguments are
counted regardless of their token type.

Signed-off-by: Stephen Kitt <skitt@redhat.com>

@romani romani added the approved label Oct 3, 2018

skitt added a commit to skitt/sevntu.checkstyle that referenced this issue Oct 18, 2018

romani added a commit that referenced this issue Nov 17, 2018

@romani romani added the bug label Nov 17, 2018

@romani romani added this to the 1.33.0 milestone Nov 17, 2018

@romani

This comment has been minimized.

Copy link
Member

commented Nov 17, 2018

Fix is merged

@romani romani closed this Nov 17, 2018

@romani

This comment has been minimized.

Copy link
Member

commented Nov 18, 2018

@skitt , we do not have strict release time, how much you can wait for release ?
Usually release happens one few changes appears merged.

@skitt

This comment has been minimized.

Copy link
Contributor Author

commented Nov 18, 2018

@skitt , we do not have strict release time, how much you can wait for release ?
Usually release happens one few changes appears merged.

Thanks for the merge! I didn’t hurry to provide the requested information, so it wouldn’t be fair to ask you to release quickly ;-). I’d obviously like to use the fix sooner rather than later, but don’t rush just for me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.