Skip to content

Commit

Permalink
Merge pull request #7 from jonas054/classes
Browse files Browse the repository at this point in the history
Classes
  • Loading branch information
bbatsov committed Jan 1, 2013
2 parents cd4e207 + 7535e9f commit c2eca33
Show file tree
Hide file tree
Showing 12 changed files with 102 additions and 61 deletions.
10 changes: 8 additions & 2 deletions lib/rubocop/cli.rb
Expand Up @@ -39,8 +39,7 @@ def run(args = ARGV)
line.chomp
end

tokens = Ripper.lex(source.join("\n"))
sexp = Ripper.sexp(source.join("\n"))
tokens, sexp = CLI.rip_source(source)

cops.each do |cop_klass|
cop = cop_klass.new
Expand All @@ -58,6 +57,13 @@ def run(args = ARGV)
return total_offences == 0 ? 0 : 1
end

def self.rip_source(source)
tokens = Ripper.lex(source.join("\n")).map { |t| Cop::Token.new(*t) }
sexp = Ripper.sexp(source.join("\n"))
Cop::Position.make_position_objects(sexp)
[tokens, sexp]
end

def show_cops_on_duty(cops)
puts "Reporting for duty:"
cops.each { |c| puts c }
Expand Down
32 changes: 19 additions & 13 deletions lib/rubocop/cop/align_parameters.rb
@@ -1,7 +1,5 @@
# encoding: utf-8

require_relative 'grammar'

module Rubocop
module Cop
class AlignParameters < Cop
Expand All @@ -12,26 +10,27 @@ def inspect(file, source, tokens, sexp)
@file = file
@tokens = tokens
@token_indexes = {}
@tokens.each_with_index { |t, ix| @token_indexes[t[0]] = ix }
@tokens.each_with_index { |t, ix| @token_indexes[t.pos] = ix }

each(:method_add_arg, sexp) do |method_add_arg|
args = get_args(method_add_arg) or next
first_arg, rest_of_args = divide_args(args)
method_name_pos = method_add_arg[1][1][-1]
method_name_ix = @token_indexes[method_name_pos]
@first_lparen_ix = method_name_ix +
@tokens[method_name_ix..-1].index { |t| t[1] == :on_lparen }
@first_lparen_ix = get_lparen_ix(method_add_arg)
pos_of_1st_arg = position_of(first_arg) or next # Give up.
rest_of_args.each do |arg|
pos = position_of(arg) or next # Give up if no position found.
if pos[0] != pos_of_1st_arg[0] && pos[1] != pos_of_1st_arg[1]
index = pos[0] - 1
add_offence(:convention, index, source[index], ERROR_MESSAGE)
if pos.lineno != pos_of_1st_arg.lineno
if pos.column != pos_of_1st_arg.column
index = pos.lineno - 1
add_offence(:convention, index, source[index], ERROR_MESSAGE)
end
end
end
end
end

private

def get_args(method_add_arg)
fcall = method_add_arg[1]
return nil if fcall[0] != :fcall
Expand Down Expand Up @@ -61,17 +60,24 @@ def divide_args(args)
[first_arg, rest_of_args]
end

def get_lparen_ix(method_add_arg)
method_name_pos = method_add_arg[1][1][-1]
method_name_ix = @token_indexes[method_name_pos]
method_name_ix +
@tokens[method_name_ix..-1].map(&:type).index(:on_lparen)
end

def position_of(sexp)
# Indentation inside a string literal is irrelevant.
return nil if sexp[0] == :string_literal

pos = find_pos_in_sexp(sexp) or return nil # Nil means not found.
ix = find_first_non_whitespace_token(pos) or return nil
@tokens[ix][0]
@tokens[ix].pos
end

