Fix Style/ZeroLengthPredicate to ignore size and length variabl…

…es (rubocop#3147) Closes rubocop#3131 `Style/ZeroLengthPredicate` cop recognized offense for variables with names `length` and `size`. eg. `size == 0` This PR fixes the issue.
Neodelf · Oct 15, 2016 · 9f79ab2 · 9f79ab2
1 parent b775292
commit 9f79ab2
Show file tree

Hide file tree

Showing 3 changed files with 137 additions and 58 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,7 @@
 * [#3120](https://github.com/bbatsov/rubocop/issues/3120): Fix `Lint/UselessAccessModifier` reporting useless access modifiers inside {Class,Module,Struct}.new blocks. ([@owst][])
 * [#3125](https://github.com/bbatsov/rubocop/issues/3125): Fix `Rails/UniqBeforePluck` to ignore `uniq` with block. ([@tejasbubane][])
 * [#3116](https://github.com/bbatsov/rubocop/issues/3116): `Style/SpaceAroundKeyword` allows `&.` method calls after `super` and `yield`. ([@segiddins][])
+* [#3131](https://github.com/bbatsov/rubocop/issues/3131): Fix `Style/ZeroLengthPredicate` to ignore `size` and `length` variables. ([@tejasbubane][])
 
 ## 0.40.0 (2016-05-09)
 

diff --git a/lib/rubocop/cop/style/zero_length_predicate.rb b/lib/rubocop/cop/style/zero_length_predicate.rb
@@ -41,15 +41,15 @@ def on_send(node)
         end
 
         def_node_matcher :zero_length_predicate, <<-END
-          {(send (send _ ${:length :size}) $:== (int $0))
-           (send (int $0) $:== (send _ ${:length :size}))
-           (send (send _ ${:length :size}) $:<  (int $1))
-           (send (int $1) $:> (send _ ${:length :size}))}
+          {(send (send (...) ${:length :size}) $:== (int $0))
+           (send (int $0) $:== (send (...) ${:length :size}))
+           (send (send (...) ${:length :size}) $:<  (int $1))
+           (send (int $1) $:> (send (...) ${:length :size}))}
         END
 
         def_node_matcher :nonzero_length_predicate, <<-END
-          {(send (send _ ${:length :size}) ${:> :!=} (int $0))
-           (send (int $0) ${:< :!=} (send _ ${:length :size}))}
+          {(send (send (...) ${:length :size}) ${:> :!=} (int $0))
+           (send (int $0) ${:< :!=} (send (...) ${:length :size}))}
         END
 
         def autocorrect(node)

diff --git a/spec/rubocop/cop/style/zero_length_predicate_spec.rb b/spec/rubocop/cop/style/zero_length_predicate_spec.rb
@@ -10,7 +10,7 @@
     inspect_source(cop, source)
   end
 
-  shared_examples 'registers offense' do |code, message, expected|
+  shared_examples 'code with offense' do |code, message, expected|
     context "when checking #{code}" do
       let(:source) { code }
 
@@ -26,55 +26,133 @@
     end
   end
 
-  it_behaves_like 'registers offense', '[1, 2, 3].length == 0',
-                  'Use `empty?` instead of `length == 0`.',
-                  '[1, 2, 3].empty?'
-  it_behaves_like 'registers offense', '[1, 2, 3].size == 0',
-                  'Use `empty?` instead of `size == 0`.',
-                  '[1, 2, 3].empty?'
-  it_behaves_like 'registers offense', '0 == [1, 2, 3].length',
-                  'Use `empty?` instead of `0 == length`.',
-                  '[1, 2, 3].empty?'
-  it_behaves_like 'registers offense', '0 == [1, 2, 3].size',
-                  'Use `empty?` instead of `0 == size`.',
-                  '[1, 2, 3].empty?'
-
-  it_behaves_like 'registers offense', '[1, 2, 3].length < 1',
-                  'Use `empty?` instead of `length < 1`.',
-                  '[1, 2, 3].empty?'
-  it_behaves_like 'registers offense', '[1, 2, 3].size < 1',
-                  'Use `empty?` instead of `size < 1`.',
-                  '[1, 2, 3].empty?'
-  it_behaves_like 'registers offense', '1 > [1, 2, 3].length',
-                  'Use `empty?` instead of `1 > length`.',
-                  '[1, 2, 3].empty?'
-  it_behaves_like 'registers offense', '1 > [1, 2, 3].size',
-                  'Use `empty?` instead of `1 > size`.',
-                  '[1, 2, 3].empty?'
-
-  it_behaves_like 'registers offense', '[1, 2, 3].length > 0',
-                  'Use `!empty?` instead of `length > 0`.',
-                  '![1, 2, 3].empty?'
-  it_behaves_like 'registers offense', '[1, 2, 3].size > 0',
-                  'Use `!empty?` instead of `size > 0`.',
-                  '![1, 2, 3].empty?'
-  it_behaves_like 'registers offense', '[1, 2, 3].length != 0',
-                  'Use `!empty?` instead of `length != 0`.',
-                  '![1, 2, 3].empty?'
-  it_behaves_like 'registers offense', '[1, 2, 3].size != 0',
-                  'Use `!empty?` instead of `size != 0`.',
-                  '![1, 2, 3].empty?'
-
-  it_behaves_like 'registers offense', '0 < [1, 2, 3].length',
-                  'Use `!empty?` instead of `0 < length`.',
-                  '![1, 2, 3].empty?'
-  it_behaves_like 'registers offense', '0 < [1, 2, 3].size',
-                  'Use `!empty?` instead of `0 < size`.',
-                  '![1, 2, 3].empty?'
-  it_behaves_like 'registers offense', '0 != [1, 2, 3].length',
-                  'Use `!empty?` instead of `0 != length`.',
-                  '![1, 2, 3].empty?'
-  it_behaves_like 'registers offense', '0 != [1, 2, 3].size',
-                  'Use `!empty?` instead of `0 != size`.',
-                  '![1, 2, 3].empty?'
+  shared_examples 'code without offense' do |code|
+    let(:source) { code }
+
+    it 'does not register any offense' do
+      expect(cop.offenses.size).to eq(0)
+    end
+  end
+
+  context 'with arrays' do
+    it_behaves_like 'code with offense', '[1, 2, 3].length == 0',
+                    'Use `empty?` instead of `length == 0`.',
+                    '[1, 2, 3].empty?'
+    it_behaves_like 'code with offense', '[1, 2, 3].size == 0',
+                    'Use `empty?` instead of `size == 0`.',
+                    '[1, 2, 3].empty?'
+    it_behaves_like 'code with offense', '0 == [1, 2, 3].length',
+                    'Use `empty?` instead of `0 == length`.',
+                    '[1, 2, 3].empty?'
+    it_behaves_like 'code with offense', '0 == [1, 2, 3].size',
+                    'Use `empty?` instead of `0 == size`.',
+                    '[1, 2, 3].empty?'
+
+    it_behaves_like 'code with offense', '[1, 2, 3].length < 1',
+                    'Use `empty?` instead of `length < 1`.',
+                    '[1, 2, 3].empty?'
+    it_behaves_like 'code with offense', '[1, 2, 3].size < 1',
+                    'Use `empty?` instead of `size < 1`.',
+                    '[1, 2, 3].empty?'
+    it_behaves_like 'code with offense', '1 > [1, 2, 3].length',
+                    'Use `empty?` instead of `1 > length`.',
+                    '[1, 2, 3].empty?'
+    it_behaves_like 'code with offense', '1 > [1, 2, 3].size',
+                    'Use `empty?` instead of `1 > size`.',
+                    '[1, 2, 3].empty?'
+
+    it_behaves_like 'code with offense', '[1, 2, 3].length > 0',
+                    'Use `!empty?` instead of `length > 0`.',
+                    '![1, 2, 3].empty?'
+    it_behaves_like 'code with offense', '[1, 2, 3].size > 0',
+                    'Use `!empty?` instead of `size > 0`.',
+                    '![1, 2, 3].empty?'
+    it_behaves_like 'code with offense', '[1, 2, 3].length != 0',
+                    'Use `!empty?` instead of `length != 0`.',
+                    '![1, 2, 3].empty?'
+    it_behaves_like 'code with offense', '[1, 2, 3].size != 0',
+                    'Use `!empty?` instead of `size != 0`.',
+                    '![1, 2, 3].empty?'
+
+    it_behaves_like 'code with offense', '0 < [1, 2, 3].length',
+                    'Use `!empty?` instead of `0 < length`.',
+                    '![1, 2, 3].empty?'
+    it_behaves_like 'code with offense', '0 < [1, 2, 3].size',
+                    'Use `!empty?` instead of `0 < size`.',
+                    '![1, 2, 3].empty?'
+    it_behaves_like 'code with offense', '0 != [1, 2, 3].length',
+                    'Use `!empty?` instead of `0 != length`.',
+                    '![1, 2, 3].empty?'
+    it_behaves_like 'code with offense', '0 != [1, 2, 3].size',
+                    'Use `!empty?` instead of `0 != size`.',
+                    '![1, 2, 3].empty?'
+  end
+
+  context 'with hashes' do
+    it_behaves_like 'code with offense', '{ a: 1, b: 2 }.size == 0',
+                    'Use `empty?` instead of `size == 0`.',
+                    '{ a: 1, b: 2 }.empty?'
+    it_behaves_like 'code with offense', '0 == { a: 1, b: 2 }.size',
+                    'Use `empty?` instead of `0 == size`.',
+                    '{ a: 1, b: 2 }.empty?'
+
+    it_behaves_like 'code with offense', '{ a: 1, b: 2 }.size != 0',
+                    'Use `!empty?` instead of `size != 0`.',
+                    '!{ a: 1, b: 2 }.empty?'
+    it_behaves_like 'code with offense', '0 != { a: 1, b: 2 }.size',
+                    'Use `!empty?` instead of `0 != size`.',
+                    '!{ a: 1, b: 2 }.empty?'
+  end
+
+  context 'with strings' do
+    it_behaves_like 'code with offense', '"string".size == 0',
+                    'Use `empty?` instead of `size == 0`.',
+                    '"string".empty?'
+    it_behaves_like 'code with offense', '0 == "string".size',
+                    'Use `empty?` instead of `0 == size`.',
+                    '"string".empty?'
+
+    it_behaves_like 'code with offense', '"string".size != 0',
+                    'Use `!empty?` instead of `size != 0`.',
+                    '!"string".empty?'
+    it_behaves_like 'code with offense', '0 != "string".size',
+                    'Use `!empty?` instead of `0 != size`.',
+                    '!"string".empty?'
+  end
+
+  context 'with collection variables' do
+    it_behaves_like 'code with offense', 'collection.size == 0',
+                    'Use `empty?` instead of `size == 0`.',
+                    'collection.empty?'
+    it_behaves_like 'code with offense', '0 == collection.size',
+                    'Use `empty?` instead of `0 == size`.',
+                    'collection.empty?'
+
+    it_behaves_like 'code with offense', 'collection.size != 0',
+                    'Use `!empty?` instead of `size != 0`.',
+                    '!collection.empty?'
+    it_behaves_like 'code with offense', '0 != collection.size',
+                    'Use `!empty?` instead of `0 != size`.',
+                    '!collection.empty?'
+  end
+
+  context 'when name of the variable is `size` or `length`' do
+    it_behaves_like 'code without offense', 'size == 0'
+    it_behaves_like 'code without offense', 'length == 0'
+
+    it_behaves_like 'code without offense', '0 == size'
+    it_behaves_like 'code without offense', '0 == length'
+
+    it_behaves_like 'code without offense', 'size <= 0'
+    it_behaves_like 'code without offense', 'length > 0'
+
+    it_behaves_like 'code without offense', '0 <= size'
+    it_behaves_like 'code without offense', '0 > length'
+
+    it_behaves_like 'code without offense', 'size != 0'
+    it_behaves_like 'code without offense', 'length != 0'
+
+    it_behaves_like 'code without offense', '0 != size'
+    it_behaves_like 'code without offense', '0 != length'
+  end
 end