Merge pull request #41 from gingermusketeer/add-settings-checks

Add settings checks

CHANGELOG.md

@@ -10,6 +10,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Fix formatting bug for `for i, in [[1, 2]] ; x ; end` issue  #45
 
 ### Changed
+Config parsing and handling:
+- No longer using `eval` to parse config files
+- Warnings have been added for invalid config keys and values
+
 The default for the following options has changed:
 - parens_in_def: ~~dynamic~~ > yes
 - last_has_comma: ~~dynamic~~ > always

lib/rufo.rb

@@ -13,6 +13,6 @@ def self.format(code, **options)
 require_relative "rufo/backport"
 require_relative "rufo/command"
 require_relative "rufo/dot_file"
+require_relative "rufo/settings"
 require_relative "rufo/formatter"
-require_relative "rufo/formatter/settings"
 require_relative "rufo/version"

lib/rufo/command.rb

@@ -158,14 +158,9 @@ def format(code, dir)
     @squiggly_warning = false
     formatter = Rufo::Formatter.new(code)
 
-    dot_rufo = @dot_file.find_in(dir)
-    if dot_rufo
-      begin
-        formatter.instance_eval(dot_rufo)
-      rescue => ex
-        STDERR.puts "Error evaluating #{dot_rufo}"
-        raise ex
-      end
+    options = @dot_file.get_config_in(dir)
+    unless options.nil?
+      formatter.init_settings(options)
     end
     formatter.format
     result = formatter.result

lib/rufo/dot_file.rb

@@ -5,12 +5,38 @@ def initialize
     @cache = {}
   end
 
+  def get_config_in(dir)
+    dot_rufo = find_in(dir)
+    if dot_rufo
+      return parse(dot_rufo)
+    end
+  end
+
   def find_in(dir)
     @cache.fetch(dir) do
       @cache[dir] = internal_find_in(dir)
     end
   end
 
+  def parse(file_contents)
+    file_contents.lines
+                 .map { |s| s.strip.split(/\s+/, 2) }
+                 .each_with_object({}) do |(name, value), acc|
+      value ||= ''
+      if value.start_with?(':')
+        value = value[1..-1].to_sym
+      elsif value == 'true'
+        value = true
+      elsif value == 'false'
+        value = false
+      else
+        $stderr.puts "Unknown config value=#{value.inspect} for #{name.inspect}"
+        next
+      end
+      acc[name.to_sym] = value
+    end
+  end
+
   def internal_find_in(dir)
     dir = File.expand_path(dir)
     file = File.join(dir, ".rufo")

lib/rufo/formatter.rb

@@ -3,6 +3,8 @@
 require "ripper"
 
 class Rufo::Formatter
+  include Rufo::Settings
+
   INDENT_SIZE = 2
 
   attr_reader :squiggly_flag
@@ -168,7 +170,7 @@ def format
 
     dedent_calls
     indent_literals
-    do_align_case_when if @align_case_when
+    do_align_case_when if align_case_when
     remove_lines_before_inline_declarations
   end
 
@@ -919,7 +921,7 @@ def visit_call_with_receiver(node)
       consume_end_of_line
 
       # If align_chained_calls is off, we still want to preserve alignment if it's already there
-      if @align_chained_calls || (@original_dot_column && @original_dot_column == current_token_column)
+      if align_chained_calls || (@original_dot_column && @original_dot_column == current_token_column)
         @name_dot_column = @dot_column || next_indent
         write_indent(@dot_column || next_indent)
       else
@@ -1031,7 +1033,7 @@ def visit_call_at_paren(node, args)
 
       if found_comma
         if needs_trailing_newline
-          write "," if @trailing_commas != :never && !block_arg
+          write "," if trailing_commas != :never && !block_arg
 
           next_token
           indent(next_indent) do
@@ -1046,7 +1048,7 @@ def visit_call_at_paren(node, args)
 
       if newline? || comment?
         if needs_trailing_newline
-          write "," if @trailing_commas == :always && want_trailing_comma
+          write "," if trailing_commas == :always && want_trailing_comma
 
           indent(next_indent) do
             consume_end_of_line
@@ -1057,7 +1059,7 @@ def visit_call_at_paren(node, args)
         end
       else
         if needs_trailing_newline && !found_comma
-          write "," if @trailing_commas == :always && want_trailing_comma
+          write "," if trailing_commas == :always && want_trailing_comma
           consume_end_of_line
           write_indent
         end
@@ -1476,7 +1478,7 @@ def visit_bodystmt(node)
 
     line = @line
 