def find_pos_in_sexp(sexp)
return sexp[2] if Array === sexp[2] && Fixnum === sexp[2][0]
return sexp[2] if Position === sexp[2]
sexp.grep(Array).each do |s|
pos = find_pos_in_sexp(s) and return pos
end
Expand All @@ -82,7 +88,7 @@ def find_first_non_whitespace_token(pos)
ix = @token_indexes[pos]
newline_found = false
start_ix = ix.downto(0) do |i|
case @tokens[i][2]
case @tokens[i].text
when '('
break i + 1 if i == @first_lparen_ix
when "\n"
Expand Down
26 changes: 25 additions & 1 deletion lib/rubocop/cop/cop.rb
Expand Up @@ -2,6 +2,30 @@

module Rubocop
module Cop
class Position < Struct.new :lineno, :column
# Does a recursive search and replaces each [lineno, column] array
# in the sexp with a Position object.
def self.make_position_objects(sexp)
if sexp[0] =~ /^@/
sexp[2] = Position.new(*sexp[2])
else
sexp.grep(Array).each { |s| make_position_objects(s) }
end
end

# The point of this class is to provide named attribute access.
# So we don't want backwards compatibility with array indexing.
undef_method :[]
end

class Token
attr_reader :pos, :type, :text

def initialize(pos, type, text)
@pos, @type, @text = Position.new(*pos), type, text
end
end

class Cop
attr_accessor :offences

Expand Down Expand Up @@ -65,7 +89,7 @@ def each(sym, sexp)
end

