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

Style/AndOr enhanced autocorrect for method a or method a,b to method(a)||method(a,b) #1259

Closed
vrthra opened this issue Aug 9, 2014 · 4 comments

Comments

@vrthra
Copy link
Contributor

vrthra commented Aug 9, 2014

I am trying to get Style/AndOr cop to autocorrect on patterns like method a,b or method c to method(a,b) || method(c). I see that there is no change in AST between the old method a,b or method c and new method(a,b)||method(c), however I am at a loss how to implement it.

I see that the or node's two child expressions can be checked for node.loc.begin if the parenthesis exists, but I dont understand what to do when it does not.

@jonas054
Copy link
Collaborator

The closing parenthesis is the easiest. You just need to find the node representing the last argument, which you do with

receiver, method_name, *args = *node

and then use insert_after in autocorrect.

The opening parenthesis is a bit trickier. You could use insert_before and give args.first.loc.expression as argument, but there will be a space that we don't want before the parenthesis. A solution might need to include creating a new Range object and using the length of the method name when calculating its starting position.

@vrthra
Copy link
Contributor Author

vrthra commented Aug 10, 2014

@jonas054 Thank you for the hints, I found that I can get it to work if I replace the method name with method( and then apply the insert_after with ')'. Does that sound Ok? or is it better to go with creating a new Range as you mentioned?

@jonas054
Copy link
Collaborator

If you can get it to work, I think it's fine. I thought that the method name was just a symbol, not a node, so that it would be necesary to construct a new Range object. It will be easier to say for sure when there's code to examine. So make a pull request when you have a solution ready, and we'll review it.

@vrthra
Copy link
Contributor Author

vrthra commented Aug 11, 2014

Here is the code to go with it

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

No branches or pull requests

2 participants