Skip to content

Commit

Permalink
Introduce force framework and convert VariableInspector into a force
Browse files Browse the repository at this point in the history
The force framework allows cops to share common logic and reduce wasted
CPU power by module mixins. A force implements custom processing and
invokes some hooks in cops that belong to the force.
  • Loading branch information
yujinakayama committed Apr 9, 2014
1 parent 4f33089 commit 4b144e1
Show file tree
Hide file tree
Showing 24 changed files with 231 additions and 91 deletions.
15 changes: 8 additions & 7 deletions lib/rubocop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,17 @@
require 'rubocop/cop/cop'
require 'rubocop/cop/commissioner'
require 'rubocop/cop/corrector'
require 'rubocop/cop/force'
require 'rubocop/cop/team'
require 'rubocop/cop/severity'

require 'rubocop/cop/variable_inspector'
require 'rubocop/cop/variable_inspector/locatable'
require 'rubocop/cop/variable_inspector/variable'
require 'rubocop/cop/variable_inspector/assignment'
require 'rubocop/cop/variable_inspector/reference'
require 'rubocop/cop/variable_inspector/scope'
require 'rubocop/cop/variable_inspector/variable_table'
require 'rubocop/cop/variable_force'
require 'rubocop/cop/variable_force/locatable'
require 'rubocop/cop/variable_force/variable'
require 'rubocop/cop/variable_force/assignment'
require 'rubocop/cop/variable_force/reference'
require 'rubocop/cop/variable_force/scope'
require 'rubocop/cop/variable_force/variable_table'

require 'rubocop/cop/mixin/annotation_comment'
require 'rubocop/cop/mixin/array_syntax'
Expand Down
21 changes: 12 additions & 9 deletions lib/rubocop/cop/commissioner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ def self.call_super(callback)
end
end

def initialize(cops, options = {})
def initialize(cops, forces, options = {})
@cops = cops
@forces = forces
@options = options
reset_errors
end
Expand All @@ -52,7 +53,8 @@ def #{callback}(node)
def investigate(processed_source)
reset_errors
prepare(processed_source)
invoke_cops_callback(processed_source)
invoke_custom_processing(@cops, processed_source)
invoke_custom_processing(@forces, processed_source)
process(processed_source.ast) if processed_source.ast
@cops.each_with_object([]) do |cop, offenses|
filename = processed_source.buffer.name
Expand All @@ -72,18 +74,19 @@ def prepare(processed_source)
@cops.each { |cop| cop.processed_source = processed_source }
end

# There are cops that require their own custom processing.
# There are cops/forces that require their own custom processing.
# If they define the #investigate method, all input parameters passed
# to the commissioner will be passed to the cop too in order to do
# its own processing.
def invoke_cops_callback(processed_source)
@cops.each do |cop|
def invoke_custom_processing(cops_or_forces, processed_source)
cops_or_forces.each do |cop|
next unless cop.respond_to?(:investigate)

filename = processed_source.buffer.name

# ignore files that are of no interest to the cop in question
next unless cop.relevant_file?(filename)
if cop.respond_to?(:relevant_file?)
# ignore files that are of no interest to the cop in question
filename = processed_source.buffer.name
next unless cop.relevant_file?(filename)
end

with_cop_error_handling(cop) do
cop.investigate(processed_source)
Expand Down
4 changes: 4 additions & 0 deletions lib/rubocop/cop/cop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ def initialize(config = nil, options = nil)
@corrections = []
end

def join_force?(force_class)
false
end

def cop_config
@config.for_cop(self)
end
Expand Down
41 changes: 41 additions & 0 deletions lib/rubocop/cop/force.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# encoding: utf-8

module Rubocop
module Cop
# A scaffold for concrete forces.
class Force
attr_reader :cops

def self.all
@all ||= []
end

def self.inherited(subclass)
all << subclass
end

def self.force_name
name.split('::').last
end

def initialize(cops)
@cops = cops
end

def name
self.class.force_name
end

def run_hook(method_name, *args)
cops.each do |cop|
next unless cop.respond_to?(method_name)
cop.send(method_name, *args)
end
end

def investigate(processed_source)
# Do custom processing and invoke #run_hook at arbitrary timing.
end
end
end
end
8 changes: 3 additions & 5 deletions lib/rubocop/cop/lint/shadowing_outer_local_variable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,13 @@ module Lint
# This is a mimic of the warning
# "shadowing outer local variable - foo" from `ruby -cw`.
class ShadowingOuterLocalVariable < Cop
include VariableInspector

MSG = 'Shadowing outer local variable - `%s`'

def investigate(processed_source)
inspect_variables(processed_source.ast)
def join_force?(force_class)
force_class == VariableForce
end

def before_declaring_variable(variable)
def before_declaring_variable(variable, variable_table)
return if variable.name.to_s.start_with?('_')

outer_local_variable = variable_table.find_variable(variable.name)
Expand Down
8 changes: 3 additions & 5 deletions lib/rubocop/cop/lint/useless_assignment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,13 @@ module Lint
# reassignments and properly handles varied cases such as branch, loop,
# rescue, ensure, etc.
class UselessAssignment < Cop
include VariableInspector

