JdtVisitor ignores optionalTagName for javadocs

[pouryafard75]

Hello, I just realized JdtVisitor ignores the optionalTagName while visiting TagElement.

Code:

/** @author john */
class C {}

Generated tree:

CompilationUnit [0,31]
    TypeDeclaration [0,31]
        Javadoc [0,19]
            TagElement [4,17]
                TextElement:  john  [11,17]
        TYPE_DECLARATION_KIND: class [21,26]
        SimpleName: C [27,28]

author is not part of the generated tree.

Since Eclipse JDT, reports this as the optionalTagName, I assume there's no need to create a seperate node for it.

However, it's still possible to concatenate it with the TextElement that comes after and generate the following:

CompilationUnit [0,30]
    TypeDeclaration [0,30]
        Javadoc [0,19]
            TagElement [4,17]
                TextElement: @author john  [11,17]
        TYPE_DECLARATION_KIND: class [20,25]
        SimpleName: C [26,27]

You can find my implementation here. This is a very naive implementation, and I am not sure if this covers all the cases. If TagElement can contain multiple TextElement, it will be problematic. Also, it assumes, that there will be always a TextElement as a child which comes after.

@tsantalis Are there any other possible scenarios? @jrfaller Please share your thoughts.

[tsantalis]

@pouryafard75 According to Eclipse JDT documentation

getTagName Returns this node's tag name, or null if none.

So, this a property of the TagElement

CompilationUnit [0,30]
    TypeDeclaration [0,30]
        Javadoc [0,19]
            TagElement [4,17]
                Identifier: @author [4, 11]
                TextElement:  john  [12,17]
        TYPE_DECLARATION_KIND: class [20,25]
        SimpleName: C [26,27]

We should update the Visitor to generate the tree as shown above. The identifier is optional, because it can be null. It is wrong to add the identifier in the TextElement

The optionalTageName seems a bit strange. I cannot find it in the documentation. The list of all tags is available in class TagElement

[jrfaller]

Hello @pouryafard75 and @tsantalis ! Thanks a lot for the detailed bug report. I tend to agree with @tsantalis it should be a separate child of a TagElement. Identifier is a good idea maybe TagName also to avoid allowing a variable identifier to move to a javadoc tag :). When you have a good implementation, don't hesitate to send a PR!

Cheers!

[tsantalis]

@pouryafard75 We can check if getTagName returns something, and if not we can check the optionalTagName property. We will use TagName as the label for this kind of nodes.

@jrfaller Please give us some time to get this change in the final 4.0.0 release of GumTree

[jrfaller]

@tsantalis sure!

[pouryafard75]

@tsantalis @jrfaller Is this structure acceptable? Please share your thoughts.

CompilationUnit [0,34]
    TypeDeclaration [0,34]
        Javadoc [0,19]
            TagElement [4,17]
                TAG_NAME: @author [4,11]
                TextElement:  john  [11,17]
        TYPE_DECLARATION_KIND: class [24,29]
        SimpleName: C [30,31]

[pouryafard75]

@jrfaller

• I think the original implementation has some minor issues regarding the offests of the TextElement (which now came to surface because of the optionalTagName 😄), for instance TextElement: john [11,17] begins from 11 even though it seems 12 is the correct beginning offset. Please confirm this.

• Shall I remove @ symbol from the TAG_NAME label?

I am using the following code snipet to test (in case you want to examine the offsets on your own)

/** @author john */
    class C {}

[tsantalis]

@pouryafard75 Is it a convention to use upper case names for AST node properties? TAG_NAME TYPE_DECLARATION_KIND

[pouryafard75]

@tsantalis Yes, I tried to be consistent with the codebase.

[jrfaller]

Hi all! Uppercase is for types that are not directly converted from the JDT AST therefore it's perfect for this case! @pouryafard75 I think the structure is good!

[pouryafard75]

@jrfaller You can close this issue.