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 #7576: update doc for SuppressWarningsHolder #11542

Merged
merged 1 commit into from Jul 13, 2022
Merged

Issue #7576: update doc for SuppressWarningsHolder #11542

merged 1 commit into from Jul 13, 2022

Conversation

Kevin222004
Copy link
Collaborator

Fixes Issue #7576
Screenshot from 2022-04-12 14-18-06

@romani
Copy link
Member

romani commented Apr 13, 2022

From issue description

Add example of config for each existing code to show examples of usage.

You need to add configs, not a java code.
Please confirm that config + code works as expected by our CLI.

@Kevin222004
Copy link
Collaborator Author

From issue description

Add example of config for each existing code to show examples of usage.

You need to add configs, not a java code. Please confirm that config + code works as expected by our CLI.

I am sorry for miss understanding

@Kevin222004
Copy link
Collaborator Author

Kevin222004 commented Apr 13, 2022

@romani I am not getting what exact violation is I have run the CLI tools with this config file and I am getting Audit done in every code example without any error

Screenshot from 2022-04-13 10-36-51

Screenshot from 2022-04-13 21-20-05

@romani
Copy link
Member

romani commented Apr 13, 2022

Try to understand

Read https://checkstyle.org/config_filters.html#SuppressWarningsFilter
Both holder and filter should be in config together

@Kevin222004
Copy link
Collaborator Author

@romani please look at this pic and review if config file is wrong
check

@romani
Copy link
Member

romani commented Apr 23, 2022

Filter is missing, please read documentation attentively, please also read code of this module

@Kevin222004
Copy link
Collaborator Author

@romani I have read the documentation but still i am not able to figure it out may be I am doing some little mistake please look at this pic and help to figure out
Screenshot from 2022-04-26 10-32-05
Screenshot from 2022-04-26 10-31-25

@Kevin222004
Copy link
Collaborator Author

I have tried without unused also

@romani
Copy link
Member

romani commented Apr 28, 2022

Please share config and code and commands that you have in images in text to let me quickly reuse it as I come to keyboard.

@Kevin222004
Copy link
Collaborator Author

Kevin222004 commented Apr 28, 2022

config file:-
https://gist.githubusercontent.com/Kevin222004/3ef73a607206dd459c64c2188d539d7b/raw/28738b739392b01baa3c892037a91674d369a832/Config.xml

code :-
class check {
@SuppressWarnings({"MemberName"})
private int J;
}

command :-
java -jar checkstyle-10.1-all.jar -c config.xml check.java

@Kevin222004
Copy link
Collaborator Author

@romani
Copy link
Member

romani commented Apr 29, 2022

Please look at #10043 as inspiration on how to use such modules. Look at other issue for this module there might be more examples.

Let me know if you still need help, but it requires me to do this during keyboard time.

@Kevin222004
Copy link
Collaborator Author

Kevin222004 commented May 1, 2022

@romani #10043 (comment) I have study this
but for the code prescence over here https://checkstyle.org/config_annotation.html#SuppressWarningsHolder
I have checked for MemberNameCheck and ConstantNameCheck: I am getting audit done without any error

for Member Name check

$ cat TestClass.java
class Test {
@SuppressWarnings({"membername"})
private int J;
}

$ cat TestConfig.xml
https://gist.githubusercontent.com/Kevin222004/7f3b4e4f1b9100abff5900be2bbfcade/raw/de1d95d2c45a154fbbe059c05d5ee0fb0a40519d/COnfig.xml

$ java -jar checkstyle-8.42-all.jar -c TestConfig.xml TestClass.java
Starting audit...
Audit done.

for Constant Name Check
$ cat TestClass.java
class Test {
@SuppressWarnings({"checkstyle:constantname"})
private static final int m = 0;
}

$ cat TestConfig.xml
https://gist.githubusercontent.com/Kevin222004/c458769ce724dea4d5dc3b5eed800842/raw/ee57495c7e89a91a113d943d5679e084a55533c4/CONFIG.xml

