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

Review: feat(comment): add an AST node for the javadoc tags #1310

Merged
merged 8 commits into from May 19, 2017

Conversation

Projects
None yet
4 participants
@tdurieux
Collaborator

tdurieux commented May 19, 2017

This one is for you @msteinbeck

@msteinbeck

This comment has been minimized.

Show comment
Hide comment
@msteinbeck

msteinbeck May 19, 2017

Contributor

@tdurieux, you are awesome :).

Contributor

msteinbeck commented May 19, 2017

@tdurieux, you are awesome :).

@tdurieux

This comment has been minimized.

Show comment
Hide comment
@tdurieux

tdurieux May 19, 2017

Collaborator

http://spoon.gforge.inria.fr is dead. The build will ne pass.

Collaborator

tdurieux commented May 19, 2017

http://spoon.gforge.inria.fr is dead. The build will ne pass.

@surli

This comment has been minimized.

Show comment
Hide comment
@surli

surli May 19, 2017

Collaborator

It seems ok for me, I'm waiting for http://spoon.gforge.inria.fr to be up again, and when checks will be ok I'll merged.

Collaborator

surli commented May 19, 2017

It seems ok for me, I'm waiting for http://spoon.gforge.inria.fr to be up again, and when checks will be ok I'll merged.

@msteinbeck

This comment has been minimized.

Show comment
Hide comment
@msteinbeck

msteinbeck May 19, 2017

Contributor

According to Oracle, the first sentence of a JavaDoc string is referred as short summary. For instance:

/**
  * This is a short summary. Now follows the long description.
  */
...

It might be useful to add getters for the short summary and long description to the AST node.

Contributor

msteinbeck commented May 19, 2017

According to Oracle, the first sentence of a JavaDoc string is referred as short summary. For instance:

/**
  * This is a short summary. Now follows the long description.
  */
...

It might be useful to add getters for the short summary and long description to the AST node.

@tdurieux

This comment has been minimized.

Show comment
Hide comment
@tdurieux

tdurieux May 19, 2017

Collaborator

Does the long description contain the short summary?

Collaborator

tdurieux commented May 19, 2017

Does the long description contain the short summary?

@msteinbeck

This comment has been minimized.

Show comment
Hide comment
@msteinbeck

msteinbeck May 19, 2017

Contributor

Does the long description contain the short summary?

