Skip to content

Issue #7571: Update doc for AnnotationUseStyle#12775

Merged
romani merged 1 commit into
checkstyle:masterfrom
naotokuwa:updateDocForAnnotationUseStyle
Mar 6, 2023
Merged

Issue #7571: Update doc for AnnotationUseStyle#12775
romani merged 1 commit into
checkstyle:masterfrom
naotokuwa:updateDocForAnnotationUseStyle

Conversation

@naotokuwa

@naotokuwa naotokuwa commented Feb 26, 2023

Copy link
Copy Markdown
Contributor

Closes #7571

The default value properties
Web Page:
Screen Shot 2023-03-05 at 4 30 04 PM
CLI:

naotokuwayama@NaotonoMacBook-Air test % 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="AnnotationUseStyle"/>
  </module>
</module>
naotokuwayama@NaotonoMacBook-Air test % cat Test.java 
@SuppressWarnings("unchecked") // OK
@Deprecated // OK
@SomeArrays({"unchecked","unused"}) // OK
public class TestOne
{

}

@SuppressWarnings(value={"unchecked"}) // Violation - parameter 'value' shouldn't be used
@Deprecated() // Violation - cannot have a closing parenthesis
@SomeArrays(value={"unchecked","unused",}) // Violation - cannot have a trailing array comma
class TestTwo {

}
naotokuwayama@NaotonoMacBook-Air test %  java $RUN_LOCALE -jar checkstyle-10.8.0-all.jar -c config.xml Test.java
Starting audit...
[ERROR] /Users/naotokuwayama/open_source/test/Test.java:9:1: Annotation style must be 'COMPACT_NO_ARRAY'. [AnnotationUseStyle]
[ERROR] /Users/naotokuwayama/open_source/test/Test.java:10:1: Annotation cannot have closing parenthesis. [AnnotationUseStyle]
[ERROR] /Users/naotokuwayama/open_source/test/Test.java:11:40: Annotation array values cannot contain trailing comma. [AnnotationUseStyle]
Audit done.
Checkstyle ends with 3 errors.

To configure the check to enforce an expanded style, with a closing parenthesis and a trailing array comma set to never.
WebPage:
2
CLI:

naotokuwayama@NaotonoMacBook-Air test % 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="AnnotationUseStyle">
        <property name="elementStyle" value="expanded"/>
        <property name="closingParens" value="never"/>
        <property name="trailingArrayComma" value="never"/>
    </module>
  </module>
</module>
naotokuwayama@NaotonoMacBook-Air test % cat Test.java                                                           
@SuppressWarnings("unchecked") // Violation - parameters should be referenced
@Deprecated // OK
@SomeArrays({"unchecked","unused"}) // Violation - parameters should be referenced
public class TestOne
{

}

@SuppressWarnings(value={"unchecked"}) // OK
@Deprecated() // Violation - cannot have a closing parenthesis
@SomeArrays(value={"unchecked","unused",}) // Violation - cannot have a trailing array comma
class TestTwo {

}

naotokuwayama@NaotonoMacBook-Air test %  java $RUN_LOCALE -jar checkstyle-10.8.0-all.jar -c config.xml Test.java
Starting audit...
[ERROR] /Users/naotokuwayama/open_source/test/Test.java:1:1: Annotation style must be 'EXPANDED'. [AnnotationUseStyle]
[ERROR] /Users/naotokuwayama/open_source/test/Test.java:3:1: Annotation style must be 'EXPANDED'. [AnnotationUseStyle]
[ERROR] /Users/naotokuwayama/open_source/test/Test.java:10:1: Annotation cannot have closing parenthesis. [AnnotationUseStyle]
[ERROR] /Users/naotokuwayama/open_source/test/Test.java:11:40: Annotation array values cannot contain trailing comma. [AnnotationUseStyle]
Audit done.
Checkstyle ends with 4 errors.

To configure the check to enforce a compact style, with always including a closing parenthesis and ignoring a trailing array comma.
WebPage:
Screen Shot 2023-03-05 at 4 30 16 PM

CLI:

naotokuwayama@NaotonoMacBook-Air test % 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="AnnotationUseStyle">
      <property name="elementStyle" value="compact"/>
      <property name="closingParens" value="always"/>
      <property name="trailingArrayComma" value="ignore"/>
    </module>
  </module>
</module>
naotokuwayama@NaotonoMacBook-Air test % cat Test.java
@SuppressWarnings("unchecked") // OK
@Deprecated // Violation - must have a closing parenthesis
@SomeArrays({"unchecked","unused"}) // OK
public class TestOne
{

}

