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

Issue #11736: Support qualified annotation names for MissingJavadocTypeCheck #11827

Conversation

stoyanK7
Copy link
Collaborator

@stoyanK7 stoyanK7 commented Jul 3, 2022

Addresses #11736
Adds support for qualified annotation names in MissingJavadocTypeCheck.
Diff Regression config: https://gist.githubusercontent.com/stoyanK7/37de7a689373daea1df3ec5f4e16efbb/raw/4f28948a768d7cf7fb3a59f6f1faf9e6152d28fa/config.xml

@stoyanK7 stoyanK7 force-pushed the issue/11736-support-qualified-annotation-names branch from 7ede563 to 66c557a Compare July 3, 2022 21:09
Copy link
Member

@nrmancuso nrmancuso left a comment

Choose a reason for hiding this comment

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

Please generate check regression report for all checks that use this method (See https://github.com/checkstyle/contribution/tree/master/checkstyle-tester#report-generation):

➜  checkstyle git:(master) grep -lR "AnnotationUtil.containsAnnotation(" | grep -iv "Test" | uniq
src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/MissingJavadocTypeCheck.java
src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocMethodCheck.java
src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/MissingJavadocMethodCheck.java
src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocTypeCheck.java
src/main/java/com/puppycrawl/tools/checkstyle/checks/annotation/MissingDeprecatedCheck.java
src/main/java/com/puppycrawl/tools/checkstyle/checks/annotation/PackageAnnotationCheck.java


Few items to start with:

@nrmancuso nrmancuso self-assigned this Jul 3, 2022
@romani
Copy link
Member

romani commented Jul 4, 2022

Please look at failed CI they will guide you on what to improve.

@stoyanK7 stoyanK7 force-pushed the issue/11736-support-qualified-annotation-names branch from 66c557a to cbb051d Compare July 4, 2022 16:32
@stoyanK7 stoyanK7 requested review from romani and nrmancuso July 4, 2022 17:07
@nrmancuso
Copy link
Member

nrmancuso commented Jul 4, 2022

No need to re-request review, please reply “done” to each item as you complete it and confirm that changes are pushed. Please generate check regression reports before we proceed.

@stoyanK7
Copy link
Collaborator Author

stoyanK7 commented Jul 4, 2022

Github, generate report

Copy link
Member

@romani romani left a comment

Choose a reason for hiding this comment

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

Items

Copy link
Member

@romani romani left a comment

Choose a reason for hiding this comment

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

Items

@stoyanK7 stoyanK7 force-pushed the issue/11736-support-qualified-annotation-names branch 2 times, most recently from c8e666b to 8aa65e4 Compare July 4, 2022 18:36
@stoyanK7 stoyanK7 force-pushed the issue/11736-support-qualified-annotation-names branch from 8aa65e4 to 71d7c0c Compare July 4, 2022 19:11
Copy link
Member

@romani romani left a comment

Choose a reason for hiding this comment

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

Item for whole PR:

@stoyanK7 stoyanK7 force-pushed the issue/11736-support-qualified-annotation-names branch from 71d7c0c to d152226 Compare July 5, 2022 09:43
Copy link
Member

@romani romani left a comment

Choose a reason for hiding this comment

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

Ok to merge

@romani romani requested a review from strkkk July 5, 2022 12:02
Copy link
Member

@nrmancuso nrmancuso left a comment

Choose a reason for hiding this comment

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

Please add test cases:

  1. With multiple, qualified skipAnnotations, i.e. MissingJavadocType: Support qualified annotation names #11736 (comment)
  2. With multiple (3+) annotations on a given type, with a few variations of number of skipAnnotations.

@stoyanK7 stoyanK7 force-pushed the issue/11736-support-qualified-annotation-names branch from d152226 to e5cad07 Compare July 6, 2022 15:01
@stoyanK7
Copy link
Collaborator Author

stoyanK7 commented Jul 6, 2022

Please add test cases:

1. With multiple, qualified `skipAnnotations`, i.e. [#11736 (comment)](https://github.com/checkstyle/checkstyle/issues/11736#issuecomment-1169486065)

2. With multiple (3+) annotations on a given type, with a few variations of number of skipAnnotations.

Done!

  1. Added testQualifiedAnnotation5() where all 3 annotations are skipped.
  2. Added testMultipleQualifiedAnnotation() where multiple annotations are used on single interface. I suppose this is what you meant?

@nrmancuso nrmancuso assigned strkkk and unassigned nrmancuso Jul 8, 2022
@strkkk strkkk merged commit 2282a7e into checkstyle:master Jul 9, 2022
@stoyanK7 stoyanK7 deleted the issue/11736-support-qualified-annotation-names branch July 9, 2022 12:47
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

Successfully merging this pull request may close these issues.

None yet

4 participants