Skip to content

review: fix class with comments has invalid DeclarationSourcePosition#modifier#1931

Merged
monperrus merged 4 commits intoINRIA:masterfrom
pvojtechovsky:positionOnCommentedClass
Apr 3, 2018
Merged

review: fix class with comments has invalid DeclarationSourcePosition#modifier#1931
monperrus merged 4 commits intoINRIA:masterfrom
pvojtechovsky:positionOnCommentedClass

Conversation

@pvojtechovsky
Copy link
Copy Markdown
Collaborator

I am experimenting with another approach to implement Sniper mode and have found a bug:
Class with sources like this

public class /*comment*/ Test {}

has wrong DeclarationSourcePosition#modifier end.

The problem is in code
/spoon-core/src/main/java/spoon/support/compiler/jdt/PositionBuilder.java (line 137)
because it doesn't count with comments. So we need some comment aware parser/tokenizer here.

I guess, I can fix it. But if anybody else would like to implement that comment parser, I would gladly do something else ;-)

@pvojtechovsky pvojtechovsky force-pushed the positionOnCommentedClass branch from 6a70bf1 to bf647b8 Compare March 31, 2018 20:23
@pvojtechovsky pvojtechovsky force-pushed the positionOnCommentedClass branch from bf647b8 to 8f22c64 Compare March 31, 2018 20:29
@pvojtechovsky
Copy link
Copy Markdown
Collaborator Author

this PR contains another fix related to source position too. The type reference: List<T> had a source position which included only List, The <T> was missing.
But List<T>[] already included everything. It looks like JDT bug.
This PR fixes source position of List<T> so it is now List<T>.
The comments are handled well in both fixes - therefore I joined them together...
List field`

@pvojtechovsky pvojtechovsky changed the title WiP: fix class with comments has invalid DeclarationSourcePosition#modifier review: fix class with comments has invalid DeclarationSourcePosition#modifier Mar 31, 2018
@tdurieux
Copy link
Copy Markdown
Collaborator

tdurieux commented Apr 2, 2018

Thank you for this fix!


@Test
public void testPositionClassWithComments() throws Exception {
final Factory build = build(new File("src/test/java/spoon/test/position/testclasses/"));
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

contract?


@Test
public void testPositionParameterTypeReference() throws Exception {
//contract: the parameterized type refernce has a source position which includes parameter types, etc.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

typo in refernce

@monperrus
Copy link
Copy Markdown
Collaborator

seems like a serious series of changes about positions :-)

@pvojtechovsky
Copy link
Copy Markdown
Collaborator Author

seems like a serious series of changes about positions :-)

the bugs becomes visible in the light of my experimenting with the sniper mode.

This is ready for merge too.

@monperrus monperrus merged commit 5a978f5 into INRIA:master Apr 3, 2018
@surli surli mentioned this pull request Jun 25, 2018
@pvojtechovsky pvojtechovsky deleted the positionOnCommentedClass branch September 1, 2018 07:25
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