@SuppressWarnings(value={"unchecked"}) // Violation - parameter 'value' shouldn't be used
@Deprecated() // OK
@SomeArrays(value={"unchecked","unused",}) // Violation - parameter 'value' shouldn't be used
class TestTwo {

}
naotokuwayama@NaotonoMacBook-Air test %  java $RUN_LOCALE -jar checkstyle-10.8.0-all.jar -c config.xml Test.java
Starting audit...
[ERROR] /Users/naotokuwayama/open_source/test/Test.java:2:1: Annotation must have closing parenthesis. [AnnotationUseStyle]
[ERROR] /Users/naotokuwayama/open_source/test/Test.java:9:1: Annotation style must be 'COMPACT'. [AnnotationUseStyle]
[ERROR] /Users/naotokuwayama/open_source/test/Test.java:11:1: Annotation style must be 'COMPACT'. [AnnotationUseStyle]
Audit done.

To configure the check to enforce a trailing array comma, with ignoring the elementStyle and a closing parenthesis.
WebPage:
4

CLI:

naotokuwayama@NaotonoMacBook-Air test % 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="AnnotationUseStyle">
        <property name="elementStyle" value="ignore"/>
        <property name="closingParens" value="ignore"/>
        <property name="trailingArrayComma" value="always"/>
    </module>
  </module>
</module>
naotokuwayama@NaotonoMacBook-Air test % cat Test.java                                                           
@SuppressWarnings("unchecked") // OK
@Deprecated // OK
@SomeArrays({"unchecked","unused"}) // Violation - must have a trailing array comma
public class TestOne
{

}

@SuppressWarnings(value={"unchecked"}) // Violation - must have a trailing array comma
@Deprecated() // OK
@SomeArrays(value={"unchecked","unused",}) // OK
class TestTwo {

}
naotokuwayama@NaotonoMacBook-Air test %  java $RUN_LOCALE -jar checkstyle-10.8.0-all.jar -c config.xml Test.java
Starting audit...
[ERROR] /Users/naotokuwayama/open_source/test/Test.java:3:34: Annotation array values must contain trailing comma. [AnnotationUseStyle]
[ERROR] /Users/naotokuwayama/open_source/test/Test.java:9:37: Annotation array values must contain trailing comma. [AnnotationUseStyle]
Audit done.
Checkstyle ends with 2 errors.

@nrmancuso

nrmancuso commented Feb 26, 2023

Copy link
Copy Markdown
Contributor

GitHub, generate site

@nrmancuso

Copy link
Copy Markdown
Contributor

@naotokuwa please share cli output by copy/pasting cat/checkstyle execution into code blocks instead of screenshot in PR description, ie:

-> ~ cat config.xml
...

Also please always include link to issue in PR description, if your PR completes work for the issue you can use "Closes #7571".

@naotokuwa

Copy link
Copy Markdown
Contributor Author

Hi, Nick @nrmancuso
I appreciate your help, and below are responses to what you pointed out.

  1. What does "Github, generate site" mean? Is there anything I can do about it?
  2. I got it. I will work on putting code blocks instead of screenshots.
  3. Include link means adding the URL link to the description, or is there any other way to do that?

I have two additional questions.

  1. When I have coding blocks and the link, should I edit the original PR or add them as a new comment?
  2. It turns out my PR could not pass the test. Is there anything I can do to make my code pass the test?

@nrmancuso

Copy link
Copy Markdown
Contributor

@naotokuwa

What does "Github, generate site" mean? Is there anything I can do about it?

"Github, generate site" will trigger a github action to generate our website; you will see a bot comment with a link soon :)

Include link means adding the URL link to the description, or is there any other way to do that?

See my example above, you literally add the phrase "Closes #7571" <- note that this links to your issue

When I have coding blocks and the link, should I edit the original PR or add them as a new comment?

Edit the PR description, the less comments in the conversation thread, the better.

It turns out my PR could not pass the test. Is there anything I can do to make my code pass the test?

The only CI that is not passing right now is Travis, this failure is on our side, you can ignore it. If any other CI fail, investigate, read error messages attentively, and update PR with fix.

@naotokuwa

Copy link
Copy Markdown
Contributor Author

@nrmancuso
Hi nick, thank you so much for explaining everything clearly.
I will edit the original PR description to have code blocks for CLI and add a link to the issue.

@naotokuwa

Copy link
Copy Markdown
Contributor Author

