Parentheses around conditional, refs turboladen#91.

lib/tailor/cli/options.rb

@@ -62,6 +62,12 @@ def self.parse!(args)
 
           opt.separator ""
           opt.separator "  * Horizontal Spacing:"
+
+          opt.on('--allow-conditional-parentheses BOOL',
+            'Check for conditionals wrapped in parentheses?  (default: true)') do |c|
+            options.style[:allow_conditional_parentheses] = c
+          end
+
           opt.on('--allow-hard-tabs BOOL',
             'Check for hard tabs?  (default: true)') do |c|
             options.style[:allow_hard_tabs] = c

lib/tailor/configuration/style.rb

@@ -26,6 +26,7 @@ def self.define_property(name)
       end
 
       define_property :allow_camel_case_methods
+      define_property :allow_conditional_parentheses
       define_property :allow_hard_tabs
       define_property :allow_screaming_snake_case_classes
       define_property :allow_trailing_line_spaces
@@ -49,6 +50,7 @@ def self.define_property(name)
       # Sets up default values.
       def initialize
         allow_camel_case_methods(false, level: :error)
+        allow_conditional_parentheses(false, level: :error)
         allow_hard_tabs(false, level: :error)
         allow_screaming_snake_case_classes(false, level: :error)
         allow_trailing_line_spaces(false, level: :error)

lib/tailor/rulers/allow_conditional_parentheses.rb

@@ -0,0 +1,66 @@
+require_relative '../ruler'
+
+class Tailor
+  module Rulers
+    class AllowConditionalParenthesesRuler < Tailor::Ruler
+      def initialize(style, options)
+        super(style, options)
+        add_lexer_observers :nl
+      end
+
+      def nl_update(current_lexed_line, lineno, column)
+        measure(current_lexed_line, lineno)
+      end
+
+      # Checks to see if a conditional is unnecessarily wrapped in parentheses.
+      #
+      # @param [Fixnum] line The current lexed line.
+      # @param [Fixnum] lineno Line the problem was found on.
+      def measure(line, lineno)
+        return if @config
+        return unless line.any? { |t| conditional?(t) }
+        if tokens_before_lparen?(line) and ! tokens_after_rparen?(line)
+          column = lparen_column(line)
+          @problems << Problem.new('conditional_parentheses', lineno, column,
+            "Parentheses around conditional expression at column #{column}.",
+            @options[:level])
+        end
+      end
+
+      private
+
+      def conditional?(token)
+        token[1] == :on_kw and %w{case if unless while}.include?(token[2])
+      end
+
+      def lparen?(token)
+        token[1] == :on_lparen
+      end
+
+      def lparen_column(tokens)
+        tokens.find { |t| lparen?(t) }[0][1] + 1
+      end
+
+      def tokens_before_lparen?(tokens)
+        without_spaces(
+          tokens.select do |t|
+            true if (conditional?(t))..(lparen?(t))
+          end.tap { |t| t.shift; t.pop }
+        ).empty?
+      end
+
+      def tokens_after_rparen?(tokens)
+        without_spaces(
+          tokens.reverse.tap do |nl|
+            nl.shift
+          end.take_while { |t| t[1] != :on_rparen }
+        ).any?
+      end
+
+      def without_spaces(tokens)
+        tokens.reject { |t| t[1] == :on_sp }
+      end
+
+    end
+  end
+end

spec/functional/conditional_parentheses_spec.rb

@@ -0,0 +1,291 @@
+require 'spec_helper'
+require_relative '../support/conditional_parentheses_cases'
+require 'tailor/critic'
+require 'tailor/configuration/style'
+
+describe 'Conditional parentheses' do
+
+  def file_name
+    self.class.description
+  end
+
+  def contents
+    CONDITIONAL_PARENTHESES[file_name] || begin
+      raise "Example not found: #{file_name}"
+    end
+  end
+
+  before do
+    Tailor::Logger.stub(:log)
+    FakeFS.activate!
+    FileUtils.touch file_name
+    File.open(file_name, 'w') { |f| f.write contents }
+  end
+
+  let(:critic) { Tailor::Critic.new }
+
+  let(:style) do
+    style = Tailor::Configuration::Style.new
+    style.trailing_newlines 0, level: :off
+    style.allow_invalid_ruby true, level: :off
+    style
+  end
+
+  context :no_parentheses do
+    it 'does not warn' do
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+    it 'does not warn when parentheses are allowed' do
+      style.allow_conditional_parentheses true, level: :error
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+    it 'does not warn when parentheses are disabled' do
+      style.allow_conditional_parentheses false, level: :off
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+  end
+
+  context :with_parentheses do
+    it 'warns' do
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to eql [{
+        :type => 'conditional_parentheses',
+        :line => 1,
+        :column=> 4,
+        :message=> 'Parentheses around conditional expression at column 4.',
+        :level=> :error
+      }]
+    end
+    it 'does not warn when parentheses are allowed' do
+      style.allow_conditional_parentheses true, level: :error
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+    it 'does not warn when parentheses are disabled' do
+      style.allow_conditional_parentheses false, level: :off
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+  end
+
+  context :with_parentheses_no_space do
+    it 'warns' do
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to eql [{
+        :type => 'conditional_parentheses',
+        :line => 1,
+        :column=> 3,
+        :message=> 'Parentheses around conditional expression at column 3.',
+        :level=> :error
+      }]
+    end
+    it 'does not warn when parentheses are allowed' do
+      style.allow_conditional_parentheses true, level: :error
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+    it 'does not warn when parentheses are disabled' do
+      style.allow_conditional_parentheses false, level: :off
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+  end
+
+  context :method_call do
+    it 'does not warn' do
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+    it 'does not warn when parentheses are allowed' do
+      style.allow_conditional_parentheses true, level: :error
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+    it 'does not warn when parentheses are disabled' do
+      style.allow_conditional_parentheses false, level: :off
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+  end
+
+  context :indented_method_call do
+    it 'does not warn' do
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+    it 'does not warn when parentheses are allowed' do
+      style.allow_conditional_parentheses true, level: :error
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+    it 'does not warn when parentheses are disabled' do
+      style.allow_conditional_parentheses false, level: :off
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+  end
+
+  context :method_call_on_parens do
+    it 'does not warn' do
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+    it 'does not warn when parentheses are allowed' do
+      style.allow_conditional_parentheses true, level: :error
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+    it 'does not warn when parentheses are disabled' do
+      style.allow_conditional_parentheses false, level: :off
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+  end
+
+  context :double_parens do
+    it 'warns by default' do
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to eql [{
+        :type => 'conditional_parentheses',
+        :line => 1,
+        :column=> 4,
+        :message=> 'Parentheses around conditional expression at column 4.',
+        :level=> :error
+      }]
+    end
+    it 'does not warn when parentheses are allowed' do
+      style.allow_conditional_parentheses true, level: :error
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+    it 'does not warn when parentheses are disabled' do
+      style.allow_conditional_parentheses false, level: :off
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+  end
+
+  context :unless_no_parentheses do
+    it 'does not warn' do
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+    it 'does not warn when parentheses are allowed' do
+      style.allow_conditional_parentheses true, level: :error
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+    it 'does not warn when parentheses are disabled' do
+      style.allow_conditional_parentheses false, level: :off
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+  end
+
+  context :unless_with_parentheses do
+    it 'warns on parentheses' do
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to eql [{
+        :type => 'conditional_parentheses',
+        :line => 1,
+        :column=> 8,
+        :message=> 'Parentheses around conditional expression at column 8.',
+        :level=> :error
+      }]
+    end
+    it 'does not warn when parentheses are allowed' do
+      style.allow_conditional_parentheses true, level: :error
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+    it 'does not warn when parentheses are disabled' do
+      style.allow_conditional_parentheses false, level: :off
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+  end
+
+  context :case_no_parentheses do
+    it 'does not warn' do
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+    it 'does not warn when parentheses are allowed' do
+      style.allow_conditional_parentheses true, level: :error
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+    it 'does not warn when parentheses are disabled' do
+      style.allow_conditional_parentheses false, level: :off
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+  end
+
+  context :case_with_parentheses do
+    it 'warns on parentheses' do
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to eql [{
+        :type => 'conditional_parentheses',
+        :line => 1,
+        :column=> 6,
+        :message=> 'Parentheses around conditional expression at column 6.',
+        :level=> :error
+      }]
+    end
+    it 'does not warn when parentheses are allowed' do
+      style.allow_conditional_parentheses true, level: :error
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+    it 'does not warn when parentheses are disabled' do
+      style.allow_conditional_parentheses false, level: :off
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+  end
+
+  context :while_no_parentheses do
+    it 'does not warn' do
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+    it 'does not warn when parentheses are allowed' do
+      style.allow_conditional_parentheses true, level: :error
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+    it 'does not warn when parentheses are disabled' do
+      style.allow_conditional_parentheses false, level: :off
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+  end
+
+  context :while_with_parentheses do
+    it 'warns on parentheses' do
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to eql [{
+        :type => 'conditional_parentheses',
+        :line => 1,
+        :column=> 7,
+        :message=> 'Parentheses around conditional expression at column 7.',
+        :level=> :error
+      }]
+    end
+    it 'does not warn when parentheses are allowed' do
+      style.allow_conditional_parentheses true, level: :error
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+    it 'does not warn when parentheses are disabled' do
+      style.allow_conditional_parentheses false, level: :off
+      critic.check_file(file_name, style.to_hash)
+      expect(critic.problems[file_name]).to be_empty
+    end
+  end
+
+end