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

Making required arguments mandatory for javadoc tags in grammar #4942

Closed
ps-sp opened this Issue Aug 12, 2017 · 6 comments

Comments

@ps-sp
Collaborator

ps-sp commented Aug 12, 2017

$ cat test.javadoc

* @author

$ java -jar master-8.2-all.jar -j test.javadoc

JAVADOC -> JAVADOC [0:0]
|--LEADING_ASTERISK -> * [0:0]
|--WS ->   [0:1]
|--JAVADOC_TAG -> JAVADOC_TAG [0:2]
|   |--AUTHOR_LITERAL -> @author [0:2]
|   `--NEWLINE -> \n [0:9]
`--EOF -> <EOF> [1:0]

$ cat test.javadoc

* @exception

$ java -jar master-8.2-all.jar -j test.javadoc

JAVADOC -> JAVADOC [0:0]
|--LEADING_ASTERISK -> * [0:0]
|--WS ->   [0:1]
|--JAVADOC_TAG -> JAVADOC_TAG [0:2]
|   |--EXCEPTION_LITERAL -> @exception [0:2]
|   `--NEWLINE -> \n [1:0]
`--EOF -> <EOF> [2:0]

$ cat test.javadoc

* @param

$ java -jar master-8.2-all.jar -j test.javadoc

JAVADOC -> JAVADOC [0:0]
|--LEADING_ASTERISK -> * [0:0]
|--WS ->   [0:1]
|--JAVADOC_TAG -> JAVADOC_TAG [0:2]
|   `--PARAM_LITERAL -> @param [0:2]
`--EOF -> <EOF> [0:8]

$ cat test.javadoc

* @return

$ java -jar master-8.2-all.jar -j test.javadoc

JAVADOC -> JAVADOC [0:0]
|--LEADING_ASTERISK -> * [0:0]
|--WS ->   [0:1]
|--JAVADOC_TAG -> JAVADOC_TAG [0:2]
|   `--RETURN_LITERAL -> @return [0:2]
`--EOF -> <EOF> [0:9]

Javadoc trees are parsed when checkstyle should rightfully throw error for such inappropriately constructed javadocs. ANTLR grammar should be updated in all the required places for this and in javadoc tags which are supposed to have atleast 1 argument, the corresponding element must not allowed to be made optional in the grammar. For example CLASS_NAME shouldn't be optional in @exception literal.

@Vladlis

This comment has been minimized.

Show comment
Hide comment
@Vladlis

Vladlis Aug 13, 2017

Member

@ps-sp , please provide links to documentation for each case

Member

Vladlis commented Aug 13, 2017

@ps-sp , please provide links to documentation for each case

@ps-sp

This comment has been minimized.

Show comment
Hide comment
@ps-sp

ps-sp Aug 13, 2017

Collaborator

@Vladlis

please provide links to documentation for each case

Can you please tell me what links are you exactly referring to ? The report for description in Javadoc tags that I provided already has hyperlinks to corresponding documentations ?

Collaborator

ps-sp commented Aug 13, 2017

@Vladlis

please provide links to documentation for each case

Can you please tell me what links are you exactly referring to ? The report for description in Javadoc tags that I provided already has hyperlinks to corresponding documentations ?

@Vladlis

This comment has been minimized.

Show comment
Hide comment
@Vladlis

Vladlis Aug 17, 2017

Member

@ps-sp
You have to prove your statement that tags from the issue description

are supposed to have atleast 1 argument

Member

Vladlis commented Aug 17, 2017

@ps-sp
You have to prove your statement that tags from the issue description

are supposed to have atleast 1 argument

@ps-sp

This comment has been minimized.

Show comment
Hide comment
@ps-sp

ps-sp Aug 17, 2017

Collaborator

@Vladlis

You have to prove your statement that tags from the issue description

It can't be proved exactly for every tag since javadoc tool results in successful site rendering even with no arguments. For example: even with no argument for @exception javadoc tool just gave a warning message and executed anyway. Though as can be seen from syntax in docs and intuition, one can only intuitively argue where an argument must be present for a javadoc tag and where it need not be.

I feel we will have to create our own list of javadoc tags where checkstyle would enforce a description based on the oracle docs and the report on javadoc tags that we have.

Collaborator

ps-sp commented Aug 17, 2017

@Vladlis

You have to prove your statement that tags from the issue description

It can't be proved exactly for every tag since javadoc tool results in successful site rendering even with no arguments. For example: even with no argument for @exception javadoc tool just gave a warning message and executed anyway. Though as can be seen from syntax in docs and intuition, one can only intuitively argue where an argument must be present for a javadoc tag and where it need not be.

I feel we will have to create our own list of javadoc tags where checkstyle would enforce a description based on the oracle docs and the report on javadoc tags that we have.

@romani romani added the approved label Aug 21, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Aug 21, 2017

Member

Issue is approved, all parts of tags should be provided by user.

Member

romani commented Aug 21, 2017

Issue is approved, all parts of tags should be provided by user.

@Vladlis Vladlis added the GSoC2017 label Aug 22, 2017

@ps-sp ps-sp changed the title from Making description mandatory for javadoc tags to Making required arguments mandatory for javadoc tags in grammar Aug 24, 2017

ps-sp added a commit to ps-sp/checkstyle that referenced this issue Aug 25, 2017

ps-sp added a commit to ps-sp/checkstyle that referenced this issue Aug 30, 2017

ps-sp added a commit to ps-sp/checkstyle that referenced this issue Aug 31, 2017

ps-sp added a commit to ps-sp/checkstyle that referenced this issue Sep 2, 2017

romani added a commit that referenced this issue Sep 5, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Sep 5, 2017

Member

fix is merged.
If smth is left - it should be separate issue.

Member

romani commented Sep 5, 2017

fix is merged.
If smth is left - it should be separate issue.

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