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

Uppercase letters to be allowed in package names in javadoc #4408

Closed
rnveach opened this Issue Jun 2, 2017 · 15 comments

Comments

@rnveach
Member

rnveach commented Jun 2, 2017

Taken from

* @see javax.swing.tree.DefaultTreeCellRenderer.getTreeCellRendererComponent()
.

$ cat TestClass.java
/**
 * @see javax.swing.tree.DefaultTreeCellRenderer.get
 */
public class TestClass {
    void method() {
    }
}

$ java -jar checkstyle-7.8-all.jar -J TestClass.java
Exception in thread "main" java.lang.IllegalArgumentException: [ERROR:1] Javadoc comment at column 0 has parse error. Unrecognized error from ANTLR parser: null
    at com.puppycrawl.tools.checkstyle.DetailNodeTreeStringPrinter.parseJavadocAsDetailNode(DetailNodeTreeStringPrinter.java:71)
    at com.puppycrawl.tools.checkstyle.AstTreeStringPrinter.parseAndPrintJavadocTree(AstTreeStringPrinter.java:117)
    at com.puppycrawl.tools.checkstyle.AstTreeStringPrinter.printJavaAndJavadocTree(AstTreeStringPrinter.java:99)
    at com.puppycrawl.tools.checkstyle.AstTreeStringPrinter.printJavaAndJavadocTree(AstTreeStringPrinter.java:103)
    at com.puppycrawl.tools.checkstyle.AstTreeStringPrinter.printJavaAndJavadocTree(AstTreeStringPrinter.java:103)
    at com.puppycrawl.tools.checkstyle.AstTreeStringPrinter.printJavaAndJavadocTree(AstTreeStringPrinter.java:103)
    at com.puppycrawl.tools.checkstyle.AstTreeStringPrinter.printJavaAndJavadocTree(AstTreeStringPrinter.java:82)
    at com.puppycrawl.tools.checkstyle.Main.runCli(Main.java:333)
    at com.puppycrawl.tools.checkstyle.Main.main(Main.java:174)

If you change get to ge than the tree prints fine with the following for the javadoc section:

