Merge pull request #1994 from pry/rubocop-fixes

Various Rubocop fixes

.rubocop.yml

@@ -55,3 +55,6 @@ Metrics/LineLength:
 
 Bundler/OrderedGems:
   Enabled: false
+
+Style/DoubleNegation:
+  Enabled: false

.rubocop_todo.yml

@@ -125,34 +125,10 @@ Security/Eval:
     - 'spec/indent_spec.rb'
     - 'spec/wrapped_module_spec.rb'
 
-# Offense count: 9
-# Configuration parameters: EnforcedStyle.
-# SupportedStyles: inline, group
-Style/AccessModifierDeclarations:
-  Exclude:
-    - 'lib/pry/command.rb'
-    - 'lib/pry/forwardable.rb'
-    - 'lib/pry/helpers/base_helpers.rb'
-    - 'lib/pry/pry_instance.rb'
-    - 'spec/editor_spec.rb'
-    - 'spec/method/patcher_spec.rb'
-
-# Offense count: 2
-# Configuration parameters: AllowedChars.
-Style/AsciiComments:
-  Exclude:
-    - 'lib/pry/pry_instance.rb'
-    - 'lib/pry/terminal.rb'
-
 # Offense count: 39
 Style/CaseEquality:
   Enabled: false
 
-# Offense count: 1
-Style/ClassVars:
-  Exclude:
-    - 'lib/pry/method/patcher.rb'
-
 # Offense count: 27
 Style/CommentedKeyword:
   Exclude:
@@ -161,24 +137,3 @@ Style/CommentedKeyword:
 # Offense count: 138
 Style/Documentation:
   Enabled: false
-
-# Offense count: 10
-Style/DoubleNegation:
-  Exclude:
-    - 'lib/pry/code.rb'
-    - 'lib/pry/command_set.rb'
-    - 'lib/pry/commands/whereami.rb'
-    - 'lib/pry/method.rb'
-    - 'lib/pry/method/weird_method_locator.rb'
-    - 'lib/pry/pager.rb'
-    - 'lib/pry/slop/option.rb'
-    - 'lib/pry/wrapped_module.rb'
-
-# Offense count: 27
-Style/EvalWithLocation:
-  Exclude:
-    - 'lib/pry/core_extensions.rb'
-    - 'lib/pry/input_completer.rb'
-    - 'spec/commands/edit_spec.rb'
-    - 'spec/commands/watch_expression_spec.rb'
-    - 'spec/method_spec.rb'

lib/pry/command.rb

@@ -411,40 +411,6 @@ class Pry
       call_safely(*(captures + args))
     end
 
