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

not understandable sevntu check #570

Open
Bananeweizen opened this issue Sep 19, 2023 · 5 comments
Open

not understandable sevntu check #570

Bananeweizen opened this issue Sep 19, 2023 · 5 comments

Comments

@Bananeweizen
Copy link
Collaborator

In 2 of my PRs I had to disable SimpleAccessorNameNotation. It's raising issues on perfectly valid names which I have checked multiple times. Also the error message and documentation don't really tell anything about how to fix it.
I suggest to completely disable it (which will happen anyway if one of those 2 PRs is merged).

@rnveach
Copy link
Member

rnveach commented Sep 19, 2023

I would report an issue to sevntu if you think it is a bug in the check.

@Bananeweizen
Copy link
Collaborator Author

I'm not sure. I don't know what it is expecting, even after reading documentation. I have a field mSomething, and a getter getSomething(). Not sure if it's confused by the "m" notation (which I don't like myself, but I try to stay consistent with existing code). If you know the check better, you may want to run one of my PRs and remove the disablement locally to see what happens.

@rnveach
Copy link
Member

rnveach commented Sep 19, 2023

Not sure if it's confused by the "m" notation

I think this is the issue. My understanding of the check, from just refreshing my memory, is (for example) a pure getter method (method that just returns a field) should be named exactly after the field it returns. It should be expecting getMSomething since there is no option to ignore prefixes.

@Bananeweizen
Copy link
Collaborator Author

In that case I would leave the disablement in my PRs and later check if we can do a mass refactoring of the field names to get rid of the m prefixes and to re-enable the check then.

@rnveach
Copy link
Member

rnveach commented Sep 19, 2023

@Bananeweizen I apologize, SimpleAccessorNameNotationCheck does have a prefix property. However, I believe it only supports one type of prefix, so if eclipse-cs uses multiple then it won't work.

$ cat TestClass.java
public class TestClass {
    private int mSomething;

    void getSomething() { // line 4
      return mSomething;
    }
}
public class TestClassAlt {
    private int mSomething;

    void getMSomething() { // line 11
      return mSomething;
    }
}

$ cat TestConfig.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">
    <property name="charset" value="UTF-8"/>
    <property name="severity" value="warning"/>
    <property name="haltOnException" value="false"/>

    <module name="TreeWalker">

<module name="com.github.sevntu.checkstyle.checks.coding.SimpleAccessorNameNotationCheck">
  <property name="prefix" value="m" />
</module>

    </module>
</module>

$ java -jar checkstyle-10.4-sevntu-1.44.1-all.jar -c TestConfig.xml TestClass.java
Starting audit...
[WARN] TestClass.java:11:5: Unexpected getter name. [SimpleAccessorNameNotation]
Audit done.

If you do remove prefix property, it does behave as I described before.

$ cat TestClass.java
public class TestClass {
    private int mSomething;

    void getSomething() { // line 4
      return mSomething;
    }
}
public class TestClassAlt {
    private int mSomething;

    void getMSomething() { // line 11
      return mSomething;
    }
}

$ cat TestConfig.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">
    <property name="charset" value="UTF-8"/>
    <property name="severity" value="warning"/>
    <property name="haltOnException" value="false"/>

    <module name="TreeWalker">

<module name="com.github.sevntu.checkstyle.checks.coding.SimpleAccessorNameNotationCheck">
</module>

    </module>
</module>

$ java -jar checkstyle-10.4-sevntu-1.44.1-all.jar -c TestConfig.xml TestClass.java
Starting audit...
[WARN] TestClass.java:4:5: Unexpected getter name. [SimpleAccessorNameNotation]
Audit done.

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

No branches or pull requests

2 participants