Issue #12409: Inconsistent allowedAbbreviations #12472

arinmodi · 2022-11-29T03:29:30Z

Closes : #12409 : Inconsistent allowedAbbreviations when a method is contains an underscore

Diff Regression config: https://gist.githubusercontent.com/arinmodi/89cc612081d60d9f9b72e090df7db612/raw/7a96b0e42c45afecc39694493ede50add32fde77/config3.xml

arinmodi · 2022-11-29T03:30:28Z

Currently, Not Focusing on CI, Once Logic is approved, I will do that

romani · 2022-11-29T05:25:27Z

Preappoved .
Please extend example in xdoc with underscore

arinmodi · 2022-11-29T06:59:01Z

Preappoved . Please extend example in xdoc with underscore

Do you mean adding example on the website ?

nrmancuso · 2022-11-29T13:46:31Z

@arinmodi please generate check regression report

arinmodi · 2022-11-29T14:08:55Z

Github, generate report

github-actions · 2022-11-29T15:21:34Z

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/13139b6_2022152055/reports/diff/index.html

arinmodi · 2022-11-29T16:25:19Z

Analysis :

If there is more Uppercase letters than specified length before "_", it is generating violation.

Without "_" it is allowing +1 character cause of Camel Case.

Is this expected behaviour ?

romani

Is this expected behaviour ?

Put all your concerns to input file, we will see it in details

Items

src/main/java/com/puppycrawl/tools/checkstyle/checks/naming/AbbreviationAsWordInNameCheck.java

...ls/checkstyle/checks/naming/abbreviationaswordinname/InputAbbreviationAsWordInNameType7.java

src/main/java/com/puppycrawl/tools/checkstyle/checks/naming/AbbreviationAsWordInNameCheck.java

romani

Items

romani

Items item

...ls/checkstyle/checks/naming/abbreviationaswordinname/InputAbbreviationAsWordInNameType7.java

romani

Items

src/main/java/com/puppycrawl/tools/checkstyle/checks/naming/AbbreviationAsWordInNameCheck.java

romani

Please explain why this is violation https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/13139b6_2022152055/reports/diff/Hbase/index.html#A1

It not more than 4 capital letters.

Items

...ls/checkstyle/checks/naming/abbreviationaswordinname/InputAbbreviationAsWordInNameType7.java

arinmodi · 2022-12-01T05:22:39Z

Please explain why this is violation https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/13139b6_2022152055/reports/diff/Hbase/index.html#A1

It not more than 4 capital letters.

Items

Because default allowed length is 3 and Without Underscore it is allowing 4 because last character is considered as the next word's first character which should be capital in camel case.

romani · 2022-12-01T16:07:48Z

We should not treat _ as part of word.
If user decided to use _ it is definitely words separation.

Please add this case to test input and we need to extend documentation about _ support.

romani

I

...ls/checkstyle/checks/naming/abbreviationaswordinname/InputAbbreviationAsWordInNameType7.java

romani

Items

src/main/java/com/puppycrawl/tools/checkstyle/checks/naming/AbbreviationAsWordInNameCheck.java

arinmodi · 2022-12-01T16:23:49Z

We should not treat _ as part of word. If user decided to use _ it is definitely words separation.

Please add this case to test input and we need to extend documentation about _ support.

getNON_ETest(), is this good example to add in test input ?

romani · 2022-12-01T16:34:16Z

is this good example to add in test input ?

Absolutely, input code should all possible combinations. Do not limit your creativity.

arinmodi · 2022-12-01T16:59:54Z

Hey I have doubt,

Suppose allowedAbbreviationLength is 3, so before "_" how many Consecutive Capital Letters should be allowed ? 3 or 4

arinmodi · 2022-12-02T17:53:33Z

@romani ?

github-actions · 2022-12-15T10:31:58Z

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/f9e5376_2022102920/reports/diff/index.html

arinmodi · 2022-12-15T10:48:28Z

Hey @romani, Updated The Code to generate violation whenever more than (allowed length + 1) found

romani · 2022-12-15T16:14:33Z

Please keep CI green.

romani

Items

src/main/java/com/puppycrawl/tools/checkstyle/checks/naming/AbbreviationAsWordInNameCheck.java

