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

HiddenField false positive on inner records #14028

Closed
fprochazka opened this issue Nov 14, 2023 · 4 comments
Closed

HiddenField false positive on inner records #14028

fprochazka opened this issue Nov 14, 2023 · 4 comments

Comments

@fprochazka
Copy link

fprochazka commented Nov 14, 2023

Link to check documentation: https://checkstyle.sourceforge.io/checks/coding/hiddenfield.html

/var/tmp $ cat config.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="HiddenField">
            <property name="ignoreConstructorParameter" value="true"/>
            <property name="setterCanReturnItsClass" value="true"/>
            <property name="ignoreSetter" value="true"/>
        </module>
    </module>
</module>
/var/tmp $ cat YOUR_FILE.java
import java.time.Clock;
import java.time.Instant;

class Scratch
{

    private final Clock clock;

    public Scratch(final Clock clock)
    {
        this.clock = clock;
    }

    public void test()
    {
        var state = new State("val", Instant.now().minusSeconds(1));
        var is = state.isFresh(clock);
        System.out.println(is);
    }

    public record State(
        String token,
        Instant expiresAt
    )
    {

        public boolean isFresh(final Clock clock) // violation
        {
            return Instant.now(clock).isBefore(expiresAt);
        }

    }

}
/var/tmp $ RUN_LOCALE="-Duser.language=en -Duser.country=US"
/var/tmp $ java $RUN_LOCALE -jar ./checkstyle-10.12.4-all.jar -c ./config.xml scratch_617.java                                                                                                                      254 ↵
Starting audit...
[ERROR] scratch_617.java:27:44: 'clock' hides a field. [HiddenField]
Audit done.
Checkstyle ends with 1 errors.

Describe what you expect in detail.

Adding static to the inner record fixes the false positive, but records don't share this of outer class, so the check is wrong in this case, because the static is AFAIK implicit.

@nrmancuso
Copy link
Member

Some comparisons with inner classes also:

➜  src cat config.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="HiddenField">
            <property name="ignoreConstructorParameter" value="true"/>
            <property name="setterCanReturnItsClass" value="true"/>
            <property name="ignoreSetter" value="true"/>
        </module>
    </module>
</module>
➜  src cat Test.java 
public class Test {
    private final int x;

    public Test(int x) {
        this.x = x;
    }

    // inner record is implicitly static, should be treated
    // the same as R2
    public record R1() {
        public void m(int x) {} // unexpected violation
    }

    public static record R2() {
        public void m(int x) {} // no violation expected
    }

    // static class, should be treated the same as R2
    public static class C1 {
        public void m(int x) {} // no violation expected
    }

    // non-static class, should have a violation
    public class C2 {
        public void m(int x) {} // violation
    }
}
➜  src javac Test.java
➜  src java -jar checkstyle-10.12.1-all.jar -c config.xml Test.java
Starting audit...
[ERROR] /home/nick/IdeaProjects/tester/src/Test.java:11:27: 'x' hides a field. [HiddenField]
[ERROR] /home/nick/IdeaProjects/tester/src/Test.java:25:27: 'x' hides a field. [HiddenField]
Audit done.
Checkstyle ends with 2 errors.

Issue is approved, @fprochazka please feel free to send a PR

@Kevin222004
Copy link
Contributor

@romani I think this issue needs to be closed know

@nrmancuso
Copy link
Member

Closed via #14128

@nrmancuso
Copy link
Member

@Kevin222004 issue will be automatically closed if you do “Closes #….” in the PR description.

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

No branches or pull requests

4 participants