def whitespace?(token)
[:on_sp, :on_ignored_nl, :on_nl].include?(token[1])
[:on_sp, :on_ignored_nl, :on_nl].include?(token.type)
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions lib/rubocop/cop/empty_lines.rb
Expand Up @@ -9,11 +9,11 @@ def inspect(file, source, tokens, sexp)
each_parent_of(:def, sexp) do |parent|
defs = parent.select { |child| child[0] == :def }
identifier_of_first_def = defs[0][1]
current_row_ix = identifier_of_first_def[-1][0] - 1
current_row_ix = identifier_of_first_def[-1].lineno - 1
# The first def doesn't need to have an empty line above it,
# so we iterate starting at index 1.
defs[1..-1].each do |child|
next_row_ix = child[1][-1][0] - 1
next_row_ix = child[1][-1].lineno - 1
if source[current_row_ix..next_row_ix].grep(/^[ \t]*$/).empty?
add_offence(:convention, next_row_ix, source[next_row_ix],
ERROR_MESSAGE)
Expand Down
15 changes: 8 additions & 7 deletions lib/rubocop/cop/grammar.rb
Expand Up @@ -4,7 +4,7 @@ module Rubocop
module Cop
class Grammar
def initialize(tokens)
@tokens_without_pos = tokens.map { |tok| tok[1..-1] }
@tokens_without_pos = tokens.map { |t| [t.type, t.text] }
process_embedded_expressions
@token_indexes = {}
@tokens_without_pos.each_with_index { |t, i|
Expand All @@ -13,7 +13,7 @@ def initialize(tokens)
}
@ix = 0
@table = {}
token_positions = tokens.map { |tok| tok[0] }
token_positions = tokens.map { |t| [t.pos.lineno, t.pos.column] }
@index_by_pos = Hash[*token_positions.each_with_index.to_a.flatten(1)]
@special = {
assign: [:on_op, '='],
Expand All @@ -31,20 +31,20 @@ def initialize(tokens)
def process_embedded_expressions
state = :outside
brace_depth = 0
@tokens_without_pos.each_with_index do |(name, _), ix|
@tokens_without_pos.each_with_index do |(type, _), ix|
case state
when :outside
state = :inside_string if name == :on_tstring_beg
state = :inside_string if type == :on_tstring_beg
when :inside_string
case name
case type
when :on_tstring_end
state = :outside
when :on_embexpr_beg
brace_depth = 1
state = :inside_expr
end
when :inside_expr
case name
case type
when :on_lbrace
brace_depth += 1
when :on_rbrace
Expand Down Expand Up @@ -76,7 +76,8 @@ def correlate(sexp, path = [])
when /^@/
# Leaves in the grammar have a corresponding token with a
# position, which we search for and advance @ix.
@ix = @index_by_pos[sexp[-1]]
@ix = @index_by_pos[[sexp[-1].lineno, sexp[-1].column]]
fail "#{sexp}\n#{@index_by_pos}" unless @ix
@table[@ix] = path + [sexp[0]]
@ix += 1
when *@special.keys
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/hash_syntax.rb
Expand Up @@ -16,7 +16,7 @@ def inspect(file, source, tokens, sexp)
end
each(:assoc_new, sexp) do |assoc_new|
if assoc_new[1][0] == :symbol_literal
index = assoc_new[1][1][1][-1][0] - 1
index = assoc_new[1][1][1][-1].lineno - 1
add_offence(:convention, index, source[index], ERROR_MESSAGE)
end
end
Expand Down
10 changes: 5 additions & 5 deletions lib/rubocop/cop/indentation.rb
Expand Up @@ -9,9 +9,9 @@ def inspect(file, source, tokens, sexp)
case_tokens = find_keywords(tokens, 'case')
when_tokens = find_keywords(tokens, 'when')
each_when(sexp) do |case_ix|
when_pos = when_tokens.shift[0]
if when_pos[1] != case_tokens[case_ix][0][1]
index = when_pos[0] - 1
when_pos = when_tokens.shift.pos
if when_pos.column != case_tokens[case_ix].pos.column
index = when_pos.lineno - 1
add_offence(:convention, index, source[index], ERROR_MESSAGE)
end
end
Expand All @@ -25,8 +25,8 @@ def find_keywords(tokens, keyword)
end

def keyword?(tokens, ix, keyword)
tokens[ix][1..-1] == [:on_kw, keyword] &&
tokens[ix - 1][1] != :on_symbeg
[tokens[ix].type, tokens[ix].text] == [:on_kw, keyword] &&
tokens[ix - 1].type != :on_symbeg
end

# Does a depth first search for :when, yielding the index of the
Expand Down
8 changes: 4 additions & 4 deletions lib/rubocop/cop/numeric_literals.rb
Expand Up @@ -7,10 +7,10 @@ class NumericLiterals < Cop
'improve their readability.'

def inspect(file, source, tokens, sexp)
tokens.each do |pos, name, text|
if [:on_int, :on_float].include?(name) &&
text.split('.').grep(/\d{6}/).any?
index = pos[0] - 1
tokens.each do |t|
if [:on_int, :on_float].include?(t.type) &&
t.text.split('.').grep(/\d{6}/).any?
index = t.pos.lineno - 1
add_offence(:convention, index, source[index], ERROR_MESSAGE)
end
end
Expand Down
11 changes: 6 additions & 5 deletions lib/rubocop/cop/space_after_comma_etc.rb
Expand Up @@ -7,15 +7,16 @@ class SpaceAfterCommaEtc < Cop

def inspect(file, source, tokens, sexp)
tokens.each_index do |ix|
pos, name, text = tokens[ix]
kind = case name
t = tokens[ix]
kind = case t.type
when :on_comma then 'comma'
when :on_label then 'colon'
when :on_op then 'colon' if text == ':'
when :on_op then 'colon' if t.text == ':'
when :on_semicolon then 'semicolon'
end
if kind and not [:on_sp, :on_ignored_nl].include?(tokens[ix + 1][1])
index = pos[0] - 1
if kind and not [:on_sp,
:on_ignored_nl].include?(tokens[ix + 1].type)
index = t.pos.lineno - 1
add_offence(:convention, index, source[index],
ERROR_MESSAGE % kind)
end
Expand Down
34 changes: 17 additions & 17 deletions lib/rubocop/cop/surrounding_space.rb
Expand Up @@ -9,58 +9,58 @@ class SurroundingSpace < Cop

def inspect(file, source, tokens, sexp)
Grammar.new(tokens).correlate(sexp).sort.each do |ix, grammar_path|
pos, name, text = tokens[ix]
case name
t = tokens[ix]
case t.type
when :on_op
unless surrounded_by_whitespace?(tokens[ix - 1, 3])
unless ok_without_spaces?(grammar_path)
index = pos[0] - 1
index = t.pos.lineno - 1
add_offence(:convention, index, source[index],
ERROR_MESSAGE + "operator '#{text}'.")
ERROR_MESSAGE + "operator '#{t.text}'.")
end
end
when :on_lbrace
unless surrounded_by_whitespace?(tokens[ix - 1, 3])
index = pos[0] - 1
index = t.pos.lineno - 1
add_offence(:convention, index, source[index],
ERROR_MESSAGE + "'{'.")
end
when :on_rbrace
unless whitespace?(tokens[ix - 1])
index = pos[0] - 1
index = t.pos.lineno - 1
add_offence(:convention, index, source[index],
"Space missing to the left of '}'.")
end
end
end
tokens.each_index do |ix|
pos, name, text = tokens[ix]
t = tokens[ix]
prev, nxt = tokens.values_at(ix - 1, ix + 1)
offence_detected = case name
offence_detected = case t.type
when :on_lbracket, :on_lparen
nxt[1] == :on_sp
nxt.type == :on_sp
when :on_rbracket, :on_rparen
if prev[1] == :on_sp
if prev.type == :on_sp
prev_ns = previous_non_space(tokens, ix)
prev_ns && tokens_on_same_row?(prev_ns,
tokens[ix]) &&
# Avoid double repoting of [ ] and ( )
prev_ns[1] != :on_lbracket &&
prev_ns[1] != :on_lparen
prev_ns.type != :on_lbracket &&
prev_ns.type != :on_lparen
end
when :on_op
text == '**' &&
t.text == '**' &&
(whitespace?(prev) || whitespace?(nxt))
end
if offence_detected
index = pos[0] - 1
kind = case name
index = t.pos.lineno - 1
kind = case t.type
when :on_lparen, :on_rparen
'inside parentheses'
when :on_lbracket, :on_rbracket
'inside square brackets'
when :on_op
"around operator #{text}"
"around operator #{t.text}"
end
add_offence(:convention, index, source[index],
"Space #{kind} detected.")
Expand All @@ -71,7 +71,7 @@ def inspect(file, source, tokens, sexp)
private