-    indent_body body, want_multiline: inside_type_body && @double_newline_inside_type == :dynamic
+    indent_body body, want_multiline: inside_type_body && double_newline_inside_type == :dynamic
 
     while rescue_body
       # [:rescue, type, name, body, more_rescue]
@@ -1818,7 +1820,7 @@ def visit_binary(node)
       write " \\"
       write_line
       write_indent(next_indent)
-    elsif first_space && @spaces_around_binary == :dynamic
+    elsif first_space && spaces_around_binary == :dynamic
       write_space first_space[2]
     else
       write_space if needs_space
@@ -1834,9 +1836,9 @@ def visit_binary(node)
                          needed_indent: needed_indent,
                          token_column: token_column,
                          base_column: base_column,
-                         preserve_whitespace: @spaces_around_binary == :dynamic
+                         preserve_whitespace: spaces_around_binary == :dynamic
     else
-      if @spaces_around_binary == :one
+      if spaces_around_binary == :one
         write " " if needs_space
       elsif first_space
         write_space first_space[2]
@@ -1977,14 +1979,14 @@ def visit_def_from_name(name, params, body)
         next_token
       end
     elsif !empty_params?(params)
-      if @parens_in_def == :yes
+      if parens_in_def == :yes
         write "("
       else
         write_space
       end
 
       visit params
-      write ")" if @parens_in_def == :yes
+      write ")" if parens_in_def == :yes
       skip_space
     end
 
@@ -2649,7 +2651,7 @@ def visit_literal_elements(elements, inside_hash: false, inside_array: false, to
     end
 
     if needs_trailing_comma
-      case @trailing_commas
+      case trailing_commas
       when :always
         write "," unless wrote_comma
       when :never
@@ -2876,7 +2878,7 @@ def visit_when(node)
 
           indent_body next_exp[1]
         else
-          if @align_case_when
+          if align_case_when
             write_space
           else
             write_space_using_setting(first_space, :one)

lib/rufo/settings.rb

@@ -0,0 +1,29 @@
+module Rufo::Settings
+  OPTIONS = {
+    spaces_around_binary: [:dynamic, :one],
+    parens_in_def: [:yes, :dynamic],
+    double_newline_inside_type: [:dynamic, :no],
+    align_case_when: [false, true],
+    align_chained_calls: [false, true],
+    trailing_commas: [:always, :never],
+  }
+
+  attr_accessor *OPTIONS.keys
+
+  def init_settings(options)
+    OPTIONS.each do |name, valid_options|
+      default = valid_options.first
+      value = options.fetch(name, default)
+      unless valid_options.include?(value)
+        $stderr.puts "Invalid value for #{name}: #{value.inspect}. Valid " \
+                     "values are: #{valid_options.map(&:inspect).join(', ')}"
+        value = default
+      end
+      self.public_send("#{name}=", value)
+    end
+    diff = options.keys - OPTIONS.keys
+    diff.each do |key|
+      $stderr.puts "Invalid config option=#{key}"
+    end
+  end
+end

spec/lib/rufo/dot_file_spec.rb

@@ -0,0 +1,27 @@
+require "spec_helper"
+
+RSpec.describe Rufo::DotFile do
+  describe '#parse' do
+    it 'parses booleans' do
+      expect(subject.parse("key true\nother false")).to eql(
+        key: true,
+        other: false,
+      )
+    end
+
+    it 'parses symbols' do
+      expect(subject.parse("key :true")).to eql(
+        key: :true,
+      )
+    end
+
+    it 'warns about config it cannot parse' do
+      result = nil
+      expect {
+        result = subject.parse("key 1")
+      }.to output(%(Unknown config value="1" for "key"\n)).to_stderr
+
+      expect(result).to eql({})
+    end
+  end
+end

spec/lib/rufo/settings_spec.rb

@@ -0,0 +1,32 @@
+require "spec_helper"
+
+RSpec.describe Rufo::Settings do
+  class TestClass
+    include Rufo::Settings
+  end
+
+  subject { TestClass.new }
+
+  describe 'settings' do
+    it 'does not output any warnings for expected settings' do
+      expect {
+        subject.init_settings(spaces_around_binary: :dynamic)
+      }.to output('').to_stderr
+    end
+
+    it 'outputs a warning for invalid config value' do
+      exp_msg = "Invalid value for spaces_around_binary: :fake. Valid values " \
+      "are: :dynamic, :one\n"
+      expect {
+        subject.init_settings(spaces_around_binary: :fake)
+      }.to output(exp_msg).to_stderr
+    end
+
+    it 'outputs a warning for invalid config option' do
+      exp_msg = "Invalid config option=fake\n"
+      expect {
+        subject.init_settings(fake: :fake_too)
+      }.to output(exp_msg).to_stderr
+    end
+  end
+end