Oracle says: "Write the first sentence as a short summary of the method, as Javadoc automatically places it in the method summary table (and index)." (http://www.oracle.com/technetwork/articles/java/index-137868.html)

So it looks like it's up to us defining how to handle the summary. I would suggest the following:

  1. A JavaDoc string provides a description. It contains anything except of tags.
  2. If desired, one can fetch the summary using a getter.
  3. If desired, one can fetch the long description without the summary using a getter.
Contributor

msteinbeck commented May 19, 2017

Does the long description contain the short summary?

Oracle says: "Write the first sentence as a short summary of the method, as Javadoc automatically places it in the method summary table (and index)." (http://www.oracle.com/technetwork/articles/java/index-137868.html)

So it looks like it's up to us defining how to handle the summary. I would suggest the following:

  1. A JavaDoc string provides a description. It contains anything except of tags.
  2. If desired, one can fetch the summary using a getter.
  3. If desired, one can fetch the long description without the summary using a getter.

tdurieux added some commits May 19, 2017

@tdurieux

This comment has been minimized.

Show comment
Hide comment
@tdurieux

tdurieux May 19, 2017

Collaborator

"Write the first sentence
Is it the first sentence or the first line?

Collaborator

tdurieux commented May 19, 2017

"Write the first sentence
Is it the first sentence or the first line?

@msteinbeck

This comment has been minimized.

Show comment
Hide comment
@msteinbeck

msteinbeck May 19, 2017

Contributor

Is it the first sentence or the first line?

As far as I know it's the first sentence delimited using a "." (dot). Consequently, sentences like: "The is a short summary, i.e., ..." lead to a wrong summary, as JavaDoc creates the following summary: "The is a short summary, i."

Contributor

msteinbeck commented May 19, 2017

Is it the first sentence or the first line?

As far as I know it's the first sentence delimited using a "." (dot). Consequently, sentences like: "The is a short summary, i.e., ..." lead to a wrong summary, as JavaDoc creates the following summary: "The is a short summary, i."

@tdurieux

This comment has been minimized.

Show comment
Hide comment
@tdurieux

tdurieux May 19, 2017

Collaborator

For me is done

Collaborator

tdurieux commented May 19, 2017

For me is done

@tdurieux tdurieux changed the title from feat(comment): add an AST node for the javadoc tags to Review: feat(comment): add an AST node for the javadoc tags May 19, 2017

@@ -840,6 +873,14 @@ public void visitCtComment(CtComment comment) {
}
}
if (comment instanceof CtJavaDoc) {
if (!((CtJavaDoc) comment).getTags().isEmpty()) {
printer.write(" *").writeln().writeTabs();

This comment has been minimized.

@surli

surli May 19, 2017

Collaborator

There's a reason to not put a space after the * here?

@surli

surli May 19, 2017

Collaborator

There's a reason to not put a space after the * here?

@spoon-bot

This comment has been minimized.

Show comment
Hide comment
@spoon-bot

spoon-bot May 19, 2017

Collaborator

Revapi Analysis results

Old API: fr.inria.gforge.spoon:spoon-core:jar:5.7.0-20170519.095031-107

New API: fr.inria.gforge.spoon:spoon-core:jar:5.7.0-SNAPSHOT

Detected changes: 6.

Change 1

Name Element
Old field spoon.support.DefaultCoreFactory.serialVersionUID
New field spoon.support.DefaultCoreFactory.serialVersionUID
Code java.field.serialVersionUIDUnchanged
Description The class changed in an incompatible way with regards to serialization but the serialVersionUID field stayed unchanged. This might be ok and/or desired but is suspicious.
Breaking source: equivalent

Change 2

Name Element
Old none
New method spoon.reflect.code.CtJavaDoc spoon.reflect.factory.CoreFactory::createJavaDoc()
Code java.method.addedToInterface
Description Method was added to an interface.
Breaking source: breaking

Change 3

Name Element
Old none
New method spoon.reflect.code.CtJavaDocTag spoon.reflect.factory.CoreFactory::createJavaDocTag()
Code java.method.addedToInterface
Description Method was added to an interface.
Breaking source: breaking

Change 4

Name Element
Old none
New method spoon.reflect.code.CtJavaDocTag spoon.reflect.factory.Factory::createJavaDocTag(java.lang.String, spoon.reflect.code.CtJavaDocTag.TagType)
Code java.method.addedToInterface
Description Method was added to an interface.
Breaking source: breaking

Change 5

Name Element
Old none
New method void spoon.reflect.visitor.CtVisitor::visitCtJavaDoc(spoon.reflect.code.CtJavaDoc)
Code java.method.addedToInterface
Description Method was added to an interface.
Breaking source: breaking

Change 6

Name Element
Old none
New method void spoon.reflect.visitor.CtVisitor::visitCtJavaDocTag(spoon.reflect.code.CtJavaDocTag)
Code java.method.addedToInterface
Description Method was added to an interface.
Breaking source: breaking
Collaborator

spoon-bot commented May 19, 2017

Revapi Analysis results

Old API: fr.inria.gforge.spoon:spoon-core:jar:5.7.0-20170519.095031-107

New API: fr.inria.gforge.spoon:spoon-core:jar:5.7.0-SNAPSHOT

Detected changes: 6.

Change 1

Name Element
Old field spoon.support.DefaultCoreFactory.serialVersionUID
New field spoon.support.DefaultCoreFactory.serialVersionUID
Code java.field.serialVersionUIDUnchanged
Description The class changed in an incompatible way with regards to serialization but the serialVersionUID field stayed unchanged. This might be ok and/or desired but is suspicious.
Breaking source: equivalent

Change 2

Name Element
Old none
New method spoon.reflect.code.CtJavaDoc spoon.reflect.factory.CoreFactory::createJavaDoc()
Code java.method.addedToInterface
Description Method was added to an interface.
Breaking source: breaking

Change 3

Name Element
Old none
New method spoon.reflect.code.CtJavaDocTag spoon.reflect.factory.CoreFactory::createJavaDocTag()
Code java.method.addedToInterface
Description Method was added to an interface.
Breaking source: breaking

Change 4

Name Element
Old none
New method spoon.reflect.code.CtJavaDocTag spoon.reflect.factory.Factory::createJavaDocTag(java.lang.String, spoon.reflect.code.CtJavaDocTag.TagType)
Code java.method.addedToInterface
Description Method was added to an interface.
Breaking source: breaking

Change 5

Name Element
Old none
New method void spoon.reflect.visitor.CtVisitor::visitCtJavaDoc(spoon.reflect.code.CtJavaDoc)
Code java.method.addedToInterface
Description Method was added to an interface.
Breaking source: breaking

Change 6

Name Element
Old none
New method void spoon.reflect.visitor.CtVisitor::visitCtJavaDocTag(spoon.reflect.code.CtJavaDocTag)
Code java.method.addedToInterface
Description Method was added to an interface.
Breaking source: breaking

@surli surli merged commit 4769715 into INRIA:master May 19, 2017

4 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.1%) to 81.616%
Details
pull_request-INRIA-spoon-master-docker exec in /tmp/tmpnkzvipqe
Details
pull_request-INRIA-spoon-master-revapi exec in /tmp/tmp2ispgctj
Details

@tdurieux tdurieux deleted the tdurieux:feat_javadoc_tags branch Jun 13, 2017

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