$ java -jar checkstyle-8.42-all.jar -c TestConfig.xml TestClass.java
Starting audit...
Audit done.

@romani
Copy link
Member

romani commented May 2, 2022

More examples of usage https://github.com/checkstyle/checkstyle/pull/11573/files

cat TestConfig.xml

Please share exact output, in comment.
Do not use links.
Please format your comment with code blocks to make it easy to read

@romani
Copy link
Member

romani commented May 4, 2022

@Kevin222004 , ping

@Kevin222004
Copy link
Collaborator Author

Kevin222004 commented May 4, 2022

code :-
class Test {
@SuppressWarnings({"membername"})
private int J;
}

config file :-

<module name="TreeWalker">

output :-
kevin@kevin-Inspiron-15-5510:~/Desktop/C_Style$ java -jar /home/kevin/Downloads/checkstyle-10.1-all.jar -c config.xml Test.java
Starting audit...
Audit done

@Kevin222004
Copy link
Collaborator Author

@Kevin222004 , ping

sorry for being late from few days

code :- class Test { @SuppressWarnings({"membername"}) private int J; }

config file :-

<module name="TreeWalker">

output :- kevin@kevin-Inspiron-15-5510:~/Desktop/C_Style$ java -jar /home/kevin/Downloads/checkstyle-10.1-all.jar -c config.xml Test.java Starting audit... Audit done.

give me a moment I will share in a good manner

@Kevin222004
Copy link
Collaborator Author

Member Name

$ cat Test.java
class Test {
  @SuppressWarnings({"membername"})
   private int J;
}

$ cat config.xml
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
          "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
          "https://checkstyle.org/dtds/configuration_1_3.dtd">

<module name="Checker">
    <property name="charset" value="UTF-8"/>

   <module name="TreeWalker">
<module name="MemberName">
   </module>

<module name="SuppressWarningsHolder" />
   </module>
  <module name="SuppressWarningsFilter" />
</module>

$ java -jar checkstyle-10.1-all.jar -c config.xml Test.java
Starting audit...
Audit done.

@Kevin222004
Copy link
Collaborator Author

@romani Is it good to review now ?

@romani
Copy link
Member

romani commented May 4, 2022

This is good example of suppression. Let's use it our examples.

@Kevin222004
Copy link
Collaborator Author

Kevin222004 commented May 4, 2022

@romani it don't need any error message ? if it is good can I update website according to it

@romani
Copy link
Member

romani commented May 4, 2022

Create few field without annotation to keep oe violation

@Kevin222004
Copy link
Collaborator Author

Kevin222004 commented May 5, 2022

@romani can you please guide on create config of code

@SuppressWarnings("paramnum")
public void needsLotsOfParameters(@SuppressWarnings("unused") int a,
  int b, int c, int d, int e, int f, int g, int h) {
  ...
}

@romani
Copy link
Member

romani commented May 5, 2022

Suppres modules are the same, just add module
https://checkstyle.org/config_sizes.html#ParameterNumber and use it's name in suppression annotation
Looks like we do not have Check for unused parameters.

@Kevin222004
Copy link
Collaborator Author

Member check:-

$ cat Test.java
class Test {
  @SuppressWarnings({"membername"})
   private int J;
}

$ cat config.xml
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
          "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
          "https://checkstyle.org/dtds/configuration_1_3.dtd">

<module name="Checker">
    <property name="charset" value="UTF-8"/>

   <module name="TreeWalker">
<module name="MemberName" />
   
<module name="SuppressWarningsHolder" />
   </module>
  <module name="SuppressWarningsFilter" />
</module>

$ java -jar checkstyle-10.1-all.jar -c config.xml Test.java
Starting audit...
Audit done.

Constant Name :-

$ cat Test.java
class Test {
   @SuppressWarnings("checkstyle:constantname")
    private static final int m = 0;
}

