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

Fix for a bug concerning multiples comments before a catch block #1074

Merged
merged 2 commits into from
Jan 3, 2017

Conversation

surli
Copy link
Collaborator

@surli surli commented Dec 24, 2016

Multiple comments placed before a catch block were ignored, see #1073. This PR contains test to spot the bug and a fix.

@monperrus
Copy link
Collaborator

OK for me. @tdurieux (expert of the comment mode), OK for you?

@surli
Copy link
Collaborator Author

surli commented Dec 25, 2016 via email

@monperrus
Copy link
Collaborator

monperrus commented Dec 25, 2016 via email

@surli
Copy link
Collaborator Author

surli commented Dec 25, 2016 via email


@Override
public void visitCtCatch(CtCatch e) {
if (comment.getPosition().getLine() <= e.getPosition().getLine()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

use sourceStart() instead of getLine

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually I used the same getLine which is used in visitCtClass which manage properly multiple inline comments. If I use here sourceStart() one comment is missing when writing something like:

try {}
// one comment
// two comment
catch {}

Copy link
Collaborator

Choose a reason for hiding this comment

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

It is strange because getLine uses sourceStart() to perform the line number

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What I understand is that sourceStart starts at the same position than the first comment, which means the equality is respected and the comment is included (it was the previous implementation). But then the sourceStart of the second comment is greater than this sourcePosition and so it's not considered.
Now with getLine either it considers all are on the same line, either it computes well the different lines using the line separator position, which is the case I think.

if (comment.getPosition().getLine() <= e.getPosition().getLine()) {
e.addComment(comment);
return;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

call super.visitCtCatch()

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This super call is directly implemented in the scan override, do we really need it here? It is missing in all the other overriding visitCtXX methods...

@monperrus monperrus merged commit ad4d458 into INRIA:master Jan 3, 2017
@surli surli deleted the bugComment branch January 5, 2017 13:37
monperrus pushed a commit to monperrus/spoon that referenced this pull request Jan 9, 2017
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.

3 participants