def tokens_on_same_row?(t1, t2)
t1[0][0] == t2[0][0]
t1.pos.lineno == t2.pos.lineno
end

def previous_non_space(tokens, ix)
Expand Down
7 changes: 5 additions & 2 deletions spec/rubocop/cops/grammar_spec.rb
Expand Up @@ -6,7 +6,8 @@ module Rubocop
module Cop
describe Grammar do
EXAMPLE = '3.times { |i| x = "#{y}#{z}}" }'
let (:grammar) { Grammar.new(Ripper.lex(EXAMPLE)) }
tokens = Ripper.lex(EXAMPLE).map { |t| Token.new(*t) }
let (:grammar) { Grammar.new(tokens) }

it "correlates token indices to grammar paths" do
method_block = [:program, :method_add_block]
Expand Down Expand Up @@ -38,7 +39,9 @@ module Cop
[[1, 29], :on_sp, " "],
[[1, 30], :on_rbrace, "}"]]

grammar.correlate(Ripper.sexp(EXAMPLE)).should == {
sexp = Ripper.sexp(EXAMPLE)
Position.make_position_objects(sexp)
grammar.correlate(sexp).should == {
0 => method_block + [:call, :@int], # 3
2 => method_block + [:call, :@ident], # times
4 => brace_block, # {
Expand Down

0 comments on commit c2eca33

Please sign in to comment.