romani

Items

...ls/checkstyle/checks/naming/abbreviationaswordinname/InputAbbreviationAsWordInNameType7.java

romani

Items

...ls/checkstyle/checks/naming/abbreviationaswordinname/InputAbbreviationAsWordInNameType7.java

romani

Fix CI violations.

Please update regression config to allow abbreviation length to be 2. To try catch some violations.

Items

...tyle/checks/naming/abbreviationaswordinname/InputAbbreviationAsWordInNameTypeSnakeStyle.java

arinmodi · 2022-12-16T15:26:04Z

Github, generate report

github-actions · 2022-12-16T15:30:46Z

Report generation job failed on phase "parse_body", step "React with rocket on run".
Link: https://github.com/checkstyle/checkstyle/actions/runs/3714339314

github-actions · 2022-12-16T16:31:00Z

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/029cc0c_2022162445/reports/diff/index.html

arinmodi · 2022-12-16T17:44:31Z

@romani, is it good now ?

romani · 2022-12-16T18:03:40Z

A way better.
Please add test code with digits as hint us at https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/029cc0c_2022162445/reports/diff/Hbase/index.html#A1 to let us be aware that it is part of word or not.
Leading number, trailing numbers, numbers in mid of word.

arinmodi · 2022-12-17T17:45:17Z

A way better. Please add test code with digits as hint us at https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/029cc0c_2022162445/reports/diff/Hbase/index.html#A1 to let us be aware that it is part of word or not. Leading number, trailing numbers, numbers in mid of word.

Done, Leading Numbers Not Possible

romani · 2022-12-17T23:20:40Z

Please add cases like TEST23456, 234VIO LATION ....

arinmodi · 2022-12-18T08:40:28Z

I think java has rule that identifier must start with letter

romani

Good job.

src/main/java/com/puppycrawl/tools/checkstyle/checks/naming/AbbreviationAsWordInNameCheck.java

arinmodi force-pushed the bug-small branch 2 times, most recently from c907723 to 13139b6 Compare November 29, 2022 11:30

romani requested changes Nov 29, 2022

View reviewed changes

arinmodi force-pushed the bug-small branch from 0cd36a7 to 879df50 Compare November 29, 2022 18:02

romani requested changes Nov 30, 2022

View reviewed changes

...ls/checkstyle/checks/naming/abbreviationaswordinname/InputAbbreviationAsWordInNameType7.java Outdated Show resolved Hide resolved

arinmodi force-pushed the bug-small branch from 1442deb to facfdf4 Compare November 30, 2022 11:12

arinmodi requested a review from romani November 30, 2022 11:13

rnveach assigned romani Nov 30, 2022

romani requested changes Nov 30, 2022

View reviewed changes

src/main/java/com/puppycrawl/tools/checkstyle/checks/naming/AbbreviationAsWordInNameCheck.java Outdated Show resolved Hide resolved

src/main/java/com/puppycrawl/tools/checkstyle/checks/naming/AbbreviationAsWordInNameCheck.java Outdated Show resolved Hide resolved

romani requested changes Nov 30, 2022

View reviewed changes

...ls/checkstyle/checks/naming/abbreviationaswordinname/InputAbbreviationAsWordInNameType7.java Outdated Show resolved Hide resolved

arinmodi force-pushed the bug-small branch 2 times, most recently from d6b91fa to cebd954 Compare December 1, 2022 13:57

romani requested changes Dec 1, 2022

View reviewed changes

...ls/checkstyle/checks/naming/abbreviationaswordinname/InputAbbreviationAsWordInNameType7.java Outdated Show resolved Hide resolved

...ls/checkstyle/checks/naming/abbreviationaswordinname/InputAbbreviationAsWordInNameType7.java Outdated Show resolved Hide resolved

romani requested changes Dec 1, 2022

View reviewed changes

src/main/java/com/puppycrawl/tools/checkstyle/checks/naming/AbbreviationAsWordInNameCheck.java Outdated Show resolved Hide resolved

arinmodi force-pushed the bug-small branch from 4dd5565 to 12db2b4 Compare December 1, 2022 17:10

arinmodi requested a review from romani December 2, 2022 09:22

romani requested changes Dec 15, 2022

