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 #14631: update SINCE_LITERAL in JavadocTokenTypes.java to new format of AST print #14631 #14738

Merged
merged 1 commit into from May 4, 2024

Conversation

SteLeo1603
Copy link
Contributor

Issue: #14631

Command used: java -jar checkstyle-10.14.2-all.jar -J Test.java | sed "s/\[[0-9]\+:[0-9]\+\]//g"

Output:

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 -> }

image

Copy link
Contributor

@MANISH-K-07 MANISH-K-07 left a comment

Choose a reason for hiding this comment

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

It is recommended to match PR title with Issue title by convention :)

Items:

* `--DESCRIPTION -> DESCRIPTION
* |--TEXT -> 3.4 RELEASE
* |--NEWLINE -> \r\n
* `--TEXT ->
Copy link
Contributor

Choose a reason for hiding this comment

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

@SteLeo1603 ,
The codeblock shown in example is just @since 3.4 RELEASE
Please remove lines 97 & 98.
The AST till line 96 is only what we need here

Copy link
Member

Choose a reason for hiding this comment

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

@SteLeo1603 , please do.

@SteLeo1603 SteLeo1603 changed the title Issue #14631: javadoc token SINCE_LITERAL ast update Issue #14631: update SINCE_LITERAL in JavadocTokenTypes.java to new format of AST print #14631 Apr 16, 2024
@SteLeo1603 SteLeo1603 changed the title Issue #14631: update SINCE_LITERAL in JavadocTokenTypes.java to new format of AST print #14631 Issue #14631: SINCE_LITERAL AST print update #14631 Apr 16, 2024
@SteLeo1603 SteLeo1603 changed the title Issue #14631: SINCE_LITERAL AST print update #14631 Issue #14631: update SINCE_LITERAL in JavadocTokenTypes.java to new format of AST print #14631 Apr 16, 2024
Copy link
Contributor

@MANISH-K-07 MANISH-K-07 left a comment

Choose a reason for hiding this comment

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

Looks great ;)

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.

items:

* |--SINCE_LITERAL -> @since
* |--WS ->
* `--DESCRIPTION -> DESCRIPTION
* |--TEXT -> 3.4 RELEASE
Copy link
Member

@romani romani Apr 28, 2024

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

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.

Ok to merge.

We will do proper structure when we come to automation of this process.

@romani romani merged commit 4ba314b into checkstyle:master May 4, 2024
113 checks passed
@SteLeo1603 SteLeo1603 deleted the javadocAstUpdate branch May 5, 2024 00:17
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

3 participants