diff --git a/lib/rubocop.rb b/lib/rubocop.rb index 760786bd7b4..931e567c834 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -16,6 +16,8 @@ require 'rubocop/cop/end_of_line' require 'rubocop/cop/numeric_literals' require 'rubocop/cop/align_parameters' +require 'rubocop/cop/def_parentheses' +require 'rubocop/cop/if_then_else' require 'rubocop/report/report' require 'rubocop/report/plain_text' diff --git a/lib/rubocop/cop/align_parameters.rb b/lib/rubocop/cop/align_parameters.rb index a2eecfd87d2..8df6ffad2e5 100644 --- a/lib/rubocop/cop/align_parameters.rb +++ b/lib/rubocop/cop/align_parameters.rb @@ -21,8 +21,7 @@ def inspect(file, source, tokens, sexp) pos = position_of(arg) or next # Give up if no position found. 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) + add_offence(:convention, pos.lineno, ERROR_MESSAGE) end end end diff --git a/lib/rubocop/cop/cop.rb b/lib/rubocop/cop/cop.rb index f4f24d29c1f..56ac41af067 100644 --- a/lib/rubocop/cop/cop.rb +++ b/lib/rubocop/cop/cop.rb @@ -24,6 +24,10 @@ class Token def initialize(pos, type, text) @pos, @type, @text = Position.new(*pos), type, text end + + def to_s + "[[#{@pos.lineno}, #{@pos.column}], #@type, #{@text.inspect}]" + end end class Cop @@ -59,8 +63,8 @@ def has_report? !@offences.empty? end - def add_offence(file, line_number, line, message) - @offences << Offence.new(file, line_number, line, message) + def add_offence(file, line_number, message) + @offences << Offence.new(file, line_number, message) end private diff --git a/lib/rubocop/cop/def_parentheses.rb b/lib/rubocop/cop/def_parentheses.rb new file mode 100644 index 00000000000..3c566e90b67 --- /dev/null +++ b/lib/rubocop/cop/def_parentheses.rb @@ -0,0 +1,38 @@ +# encoding: utf-8 + +module Rubocop + module Cop + class DefParentheses < Cop + ERROR_MESSAGE = ['Use def with parentheses when there are arguments.', + "Omit the parentheses in defs when the method " + + "doesn't accept any arguments."] + EMPTY_PARAMS = [:params, nil, nil, nil, nil, nil] + + def inspect(file, source, tokens, sexp) + each(:def, sexp) do |def_sexp| + pos = def_sexp[1][-1] + case def_sexp[2][0] + when :params + if def_sexp[2] != EMPTY_PARAMS + add_offence(:convention, pos.lineno, ERROR_MESSAGE[0]) + end + when :paren + if def_sexp[2][1] == EMPTY_PARAMS + method_name_ix = tokens.index { |t| t.pos == pos } + start = method_name_ix + 1 + rparen_ix = start + tokens[start..-1].index { |t| t.text == ')' } + first_body_token = tokens[(rparen_ix + 1)..-1].find do |t| + not whitespace?(t) + end + if first_body_token.pos.lineno > pos.lineno + # Only report offence if there's a line break after + # the empty parens. + add_offence(:convention, pos.lineno, ERROR_MESSAGE[1]) + end + end + end + end + end + end + end +end diff --git a/lib/rubocop/cop/empty_lines.rb b/lib/rubocop/cop/empty_lines.rb index 594e0c10417..522c0885708 100644 --- a/lib/rubocop/cop/empty_lines.rb +++ b/lib/rubocop/cop/empty_lines.rb @@ -15,8 +15,7 @@ def inspect(file, source, tokens, sexp) defs[1..-1].each do |child| 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) + add_offence(:convention, next_row_ix + 1, ERROR_MESSAGE) end current_row_ix = next_row_ix end diff --git a/lib/rubocop/cop/encoding.rb b/lib/rubocop/cop/encoding.rb index ec9e000f216..4529bd8f7ac 100644 --- a/lib/rubocop/cop/encoding.rb +++ b/lib/rubocop/cop/encoding.rb @@ -9,7 +9,7 @@ class Encoding < Cop def inspect(file, source, tokens, sexp) unless source[0] =~ /#.*coding: (UTF|utf)-8/ message = sprintf(ERROR_MESSAGE) - add_offence(:convention, 0, 0, message) + add_offence(:convention, 1, message) end end end diff --git a/lib/rubocop/cop/end_of_line.rb b/lib/rubocop/cop/end_of_line.rb index 6c5863d6ae4..33d14cce1a4 100644 --- a/lib/rubocop/cop/end_of_line.rb +++ b/lib/rubocop/cop/end_of_line.rb @@ -8,7 +8,7 @@ class EndOfLine < Cop def inspect(file, source, tokens, sexp) source.each_with_index do |line, index| if line =~ /\r$/ - add_offence(:convention, index, line, ERROR_MESSAGE) + add_offence(:convention, index + 1, ERROR_MESSAGE) end end end diff --git a/lib/rubocop/cop/hash_syntax.rb b/lib/rubocop/cop/hash_syntax.rb index 1e2585ac650..91b296c39f5 100644 --- a/lib/rubocop/cop/hash_syntax.rb +++ b/lib/rubocop/cop/hash_syntax.rb @@ -16,8 +16,8 @@ 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].lineno - 1 - add_offence(:convention, index, source[index], ERROR_MESSAGE) + add_offence(:convention, assoc_new[1][1][1][-1].lineno, + ERROR_MESSAGE) end end end diff --git a/lib/rubocop/cop/if_then_else.rb b/lib/rubocop/cop/if_then_else.rb new file mode 100644 index 00000000000..57451ae62f2 --- /dev/null +++ b/lib/rubocop/cop/if_then_else.rb @@ -0,0 +1,49 @@ +# encoding: utf-8 + +module Rubocop + module Cop + class IfThenElse < Cop + ERROR_MESSAGE = { + multiline_if_then: + 'Never use then for multi-line if/unless.', + one_liner: + 'Favor the ternary operator (?:) over if/then/else/end constructs.', + semicolon: + 'Never use if x; Use the ternary operator instead.' + } + + def inspect(file, source, tokens, sexp) + tokens.each_with_index do |t, ix| + if t.type == :on_kw && ['if', 'unless'].include?(t.text) + error = ERROR_MESSAGE[kind_of_if(tokens, ix + 1)] + add_offence(:convention, t.pos.lineno, error) if error + end + end + end + + def kind_of_if(tokens, ix) + then_found = false + tokens[ix..-1].each do |t| + case t.type + when :on_kw + case t.text + when 'then' then then_found = true + when 'end' then return :one_liner + end + when :on_ignored_nl, :on_nl + break + when :on_semicolon + return :semicolon + when :on_comment + break if t.text =~ /\n/ + when :on_sp + nil + else + then_found = false + end + end + then_found ? :multiline_if_then : nil + end + end + end +end diff --git a/lib/rubocop/cop/indentation.rb b/lib/rubocop/cop/indentation.rb index 6c2fe612943..8006aa88440 100644 --- a/lib/rubocop/cop/indentation.rb +++ b/lib/rubocop/cop/indentation.rb @@ -11,8 +11,7 @@ def inspect(file, source, tokens, sexp) each_when(sexp) do |case_ix| 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) + add_offence(:convention, when_pos.lineno, ERROR_MESSAGE) end end end diff --git a/lib/rubocop/cop/line_length.rb b/lib/rubocop/cop/line_length.rb index 7e3916f04fb..797779042e7 100644 --- a/lib/rubocop/cop/line_length.rb +++ b/lib/rubocop/cop/line_length.rb @@ -10,7 +10,7 @@ def inspect(file, source, tokens, sexp) source.each_with_index do |line, index| if line.length > MAX_LINE_LENGTH message = sprintf(ERROR_MESSAGE, line.length, MAX_LINE_LENGTH) - add_offence(:convention, index, line, message) + add_offence(:convention, index + 1, message) end end end diff --git a/lib/rubocop/cop/numeric_literals.rb b/lib/rubocop/cop/numeric_literals.rb index 2f29417c3e3..bfe9b464004 100644 --- a/lib/rubocop/cop/numeric_literals.rb +++ b/lib/rubocop/cop/numeric_literals.rb @@ -10,8 +10,7 @@ def inspect(file, source, tokens, sexp) 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) + add_offence(:convention, t.pos.lineno, ERROR_MESSAGE) end end end diff --git a/lib/rubocop/cop/offence.rb b/lib/rubocop/cop/offence.rb index b021e4bed9a..618369bc110 100644 --- a/lib/rubocop/cop/offence.rb +++ b/lib/rubocop/cop/offence.rb @@ -3,14 +3,13 @@ module Rubocop module Cop class Offence - attr_accessor :severity, :line_number, :line, :message + attr_accessor :severity, :line_number, :message SEVERITIES = [:refactor, :convention, :warning, :error, :fatal] - def initialize(severity, line_number, line, message) + def initialize(severity, line_number, message) @severity = severity @line_number = line_number - @line = line @message = message end diff --git a/lib/rubocop/cop/space_after_comma_etc.rb b/lib/rubocop/cop/space_after_comma_etc.rb index 0e3bec42d5a..86595aeb5c0 100644 --- a/lib/rubocop/cop/space_after_comma_etc.rb +++ b/lib/rubocop/cop/space_after_comma_etc.rb @@ -16,9 +16,7 @@ def inspect(file, source, tokens, sexp) end 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) + add_offence(:convention, t.pos.lineno, ERROR_MESSAGE % kind) end end end diff --git a/lib/rubocop/cop/surrounding_space.rb b/lib/rubocop/cop/surrounding_space.rb index d5d0300394c..3162c325c2a 100644 --- a/lib/rubocop/cop/surrounding_space.rb +++ b/lib/rubocop/cop/surrounding_space.rb @@ -14,21 +14,17 @@ def inspect(file, source, tokens, sexp) when :on_op unless surrounded_by_whitespace?(tokens[ix - 1, 3]) unless ok_without_spaces?(grammar_path) - index = t.pos.lineno - 1 - add_offence(:convention, index, source[index], + add_offence(:convention, t.pos.lineno, ERROR_MESSAGE + "operator '#{t.text}'.") end end when :on_lbrace unless surrounded_by_whitespace?(tokens[ix - 1, 3]) - index = t.pos.lineno - 1 - add_offence(:convention, index, source[index], - ERROR_MESSAGE + "'{'.") + add_offence(:convention, t.pos.lineno, ERROR_MESSAGE + "'{'.") end when :on_rbrace unless whitespace?(tokens[ix - 1]) - index = t.pos.lineno - 1 - add_offence(:convention, index, source[index], + add_offence(:convention, t.pos.lineno, "Space missing to the left of '}'.") end end @@ -53,7 +49,6 @@ def inspect(file, source, tokens, sexp) (whitespace?(prev) || whitespace?(nxt)) end if offence_detected - index = t.pos.lineno - 1 kind = case t.type when :on_lparen, :on_rparen 'inside parentheses' @@ -62,8 +57,7 @@ def inspect(file, source, tokens, sexp) when :on_op "around operator #{t.text}" end - add_offence(:convention, index, source[index], - "Space #{kind} detected.") + add_offence(:convention, t.pos.lineno, "Space #{kind} detected.") end end end diff --git a/lib/rubocop/cop/tab.rb b/lib/rubocop/cop/tab.rb index 8510a4ba564..ae58e3d43aa 100644 --- a/lib/rubocop/cop/tab.rb +++ b/lib/rubocop/cop/tab.rb @@ -8,7 +8,7 @@ class Tab < Cop def inspect(file, source, tokens, sexp) source.each_with_index do |line, index| if line =~ /^ *\t/ - add_offence(:convention, index, line, ERROR_MESSAGE) + add_offence(:convention, index + 1, ERROR_MESSAGE) end end end diff --git a/lib/rubocop/cop/trailing_whitespace.rb b/lib/rubocop/cop/trailing_whitespace.rb index 4124c456ee6..fca1b72a0d9 100644 --- a/lib/rubocop/cop/trailing_whitespace.rb +++ b/lib/rubocop/cop/trailing_whitespace.rb @@ -8,7 +8,7 @@ class TrailingWhitespace < Cop def inspect(file, source, tokens, sexp) source.each_with_index do |line, index| if line =~ /.*[ \t]+$/ - add_offence(:convention, index, line, ERROR_MESSAGE) + add_offence(:convention, index + 1, ERROR_MESSAGE) end end end diff --git a/lib/rubocop/report/emacs_style.rb b/lib/rubocop/report/emacs_style.rb index 76f680114d7..dd4efb3b8a9 100644 --- a/lib/rubocop/report/emacs_style.rb +++ b/lib/rubocop/report/emacs_style.rb @@ -7,7 +7,7 @@ class EmacsStyle < PlainText # Generates a string representation of the report def generate report = entries.map do |e| - "#@filename:#{e.line_number + 1}: #{e.encode_severity}: #{e.message}" + "#@filename:#{e.line_number}: #{e.encode_severity}: #{e.message}" end report.join("\n") end diff --git a/spec/rubocop/cli_spec.rb b/spec/rubocop/cli_spec.rb index 9a113d5d26e..07d797a6dd2 100644 --- a/spec/rubocop/cli_spec.rb +++ b/spec/rubocop/cli_spec.rb @@ -39,7 +39,7 @@ module Report begin cli.run(['example.rb']).should == 1 $stdout.string.should == ['== example.rb ==', - 'C: 1: Trailing whitespace detected.', + 'C: 2: Trailing whitespace detected.', '', '1 files inspected, 1 offences detected', ''].join("\n") diff --git a/spec/rubocop/cops/align_parameters_spec.rb b/spec/rubocop/cops/align_parameters_spec.rb index e18667be551..fca6d29e39d 100644 --- a/spec/rubocop/cops/align_parameters_spec.rb +++ b/spec/rubocop/cops/align_parameters_spec.rb @@ -38,7 +38,7 @@ module Cop it "doesn't get confused by a symbol argument" do inspect_source(align, '', - ['add_offence(:convention, index, source[index],', + ['add_offence(:convention, index,', ' ERROR_MESSAGE % kind)']) align.offences.map(&:message).should == [] end @@ -54,7 +54,7 @@ module Cop 'func3(*a)', ]) align.offences.map(&:to_s).should == - ['C: 4: Align the parameters of a method call if they span more ' + + ['C: 5: Align the parameters of a method call if they span more ' + 'than one line.'] end diff --git a/spec/rubocop/cops/cop_spec.rb b/spec/rubocop/cops/cop_spec.rb index dca49cad2cf..5e04c6999e2 100644 --- a/spec/rubocop/cops/cop_spec.rb +++ b/spec/rubocop/cops/cop_spec.rb @@ -16,13 +16,13 @@ module Cop end it 'keeps track of offences' do - cop.add_offence('file', 0, 'line', 'message') + cop.add_offence('file', 1, 'message') cop.offences.size.should == 1 end it 'will report registered offences' do - cop.add_offence('file', 0, 'line', 'message') + cop.add_offence('file', 1, 'message') cop.has_report?.should be_true end diff --git a/spec/rubocop/cops/def_parentheses_spec.rb b/spec/rubocop/cops/def_parentheses_spec.rb new file mode 100644 index 00000000000..eb174d6300d --- /dev/null +++ b/spec/rubocop/cops/def_parentheses_spec.rb @@ -0,0 +1,48 @@ +# encoding: utf-8 + +require 'spec_helper' + +module Rubocop + module Cop + describe DefParentheses do + let (:def_par) { DefParentheses.new } + + it 'reports an offence for def with parameters but no parens' do + src = ['def func a, b', + 'end'] + inspect_source(def_par, '', src) + def_par.offences.map(&:message).should == + ['Use def with parentheses when there are arguments.'] + end + + it 'reports an offence for def with empty parens' do + src = ['def func()', + 'end'] + inspect_source(def_par, '', src) + def_par.offences.map(&:message).should == + ["Omit the parentheses in defs when the method doesn't accept any " + + "arguments."] + end + + it 'accepts def with arg and parens' do + src = ['def func(a)', + 'end'] + inspect_source(def_par, '', src) + def_par.offences.map(&:message).should == [] + end + + it 'accepts def with no args and no parens' do + src = ['def func', + 'end'] + inspect_source(def_par, '', src) + def_par.offences.map(&:message).should == [] + end + + it 'accepts empty parentheses in one liners' do + src = ["def to_s() join '/' end"] + inspect_source(def_par, '', src) + def_par.offences.map(&:message).should == [] + end + end + end +end diff --git a/spec/rubocop/cops/empty_lines_spec.rb b/spec/rubocop/cops/empty_lines_spec.rb index 3184a8abb04..33b477284f7 100644 --- a/spec/rubocop/cops/empty_lines_spec.rb +++ b/spec/rubocop/cops/empty_lines_spec.rb @@ -22,8 +22,7 @@ module Cop ' end', 'end']) empty_lines.offences.size.should == 2 - empty_lines.offences.map(&:line).sort.should == [' def o', - ' def p'] + empty_lines.offences.map(&:line_number).sort.should == [7, 11] end # Only one def, so rule about empty line *between* defs does not diff --git a/spec/rubocop/cops/if_then_else_spec.rb b/spec/rubocop/cops/if_then_else_spec.rb new file mode 100644 index 00000000000..383123e008f --- /dev/null +++ b/spec/rubocop/cops/if_then_else_spec.rb @@ -0,0 +1,74 @@ +# encoding: utf-8 + +require 'spec_helper' + +module Rubocop + module Cop + describe IfThenElse do + let (:if_then_else) { IfThenElse.new } + + # if + + it 'registers an offence for then in multiline if' do + inspect_source(if_then_else, '', ['if cond then', + 'end', + "if cond then\t", + 'end', + "if cond then ", + 'end', + 'if cond then # bad', + 'end']) + if_then_else.offences.size.should == 4 + end + + it 'accepts multiline if without then' do + inspect_source(if_then_else, '', ['if cond', + 'end']) + if_then_else.offences.map(&:message).sort.should == [] + end + + it 'registers an offence for one line if/then/end' do + inspect_source(if_then_else, '', ['if cond then run else dont end']) + if_then_else.offences.map(&:message).sort.should == + ['Favor the ternary operator (?:) over if/then/else/end constructs.'] + end + + it 'registers an offence for one line if/;/end' do + inspect_source(if_then_else, '', ['if cond; run else dont end']) + if_then_else.offences.map(&:message).sort.should == + ['Never use if x; Use the ternary operator instead.'] + end + + it 'accepts table style if/then/elsif/ends' do + inspect_source(if_then_else, '', + ['if @io == $stdout then str << "$stdout"', + 'elsif @io == $stdin then str << "$stdin"', + 'elsif @io == $stderr then str << "$stderr"', + 'else str << @io.class.to_s', + 'end']) + if_then_else.offences.map(&:message).sort.should == [] + end + + # unless + + it 'registers an offence for then in multiline unless' do + inspect_source(if_then_else, '', ['unless cond then', + 'end']) + if_then_else.offences.map(&:message).sort.should == + ['Never use then for multi-line if/unless.'] + end + + it 'accepts multiline unless without then' do + inspect_source(if_then_else, '', ['unless cond', + 'end']) + if_then_else.offences.map(&:message).sort.should == [] + end + + it 'registers an offence for one line unless/then/ends' do + inspect_source(if_then_else, '', ['unless cond then run end']) + if_then_else.offences.map(&:message).sort.should == + ['Favor the ternary operator (?:) over if/then/else/end constructs.'] + end + end + end +end diff --git a/spec/rubocop/cops/offence_spec.rb b/spec/rubocop/cops/offence_spec.rb index dd5d2ebeab7..51fc6b909ae 100644 --- a/spec/rubocop/cops/offence_spec.rb +++ b/spec/rubocop/cops/offence_spec.rb @@ -6,16 +6,15 @@ module Rubocop module Cop describe Offence do it 'has a few required attributes' do - offence = Offence.new(:convention, 1, 'line', 'message') + offence = Offence.new(:convention, 1, 'message') offence.severity.should == :convention offence.line_number.should == 1 - offence.line.should == 'line' offence.message.should == 'message' end it 'overrides #to_s' do - offence = Offence.new(:convention, 1, 'line', 'message') + offence = Offence.new(:convention, 1, 'message') offence.to_s.should == 'C: 1: message' end diff --git a/spec/rubocop/reports/emacs_style_spec.rb b/spec/rubocop/reports/emacs_style_spec.rb index dbcd6b8a667..5745d21a300 100644 --- a/spec/rubocop/reports/emacs_style_spec.rb +++ b/spec/rubocop/reports/emacs_style_spec.rb @@ -10,8 +10,8 @@ module Report it 'displays parsable text' do cop = Cop::Cop.new - cop.add_offence(:convention, 0, 'line', 'message 1') - cop.add_offence(:fatal, 10, 'line', 'message 2') + cop.add_offence(:convention, 1, 'message 1') + cop.add_offence(:fatal, 11, 'message 2') emacs_style << cop diff --git a/spec/rubocop/reports/report_spec.rb b/spec/rubocop/reports/report_spec.rb index fd75e299223..a0321e855ba 100644 --- a/spec/rubocop/reports/report_spec.rb +++ b/spec/rubocop/reports/report_spec.rb @@ -17,7 +17,7 @@ module Report it 'keeps track of offences' do cop = Cop::Cop.new - cop.add_offence(:convention, 0, 'line', 'message') + cop.add_offence(:convention, 1, 'message') report << cop