$ cat config.xml
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
          "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
          "https://checkstyle.org/dtds/configuration_1_3.dtd">

<module name="Checker">
    <property name="charset" value="UTF-8"/>

   <module name="TreeWalker">
<module name="ConstantName" />
   
<module name="SuppressWarningsHolder" />
   </module>
  <module name="SuppressWarningsFilter" />
</module>

$ java -jar checkstyle-10.1-all.jar -c config.xml Test.java
Starting audit...
Audit done.

ParameterNumber:-

$ cat Test.java
class Test {
    @SuppressWarnings("paramnum")
     public void needsLotsOfParameters(@SuppressWarnings("unused") int a,
     int b, int c, int d, int e, int f, int g, int h) {
        //  ...
     }
}

$ cat config.xml
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
          "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
          "https://checkstyle.org/dtds/configuration_1_3.dtd">

<module name="Checker">
    <property name="charset" value="UTF-8"/>

   <module name="TreeWalker">
<module name="ParameterNumber" />
   
<module name="SuppressWarningsHolder" />
   </module>
  <module name="SuppressWarningsFilter" />
</module>

$ java -jar checkstyle-10.1-all.jar -c config.xml Test.java
Starting audit...
[ERROR] /home/kevin/Desktop/C_Style/Test.java:3:15: More than 7 parameters (found 8). [ParameterNumber]
Audit done.
Checkstyle ends with 1 errors.

@Kevin222004
Copy link
Collaborator Author

Screenshot from 2022-05-07 10-27-55

@Kevin222004
Copy link
Collaborator Author

Screenshot from 2022-05-07 10-28-04

@romani
Copy link
Member

romani commented May 7, 2022

Please fix indentation in xml config to let users read it with pleasure.

@Kevin222004
Copy link
Collaborator Author

Please fix indentation in xml config to let users read it with pleasure.

Please look at this pic is it fine now

Screenshot from 2022-05-07 11-18-42
Screenshot from 2022-05-07 11-18-50

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.

Please update all configs to have holder and check. User need ready to copy paste examples to to play with.

Please update all to have config that is following by java code to show how it works. People love examples more than reading boring documentation.

Items:

@romani romani removed their assignment Jun 28, 2022
@romani
Copy link
Member

romani commented Jun 28, 2022

GitHub, generate website

@Kevin222004
Copy link
Collaborator Author

Is there any changes still required

@romani
Copy link
Member

romani commented Jul 2, 2022

GitHub, generate website

@Kevin222004 Kevin222004 requested a review from romani July 4, 2022 03:43
@romani
Copy link
Member

romani commented Jul 4, 2022

GitHub, generate website

@romani
Copy link
Member

romani commented Jul 4, 2022

@Kevin222004, please start dealing with other issue meanwhile, this PR in final phase of review.

Copy link
Member

@rnveach rnveach left a comment

Choose a reason for hiding this comment

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

I can only do partials right now.

@rnveach
Copy link
Member

rnveach commented Jul 7, 2022

GitHub, generate website

@github-actions
Copy link
Contributor

github-actions bot commented Jul 7, 2022

@rnveach rnveach assigned romani and unassigned rnveach Jul 8, 2022
@Kevin222004
Copy link
Collaborator Author

#11542 (comment) should I create a new issue for this now

@Kevin222004
Copy link
Collaborator Author

Screenshot from 2022-07-11 14-02-39

@romani
Copy link
Member

romani commented Jul 12, 2022

I pushed right version, i will merge after CI pass.

@romani
Copy link
Member

romani commented Jul 12, 2022

@Kevin222004 , please fix Teamcity violation

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.

Thanks a lot for update

@romani romani merged commit 0b78aed into checkstyle:master Jul 13, 2022
@Kevin222004
Copy link
Collaborator Author

Thanks a lot for update

Welcome and Thanks to you also sir to stay for a long for easy issue

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