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

Use token helpers from rubocop-ast #8729

Merged
merged 1 commit into from Sep 18, 2020

Conversation

fatkodima
Copy link
Contributor

Followup of #8450
This pr uses new apis from rubocop/rubocop-ast#92

cc @marcandre

tokens = processed_source.tokens
begin_index = index_of_first_token(node)
return unless tokens[begin_index].left_brace?
begin_index = processed_source.send(:first_token_index, node)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should have made this methods public also, I think.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe, but I am not sure this is a good example here. Why not call tokens_within, use those and yield those?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, updated.
Do you have an idea on why CI fails on windows only?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure and I've got to run 🏃‍♂️

Copy link
Contributor

Choose a reason for hiding this comment

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

Released 0.4.2; if you bump the requirement this should be good to go 👍

check(tokens[end_index - 1], tokens[end_index])
end
check(tokens[0], tokens[1])
check(tokens[-2], tokens[-1]) if tokens.size > 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Previous condition translates to if tokens.size > 2, no? (technically != 2 but can't be < 2, right?). Still I don't see how that can create an issue, it's just an optimization...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to > 2, but the same offense.
Funny, that I tested on ruby 2.7 on mac and saw 1 offense on one of the test files, but on CI (non windows) no offenses.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, finally, I have figured it out - rubocop/rubocop-ast#116

The problem is that sorting is not stable (and this sometimes can change the relative order of tokens like @text="" @pos=123...123 and @text="}" @pos=123..124) and this can lead to incorrect tokens returned by ProcessedSource#tokens_within.

@mergify mergify bot merged commit 032e8a8 into rubocop:master Sep 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants