Skip to content

Issue #7176: Add AnnotatedMethodVisibilityModifierCheck#19164

Open
gianmarcoschifone wants to merge 1 commit into
checkstyle:masterfrom
gianmarcoschifone:7176
Open

Issue #7176: Add AnnotatedMethodVisibilityModifierCheck#19164
gianmarcoschifone wants to merge 1 commit into
checkstyle:masterfrom
gianmarcoschifone:7176

Conversation

@gianmarcoschifone

@gianmarcoschifone gianmarcoschifone commented Mar 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

@gianmarcoschifone gianmarcoschifone force-pushed the 7176 branch 6 times, most recently from 4f3a4e1 to 7170a03 Compare March 10, 2026 16:31
@gianmarcoschifone

gianmarcoschifone commented Mar 10, 2026

Copy link
Copy Markdown
Member Author

GitHub, generate website

@gianmarcoschifone

gianmarcoschifone commented Mar 10, 2026

Copy link
Copy Markdown
Member Author

GitHub, generate report

@github-actions

github-actions Bot commented Mar 10, 2026

Copy link
Copy Markdown
Contributor

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

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/7170a03_20260310170124/checks.html#Standard_Checks

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/7170a03_20260310170124/checks/modifier/annotatedmethodvisibilitymodifier.html#

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/7170a03_20260310170124/checks/modifier/index.html#Modifiers_Checks

@github-actions

github-actions Bot commented Mar 10, 2026

Copy link
Copy Markdown
Contributor

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

@gianmarcoschifone

gianmarcoschifone commented Mar 10, 2026

Copy link
Copy Markdown
Member Author

GitHub, generate website

@github-actions

github-actions Bot commented Mar 10, 2026

Copy link
Copy Markdown
Contributor

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

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/f5dc96b_20260310193109/checks.html#Standard_Checks

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/f5dc96b_20260310193109/checks/modifier/annotatedmethodvisibilitymodifier.html#

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/f5dc96b_20260310193109/checks/modifier/index.html#Modifiers_Checks

@gianmarcoschifone

gianmarcoschifone commented Mar 10, 2026

Copy link
Copy Markdown
Member Author

Hi @romani , PR is ready for review.
I analyzed the diff report and violations are regular.

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

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/f5dc96b_20260310193109/checks/modifier/annotatedmethodvisibilitymodifier.html#

@gianmarcoschifone gianmarcoschifone force-pushed the 7176 branch 2 times, most recently from 91c9fdf to 2281c83 Compare March 16, 2026 10:56
romani
romani requested changes May 25, 2026
View reviewed changes

@romani romani left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please rebase.

items:

Comment thread .../com/puppycrawl/tools/checkstyle/checks/modifier/AnnotatedMethodVisibilityModifierCheck.java Outdated
* element, its visibility modifier must match one of the allowed values.
* </p>
*
* @since 13.4.0

@romani romani May 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

13.5.0

romani
romani requested changes May 25, 2026
View reviewed changes

@romani romani left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we fully targeting to Methods, do we need extra tokens for types ?

Comment thread .../com/puppycrawl/tools/checkstyle/checks/modifier/AnnotatedMethodVisibilityModifierCheck.java
TokenTypes.IMPORT,
// tokens that can have annotations
TokenTypes.CLASS_DEF,
TokenTypes.INTERFACE_DEF,

@romani romani May 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

something wrong with name of Check.

Method Visibility vs target Class.

@gianmarcoschifone gianmarcoschifone May 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check should target any token that can have a visibility modifier, not just method, as reported in #7176.
Could we rename the check to AnnotatedVisibilityModifier?

@gianmarcoschifone gianmarcoschifone mentioned this pull request Jun 3, 2026
Open
@romani

romani commented Jun 27, 2026

Copy link
Copy Markdown
Member

@gianmarcoschifone , please rebase and make it versioned 13.8.0 , I will make sue review if finished in July

@gianmarcoschifone

gianmarcoschifone commented Jun 28, 2026

Copy link
Copy Markdown
Member Author

@romani done

@romani

romani commented Jun 29, 2026

Copy link
Copy Markdown
Member

CI is not happy , please address

@gianmarcoschifone

gianmarcoschifone commented Jun 29, 2026

Copy link
Copy Markdown
Member Author

@romani I think this check will need some changes because what I tried was attempting to handle unqualified annotations through package and imports, this brought me to add non compilable resources

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

Reviewers

@romani romani romani requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@gianmarcoschifone @romani