Merge pull request #2014 from gotrevor/TrivalPredicate_AllowPredicate…

…s_bugfix Fix `Style/TrivialAccessors` to support AllowPredicates: false
rubocop · Jul 5, 2015 · 2b94201 · 2b94201
2 parents 86775e9 + e771f4b
commit 2b94201
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,7 @@
 
 ### Bugs fixed
 
+* [#2014](https://github.com/bbatsov/rubocop/pull/2014): Fix `Style/TrivialAccessors` to support AllowPredicates: false. ([@gotrevor][])
 * [#1988](https://github.com/bbatsov/rubocop/issues/1988): Fix bug in `Style/ParallelAssignment` when assigning from `Module::CONSTANT`. ([@rrosenblum][])
 * [#1995](https://github.com/bbatsov/rubocop/pull/1995): Improve message for `Rails/TimeZone`. ([@palkan][])
 * [#1977](https://github.com/bbatsov/rubocop/issues/1977): Fix bugs in `Rails/Date` and `Rails/TimeZone` when using namespaced Time/Date. ([@palkan][])

diff --git a/lib/rubocop/cop/style/trivial_accessors.rb b/lib/rubocop/cop/style/trivial_accessors.rb
@@ -100,7 +100,7 @@ def allowed_reader?(method_name)
         def names_match?(method_name, body)
           ivar_name, = *body
 
-          method_name.to_s.chomp('=') == ivar_name[1..-1]
+          method_name.to_s.sub(/[=?]$/, '') == ivar_name[1..-1]
         end
 
         def trivial_accessor_kind(method_name, args, body)
@@ -127,6 +127,7 @@ def autocorrect(node)
         def autocorrect_instance(node)
           method_name, args, body = *node
           unless names_match?(method_name, body) &&
+                 !predicate?(method_name) &&
                  (kind = trivial_accessor_kind(method_name, args, body))
             return
           end

diff --git a/spec/rubocop/cop/style/trivial_accessors_spec.rb b/spec/rubocop/cop/style/trivial_accessors_spec.rb
@@ -179,6 +179,18 @@
     end
   end
 
+  context 'disallow predicates' do
+    let(:cop_config) { { 'AllowPredicates' => false } }
+
+    it 'does not accept predicate-like reader' do
+      inspect_source(cop,
+                     ['def foo?',
+                      '  @foo',
+                      'end'])
+      expect(cop.offenses.size).to eq(1)
+    end
+  end
+
   context 'allow predicates' do
     let(:cop_config) { { 'AllowPredicates' => true } }
 
@@ -192,7 +204,7 @@
   end
 
   context 'with whitelist' do
-    let(:cop_config) { { 'Whitelist' => ['to_foo', 'bar='] } }
+    let(:cop_config) { { 'Whitelist' => ['to_foo', 'bar=', 'foo?'] } }
 
     it 'accepts whitelisted reader' do
       inspect_source(cop,
@@ -209,6 +221,19 @@
                       ' end'])
       expect(cop.offenses).to be_empty
     end
+
+    context 'with AllowPredicates: false' do
+      let(:cop_config) { { 'AllowPredicates' => false } }
+
+      it 'accepts whitelisted predicate' do
+        pending 'Whitelist should override AllowPredicates'
+        inspect_source(cop,
+                       [' def foo?',
+                        '   @foo',
+                        ' end'])
+        expect(cop.offenses).to be_empty
+      end
+    end
   end
 
   context 'with DSL allowed' do
@@ -270,6 +295,20 @@
       end
     end
 
+    context 'predicate reader, with AllowPredicates: false' do
+      let(:cop_config) { { 'AllowPredicates' => false } }
+      let(:source) do
+        ['def foo?',
+         '  @foo',
+         'end']
+      end
+
+      it 'does not autocorrect' do
+        expect(autocorrect_source(cop, source)).to eq(source.join("\n"))
+        expect(cop.offenses.map(&:corrected?)).to eq [false]
+      end
+    end
+
     context 'trivial writer' do
       let(:source) { trivial_writer }