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

Inconsistent allowedAbbreviations when a method contains an underscore #12409

Closed
NickPaul41 opened this issue Nov 14, 2022 · 9 comments · Fixed by #12472
Closed

Inconsistent allowedAbbreviations when a method contains an underscore #12409

NickPaul41 opened this issue Nov 14, 2022 · 9 comments · Fixed by #12472

Comments

@NickPaul41
Copy link

NickPaul41 commented Nov 14, 2022

https://checkstyle.org/config_naming.html#AbbreviationAsWordInName

/var/tmp $ javac Test.java
/var/tmp $ cat config.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="AbbreviationAsWordInName">
            <property name="allowedAbbreviations" value="ORDER, OBSERVATION, UNDERSCORE, TEST" />
        </module>
    </module>
</module>
/var/tmp $ cat Test.java
public class Test {
  void getTEST() {
  } // WORKING

  void getORDER_OBSERVATION() {} // NOT WORKING

  void getUNDERSCORE() {} // WORKING

  void getTEST_OBSERVATION() {} // WORKING

  void getTEST_UNDERSCORE() {} // WORKING

  void getORDER() {} // WORKING

  void getOBSERVATION() {} // WORKING

  void getORDER_UNDERSCORE() {} // NOT WORKING
}
/var/tmp $ RUN_LOCALE="-Duser.language=en -Duser.country=US"
/var/tmp $ java $RUN_LOCALE -jar checkstyle-10.4-all.jar -c config.xml Test.java
[ERROR] /var/tmp/Test.java:5:8: Abbreviation in name 'getORDER_OBSERVATION' must contain no more than '4' consecutive capital letters. [AbbreviationAsWordInName]
[ERROR] /var/tmp/Test.java:17:8: Abbreviation in name 'getORDER_UNDERSCORE' must contain no more than '4' consecutive capital letters. [AbbreviationAsWordInName]

There are four words in allowedAbbreviations when defining a method that has an underscore between different combinations, some are allowed and others are not. It is unclear to me why getTEST_OBSERVATION works and getORDER_OBSERVATION throws an an error.


@rnveach
Copy link
Member

rnveach commented Nov 14, 2022

I am just talking about this from the code and the reason, not issue approval.

This seems similar to #8863 but this issue has a better report.

getORDER_OBSERVATION is not seen as ORDER to the code, but is seen as ORDE. The R is most likely seen as the next letter of the next word. If the name was "ORDERest", it sees "ORDE" and "Rest". If you add ORDE to the list of abbreviations, all the violations go away.

@romani
Copy link
Member

romani commented Nov 19, 2022

@arinmodi
Copy link
Contributor

@romani,

Config:

AbbreviationAsWordInName
allowedAbbreviationLength = 5
allowedAbbreviations = NUMBER, MARAZMATIC, VARIABLE
ignoreFinal = (default)true
ignoreStatic = (default)true
ignoreStaticFinal = (default)true
ignoreOverriddenMethods = (default)true
tokens = CLASS_DEF, VARIABLE_DEF, METHOD_DEF, ENUM_DEF, ENUM_CONSTANT_DEF, PARAMETER_DEF, \
         INTERFACE_DEF, ANNOTATION_DEF


Are Following Violations or Not, According To Current Code it's not violation but I think it should be violation ?

                int marazmaticVARIABLEName = 2;
                int MARAZMATICVariableName = 1;

@romani
Copy link
Member

romani commented Nov 28, 2022

I should not be violation, as all capitalized words are in allowed list

@arinmodi
Copy link
Contributor

I should not be violation, as all capitalized words are in allowed list

If you see closely, it's not "VARIABLE" but it is "VARIABLEN" which is not part of allowed list. Still not violation ?

@rnveach
Copy link
Member

rnveach commented Nov 29, 2022

@arinmodi Please provide us a Checkstyle run of the example. I agree with @romani 's take that it shouldn't be a violation. Consecutive uppercase letters end 1 before it switches to lowercase. See my example on ORDERest above.

@romani
Copy link
Member

romani commented Nov 29, 2022

N is part of next word.
In camel case notation it is like this.

@arinmodi
Copy link
Contributor

Got it

arinmodi added a commit to arinmodi/checkstyle that referenced this issue Nov 29, 2022
arinmodi added a commit to arinmodi/checkstyle that referenced this issue Nov 29, 2022
arinmodi added a commit to arinmodi/checkstyle that referenced this issue Nov 29, 2022
arinmodi added a commit to arinmodi/checkstyle that referenced this issue Nov 29, 2022
arinmodi added a commit to arinmodi/checkstyle that referenced this issue Nov 29, 2022
arinmodi added a commit to arinmodi/checkstyle that referenced this issue Nov 29, 2022
arinmodi added a commit to arinmodi/checkstyle that referenced this issue Nov 29, 2022
arinmodi added a commit to arinmodi/checkstyle that referenced this issue Nov 29, 2022
arinmodi added a commit to arinmodi/checkstyle that referenced this issue Nov 30, 2022
arinmodi added a commit to arinmodi/checkstyle that referenced this issue Nov 30, 2022
arinmodi added a commit to arinmodi/checkstyle that referenced this issue Dec 1, 2022
arinmodi added a commit to arinmodi/checkstyle that referenced this issue Dec 1, 2022
arinmodi added a commit to arinmodi/checkstyle that referenced this issue Dec 1, 2022
arinmodi added a commit to arinmodi/checkstyle that referenced this issue Dec 1, 2022
arinmodi added a commit to arinmodi/checkstyle that referenced this issue Dec 1, 2022
arinmodi added a commit to arinmodi/checkstyle that referenced this issue Dec 1, 2022
arinmodi added a commit to arinmodi/checkstyle that referenced this issue Dec 5, 2022
arinmodi added a commit to arinmodi/checkstyle that referenced this issue Dec 15, 2022
arinmodi added a commit to arinmodi/checkstyle that referenced this issue Dec 15, 2022
arinmodi added a commit to arinmodi/checkstyle that referenced this issue Dec 15, 2022
arinmodi added a commit to arinmodi/checkstyle that referenced this issue Dec 15, 2022
arinmodi added a commit to arinmodi/checkstyle that referenced this issue Dec 15, 2022
arinmodi added a commit to arinmodi/checkstyle that referenced this issue Dec 16, 2022
arinmodi added a commit to arinmodi/checkstyle that referenced this issue Dec 16, 2022
arinmodi added a commit to arinmodi/checkstyle that referenced this issue Dec 18, 2022
arinmodi added a commit to arinmodi/checkstyle that referenced this issue Dec 19, 2022
@rnveach
Copy link
Member

rnveach commented Dec 31, 2022

Fix was merged

@rnveach rnveach added the bug label Dec 31, 2022
@rnveach rnveach added this to the 10.6.0 milestone Dec 31, 2022
@romani romani changed the title Inconsistent allowedAbbreviations when a method is contains an underscore. Inconsistent allowedAbbreviations when a method contains an underscore Dec 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants