Merge pull request #8 from jonas054/more_rules

More rules

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'

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

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

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

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

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

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

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

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

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

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

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

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
 

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

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

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

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

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

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")

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
 

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

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

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