diff --git a/CHANGELOG.md b/CHANGELOG.md index ad7e32a2b909..700a87a5a050 100644 --- 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 index db7840d72d56..69699ae3ddda 100644 --- 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 index 36912de1b313..10a171445b6d 100644 --- 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