Hi, Nick @nrmancuso
I finished editing the PR description so that it has a link to the issue and code blocks of CLI results.
Screen Shot 2023-02-26 at 12 37 57 PM
I failed 1 test, but that was about I do not have a reference to the original issue which I solved above.
Is there anything I missed to solve this test, or I am fine?

@nrmancuso

Copy link
Copy Markdown
Contributor

@naotokuwa CI is restarted, it should pass now that PR description is updated

@naotokuwa

Copy link
Copy Markdown
Contributor Author

@nrmancuso Thank you! It seems all tests passed except for Travis!

@romani

romani commented Feb 27, 2023

Copy link
Copy Markdown
Member

GitHub, generate website

@romani

romani commented Feb 27, 2023

Copy link
Copy Markdown
Member

@Vyom-Yadav , can you help to review this PR first?

@romani

romani commented Feb 27, 2023

Copy link
Copy Markdown
Member

@naotokuwa, can you rebase on latest master? I am not sure why our action to generate website is not working.

@naotokuwa

naotokuwa commented Feb 27, 2023

Copy link
Copy Markdown
Contributor Author

Hi, Romani @romani
Thank you for helping me.
Rebasing on the latest master means doing steps (5) to step(9) in the following link?
https://checkstyle.org/beginning_development.html
I did that before sending the PR but I will try again if the above step is correct.
Moreover, after rebasing my code on the master, how can I change a PR?
I know that I must not create another PR.

@nrmancuso

nrmancuso commented Feb 27, 2023

Copy link
Copy Markdown
Contributor

@naotokuwa

git rebase upstream/master
git push —force

@naotokuwa

Copy link
Copy Markdown
Contributor Author

Hi, Nick. @nrmancuso
Thank you for helping me, and I tried what you told me at the local branch. Please correct me if I did it the wrong way.
Seems my code is already updated. Is there anything I can do about this problem?

Screen Shot 2023-02-27 at 4 10 49 AM

@nrmancuso

Copy link
Copy Markdown
Contributor

Please fetch first, git fetch upstream master

@naotokuwa naotokuwa force-pushed the updateDocForAnnotationUseStyle branch from a04e238 to a13070f Compare February 27, 2023 12:17
@nrmancuso

nrmancuso commented Feb 27, 2023

Copy link
Copy Markdown
Contributor

Github, generate site

@naotokuwa

Copy link
Copy Markdown
Contributor Author

Hi, @nrmancuso
Thank you for your help. Below is what I did and the result. It turns out my PR does not follow the latest version.
Screen Shot 2023-02-27 at 4 20 21 AM
Screen Shot 2023-02-27 at 4 20 38 AM

If you have an idea, could you tell me what my issue was for further contribution? I followed the below link and sent a PR.
Is it because I did not fetch it?
https://checkstyle.org/beginning_development.html

@nrmancuso

Copy link
Copy Markdown
Contributor

We are constantly merging new commits, so it is safe to assume that your local branch is never up to date :) Whenever I come to work on a PR branch, I always do:

git fetch —all
git rebase upstream/master
git push —force

It is a good practice to always keep your PR branches up to date to avoid having strange issues that might be hard to diagnose. Also, when you are creating a new branch, make sure that master is up to date too, before you create it:

git checkout master
git pull upstream master
git push (optional, updates the master branch in your fork)

then check out your new branch.

@nrmancuso

Copy link
Copy Markdown
Contributor

@naotokuwa site generation is failing because you have renamed your fork. Your fork should be named checkstyle just like main repo.

@naotokuwa

Copy link
Copy Markdown
Contributor Author

@nrmancuso
Thank you so much for your detailed explanation regarding the git issue I had.
For the naming issue, things I should do is just change the name of the folk repository to "checkstyle" on GitHub?
I thought I could change the name to anything I want, sorry for that.

@nrmancuso

nrmancuso commented Feb 27, 2023

Copy link
Copy Markdown
Contributor

@naotokuwa a lot of our automation assumes that your branch is named the same as the main repo, it used to be more inconvenient to rename a forked repo so no one really did it in the past. Rename your fork, give it a few minutes, then comment in this PR with “GitHub, generate site”

@naotokuwa

Copy link
Copy Markdown
Contributor Author

@nrmancuso
I will try renaming. Thank you.

@naotokuwa

Copy link
Copy Markdown
Contributor Author

GitHub, generate site

@github-actions

github-actions Bot commented Feb 27, 2023

Copy link
Copy Markdown
Contributor

@romani

romani commented Feb 27, 2023

