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 autocorrect clobbering #569

Merged
merged 3 commits into from Oct 15, 2013

Conversation

jonas054
Copy link
Collaborator

While working on #553 and testing --auto-correct on lage code bases, I noticed that we would get errors from Parser because two corrections would conflict with each other. It seems that we have been rather lazy when implementing our auto-corrections, using the replace method everywhere. This PR tries to fix the problem by using more precise corrections on smaller ranges. There are also a couple of cases of double reporting that had to be fixed, since they would also lead to clobbering errors.

@bbatsov
Copy link
Collaborator

bbatsov commented Oct 15, 2013

Looks good. You'll have to rebase, though.

The StringLiterals cop shold not report offences for character literals.
Calling corrector.replace with rather big ranges increases the risk for clobbering, i.e. conflicting corrections. It's better to use insert_before, insert_after, and remove when possible, and replace only when it's relevant.
@jonas054
Copy link
Collaborator Author

Rebased.

@jonas054
Copy link
Collaborator Author

Hmm. What happened with the Rubinius run on Travis?

bbatsov added a commit that referenced this pull request Oct 15, 2013
@bbatsov bbatsov merged commit 2ad1157 into rubocop:master Oct 15, 2013
@jonas054 jonas054 deleted the fix_autocorrect_clobbering branch November 3, 2013 11:45
jeffcarbs added a commit to jeffcarbs/rubocop that referenced this pull request Nov 23, 2014
When there were multiple offenses for the SymbolProc cop (e.g.
`coll.map { |s| s.upcase }.map { |s| s.downcase }`) the cop would
raise an error: `Parser::Source::Rewriter detected clobbering`.

Digging into relevant commits I found some inspiration from
@jonas054 (rubocop#569). This commit changes the autocorrect for the
`SymbolProc` cop to be a little more precise in determining what to
correct. There's a new spec that I wrote that fails on master and
passes on this branch.
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.

None yet

2 participants