Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Pry::CodeObject, Pry::WrappedModule: introduce safe_to_evaluate? method

Unsure where to put safe_to_evaluate? method, so it's currently duplicated in
both the mentioned modules. If we can simply switch to a defined?(str) != "method" test
then we could put the method on Pry::Method.looks_like_a_method?(str, binding) and call it from both locations, avoiding duplication.

https://www.evernote.com/shard/s130/sh/4e839419-387e-43ab-81d5-06f87aa2cbac/2c640432e3ecccb2035c69c347f63af1
  • Loading branch information...
commit 470c938210bf43997e4ac6f58be1b8cecbe8d403 1 parent f1654d8
@banister banister authored
Showing with 37 additions and 32 deletions.
  1. +25 −7 lib/pry/code_object.rb
  2. +12 −25 lib/pry/wrapped_module.rb
View
32 lib/pry/code_object.rb
@@ -67,7 +67,9 @@ def binding_lookup
# lookup variables and constants and `self` that are not modules
def default_lookup
- if variable_or_constant_or_self?(str)
+
+ # we skip instance methods as we want those to fall through to method_or_class_lookup()
+ if safe_to_evaluate?(str) && !looks_like_an_instance_method?(str)
obj = target.eval(str)
# restrict to only objects we KNOW for sure support the full API
@@ -90,7 +92,7 @@ def method_or_class_lookup
# Pry::Method.from_binding when str is nil.
# Once we refactor Pry::Method.from_str() so it doesnt lookup
# from bindings, we can get rid of this check
- return nil if !str || str.empty?
+ return nil if str.to_s.empty?
obj = if str =~ /::(?:\S+)\Z/
Pry::WrappedModule.from_str(str,target) || Pry::Method.from_str(str, target)
@@ -107,12 +109,28 @@ def sourcable_object?(obj)
[::Proc, ::Method, ::UnboundMethod].any? { |o| obj.is_a?(o) }
end
- # Whether `str` represents `self` or a variable (or constant) when looked up
- # in the context of the `target` binding. This is used to
- # distinguish it from methods or expressions.
+
+ # Returns true if `str` looks like a method, i.e Klass#method
+ # We need to consider this case because method lookups should fall
+ # through to the `method_or_class_lookup()` method but a
+ # defined?() on a "Klass#method` string will see the `#` as a
+ # comment and only evaluate the `Klass` part.
+ # @param [String] str
+ # @return [Boolean] Whether the string looks like an instance method.
+ def looks_like_an_instance_method?(str)
+ str =~ /\S#\S/
+ end
+
+ # We use this method to decide whether code is safe to eval. Method's are
+ # generally not, but everything else is.
+ # TODO: is just checking != "method" enough??
+ # TODO: see duplication of this method in Pry::WrappedModule
# @param [String] str The string to lookup
- def variable_or_constant_or_self?(str)
- str.strip == "self" || str !~ /\S#\S/ && target.eval("defined? #{str} ") =~ /variable|constant/
+ # @return [Boolean]
+ def safe_to_evaluate?(str)
+ return true if str.strip == "self"
+ kind = target.eval("defined?(#{str})")
+ kind =~ /variable|constant/
end
def target_self
View
37 lib/pry/wrapped_module.rb
@@ -27,7 +27,7 @@ class WrappedModule
# @example
# Pry::WrappedModule.from_str("Pry::Code")
def self.from_str(mod_name, target=TOPLEVEL_BINDING)
- if variable_or_constant_or_self_from_binding_is_a_module?(target, mod_name)
+ if safe_to_evaluate?(mod_name, target)
Pry::WrappedModule.new(target.eval(mod_name))
else
nil
@@ -39,30 +39,17 @@ def self.from_str(mod_name, target=TOPLEVEL_BINDING)
class << self
private
- # Check whether the variable `mod_name` in binding `target` is a variable
- # or a constant. If we dont limit to variables/constants then `from_str` could end up
- # executing methods which is not good, i.e `show-source pry`
- # @param [Binding] target
- # @param [String] mod_name The string to lookup in the binding.
- # @return [Boolean] Whether the string represents a variable or constant.
- def variable_or_constant_or_self?(target, mod_name)
- return true if mod_name.strip == "self"
-
- kind = target.eval("defined?(#{mod_name})")
- kind == "constant" || kind =~ /variable/
- end
-
- # Verify that the looked up string represents 1. a variable or
- # constant and 2. Is a module.
- # @param [Binding] target
- # @param [String] mod_name The string to look up in the binding.
- # @return [Boolean] Whether the string represents a module.
- def variable_or_constant_or_self_from_binding_is_a_module?(target, mod_name)
- if variable_or_constant_or_self?(target, mod_name)
- target.eval(mod_name).is_a?(Module)
- else
- nil
- end
+ # We use this method to decide whether code is safe to eval. Method's are
+ # generally not, but everything else is.
+ # TODO: is just checking != "method" enough??
+ # TODO: see duplication of this method in Pry::CodeObject
+ # @param [String] str The string to lookup.
+ # @param [Binding] target Where the lookup takes place.
+ # @return [Boolean]
+ def safe_to_evaluate?(str, target)
+ return true if str.strip == "self"
+ kind = target.eval("defined?(#{str})")
+ kind =~ /variable|constant/
end
end
Please sign in to comment.
Something went wrong with that request. Please try again.