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.

Sign up for GitHub

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

Jump to bottom

Issue #14631: update SINCE_LITERAL in JavadocTokenTypes.java to new format of AST print #14631 #14738

Merged
merged 1 commit into from May 4, 2024
Merged

Issue #14631: update SINCE_LITERAL in JavadocTokenTypes.java to new format of AST print #14631 #14738

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/main/java/com/puppycrawl/tools/checkstyle/api/JavadocTokenTypes.java
View file
Open in desktop
Expand Up @@ -89,11 +89,11 @@ public final class JavadocTokenTypes {
* <pre>{@code @since 3.4 RELEASE}</pre>
* <b>Tree:</b>
* <pre>{@code
* |--JAVADOC_TAG[3x0] : [@since 3.4 RELEASE]
* |--SINCE_LITERAL[3x0] : [@since]
* |--WS[3x6] : [ ]
* |--DESCRIPTION[3x7] : [3.4 RELEASE]
* |--TEXT[3x7] : [3.4 RELEASE]
* JAVADOC_TAG -> JAVADOC_TAG
* |--SINCE_LITERAL -> @since
* |--WS ->
* `--DESCRIPTION -> DESCRIPTION
* |--TEXT -> 3.4 RELEASE
Copy link
Member

@romani romani Apr 28, 2024 •
edited

Choose a reason for hiding this comment

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

this line should start with " `".

I understand that we skip \r\n lines that are windows specific, but not sure why we missed TEXT -->

please share input file file with content of javadoc,
I will rerun your cmd on my side to check AST.

Copy link
Contributor

Choose a reason for hiding this comment

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

@romani , I thought the `TEXT --> is only when the tag is last of javadoc content (i.e. part of tag before EOF) as evident from #14632 (comment).
Since we trim the AST to only one specific tag and it's description, isn't this line redundant here for our docs?

Copy link
Member

Choose a reason for hiding this comment

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

Can we use -j, --javadocTree https://checkstyle.org/cmdline.html ?
So we will provide exactly a content that is in example.

Copy link
Contributor

Choose a reason for hiding this comment

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

E:\MANISH\Workspace>cat Test.java
/**
 * @since 3.4 RELEASE
 */

E:\MANISH\Workspace>java -jar checkstyle-10.14.2-all.jar -j Test.java | sed "s/\[[0-9]\+:[0-9]\+\]//g"
JAVADOC -> JAVADOC
|--TEXT -> /**
|--NEWLINE -> \n
|--LEADING_ASTERISK ->  *
|--WS ->
|--JAVADOC_TAG -> JAVADOC_TAG
|   |--SINCE_LITERAL -> @since
|   |--WS ->
|   `--DESCRIPTION -> DESCRIPTION
|       |--TEXT -> 3.4 RELEASE
|       |--NEWLINE -> \n
|       |--LEADING_ASTERISK ->  *
|       |--TEXT -> /
|       `--NEWLINE -> \n
`--EOF -> <EOF>

E:\MANISH\Workspace>

@romani , still would be the same thing I believe.

but not sure why we missed TEXT -->

The Text -> part that you were referring to is at EOF only (here by EOF, I mean end of javadoc parse file)

Copy link
Member

Choose a reason for hiding this comment

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

From doc:

The file has to contain only Javadoc comment content excluding '/**' and '*/' at the beginning and at the end respectively. It can only be used on a single file and cannot be combined with other options.

Please try without java comments '/**'

Copy link
Contributor

Choose a reason for hiding this comment

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

Please try without java comments '/**'

Yes @romani , that was what I did by default for Ast using -j.
I have added the comment headers /** later on only to show the difference where Text -> is displayed..

The Text -> part that you were referring to is at EOF only (here by EOF, I mean end of javadoc parse file)

E:\MANISH\Workspace>cat Test.java
* @since 3.4 RELEASE

E:\MANISH\Workspace>java -jar checkstyle-10.14.2-all.jar -j Test.java | sed "s/\[[0-9]\+:[0-9]\+\]//g"
JAVADOC -> JAVADOC
|--LEADING_ASTERISK -> *
|--WS ->
|--JAVADOC_TAG -> JAVADOC_TAG
|   |--SINCE_LITERAL -> @since
|   |--WS ->
|   `--DESCRIPTION -> DESCRIPTION
|       |--TEXT -> 3.4 RELEASE
|       `--NEWLINE -> \n
`--EOF -> <EOF>

E:\MANISH\Workspace>cat Test.java
@since 3.4 RELEASE

E:\MANISH\Workspace>java -jar checkstyle-10.14.2-all.jar -j Test.java | sed "s/\[[0-9]\+:[0-9]\+\]//g"
JAVADOC -> JAVADOC
|--JAVADOC_TAG -> JAVADOC_TAG
|   |--SINCE_LITERAL -> @since
|   |--WS ->
|   `--DESCRIPTION -> DESCRIPTION
|       |--TEXT -> 3.4 RELEASE
|       `--NEWLINE -> \n
`--EOF -> <EOF>

E:\MANISH\Workspace>

Copy link
Contributor

Choose a reason for hiding this comment

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

To answer your question at #14738 (comment), from what I understand, the parser only knows what a leading asterix is. It doesn't know what '/**' and '/' are so, it simply flushes them as text.
The start of javadoc chunk and after the last tag (i.e. just before end of javadoc file)

|--TEXT -> /** and |--TEXT -> /

We are only providing examples of AST for one specific tag, so I trimmed this part in the first PR itself to begin with :)

Copy link
Member

Choose a reason for hiding this comment

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

We just need to remove newline character from javadoc snippet file

rivanov@p5510:/var/tmp$ cat Test.java 
* @since 3.4 RELEASErivanov@p5510:/var/tmp$ java -jar checkstyle-10.15.0-all.jar -j Test.java
JAVADOC -> JAVADOC [0:0]
|--LEADING_ASTERISK -> * [0:0]
|--WS ->   [0:1]
|--JAVADOC_TAG -> JAVADOC_TAG [0:2]
|   |--SINCE_LITERAL -> @since [0:2]
|   |--WS ->   [0:8]
|   `--DESCRIPTION -> DESCRIPTION [0:9]
|       `--TEXT -> 3.4 RELEASE [0:9]
`--EOF -> <EOF> [0:20]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this line should start with " `".

I understand that we skip \r\n lines that are windows specific, but not sure why we missed TEXT -->

please share input file file with content of javadoc, I will rerun your cmd on my side to check AST.

SteLeo@DESKTOP-11C7U5R MINGW64 ~/Test_tutorial
$ cat Test.java
/**
 * Some javadoc
 *
 * @since 3.4 RELEASE
 */
public class Test {

}

SteLeo@DESKTOP-11C7U5R MINGW64 ~/Test_tutorial
$ java -jar checkstyle-10.14.2-all.jar -J Test.java | sed "s/\[[0-9]\+:[0-9]\+\]//g"
COMPILATION_UNIT -> COMPILATION_UNIT
`--CLASS_DEF -> CLASS_DEF
    |--MODIFIERS -> MODIFIERS
    |   |--BLOCK_COMMENT_BEGIN -> /*
    |   |   |--COMMENT_CONTENT -> *\r\n * Some javadoc\r\n *\r\n * @since 3.4 RELEASE\r\n
    |   |   |   `--JAVADOC -> JAVADOC
    |   |   |       |--NEWLINE -> \r\n
    |   |   |       |--LEADING_ASTERISK ->  *
    |   |   |       |--TEXT ->  Some javadoc
    |   |   |       |--NEWLINE -> \r\n
    |   |   |       |--LEADING_ASTERISK ->  *
    |   |   |       |--NEWLINE -> \r\n
    |   |   |       |--LEADING_ASTERISK ->  *
    |   |   |       |--WS ->
    |   |   |       |--JAVADOC_TAG -> JAVADOC_TAG
    |   |   |       |   |--SINCE_LITERAL -> @since
    |   |   |       |   |--WS ->
    |   |   |       |   `--DESCRIPTION -> DESCRIPTION
    |   |   |       |       |--TEXT -> 3.4 RELEASE
    |   |   |       |       |--NEWLINE -> \r\n
    |   |   |       |       `--TEXT ->
    |   |   |       `--EOF -> <EOF>
    |   |   `--BLOCK_COMMENT_END -> */
    |   `--LITERAL_PUBLIC -> public
    |--LITERAL_CLASS -> class
    |--IDENT -> Test
    `--OBJBLOCK -> OBJBLOCK
        |--LCURLY -> {
        `--RCURLY -> }

SteLeo@DESKTOP-11C7U5R MINGW64 ~/Test_tutorial
$

I use Git Bash as my working terminal which supports essential linux commands and combines with windows overall setting

* }</pre>
*
* @see
Expand Down