-    # Pass a block argument to a command.
-    # @param [String] arg_string The arguments (as a string) passed to the command.
-    #   We inspect these for a '| do' or a '| {' and if we find it we use it
-    #   to start a block input sequence. Once we have a complete
-    #   block, we save it to an accessor that can be retrieved from the command context.
-    #   Note that if we find the '| do' or '| {' we delete this and the
-    #   elements following it from `arg_string`.
-    def pass_block(arg_string)
-      # Workaround for weird JRuby bug where rindex in this case can return nil
-      # even when there's a match.
-      arg_string.scan(/\| *(?:do|\{)/)
-      block_index = $LAST_MATCH_INFO && $LAST_MATCH_INFO.offset(0)[0]
-
-      return unless block_index
-
-      block_init_string = arg_string.slice!(block_index..-1)[1..-1]
-      prime_string = "proc #{block_init_string}\n"
-
-      block_string =
-        if !Pry::Code.complete_expression?(prime_string)
-          pry_instance.r(target, prime_string)
-        else
-          prime_string
-        end
-
-      begin
-        self.command_block = target.eval(block_string)
-      rescue Pry::RescuableException
-        raise CommandError, "Incomplete block definition."
-      end
-    end
-
-    private :pass_block
-
     # Run the command with the given `args`.
     #
     # This is a public wrapper around `#call` which ensures all preconditions
@@ -482,6 +448,38 @@ class Pry
 
     private
 
+    # Pass a block argument to a command.
+    # @param [String] arg_string The arguments (as a string) passed to the command.
+    #   We inspect these for a '| do' or a '| {' and if we find it we use it
+    #   to start a block input sequence. Once we have a complete
+    #   block, we save it to an accessor that can be retrieved from the command context.
+    #   Note that if we find the '| do' or '| {' we delete this and the
+    #   elements following it from `arg_string`.
+    def pass_block(arg_string)
+      # Workaround for weird JRuby bug where rindex in this case can return nil
+      # even when there's a match.
+      arg_string.scan(/\| *(?:do|\{)/)
+      block_index = $LAST_MATCH_INFO && $LAST_MATCH_INFO.offset(0)[0]
+
+      return unless block_index
+
+      block_init_string = arg_string.slice!(block_index..-1)[1..-1]
+      prime_string = "proc #{block_init_string}\n"
+
+      block_string =
+        if !Pry::Code.complete_expression?(prime_string)
+          pry_instance.r(target, prime_string)
+        else
+          prime_string
+        end
+
+      begin
+        self.command_block = target.eval(block_string)
+      rescue Pry::RescuableException
+        raise CommandError, "Incomplete block definition."
+      end
+    end
+
     def find_hooks(event)
       event_name = "#{event}_#{command_name}"
       (hooks || self.class.hooks).get_hooks(event_name).values

lib/pry/core_extensions.rb

@@ -84,7 +84,7 @@ class Object
       return class_eval { binding } if Pry::Helpers::Platform.jruby? && name.nil?
 
       # class_eval sets both self and the default definee to this class.
-      return class_eval("binding")
+      return class_eval("binding", __FILE__, __LINE__)
     end
 
     unless self.class.method_defined?(:__pry__)

lib/pry/editor.rb

@@ -36,8 +36,6 @@ class Pry
       end
     end
 
-    private
-
     # Generate the string that's used to start the editor. This includes
     # all the flags we want as well as the file and line number we
     # want to open at.
@@ -54,6 +52,8 @@ class Pry
       end
     end
 
+    private
+
     # Start the editor running, using the calculated invocation string
     def open_editor(editor_invocation)
       # Note we dont want to use Pry.config.system here as that

lib/pry/forwardable.rb

@@ -17,7 +17,9 @@ class Pry
           instance_variable_get(target).__send__(private_delegate, *a, &b)
         end
       end
-      class_eval { private(*private_delegates) }
+      class_eval do
+        private(*private_delegates) # rubocop:disable Style/AccessModifierDeclarations
+      end
     end
   end
 end

lib/pry/helpers/base_helpers.rb

@@ -23,7 +23,6 @@ class Pry
         (Module === obj ? Module : Object).instance_method(method)
           .bind(obj).call(*args, &block)
       end
-      public :safe_send
 
       def find_command(name, set = Pry::Commands)
         command_match = set.find do |_, command|

lib/pry/helpers/platform.rb

@@ -5,7 +5,6 @@ class Pry
     # Contains methods for querying the platform that Pry is running on
     # @api public
     # @since v0.12.0
-    # rubocop:disable Style/DoubleNegation
     module Platform
       # @return [Boolean]
       def self.mac_osx?
@@ -47,14 +46,13 @@ class Pry
 
       # @return [Boolean]
       def self.mri_19?
-        !!(mri? && RUBY_VERSION.start_with?('1.9'))
+        mri? && RUBY_VERSION.start_with?('1.9')
       end
 
       # @return [Boolean]
       def self.mri_2?
-        !!(mri? && RUBY_VERSION.start_with?('2'))
+        mri? && RUBY_VERSION.start_with?('2')
       end
     end
-    # rubocop:enable Style/DoubleNegation
   end
 end

lib/pry/input_completer.rb

@@ -115,8 +115,12 @@ class Pry
           receiver = Regexp.last_match(1)
           message = Regexp.quote(Regexp.last_match(2))
           begin
-            candidates = eval("#{receiver}.constants.collect(&:to_s)", bind)
-            candidates |= eval("#{receiver}.methods.collect(&:to_s)", bind)
+            candidates = eval(
+              "#{receiver}.constants.collect(&:to_s)", bind, __FILE__, __LINE__
+            )
+            candidates |= eval(
+              "#{receiver}.methods.collect(&:to_s)", bind, __FILE__, __LINE__
+            )
           rescue Pry::RescuableException
             candidates = []
           end
@@ -153,15 +157,17 @@ class Pry
           receiver = Regexp.last_match(1)
           message = Regexp.quote(Regexp.last_match(2))
 
-          gv = eval("global_variables", bind).collect(&:to_s)
-          lv = eval("local_variables", bind).collect(&:to_s)
-          cv = eval("self.class.constants", bind).collect(&:to_s)
+          gv = eval("global_variables", bind, __FILE__, __LINE__).collect(&:to_s)
+          lv = eval("local_variables", bind, __FILE__, __LINE__).collect(&:to_s)
+          cv = eval("self.class.constants", bind, __FILE__, __LINE__).collect(&:to_s)
 
           if (gv | lv | cv).include?(receiver) || /^[A-Z]/ =~ receiver && /\./ !~ receiver
             # foo.func and foo is local var. OR
             # Foo::Bar.func
             begin
-              candidates = eval("#{receiver}.methods", bind).collect(&:to_s)
+              candidates = eval(
+                "#{receiver}.methods", bind, __FILE__, __LINE__
+              ).collect(&:to_s)
             rescue Pry::RescuableException
               candidates = []
             end
@@ -195,11 +201,13 @@ class Pry
           candidates = eval(
             "methods | private_methods | local_variables | " \
             "self.class.constants | instance_variables",
-            bind
+            bind, __FILE__, __LINE__ - 2
           ).collect(&:to_s)
 
-          if eval("respond_to?(:class_variables)", bind)
-            candidates += eval("class_variables", bind).collect(&:to_s)
+          if eval("respond_to?(:class_variables)", bind, __FILE__, __LINE__)
+            candidates += eval(
+              "class_variables", bind, __FILE__, __LINE__
+            ).collect(&:to_s)
           end
           candidates =
             (candidates | ReservedWords | custom_completions)

lib/pry/method/patcher.rb

@@ -3,7 +3,9 @@ class Pry
     class Patcher
       attr_accessor :method
 
+      # rubocop:disable Style/ClassVars
       @@source_cache = {}
+      # rubocop:enable Style/ClassVars
 
       def initialize(method)
         @method = method

lib/pry/pry_instance.rb

@@ -349,9 +349,9 @@ class Pry
 
     throw(:breakout) if current_binding.nil?
   end
-  private :handle_line
+  private :handle_line # rubocop:disable Style/AccessModifierDeclarations
 
-  # Potentially deprecated — Use `Pry::REPL.new(pry, :target => target).start`
+  # Potentially deprecated. Use `Pry::REPL.new(pry, :target => target).start`
   # (If nested sessions are going to exist, this method is fine, but a goal is
   # to come up with an alternative to nested sessions altogether.)
   def repl(target = nil)
@@ -401,7 +401,7 @@ class Pry
       @eval_string.force_encoding(val.encoding)
     end
   end
-  private :ensure_correct_encoding!
+  private :ensure_correct_encoding! # rubocop:disable Style/AccessModifierDeclarations
 
   # If the given line is a valid command, process it in the context of the
   # current `eval_string` and binding.
@@ -591,13 +591,13 @@ class Pry
       prompt_proc.call(conf.object, conf.nesting_level, conf.pry_instance)
     end
   end
-  private :generate_prompt
+  private :generate_prompt # rubocop:disable Style/AccessModifierDeclarations
 
   # the array that the prompt stack is stored in
   def prompt_stack
     @prompt_stack ||= []
   end
-  private :prompt_stack
+  private :prompt_stack # rubocop:disable Style/AccessModifierDeclarations
 
   # Pushes the current prompt onto a stack that it can be restored from later.
   # Use this if you wish to temporarily change the prompt.

lib/pry/slop/option.rb

@@ -56,7 +56,7 @@ class Pry
         @config.each_key do |key|
           predicate = :"#{key}?"
           unless self.class.method_defined?(predicate)
-            self.class.__send__(:define_method, predicate) { !!@config[key] }
+            self.class.__send__(:define_method, predicate) { !@config.key?(key) }
           end
         end
       end

lib/pry/terminal.rb

@@ -25,7 +25,7 @@ class Pry
       end
 
       def actual_screen_size
-        # The best way, if possible (requires non-jruby ≥1.9 or io-console gem)
+        # The best way, if possible (requires non-jruby >=1.9 or io-console gem)
         screen_size_according_to_io_console ||
           # Fall back to the old standby, though it might be stale:
           screen_size_according_to_env ||

spec/commands/edit_spec.rb

@@ -773,11 +773,13 @@ describe "edit" do
     before do
       @t = pry_tester
       class BinkyWink
-        eval %(
+        # rubocop:disable Style/EvalWithLocation
+        eval <<-RUBY
           def m1
             binding
           end
-        )
+        RUBY
+        # rubocop:enable Style/EvalWithLocation
 
         def m2
           _foo = :jeremy_jones
@@ -821,10 +823,9 @@ describe "edit" do
     before do
       @t = pry_tester
       class TrinkyDink
-        eval %(
-          def m
-          end
-        )
+        # rubocop:disable Style/EvalWithLocation
+        eval('def m; end')
+        # rubocop:enable Style/EvalWithLocation
       end
     end
 

spec/commands/watch_expression_spec.rb

@@ -3,8 +3,8 @@ describe "watch expression" do
   # 1) Create an instance of pry that can use for multiple calls
   # 2) Exercise the after_eval hook
   # 3) Return the output
-  def eval(expr)
-    output = @tester.eval expr
+  def watch_eval(expr)
+    output = @tester.eval(expr)
     @tester.pry.hooks.exec_hook :after_eval, nil, @tester.pry
     output
   end
@@ -12,27 +12,28 @@ describe "watch expression" do
   before do
     @tester = pry_tester
     @tester.pry.hooks.clear_event_hooks(:after_eval)
-    eval "watch --delete"
+    watch_eval('watch --delete')
   end
 
   it "registers the after_eval hook" do
-    eval 'watch 1+1'
+    watch_eval('watch 1+1')
+    watch_eval('')
     expect(@tester.pry.hooks.hook_exists?(:after_eval, :watch_expression)).to eq(true)
   end
 
   it "prints no watched expressions" do
-    expect(eval('watch')).to match(/No watched expressions/)
+    expect(watch_eval('watch')).to match(/No watched expressions/)
   end
 
   it "watches an expression" do
-    eval "watch 1+1"
-    expect(eval('watch')).to match(/=> 2/)
+    watch_eval 'watch 1+1'
+    expect(watch_eval('watch')).to match(/=> 2/)
   end
 
   it "watches a local variable" do
-    eval 'foo = :bar'
-    eval 'watch foo'
-    expect(eval('watch')).to match(/=> :bar/)
+    watch_eval('foo = :bar')
+    watch_eval('watch foo')
+    expect(watch_eval('watch')).to match(/=> :bar/)
   end
 
   it "prints when an expression changes" do
@@ -100,17 +101,17 @@ describe "watch expression" do
 
   describe "deleting expressions" do
     before do
-      eval 'watch :keeper'
-      eval 'watch :delete'
-      eval 'watch -d 2'
+      watch_eval('watch :keeper')
+      watch_eval('watch :delete')
+      watch_eval('watch -d 2')
     end
 
     it "keeps keeper" do
-      expect(eval('watch')).to match(/keeper/)
+      expect(watch_eval('watch')).to match(/keeper/)
     end
 
     it "deletes delete" do
-      expect(eval('watch')).not_to match(/delete/)
+      expect(watch_eval('watch')).not_to match(/delete/)
     end
   end
 end

spec/editor_spec.rb

@@ -1,12 +1,6 @@
 require 'pathname'
 
 describe Pry::Editor do
-  class Pry
-    class Editor
-      public :build_editor_invocation_string
-    end
-  end
-
   before do
     # OS-specific tempdir name. For GNU/Linux it's "tmp", for Windows it's
     # something "Temp".

spec/method/patcher_spec.rb

@@ -25,7 +25,9 @@ describe Pry::Method::Patcher do
   end
 
   it "should preserve visibility" do
-    class << @x; private :test; end
+    class << @x
+      private :test # rubocop:disable Style/AccessModifierDeclarations
+    end
     expect(@method.visibility).to eq :private
     @method.redefine "def @x.test; :after; end\n"
     expect(Pry::Method(@x.method(:test)).visibility).to eq :private

spec/method_spec.rb

@@ -638,13 +638,19 @@ describe Pry::Method do
     # keyword args are only on >= Ruby 2.1
     if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new("2.1")
       it 'should print the name of keyword args, with :? after the arg name' do
-        eval %{def @class.keyword(keyword_arg: "") end}
+        eval <<-RUBY, binding, __FILE__, __LINE__ + 1
+          def @class.keyword(keyword_arg: '')
+          end
+        RUBY
         signature = Pry::Method.new(@class.method(:keyword)).signature
         expect(signature).to eq("keyword(keyword_arg:?)")
       end
 
       it 'should print the name of keyword args, with : after the arg name' do
-        eval %{def @class.required_keyword(required_key:) end}
+        eval <<-RUBY, binding, __FILE__, __LINE__ + 1
+          def @class.required_keyword(required_key:)
+          end
+        RUBY
         signature = Pry::Method.new(@class.method(:required_keyword)).signature
         expect(signature).to eq("required_keyword(required_key:)")
       end