|  |  |  `--JAVADOC -> \n * @see javax.swing.tree.DefaultTreeCellRenderer.ge\n <EOF> [1:3]
|  |  |      |--NEWLINE -> \n [1:3]
|  |  |      |--LEADING_ASTERISK ->  * [2:0]
|  |  |      |--WS ->  [2:2]
|  |  |      |--JAVADOC_TAG -> @see javax.swing.tree.DefaultTreeCellRenderer.ge\n  [2:3]
|  |  |      |  |--SEE_LITERAL -> @see [2:3]
|  |  |      |  |--WS ->  [2:7]
|  |  |      |  |--REFERENCE -> javax.swing.tree.DefaultTreeCellRenderer. [2:8]
|  |  |      |  |  |--PACKAGE -> javax.swing.tree [2:8]
|  |  |      |  |  |--DOT -> . [2:24]
|  |  |      |  |  |--CLASS -> DefaultTreeCellRenderer [2:25]
|  |  |      |  |  `--DOT -> . [2:48]
|  |  |      |  `--DESCRIPTION -> ge\n  [2:50]
|  |  |      |      |--TEXT -> ge [2:50]
|  |  |      |      |--NEWLINE -> \n [2:52]
|  |  |      |      `--TEXT ->  [3:0]
|  |  |      `--EOF -> <EOF> [3:1]

To me, this makes no sense why the parser is failing because of the word get.
One possibly explanation is we may be too strict on casing for package and class, but then why doesn't ge fail the same.

Looking at tree output, to me ge shouldn't be part of description but in reference.

@MEZk

This comment has been minimized.

Show comment
Hide comment
@MEZk

MEZk Jun 4, 2017

Contributor

@rnveach

but then why doesn't ge fail the same.

DefaultTreeCellRenderer recognized as CLASS. JavadocLexer thinks that after CLASS there should be HASH which is followed by a class member, but in your example there is DOT followed by get. If get should be a part of the reference, then should it be a part of PACKAGE? If it should be a part of the PACKAGE than how can we distinguish CLASS from PACKAGE?

In order to allow get to be a part of PACKAGE we need to change the grammar. Something like this:

PACKAGE: [a-zA-Z_$] ([a-zA-Z0-9_$] | '.')+ [a-zA-Z0-9_$] {referenceCatched = true;};

For the input file:

/**
 * @see javax.swing.tree.DefaultTreeCellRenderer.get
 */
public class InputJavadocParserFailure {
    void method() {
    }
}

The tree will be:

CLASS_DEF -> CLASS_DEF [4:0]
|--MODIFIERS -> MODIFIERS [4:0]
|   |--BLOCK_COMMENT_BEGIN -> /* [1:0]
|   |   |--COMMENT_CONTENT -> *\n * @see javax.swing.tree.DefaultTreeCellRenderer.get\n  [1:2]
|   |   |   `--JAVADOC -> \n * @see javax.swing.tree.DefaultTreeCellRenderer.get\n <EOF> [1:3]
|   |   |       |--NEWLINE -> \n [1:3]
|   |   |       |--LEADING_ASTERISK ->  * [2:0]
|   |   |       |--WS ->   [2:2]
|   |   |       |--JAVADOC_TAG -> @see javax.swing.tree.DefaultTreeCellRenderer.get\n  [2:3]
|   |   |       |   |--SEE_LITERAL -> @see [2:3]
|   |   |       |   |--WS ->   [2:7]
|   |   |       |   |--REFERENCE -> javax.swing.tree.DefaultTreeCellRenderer.get [2:8]
|   |   |       |   |   `--PACKAGE -> javax.swing.tree.DefaultTreeCellRenderer.get [2:8]
|   |   |       |   |--NEWLINE -> \n [2:52]
|   |   |       |   `--WS ->   [3:0]
|   |   |       `--EOF -> <EOF> [3:1]
|   |   `--BLOCK_COMMENT_END -> */ [3:1]
|   `--LITERAL_PUBLIC -> public [4:0]
|--LITERAL_CLASS -> class [4:7]
|--IDENT -> InputJavadocParserFailure [4:13]
`--OBJBLOCK -> OBJBLOCK [4:39]
    |--LCURLY -> { [4:39]
    |--METHOD_DEF -> METHOD_DEF [5:4]
    |   |--MODIFIERS -> MODIFIERS [5:4]
    |   |--TYPE -> TYPE [5:4]
    |   |   `--LITERAL_VOID -> void [5:4]
    |   |--IDENT -> method [5:9]
    |   |--LPAREN -> ( [5:15]
    |   |--PARAMETERS -> PARAMETERS [5:16]
    |   |--RPAREN -> ) [5:16]
    |   `--SLIST -> { [5:18]
    |       `--RCURLY -> } [6:4]
    `--RCURLY -> } [7:0]

As it can be seen, get became the part of PACKAGE.

Contributor

MEZk commented Jun 4, 2017

@rnveach

but then why doesn't ge fail the same.

DefaultTreeCellRenderer recognized as CLASS. JavadocLexer thinks that after CLASS there should be HASH which is followed by a class member, but in your example there is DOT followed by get. If get should be a part of the reference, then should it be a part of PACKAGE? If it should be a part of the PACKAGE than how can we distinguish CLASS from PACKAGE?

In order to allow get to be a part of PACKAGE we need to change the grammar. Something like this:

PACKAGE: [a-zA-Z_$] ([a-zA-Z0-9_$] | '.')+ [a-zA-Z0-9_$] {referenceCatched = true;};

For the input file:

/**
 * @see javax.swing.tree.DefaultTreeCellRenderer.get
 */
public class InputJavadocParserFailure {
    void method() {
    }
}

The tree will be:

CLASS_DEF -> CLASS_DEF [4:0]
|--MODIFIERS -> MODIFIERS [4:0]
|   |--BLOCK_COMMENT_BEGIN -> /* [1:0]
|   |   |--COMMENT_CONTENT -> *\n * @see javax.swing.tree.DefaultTreeCellRenderer.get\n  [1:2]
|   |   |   `--JAVADOC -> \n * @see javax.swing.tree.DefaultTreeCellRenderer.get\n <EOF> [1:3]
|   |   |       |--NEWLINE -> \n [1:3]
|   |   |       |--LEADING_ASTERISK ->  * [2:0]
|   |   |       |--WS ->   [2:2]
|   |   |       |--JAVADOC_TAG -> @see javax.swing.tree.DefaultTreeCellRenderer.get\n  [2:3]
|   |   |       |   |--SEE_LITERAL -> @see [2:3]
|   |   |       |   |--WS ->   [2:7]
|   |   |       |   |--REFERENCE -> javax.swing.tree.DefaultTreeCellRenderer.get [2:8]
|   |   |       |   |   `--PACKAGE -> javax.swing.tree.DefaultTreeCellRenderer.get [2:8]
|   |   |       |   |--NEWLINE -> \n [2:52]
|   |   |       |   `--WS ->   [3:0]
|   |   |       `--EOF -> <EOF> [3:1]
|   |   `--BLOCK_COMMENT_END -> */ [3:1]
|   `--LITERAL_PUBLIC -> public [4:0]
|--LITERAL_CLASS -> class [4:7]
|--IDENT -> InputJavadocParserFailure [4:13]
`--OBJBLOCK -> OBJBLOCK [4:39]
    |--LCURLY -> { [4:39]
    |--METHOD_DEF -> METHOD_DEF [5:4]
    |   |--MODIFIERS -> MODIFIERS [5:4]
    |   |--TYPE -> TYPE [5:4]
    |   |   `--LITERAL_VOID -> void [5:4]
    |   |--IDENT -> method [5:9]
    |   |--LPAREN -> ( [5:15]
    |   |--PARAMETERS -> PARAMETERS [5:16]
    |   |--RPAREN -> ) [5:16]
    |   `--SLIST -> { [5:18]
    |       `--RCURLY -> } [6:4]
    `--RCURLY -> } [7:0]

As it can be seen, get became the part of PACKAGE.

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Jun 4, 2017

Member

If get should be a part of the reference, then should it be a part of PACKAGE?

Originally I was thinking get should be class and the rest package. I don't think we can assume all users will write packages in lower case. I made this mistake before joining checkstyle.

If it should be a part of the PACKAGE than how can we distinguish CLASS from PACKAGE?

We can't make this distinction in Java either.

abc.def.ghi

abc can be a variable, and def be fields of the class.
def can be a class and abc could be the package it is in.
etc.

If we are too strict in grammar, users will get meaningless exceptions and not be sure what the problem is in their documentation without diving into our code.

Member

rnveach commented Jun 4, 2017

If get should be a part of the reference, then should it be a part of PACKAGE?

Originally I was thinking get should be class and the rest package. I don't think we can assume all users will write packages in lower case. I made this mistake before joining checkstyle.

If it should be a part of the PACKAGE than how can we distinguish CLASS from PACKAGE?

We can't make this distinction in Java either.

abc.def.ghi

abc can be a variable, and def be fields of the class.
def can be a class and abc could be the package it is in.
etc.

If we are too strict in grammar, users will get meaningless exceptions and not be sure what the problem is in their documentation without diving into our code.

@ps-sp

This comment has been minimized.

Show comment
Hide comment
@ps-sp

ps-sp Jun 5, 2017

Collaborator

@rnveach @MEZk

This tutorial on naming packages suggests that package names should be all lowercase by convention: https://docs.oracle.com/javase/tutorial/java/package/namingpkgs.html

But the grammar page suggests that any identifier can be used:
https://docs.oracle.com/javase/specs/jls/se8/html/jls-7.html#jls-7.4.1

Collaborator

ps-sp commented Jun 5, 2017

@rnveach @MEZk

This tutorial on naming packages suggests that package names should be all lowercase by convention: https://docs.oracle.com/javase/tutorial/java/package/namingpkgs.html

But the grammar page suggests that any identifier can be used:
https://docs.oracle.com/javase/specs/jls/se8/html/jls-7.html#jls-7.4.1

@MEZk

This comment has been minimized.

Show comment
Hide comment
@MEZk

MEZk Jun 6, 2017

Contributor

This tutorial on naming packages suggests that package names should be all lowercase by convention:

Convention is not a law. It it just suggestion. Language specification is what we must follow.

Contributor

MEZk commented Jun 6, 2017

This tutorial on naming packages suggests that package names should be all lowercase by convention:

Convention is not a law. It it just suggestion. Language specification is what we must follow.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jun 7, 2017

Member

Convention is not a law. It it just suggestion. Language specification is what we must follow.

Absolutely correct. Conventions are tending to change. Users have very different contexts so all that is compilable and match specification should be supported, in worse case ignored from validation, but not crashes for sure.

Member

romani commented Jun 7, 2017

Convention is not a law. It it just suggestion. Language specification is what we must follow.

Absolutely correct. Conventions are tending to change. Users have very different contexts so all that is compilable and match specification should be supported, in worse case ignored from validation, but not crashes for sure.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jun 7, 2017

Member

Expected @see format is at http://www.oracle.com/technetwork/java/javase/documentation/index-137868.html#multiple@see

We can not recognize:
@see package.Class and @see package
In modern Java it is become ok to name types from lowercase.
Examples:

  • import lombok.val; - javadoc , see bug on our side for this class usage #4338

So we need to make new token PACKAGE_CLASS that will be be used for cases where we are not sure (only this two cases). Check will do it's custom processing base on user configuration to follow convention on type names or not.

example in issue description is not "valid" javadoc (....#get should be), but issue is valid as we should not fail with exception as it should be treated as @see package.Class or @see package cases.

Member

romani commented Jun 7, 2017

Expected @see format is at http://www.oracle.com/technetwork/java/javase/documentation/index-137868.html#multiple@see

We can not recognize:
@see package.Class and @see package
In modern Java it is become ok to name types from lowercase.
Examples:

  • import lombok.val; - javadoc , see bug on our side for this class usage #4338

So we need to make new token PACKAGE_CLASS that will be be used for cases where we are not sure (only this two cases). Check will do it's custom processing base on user configuration to follow convention on type names or not.

example in issue description is not "valid" javadoc (....#get should be), but issue is valid as we should not fail with exception as it should be treated as @see package.Class or @see package cases.

@romani romani added the approved label Jun 8, 2017

@ps-sp

This comment has been minimized.

Show comment
Hide comment
@ps-sp

ps-sp Jun 19, 2017

Collaborator

@rnveach

but then why doesn't ge fail the same.

This is because get is taken in by ANTLR as a package token which by design has to be atleast 3 letters in length and the reference rule can only process one package . On the other hand ge is taken in as: g: char , e: char and hence can be attributed to the description rule which accepts text.

Collaborator

ps-sp commented Jun 19, 2017

@rnveach

but then why doesn't ge fail the same.

This is because get is taken in by ANTLR as a package token which by design has to be atleast 3 letters in length and the reference rule can only process one package . On the other hand ge is taken in as: g: char , e: char and hence can be attributed to the description rule which accepts text.

@ps-sp

This comment has been minimized.

Show comment
Hide comment
@ps-sp

ps-sp Jun 22, 2017

Collaborator

PACKAGE token is used only in reference rule of the parser and all the tags using the reference rules (@link, @see, @value, @linkplain ) have the standard package.class#member notation for referencing members which is exactly identical to that of @see tag. So, should we delete the PACKAGE token and replace it with PACKAGE_CLASS token as we can never separate a package from a class without context ?

Collaborator

ps-sp commented Jun 22, 2017

PACKAGE token is used only in reference rule of the parser and all the tags using the reference rules (@link, @see, @value, @linkplain ) have the standard package.class#member notation for referencing members which is exactly identical to that of @see tag. So, should we delete the PACKAGE token and replace it with PACKAGE_CLASS token as we can never separate a package from a class without context ?

@ps-sp

This comment has been minimized.

Show comment
Hide comment
@ps-sp

ps-sp Jun 22, 2017

Collaborator

Consider this input:

package com.puppycrawl.tools.checkstyle.checks.javadoc;

/**
 * @see javax.swing.tree.DefaultTreeCellRenderer.getTreeCellRendererComponent()
 */
class InputTestInvalidAtSeeReference {
}

Currently, few of the UTs have this as input file and those UTs assert an ANTLR error for parsing this file. With the modifications I have made to the grammar, I get the following parse tree for this input file:

PACKAGE_DEF -> package [1:0]
|--ANNOTATIONS -> ANNOTATIONS [1:46]
|--DOT -> . [1:46]
|   |--DOT -> . [1:39]
|   |   |--DOT -> . [1:28]
|   |   |   |--DOT -> . [1:22]
|   |   |   |   |--DOT -> . [1:11]
|   |   |   |   |   |--IDENT -> com [1:8]
|   |   |   |   |   `--IDENT -> puppycrawl [1:12]
|   |   |   |   `--IDENT -> tools [1:23]
|   |   |   `--IDENT -> checkstyle [1:29]
|   |   `--IDENT -> checks [1:40]
|   `--IDENT -> javadoc [1:47]
`--SEMI -> ; [1:54]
CLASS_DEF -> CLASS_DEF [6:0]
|--MODIFIERS -> MODIFIERS [6:0]
|--BLOCK_COMMENT_BEGIN -> /* [3:0]
|   |--COMMENT_CONTENT -> *\r\n * @see javax.swing.tree.DefaultTreeCellRenderer.getTreeCellRendererComponent()\r\n  [3:2]
|   |   `--JAVADOC -> JAVADOC [3:3]
|   |       |--NEWLINE -> \r\n [3:3]
|   |       |--LEADING_ASTERISK ->  * [4:0]
|   |       |--WS ->   [4:2]
|   |       |--JAVADOC_TAG -> JAVADOC_TAG [4:3]
|   |       |   |--SEE_LITERAL -> @see [4:3]
|   |       |   |--WS ->   [4:7]
|   |       |   |--REFERENCE -> REFERENCE [4:8]
|   |       |   |   `--PACKAGE_CLASS -> javax.swing.tree.DefaultTreeCellRenderer.getTreeCellRendererComponent [4:8]
|   |       |   `--DESCRIPTION -> DESCRIPTION [4:78]
|   |       |       |--TEXT -> () [4:78]
|   |       |       |--NEWLINE -> \r\n [4:80]
|   |       |       `--TEXT ->   [5:0]
|   |       `--EOF -> <EOF> [5:1]
|   `--BLOCK_COMMENT_END -> */ [5:1]
|--LITERAL_CLASS -> class [6:0]
|--IDENT -> InputTestInvalidAtSeeReference [6:6]
`--OBJBLOCK -> OBJBLOCK [6:37]
    |--LCURLY -> { [6:37]
    `--RCURLY -> } [7:0]

What sort of behavior or parse tree do we want for this ? Should we generate a parse tree for this or it should continue to result in ANTLR error. If this still is supposed to result in an ANTLR error then should presence of parenthesis after reference lead to an ANTLR error ? If not, what exactly should the ANTLR error stem from for such inputs.

Collaborator

ps-sp commented Jun 22, 2017

Consider this input:

package com.puppycrawl.tools.checkstyle.checks.javadoc;

/**
 * @see javax.swing.tree.DefaultTreeCellRenderer.getTreeCellRendererComponent()
 */
class InputTestInvalidAtSeeReference {
}

Currently, few of the UTs have this as input file and those UTs assert an ANTLR error for parsing this file. With the modifications I have made to the grammar, I get the following parse tree for this input file:

PACKAGE_DEF -> package [1:0]
|--ANNOTATIONS -> ANNOTATIONS [1:46]
|--DOT -> . [1:46]
|   |--DOT -> . [1:39]
|   |   |--DOT -> . [1:28]
|   |   |   |--DOT -> . [1:22]
|   |   |   |   |--DOT -> . [1:11]
|   |   |   |   |   |--IDENT -> com [1:8]
|   |   |   |   |   `--IDENT -> puppycrawl [1:12]
|   |   |   |   `--IDENT -> tools [1:23]
|   |   |   `--IDENT -> checkstyle [1:29]
|   |   `--IDENT -> checks [1:40]
|   `--IDENT -> javadoc [1:47]
`--SEMI -> ; [1:54]
CLASS_DEF -> CLASS_DEF [6:0]
|--MODIFIERS -> MODIFIERS [6:0]
|--BLOCK_COMMENT_BEGIN -> /* [3:0]
|   |--COMMENT_CONTENT -> *\r\n * @see javax.swing.tree.DefaultTreeCellRenderer.getTreeCellRendererComponent()\r\n  [3:2]
|   |   `--JAVADOC -> JAVADOC [3:3]
|   |       |--NEWLINE -> \r\n [3:3]
|   |       |--LEADING_ASTERISK ->  * [4:0]
|   |       |--WS ->   [4:2]
|   |       |--JAVADOC_TAG -> JAVADOC_TAG [4:3]
|   |       |   |--SEE_LITERAL -> @see [4:3]
|   |       |   |--WS ->   [4:7]
|   |       |   |--REFERENCE -> REFERENCE [4:8]
|   |       |   |   `--PACKAGE_CLASS -> javax.swing.tree.DefaultTreeCellRenderer.getTreeCellRendererComponent [4:8]
|   |       |   `--DESCRIPTION -> DESCRIPTION [4:78]
|   |       |       |--TEXT -> () [4:78]
|   |       |       |--NEWLINE -> \r\n [4:80]
|   |       |       `--TEXT ->   [5:0]
|   |       `--EOF -> <EOF> [5:1]
|   `--BLOCK_COMMENT_END -> */ [5:1]
|--LITERAL_CLASS -> class [6:0]
|--IDENT -> InputTestInvalidAtSeeReference [6:6]
`--OBJBLOCK -> OBJBLOCK [6:37]
    |--LCURLY -> { [6:37]
    `--RCURLY -> } [7:0]

What sort of behavior or parse tree do we want for this ? Should we generate a parse tree for this or it should continue to result in ANTLR error. If this still is supposed to result in an ANTLR error then should presence of parenthesis after reference lead to an ANTLR error ? If not, what exactly should the ANTLR error stem from for such inputs.

ps-sp added a commit to ps-sp/checkstyle that referenced this issue Jun 22, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jun 23, 2017

Member

@Vladlis and @sabaka , please address question from your student.

Member

romani commented Jun 23, 2017

@Vladlis and @sabaka , please address question from your student.

@Vladlis Vladlis moved this from To Do to In Progress in Javadoc style coverage and parser optimization Jun 27, 2017

@Vladlis

This comment has been minimized.

Show comment
Hide comment
@Vladlis

Vladlis Jun 27, 2017

Member

@ps-sp , this case should definitely result in error as description has to be separated from reference by a whitespace. Let's stay with an existing errors asserted in UTs.

Member

Vladlis commented Jun 27, 2017

@ps-sp , this case should definitely result in error as description has to be separated from reference by a whitespace. Let's stay with an existing errors asserted in UTs.

ps-sp added a commit to ps-sp/checkstyle that referenced this issue Jun 27, 2017

ps-sp added a commit to ps-sp/checkstyle that referenced this issue Jun 28, 2017

ps-sp added a commit to ps-sp/checkstyle that referenced this issue Jun 28, 2017

Issue #4408: Uppercase letters to be allowed in package names in java…
…doc. Replaced PACKAGE token in JavadocLexer with PACKAGE_CLASS

ps-sp added a commit to ps-sp/checkstyle that referenced this issue Jun 28, 2017

Issue #4408: Uppercase letters to be allowed in package names in java…
…doc. Replaced PACKAGE token in JavadocLexer with PACKAGE_CLASS
@ps-sp

This comment has been minimized.

Show comment
Hide comment
@ps-sp

ps-sp Jun 28, 2017

Collaborator

As description has to be separated from reference by a whitespace. Let's stay with an existing errors asserted in UTs.

Please consider this input:

* @serial includeDescription

Following is the output javadoc tree generated for it which is the same for my checkstyle master and my patch:

JAVADOC -> JAVADOC [0:0]
|--LEADING_ASTERISK -> * [0:0]
|--WS ->   [0:1]
|--JAVADOC_TAG -> JAVADOC_TAG [0:2]
|   |--SERIAL_LITERAL -> @serial [0:2]
|   |--WS ->   [0:9]
|   |--LITERAL_INCLUDE -> include [0:10]
|   `--DESCRIPTION -> DESCRIPTION [0:17]
|       `--TEXT -> Description [0:17]
`--EOF -> <EOF> [0:28]

This is because WS is not ascertained before DESCRIPTION in the JavadocParser here. There were few other places too where WS had not been ascertained.

In my patch I have ascertained WS everywhere but for SERIAL_LITERAL , since it was failing two UTs in JavadocParseTreeTest. (Using alternative

| SERIAL_LITERAL (WS | NEWLINE)* (LITERAL_INCLUDE | LITERAL_EXCLUDE)? (WS | NEWLINE)* ((WS | NEWLINE)+ description)? 

instead of the present one was failing serial and testAllStandardJavadocTags )

Please confirm the accepted behaviour for SERIAL_LITERAL. I think it should also have a space.

Collaborator

ps-sp commented Jun 28, 2017

As description has to be separated from reference by a whitespace. Let's stay with an existing errors asserted in UTs.

Please consider this input:

* @serial includeDescription

Following is the output javadoc tree generated for it which is the same for my checkstyle master and my patch:

JAVADOC -> JAVADOC [0:0]
|--LEADING_ASTERISK -> * [0:0]
|--WS ->   [0:1]
|--JAVADOC_TAG -> JAVADOC_TAG [0:2]
|   |--SERIAL_LITERAL -> @serial [0:2]
|   |--WS ->   [0:9]
|   |--LITERAL_INCLUDE -> include [0:10]
|   `--DESCRIPTION -> DESCRIPTION [0:17]
|       `--TEXT -> Description [0:17]
`--EOF -> <EOF> [0:28]

This is because WS is not ascertained before DESCRIPTION in the JavadocParser here. There were few other places too where WS had not been ascertained.

In my patch I have ascertained WS everywhere but for SERIAL_LITERAL , since it was failing two UTs in JavadocParseTreeTest. (Using alternative

| SERIAL_LITERAL (WS | NEWLINE)* (LITERAL_INCLUDE | LITERAL_EXCLUDE)? (WS | NEWLINE)* ((WS | NEWLINE)+ description)? 

instead of the present one was failing serial and testAllStandardJavadocTags )

Please confirm the accepted behaviour for SERIAL_LITERAL. I think it should also have a space.

@Vladlis

This comment has been minimized.

Show comment
Hide comment
@Vladlis

Vladlis Jun 28, 2017

Member

@ps-sp, yes, looks like WS is missed before description in some tags. Why do think SERIAL_LITERAL can be an exclusion?

Member

Vladlis commented Jun 28, 2017

@ps-sp, yes, looks like WS is missed before description in some tags. Why do think SERIAL_LITERAL can be an exclusion?

@Vladlis Vladlis moved this from In Progress to Review in Javadoc style coverage and parser optimization Jun 28, 2017

@ps-sp

This comment has been minimized.

Show comment
Hide comment
@ps-sp

ps-sp Jun 29, 2017

Collaborator

@Vladlis No. I don't think it should be, rather there should be a space before description everywhere ! I will update PR

Collaborator

ps-sp commented Jun 29, 2017

@Vladlis No. I don't think it should be, rather there should be a space before description everywhere ! I will update PR

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

Issue #4408: Uppercase letters to be allowed in package names in java…
…doc. Replaced PACKAGE token in JavadocLexer with PACKAGE_CLASS

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

Issue #4408: Uppercase letters to be allowed in package names in java…
…doc. Replaced PACKAGE token in JavadocLexer with PACKAGE_CLASS

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

Issue #4408: Uppercase letters to be allowed in package names in java…
…doc. Replaced PACKAGE token in JavadocLexer with PACKAGE_CLASS

ps-sp added a commit to ps-sp/checkstyle that referenced this issue Jul 3, 2017

Issue #4408: Uppercase letters to be allowed in package names in java…
…doc. Replaced PACKAGE token in JavadocLexer with PACKAGE_CLASS

ps-sp added a commit to ps-sp/checkstyle that referenced this issue Jul 3, 2017

Issue #4408: Uppercase letters to be allowed in package names in java…
…doc. Replaced PACKAGE token in JavadocLexer with PACKAGE_CLASS

@Vladlis Vladlis changed the title from Javadoc parser exception on see with 'get' to Uppercase letters to be allowed in package names in javadoc Jul 6, 2017

ps-sp added a commit to ps-sp/checkstyle that referenced this issue Jul 6, 2017

ps-sp added a commit to ps-sp/checkstyle that referenced this issue Jul 6, 2017

Issue #4408: Uppercase letters to be allowed in package names in java…
…doc. Replaced PACKAGE token in JavadocLexer with PACKAGE_CLASS

ps-sp added a commit to ps-sp/checkstyle that referenced this issue Jul 9, 2017

ps-sp added a commit to ps-sp/checkstyle that referenced this issue Jul 9, 2017

Issue #4408: Uppercase letters to be allowed in package names in java…
…doc. Replaced PACKAGE token in JavadocLexer with PACKAGE_CLASS

ps-sp added a commit to ps-sp/checkstyle that referenced this issue Jul 9, 2017

ps-sp added a commit to ps-sp/checkstyle that referenced this issue Jul 9, 2017

ps-sp added a commit to ps-sp/checkstyle that referenced this issue Jul 12, 2017

Issue #4408: Uppercase letters to be allowed in package names in java…
…doc. Replaced PACKAGE token in JavadocLexer with PACKAGE_CLASS

ps-sp added a commit to ps-sp/checkstyle that referenced this issue Jul 12, 2017

ps-sp added a commit to ps-sp/checkstyle that referenced this issue Jul 12, 2017

Issue #4408: Uppercase letters to be allowed in package names in java…
…doc. Replaced PACKAGE token in JavadocLexer with PACKAGE_CLASS

ps-sp added a commit to ps-sp/checkstyle that referenced this issue Jul 12, 2017

Merge pull request #2 from ps-sp/issue_4408_2
Issue #4408: Updated javadoc parser grammar to enforce WS before desc…

ps-sp added a commit to ps-sp/checkstyle that referenced this issue Jul 12, 2017

ps-sp added a commit to ps-sp/checkstyle that referenced this issue Jul 12, 2017

Issue #4408: Uppercase letters to be allowed in package names in java…
…doc. Replaced PACKAGE token in JavadocLexer with PACKAGE_CLASS

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

Issue #4408: Uppercase letters to be allowed in package names in java…
…doc. Replaced PACKAGE token in JavadocLexer with PACKAGE_CLASS

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

Issue #4408: Uppercase letters to be allowed in package names in java…
…doc. Replaced PACKAGE token in JavadocLexer with PACKAGE_CLASS

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

Issue #4408: Uppercase letters to be allowed in package names in java…
…doc. Replaced PACKAGE token in JavadocLexer with PACKAGE_CLASS

@Vladlis Vladlis moved this from Blocked to In Progress in Javadoc style coverage and parser optimization Aug 30, 2017

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

Issue #4408: Uppercase letters to be allowed in package names in java…
…doc. Replaced PACKAGE token in JavadocLexer with PACKAGE_CLASS

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

Issue #4408: Uppercase letters to be allowed in package names in java…
…doc. Replaced PACKAGE token in JavadocLexer with PACKAGE_CLASS

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

Issue #4408: Uppercase letters to be allowed in package names in java…
…doc. Replaced PACKAGE token in JavadocLexer with PACKAGE_CLASS

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

Issue #4408: Uppercase letters to be allowed in package names in java…
…doc. Replaced PACKAGE token in JavadocLexer with PACKAGE_CLASS

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

Issue #4408: Uppercase letters to be allowed in package names in java…
…doc. Replaced PACKAGE token in JavadocLexer with PACKAGE_CLASS

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

Issue #4408: Uppercase letters to be allowed in package names in java…
…doc. Replaced PACKAGE token in JavadocLexer with PACKAGE_CLASS

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

Issue #4408: Uppercase letters to be allowed in package names in java…
…doc. Replaced PACKAGE token in JavadocLexer with PACKAGE_CLASS. Removed CLASS and DOT tokens from JavadocLexer

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

Issue #4408: Uppercase letters to be allowed in package names in java…
…doc. Replaced PACKAGE token in JavadocLexer with PACKAGE_CLASS. Removed CLASS and DOT tokens from JavadocLexer

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

Issue #4408: Uppercase letters to be allowed in package names in java…
…doc. Replaced PACKAGE token in JavadocLexer with PACKAGE_CLASS. Removed CLASS and DOT tokens from JavadocLexer

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

Issue #4408: Uppercase letters to be allowed in package names in java…
…doc. Replaced PACKAGE token in JavadocLexer with PACKAGE_CLASS. Removed CLASS and DOT tokens from JavadocLexer

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

Issue #4408: Uppercase letters to be allowed in package names in java…
…doc. Replaced PACKAGE token in JavadocLexer with PACKAGE_CLASS. Removed CLASS and DOT tokens from JavadocLexer
@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Sep 17, 2017

Member

fix is merged

Member

romani commented Sep 17, 2017

fix is merged

@romani romani closed this Sep 17, 2017

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