[Fix #3032] Autocorrect predicates without space before args - e.g. is_a?Integer #3033

graemeboy · 2016-04-12T00:41:30Z

Summary of problem:

Using rubocop autocorrect on the following lines of code:

false or 'a'.is_a?String
=> false || 'a'.is_a?(tring)

'1'.is_a?Integer or 1.is_a?Integer
=> '1'.is_a?(nteger) || 1.is_a?(nteger)

PR adds six example cases to the test specs, and some code that makes all pass.

rrosenblum · 2016-04-12T02:20:05Z

lib/rubocop/cop/style/and_or.rb

@@ -119,7 +119,9 @@ def correctable_send?(node)
        def whitespace_before_arg(node)
          sb = node.source_range.source_buffer
          begin_paren = node.loc.selector.end_pos
-          Parser::Source::Range.new(sb, begin_paren, begin_paren + 1)
+          end_paren = begin_paren
+          end_paren += 1 unless node.source_range.source.to_s =~ /\?[A-Za-z]/


This can be simplified to end_paren += 1 unless node.source =~ /\?[A-Za-z]/.

rrosenblum · 2016-04-12T02:27:09Z

This is an interesting issue. I guess this could happen for any method that ends in ? and then does not use a space. I think the regex may need to be tweaked to look for any non white space character.

I think that this would fall into your same error condition, but I don't think that it would be caught by the current regex.

@foo = Integer
bar.is_a?@foo

There are a few test failures for line lengths in the tests being too long.

Aside from a few small tweaks, it looks pretty good to me.

graemeboy · 2016-04-12T18:33:54Z

♻️

bbatsov · 2016-04-13T13:41:09Z

lib/rubocop/cop/style/and_or.rb

@@ -119,7 +119,9 @@ def correctable_send?(node)
        def whitespace_before_arg(node)
          sb = node.source_range.source_buffer
          begin_paren = node.loc.selector.end_pos
-          Parser::Source::Range.new(sb, begin_paren, begin_paren + 1)
+          end_paren = begin_paren
+          end_paren += 1 unless node.source =~ /\?[!\S]/


I'd add some comment here as it's not obvious what you're checking for.

bbatsov · 2016-04-13T13:44:29Z

You should also add a changelog entry mentioning this fix.

bbatsov · 2016-04-13T16:05:33Z

CHANGELOG.md

@@ -10,7 +10,7 @@
 * [#2993](https://github.com/bbatsov/rubocop/pull/2993): `Style/SpaceAfterColon` now checks optional keyword arguments. ([@owst][])

 ### Bug fixes
-
+* [#3033](https://github.com/bbatsov/rubocop/pull/3033): Fix autocorrecting parentheses for predicate methods without space before args. ([@graemeboy][])


The PR says this fixes #3032.

The PR itself is #3033, ha. Thanks :)

I know. But we typically refer here to the bug reports.

graemeboy · 2016-04-13T20:47:19Z

@bbatsov ♻️

bbatsov · 2016-04-13T20:49:28Z

👍

rrosenblum reviewed Apr 12, 2016
View reviewed changes

bbatsov reviewed Apr 13, 2016
View reviewed changes

Fix autocorrecting predicates with args without spaces

cdda2ee

bbatsov merged commit 96728e4 into rubocop:master Apr 13, 2016

jonas054 mentioned this pull request Apr 23, 2016

Autocorrect "or" and "and" expressions with predicates that don't have spaces #3032

Closed

Neodelf pushed a commit to Neodelf/rubocop that referenced this pull request Oct 15, 2016

Fix autocorrecting predicates with args without spaces (rubocop#3033)

71f6010

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fix #3032] Autocorrect predicates without space before args - e.g. is_a?Integer #3033

[Fix #3032] Autocorrect predicates without space before args - e.g. is_a?Integer #3033

graemeboy commented Apr 12, 2016 •

edited

Loading

rrosenblum Apr 12, 2016

rrosenblum commented Apr 12, 2016

graemeboy commented Apr 12, 2016

bbatsov Apr 13, 2016

bbatsov commented Apr 13, 2016

bbatsov Apr 13, 2016

graemeboy Apr 13, 2016

bbatsov Apr 13, 2016

graemeboy commented Apr 13, 2016

bbatsov commented Apr 13, 2016

[Fix #3032] Autocorrect predicates without space before args - e.g. is_a?Integer #3033

[Fix #3032] Autocorrect predicates without space before args - e.g. is_a?Integer #3033

Conversation

graemeboy commented Apr 12, 2016 • edited Loading

rrosenblum Apr 12, 2016

Choose a reason for hiding this comment

rrosenblum commented Apr 12, 2016

graemeboy commented Apr 12, 2016

bbatsov Apr 13, 2016

Choose a reason for hiding this comment

bbatsov commented Apr 13, 2016

bbatsov Apr 13, 2016

Choose a reason for hiding this comment

graemeboy Apr 13, 2016

Choose a reason for hiding this comment

bbatsov Apr 13, 2016

Choose a reason for hiding this comment

graemeboy commented Apr 13, 2016

bbatsov commented Apr 13, 2016

graemeboy commented Apr 12, 2016 •

edited

Loading