View reviewed changes

src/main/java/com/puppycrawl/tools/checkstyle/checks/naming/AbbreviationAsWordInNameCheck.java Outdated Show resolved Hide resolved

arinmodi force-pushed the bug-small branch from f9e5376 to 8ec3472 Compare December 15, 2022 16:20

romani requested changes Dec 15, 2022

View reviewed changes

...ls/checkstyle/checks/naming/abbreviationaswordinname/InputAbbreviationAsWordInNameType7.java Outdated Show resolved Hide resolved

...ls/checkstyle/checks/naming/abbreviationaswordinname/InputAbbreviationAsWordInNameType7.java Outdated Show resolved Hide resolved

romani requested changes Dec 15, 2022

View reviewed changes

...ls/checkstyle/checks/naming/abbreviationaswordinname/InputAbbreviationAsWordInNameType7.java Outdated Show resolved Hide resolved

arinmodi force-pushed the bug-small branch 3 times, most recently from dfcf457 to aad25d9 Compare December 15, 2022 19:55

romani requested changes Dec 16, 2022

View reviewed changes

arinmodi force-pushed the bug-small branch from aad25d9 to 029cc0c Compare December 16, 2022 15:18

arinmodi force-pushed the bug-small branch from 029cc0c to d4d5068 Compare December 16, 2022 22:43

Issue checkstyle#12409: Inconsistentency In Allowed Abbreviations

e2bf316

arinmodi force-pushed the bug-small branch from d4d5068 to e2bf316 Compare December 18, 2022 08:45

romani approved these changes Dec 18, 2022

View reviewed changes

romani requested a review from rnveach December 18, 2022 15:59

romani assigned rnveach and unassigned romani Dec 18, 2022

rnveach reviewed Dec 19, 2022

View reviewed changes

src/main/java/com/puppycrawl/tools/checkstyle/checks/naming/AbbreviationAsWordInNameCheck.java Show resolved Hide resolved

rnveach approved these changes Dec 31, 2022

View reviewed changes

rnveach merged commit d12ffc7 into checkstyle:master Dec 31, 2022

Issue #12409: Inconsistent allowedAbbreviations #12472

Issue #12409: Inconsistent allowedAbbreviations #12472

Conversation

arinmodi commented Nov 29, 2022 • edited

arinmodi commented Nov 29, 2022

romani commented Nov 29, 2022 • edited

arinmodi commented Nov 29, 2022

nrmancuso commented Nov 29, 2022

arinmodi commented Nov 29, 2022

github-actions bot commented Nov 29, 2022

arinmodi commented Nov 29, 2022

romani left a comment • edited

Choose a reason for hiding this comment

romani left a comment

Choose a reason for hiding this comment

romani left a comment

Choose a reason for hiding this comment

romani left a comment

Choose a reason for hiding this comment

romani left a comment • edited

Choose a reason for hiding this comment

arinmodi commented Dec 1, 2022

romani commented Dec 1, 2022 • edited

romani left a comment

Choose a reason for hiding this comment

romani left a comment

Choose a reason for hiding this comment

arinmodi commented Dec 1, 2022

romani commented Dec 1, 2022

arinmodi commented Dec 1, 2022

arinmodi commented Dec 2, 2022

github-actions bot commented Dec 15, 2022

arinmodi commented Dec 15, 2022

romani commented Dec 15, 2022

romani left a comment

Choose a reason for hiding this comment

romani left a comment

Choose a reason for hiding this comment

romani left a comment

Choose a reason for hiding this comment

romani left a comment • edited

Choose a reason for hiding this comment

arinmodi commented Dec 16, 2022

github-actions bot commented Dec 16, 2022

github-actions bot commented Dec 16, 2022

arinmodi commented Dec 16, 2022

romani commented Dec 16, 2022

arinmodi commented Dec 17, 2022

romani commented Dec 17, 2022

arinmodi commented Dec 18, 2022

romani left a comment

Choose a reason for hiding this comment

arinmodi commented Nov 29, 2022 •

edited

romani commented Nov 29, 2022 •

edited

romani left a comment •

edited

romani left a comment •

edited

romani commented Dec 1, 2022 •

edited

romani left a comment •

edited