MSG = 'Useless assignment to variable - %s'

def investigate(processed_source)
inspect_variables(processed_source.ast)
def join_force?(force_class)
force_class == VariableForce
end

def after_leaving_scope(scope)
def after_leaving_scope(scope, variable_table)
scope.variables.each_value do |variable|
check_for_unused_assignments(variable)
check_for_unused_block_local_variable(variable)
Expand Down
10 changes: 9 additions & 1 deletion lib/rubocop/cop/team.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def inspect_file(processed_source)
return Lint::Syntax.offenses_from_diagnostics(diagnostics)
end

commissioner = Commissioner.new(cops)
commissioner = Commissioner.new(cops, forces)
offenses = commissioner.investigate(processed_source)
process_commissioner_errors(
processed_source.file_path, commissioner.errors)
Expand All @@ -50,6 +50,14 @@ def cops
end
end

def forces
@forces ||= Force.all.each_with_object([]) do |force_class, forces|
joining_cops = cops.select { |cop| cop.join_force?(force_class) }
next if joining_cops.empty?
forces << force_class.new(joining_cops)
end
end

private

def cop_enabled?(cop_class)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,27 @@

module Rubocop
module Cop
# This module provides a way to track local variables and scopes of Ruby.
# This is intended to be used as mix-in, and the user class may override
# some of the hook methods.
module VariableInspector
# This force provides a way to track local variables and scopes of Ruby.
# Cops intertact with this force need to override some of the hook methods.
#
# def before_entering_scope(scope, variable_table)
# end
#
# def after_entering_scope(scope, variable_table)
# end
#
# def before_leaving_scope(scope, variable_table)
# end
#
# def after_leaving_scope(scope, variable_table)
# end
#
# def before_declaring_variable(variable, variable_table)
# end
#
# def after_declaring_variable(variable, variable_table)
# end
class VariableForce < Force # rubocop:disable ClassLength
VARIABLE_ASSIGNMENT_TYPE = :lvasgn
REGEXP_NAMED_CAPTURE_TYPE = :match_with_lvasgn
VARIABLE_ASSIGNMENT_TYPES =
Expand Down Expand Up @@ -42,27 +59,26 @@ module VariableInspector
TWISTED_SCOPE_TYPES = [:block, :class, :sclass, :defs].freeze
SCOPE_TYPES = (TWISTED_SCOPE_TYPES + [:top_level, :module, :def]).freeze

def self.wrap_with_top_level_node(node)
# This is a custom node type, not defined in Parser.
Parser::AST::Node.new(:top_level, [node])
end

def variable_table
@variable_table ||= VariableTable.new(self)
end

# Starting point.
def inspect_variables(root_node)
def investigate(processed_source)
root_node = processed_source.ast
return unless root_node

# Wrap the root node with :top_level scope node.
top_level_node = wrap_with_top_level_node(root_node)
top_level_node = self.class.wrap_with_top_level_node(root_node)

inspect_variables_in_scope(top_level_node)
end

def wrap_with_top_level_node(node)
# This is a custom node type, not defined in Parser.
Parser::AST::Node.new(:top_level, [node])
end

module_function :wrap_with_top_level_node

# This is called for each scope recursively.
def inspect_variables_in_scope(scope_node)
variable_table.push_scope(scope_node)
Expand Down Expand Up @@ -339,24 +355,19 @@ def scanned_nodes
@scanned_nodes ||= []
end

# Hooks

def before_entering_scope(scope)
end

def after_entering_scope(scope)
end

def before_leaving_scope(scope)
end

def after_leaving_scope(scope)
end

def before_declaring_variable(variable_variable)
end

def after_declaring_variable(variable_variable)
# Hooks invoked by VariableTable.
[
:before_entering_scope,
:after_entering_scope,
:before_leaving_scope,
:after_leaving_scope,
:before_declaring_variable,
:after_declaring_variable
].each do |hook|
define_method(hook) do |arg|
# Invoke hook in cops.
run_hook(hook, arg, variable_table)
end
end

# Post condition loops
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

module Rubocop
module Cop
module VariableInspector
class VariableForce
# This class represents each assignment of a variable.
class Assignment
include Locatable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

module Rubocop
module Cop
module VariableInspector
class VariableForce
# This module provides a way to locate the conditional branch the node is
# in. This is intended to be used as mix-in.
module Locatable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

module Rubocop
module Cop
module VariableInspector
class VariableForce
# This class represents each reference of a variable.
class Reference
include Locatable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

module Rubocop
module Cop
module VariableInspector
class VariableForce
# A Scope represents a context of local variable visibility.
# This is a place where local variables belong to.
# A scope instance holds a scope node and variable entries.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

module Rubocop
module Cop
module VariableInspector
class VariableForce
# A Variable represents existance of a local variable.
# This holds a variable declaration node,
# and some states of the variable.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

module Rubocop
module Cop
module VariableInspector
class VariableForce
# A VariableTable manages the lifetime of all scopes and local variables
# in a program.
# This holds scopes as stack structure, and provides a way to add local
Expand Down

0 comments on commit 4b144e1

Please sign in to comment.