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

feat: add CtStatement#comment to comment code #3494

Merged
merged 12 commits into from Jul 29, 2020

Conversation

LakshyAAAgrawal
Copy link
Contributor

Add "comment" method to comment out a statement.

In case of a CtBlock, if the parent is a method(the block defines a method body) then the statements within the block are commented out, otherwise, the statement itself is commented out. In case of multiline statements, a block comment is created, otherwise an inline comment is created.

Currently working on testcases for all the child classes of CtStatement.

Fixes #2013

@monperrus
Copy link
Collaborator

Thanks for the proposal.

There is a lot of code duplication here.

Here is a tentative solution, by putting adding a single method in CtElementImpl#comment

if (this instanceof CtStatement && getParent() instanceof CtBlock) {
 // do it
} else {
  throw new UnsupportedOperationException()
}

This would make this feature more understandable and maintainable.

WDYT?

@monperrus
Copy link
Collaborator

Thanks Benjamin.

@MartinWitt
Copy link
Collaborator

MartinWitt commented Jul 16, 2020

Couldn't the instanceOf be replaced with a visitor? Less usage of instanceOf is always better

@monperrus
Copy link
Collaborator

monperrus commented Jul 16, 2020 via email

@LakshyAAAgrawal
Copy link
Contributor Author

Thank you very much for the kind suggestions.

I will make the following changes:

  1. Remove CtStatementImpl#comment
  2. Add CtElementImpl#comment as described above
  3. Keep CtBlock#comment - The implementation for this instance is different from above, since in case of the block being a method body, the inner statements are commented out instead of commenting out the block itself
  4. Keep CtStatament#comment - I believe that #comment must be a part of the interface CtStatement but not CtElement.

Kindly let me know about your views on these and I will go ahead with the implementation.

@monperrus
Copy link
Collaborator

yes, and remove changes in CtClassImpl, CtUnaryOperatorImpl, CtInvocationImpl, CtConstructorCallImpl, etc.

thanks!

@LakshyAAAgrawal LakshyAAAgrawal changed the title WIP: feat: add CtStatement#comment review: feat: add CtStatement#comment Jul 19, 2020
@monperrus
Copy link
Collaborator

Thanks a lot!

In Spoon, we have one comment line per test that states the test intention, the contract that is being tested. By convention, it starts with // contract: eg

// contract: a single statement can be commented out

Could you add this contract line in each test?

Thanks!

@LakshyAAAgrawal
Copy link
Contributor Author

I have added the contract statements.

@monperrus
Copy link
Collaborator

Thanks a lot. LGTM, will merge.

@monperrus monperrus changed the title review: feat: add CtStatement#comment feat: add CtStatement#comment to comment code Jul 29, 2020
@monperrus monperrus merged commit 92dcea2 into INRIA:master Jul 29, 2020
@monperrus
Copy link
Collaborator

Thanks a lot @LakshyAAAgrawal for your contribution!

@monperrus monperrus mentioned this pull request Oct 14, 2020
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.

add method CtStatement#comment()
3 participants