This repository has been archived by the owner. It is now read-only.
Permalink
Browse files

Properly resolve method receiver definitions.

The various parts of ruby-lint that deal with method calls on receiver
objects/constants are now able to do so without breaking things. Having said
that, I'm currently not very happy with the code of
RubyLint::Helper::Scoping#resolve_method_receiver as I feel it's too complex
and tries to do too much on its own. I plan on breaking this method into
smaller methods and cleaning it up in the following commits.

Signed-off-by: Yorick Peterse <yorickpeterse@gmail.com>
  • Loading branch information...
1 parent 084ec5e commit 2e9a7af4a1d07fc8601491c2a87bb34f01a1deca @YorickPeterse committed Nov 17, 2012
@@ -15,80 +15,18 @@ class MethodValidation < RubyLint::Callback
#
DESCRIPTION = 'Validates method calls and the specified parameters.'
- ##
- # Hash containing the various Ruby classes that are used to represent
- # various types.
- #
- # @return [Hash]
- #
- TYPE_CLASSES = {
- :string => 'String',
- :integer => 'Fixnum', # Fixnum and Bignum share the same methods.
- :float => 'Float',
- :symbol => 'Symbol',
- :array => 'Array',
- :hash => 'Hash',
- :brace_block => 'Proc',
- :lambda => 'Proc',
- :regexp => 'Regexp',
- :range => 'Range'
- }
-
##
# Called when a method call is found.
#
# @param [RubyLint::Token::MethodToken] token
#
def on_method(token)
- # Method called on a receiver (e.g. `String.new`).
if token.receiver
- receiver_name = token.receiver.name
- receiver_scope = scope
- receiver_type = token.receiver.type
- method_type = :instance_method
-
- if receiver_name.is_a?(Array)
- return unless valid_constant_path?(token.receiver)
-
- receiver_scope = resolve_definition(receiver_name)
- end
-
- # Method calls on variables such as `name.upcase`.
- if token.receiver.is_a?(Token::VariableToken) \
- and receiver_type != :constant \
- and receiver_type != :constant_path
- value = receiver_scope.lookup(receiver_type, receiver_name)
-
- if !value.nil? and !value.token.value.nil?
- value = value.token.value
- receiver_type = TYPE_CLASSES[value.type]
- end
-
- # Extract the class from a method call.
- if value.respond_to?(:receiver)
- while value.respond_to?(:receiver) and !value.receiver.nil?
- value = value.receiver
- end
-
- receiver_type = value.name
- end
-
- # Methods called directly on a type such as `'name'.upcase`.
- elsif TYPE_CLASSES[receiver_type]
- receiver_type = TYPE_CLASSES[receiver_type]
-
- # Everything else.
- else
- method_type = :method
- receiver_type = receiver_name.is_a?(Array) \
- ? receiver_name[-1] \
- : receiver_name
- end
-
- # Retrieve the constant to check for the existence of the method.
- found = receiver_scope.lookup(:constant, receiver_type)
+ method_type, definition = resolve_method_receiver(token.receiver)
- if found and !definition_exists?(method_type, token, found)
+ if definition \
+ and method_type \
+ and !definition_exists?(method_type, token, definition)
if method_type == :instance_method
error = "undefined instance method #{token.name}"
else
@@ -47,12 +47,15 @@ def on_finish
# @param [RubyLint::Token::MethodDefinitionToken] token
#
def on_method_definition(token)
- @scopes << scope.lookup(
- token.receiver ? :method : :instance_method,
- token.name
- )
+ definition = scope
+ method_type = :instance_method
- @call_types << :instance_method
+ if token.receiver
+ method_type, definition = resolve_method_receiver(token.receiver)
+ end
+
+ @scopes << definition.lookup(method_type, token.name)
+ @call_types << method_type
call_method(:on_new_scope)
end
@@ -5,8 +5,8 @@ module Helper
# easily access scoping related information in subclasses of
# {RubyLint::Callback}.
#
- # Note that unlike {RubyLint::Helper::DefinitionResolver} this method does not
- # automatically update the `@scopes` array mentioned below, it merely
+ # Note that unlike {RubyLint::Helper::DefinitionResolver} this method does
+ # not automatically update the `@scopes` array mentioned below, it merely
# creates the required variables and provides a few helper methods.
#
# ## Methods
@@ -34,6 +34,25 @@ module Helper
# definition list of the current block of code that's being analyzed.
#
module Scoping
+ ##
+ # Hash containing the various Ruby classes that are used to represent
+ # various types.
+ #
+ # @return [Hash]
+ #
+ TYPE_CLASSES = {
+ :string => 'String',
+ :integer => 'Fixnum', # Fixnum and Bignum share the same methods.
+ :float => 'Float',
+ :symbol => 'Symbol',
+ :array => 'Array',
+ :hash => 'Hash',
+ :brace_block => 'Proc',
+ :lambda => 'Proc',
+ :regexp => 'Regexp',
+ :range => 'Range'
+ }
+
##
# @see RubyLint::Callback#initialize
#
@@ -115,6 +134,62 @@ def definition_exists?(type, token, scope = scope)
end
end
+ ##
+ # Retrieves the class for the method receiver along with the method call
+ # type. The method type is set in the first index, the definition in the
+ # second one.
+ #
+ # @param [RubyLint::Token::Token] token
+ # @return [Array|NilClass]
+ #
+ def resolve_method_receiver(token)
+ receiver_name = token.name
+ receiver_scope = scope
+ receiver_type = token.type
+ method_type = :instance_method
+
+ if receiver_name.is_a?(Array)
+ return unless valid_constant_path?(token)
+
+ receiver_scope = resolve_definition(receiver_name)
+ end
+
+ if receiver_name == 'self' and @namespace
+ receiver_type = @namespace[-1]
+
+ # Method calls on variables such as `name.upcase`.
+ elsif token.is_a?(Token::VariableToken) \
+ and receiver_type != :constant \
+ and receiver_type != :constant_path
+ value = receiver_scope.lookup(receiver_type, receiver_name)
+
+ if !value.nil? and !value.token.value.nil?
+ value = value.token.value
+ receiver_type = TYPE_CLASSES[value.type]
+ end
+
+ if value.respond_to?(:receiver)
+ while value.respond_to?(:receiver) and value.receiver
+ value = value.receiver
+ end
+
+ receiver_type = value.name
+ end
+
+ # Methods called directly on a type such as `'name'.upcase`.
+ elsif TYPE_CLASSES[receiver_type]
+ receiver_type = TYPE_CLASSES[receiver_type]
+
+ else
+ method_type = :method
+ receiver_type = receiver_name.is_a?(Array) \
+ ? receiver_name[-1] \
+ : receiver_name
+ end
+
+ return [method_type, receiver_scope.lookup(:constant, receiver_type)]
+ end
+
##
# Returns the call type to use for method calls.
#
@@ -0,0 +1,39 @@
+module RubyLint
+ module Spec
+ ##
+ # Helper module that removes some common cruft from specifications.
+ #
+ module Helper
+ ##
+ # Parses the given file path and returns an array of tokens.
+ #
+ # @param [String] file The file to parse.
+ # @return [Array]
+ #
+ def parse_file(file)
+ return Parser.new(File.read(file), file).parse
+ end
+
+ ##
+ # Creates a new instance of {RubyLint::Iterator} and binds all the
+ # available analysis classes to it.
+ #
+ # @param [RubyLint::Report] report An optional report to assign to the
+ # iterator.
+ # @return [RubyLint::Iterator]
+ #
+ def create_iterator(report = nil)
+ iterator = RubyLint::Iterator.new(report)
+
+ iterator.bind(RubyLint::Analyze::Definitions)
+ iterator.bind(RubyLint::Analyze::CodingStyle)
+ iterator.bind(RubyLint::Analyze::MethodValidation)
+ iterator.bind(RubyLint::Analyze::ShadowingVariables)
+ iterator.bind(RubyLint::Analyze::UndefinedVariables)
+ iterator.bind(RubyLint::Analyze::UnusedVariables)
+
+ return iterator
+ end
+ end # Helper
+ end # Spec
+end # RubyLint
@@ -0,0 +1,20 @@
+def IO.default_console_size
+ [
+ ENV["LINES"].to_i.nonzero? || 25,
+ ENV["COLUMNS"].to_i.nonzero? || 80,
+ ]
+end
+
+begin
+ require 'io/console'
+rescue LoadError
+ class IO
+ alias console_size default_console_size
+ end
+else
+ def IO.console_size
+ console.winsize
+ rescue NoMethodError
+ default_console_size
+ end
+end
View
@@ -1,5 +1,6 @@
require File.expand_path('../../lib/ruby-lint', __FILE__)
require 'bacon'
+require 'ruby-lint/spec/helper'
RubyLint::FIXTURES = File.expand_path('../fixtures', __FILE__)
@@ -0,0 +1,23 @@
+require File.expand_path('../../../../helper', __FILE__)
+
+describe 'Analyze code that defines data in a begin/rescue statement' do
+ extend RubyLint::Spec::Helper
+
+ it 'Define two class methods on IO' do
+ tokens = parse_file(File.join(RubyLint::FIXTURES, 'stdlib/size.rb'))
+ iterator = create_iterator
+
+ iterator.run(tokens)
+
+ scope = iterator.storage[:scope]
+ io = scope.lookup(:constant, 'IO')
+
+ io.lookup(:method, 'default_console_size') \
+ .class \
+ .should == RubyLint::Definition
+
+ io.lookup(:method, 'console_size') \
+ .class \
+ .should == RubyLint::Definition
+ end
+end
@@ -1,17 +1,12 @@
require File.expand_path('../../../../helper', __FILE__)
-describe 'Complex code analysis' do
- it 'Analyze fixtures/stdlib/un.rb' do
- code = File.read(File.join(RubyLint::FIXTURES, 'stdlib/un.rb'))
- tokens = RubyLint::Parser.new(code).parse
- iterator = RubyLint::Iterator.new
+describe 'Analyze a list of global methods defined in un.rb' do
+ extend RubyLint::Spec::Helper
+
+ it 'All the methods should be defined as instance methods' do
+ tokens = parse_file(File.join(RubyLint::FIXTURES, 'stdlib/un.rb'))
+ iterator = create_iterator
- iterator.bind(RubyLint::Analyze::Definitions)
- iterator.bind(RubyLint::Analyze::CodingStyle)
- iterator.bind(RubyLint::Analyze::MethodValidation)
- iterator.bind(RubyLint::Analyze::ShadowingVariables)
- iterator.bind(RubyLint::Analyze::UndefinedVariables)
- iterator.bind(RubyLint::Analyze::UnusedVariables)
iterator.run(tokens)
scope = iterator.storage[:scope]
View
@@ -25,6 +25,8 @@ task :stdlib do
iterator = RubyLint::Iterator.new
RubyLint.options.analyzers.each { |const| iterator.bind(const) }
+
+ iterator.run(tokens)
end
end
end

0 comments on commit 2e9a7af

Please sign in to comment.