Copy link
Copy Markdown
Member

Git commands that usually required https://checkstyle.org/beginning_development.html#Starting_Development

@naotokuwa

Copy link
Copy Markdown
Contributor Author

Hi, @romani
Thank you for the further information!

@nrmancuso

nrmancuso commented Mar 3, 2023

Copy link
Copy Markdown
Contributor

@naotokuwa you should keep one commit in your feature branch, and keep squashing new/ fixing up requested changes into it. Try to drop the commit from the other contributor, squash your commits into one, and rebase again. You can use interactive rebase for this, as you’ve mentioned above.

@romani

romani commented Mar 3, 2023

Copy link
Copy Markdown
Member

GitHub, generate website

@naotokuwa naotokuwa force-pushed the updateDocForAnnotationUseStyle branch from 0b00dfe to f8d3e86 Compare March 3, 2023 20:48
@naotokuwa

naotokuwa commented Mar 3, 2023

Copy link
Copy Markdown
Contributor Author

@nrmancuso
Thank you for your clear instruction. I could make it only 1 commit and I am starting updating the PR description.

@naotokuwa

Copy link
Copy Markdown
Contributor Author

I updated the codes, commit and PR description, but some of CI tests failed. Before pushing the new change to the remote, is there anyway I can confirm CI won't fail in addition to "mvn verify"?
I also did fetch and rebase in the local repository, then pushed to here.

@romani

romani commented Mar 4, 2023

Copy link
Copy Markdown
Member

GitHub, generate website

@romani

romani commented Mar 4, 2023

Copy link
Copy Markdown
Member

If no link to website posted by bot, please rebase PR on latest our master.

@naotokuwa naotokuwa force-pushed the updateDocForAnnotationUseStyle branch from f8d3e86 to 6aacbce Compare March 4, 2023 03:04
@naotokuwa

Copy link
Copy Markdown
Contributor Author

GitHub, generate website

@Vyom-Yadav

Copy link
Copy Markdown
Member

Github, generate site

@github-actions

github-actions Bot commented Mar 4, 2023

Copy link
Copy Markdown
Contributor

@romani

romani commented Mar 4, 2023

Copy link
Copy Markdown
Member

@naotokuwa , please add example for compact_no_array

@naotokuwa

Copy link
Copy Markdown
Contributor Author

Hi , @romani
For COMPACT_NO_ARRAY, I have already covered that on default settings. Do I need to add an example which has COMPACT_NO_ARRAY in the property?
Is that ok that number of examples becomes 5?

@romani

romani commented Mar 4, 2023

Copy link
Copy Markdown
Member

No need, sorry Missed it.

@romani

romani commented Mar 4, 2023

Copy link
Copy Markdown
Member

@naotokuwa

naotokuwa commented Mar 4, 2023

Copy link
Copy Markdown
Contributor Author

@romani
Thank you for your review. For checking whether my code will pass a CI test or not, is there anything that I can do in local repository?

@romani

romani commented Mar 4, 2023

Copy link
Copy Markdown
Member

@naotokuwa naotokuwa force-pushed the updateDocForAnnotationUseStyle branch from 6aacbce to 5b8c272 Compare March 4, 2023 05:56
@naotokuwa

Copy link
Copy Markdown
Contributor Author

Github, generate site

@naotokuwa

Copy link
Copy Markdown
Contributor Author

@romani
Thank you for your instruction. I figured out the CI issue. Right now, although I fetched and rebased on master, it does not give me a new site. Do you have any idea about why "Github, generate site" does not work?

@romani

romani commented Mar 4, 2023

Copy link
Copy Markdown
Member

Github, generate site

@github-actions

github-actions Bot commented Mar 4, 2023

Copy link
Copy Markdown
Contributor

@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.

awesome

last updates for clarity what is parameter of annotation:

@naotokuwa naotokuwa force-pushed the updateDocForAnnotationUseStyle branch from 5b8c272 to 18fef83 Compare March 6, 2023 00:28
@naotokuwa

Copy link
Copy Markdown
Contributor Author

Github, generate site

1 similar comment
@romani

romani commented Mar 6, 2023

Copy link
Copy Markdown
Member

Github, generate site

@github-actions

github-actions Bot commented Mar 6, 2023

Copy link
Copy Markdown
Contributor

@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.

Ok to merge.

Thanks a lot!
Please help us to fix more issues.

@romani romani merged commit 6fad92a into checkstyle:master Mar 6, 2023
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.

Update documentation for AnnotationUseStyle

4 participants