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

Type javadoc comment is added to compilation unit if it's at the very start of the file #3300

Open
slarse opened this issue Mar 11, 2020 · 9 comments
Labels

Comments

@slarse
Copy link
Collaborator

slarse commented Mar 11, 2020

Hi!

I found some behavior that I think is a bug. If a file starts with a Javadoc comment (i.e. no package statement, and the very first byte is a /), then that comment is attached to the compilation unit instead of the type. For example, this file will have the comment attached to the CU:

/**
 * A comment!
 */
class Cls {}

But this file will have it attached to the type (note leading space before first /):

 /**
 * A comment!
 */
class Cls {}

I would expect both those examples to have the comment attached to the type. The problem is that, in the first case, the source start position of the type is set to 30 bytes, and the source start of the comment is set to 0 bytes, so the check below fails on the type/comment pair, and the type is not set as the comment's parent.

boolean betweenElementPosition = this.isCommentBetweenElementPosition(element);

In the second case, the source start position of the type is set to 1 byte, and so is the start position of the comment, so the above check passes and the comment is added to the type.

A workaround is to use a VirtualFile as a resource, as then the case below is hit and the comment is added to the type regardless of position.

if (commentParent instanceof CtPackageDeclaration || commentParent instanceof CtCompilationUnit) {
File file = spoonUnit.getFile();
if (file == null) {
//it is a virtual compilation unit - e.g. Snipet compilation unit
//all such comments belongs to declared type
List<CtType<?>> types = spoonUnit.getDeclaredTypes();
if (types.size() > 0) {
types.get(0).addComment(comment);
return;
}

I'm not sure if this is a bug in Spoon, or in JDT. If the source start for the type was set to 0 in the first example, then it would be fine, but it's set to 30, which breaks it all. Anyway, here's a small test case that reproduces the issue.

@Test
public void commentIsAttachedToType_whenCommentAtStartOfFile() {
	Launcher launcher = new Launcher();
	launcher.addInputResource("Cls.java");
	CtModel model = launcher.buildModel();

	CtType<?> type = model.getAllTypes().iterator().next();

	assertEquals(1, type.getComments().size());
}

With Cls.java being this file:

/**
 * A comment!
 */
class Cls {}

I'm well aware that this is a corner case of corner cases and may not be worth fixing at all, but after spending over an hour figuring out where my comments were disappearing to I wanted to report it anyway :)

@monperrus
Copy link
Collaborator

Thanks for the bug report.

Could you make a PR with a failing test case? That's super helpful.

@slarse
Copy link
Collaborator Author

slarse commented Mar 13, 2020

Sure, I'm a bit busy right now but I'll add it over the weekend. Any idea of which test class it's appropriate to put it in? Otherwise I'll just pick a spot that seems good enough and you can move it if you think appropriate.

@monperrus
Copy link
Collaborator

I can reproduce the bug, it is a rare one because it only happens for classes in the default package (with no package declaration)

@monperrus
Copy link
Collaborator

Failing test case in #3315

@slarse
Copy link
Collaborator Author

slarse commented Mar 27, 2020

@monperrus Oh wow, sorry, I completely forgot about this :(. Next time I report a bug I'll add the test case PR at the same time.

I can reproduce the bug, it is a rare one because it only happens for classes in the default package (with no package declaration)

I don't think that's entirely the case. I'm fairly certain the bug is entirely dependent on the comment starting at the very first byte of the file. For example, with this file, the comment is nowhere to be found in the resulting tree:

/**
 * A comment!
 */

package test;

class Cls {}

So I don't think it's related to the package statement itself, but rather that putting the package statement at the top causes the comment to not start at the first byte of the file.

I've come across several projects that have the license included in each file, above the package statement, and those licenses always simply disappear for me. Obviously it's debatable what element the comment belongs to if it appears above the package statement, but I think it would be reasonable for it to belong to the closest defined type or the package declaration.

@slarse
Copy link
Collaborator Author

slarse commented Mar 27, 2020

Hmm, actually, putting any comment above the package declaration causes it to vanish, regardless of placement. So I may either be incorrect, or it's expected behavior that comments above package declarations aren't attached to any tree element, or that's a separate bug.

@monperrus
Copy link
Collaborator

it's expected behavior that comments above package declarations aren't attached to any tree element,

no, it's a bug, probably with the same cause.

I've added you as collaborator, you can add the new failing test case in branch https://github.com/monperrus/spoon/tree/fix-3300

thanks!

@slarse
Copy link
Collaborator Author

slarse commented Mar 27, 2020

On it!

@slarse
Copy link
Collaborator Author

slarse commented Mar 27, 2020

@monperrus Done. I thought about it for a while and thought it would make most sense for the comment to be attached to the package declaration, but I think it would also make sense to attach it to the closest type declaration. Edit at your preference!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants