diff --git a/features/valid_ruby.feature b/features/valid_ruby.feature index 3a5e607..4b8e8df 100644 --- a/features/valid_ruby.feature +++ b/features/valid_ruby.feature @@ -7,7 +7,7 @@ Feature: Valid Ruby Given a file named "extra_end.rb" with: """ def a_method - puts "stuff" + puts 'stuff' end end diff --git a/lib/tailor/cli/options.rb b/lib/tailor/cli/options.rb index 9edd916..eef27b4 100644 --- a/lib/tailor/cli/options.rb +++ b/lib/tailor/cli/options.rb @@ -75,6 +75,12 @@ def self.parse!(args) options.style[:allow_trailing_line_spaces] = c end + opt.on('--allow-unnecessary-double-quotes BOOL', + 'Check for unnecessary use of double quotes?', + '(default: false)') do |c| + options.style[:allow_unnecessary_double_quotes] = c + end + opt.on('--indentation-spaces NUMBER', INTEGER_OR_OFF, 'Spaces to expect indentation. (default: 2)') do |c| options.style[:indentation_spaces] = c diff --git a/lib/tailor/configuration/style.rb b/lib/tailor/configuration/style.rb index 873fcbb..ad3eed0 100644 --- a/lib/tailor/configuration/style.rb +++ b/lib/tailor/configuration/style.rb @@ -29,6 +29,7 @@ def self.define_property(name) define_property :allow_hard_tabs define_property :allow_screaming_snake_case_classes define_property :allow_trailing_line_spaces + define_property :allow_unnecessary_double_quotes define_property :allow_invalid_ruby define_property :indentation_spaces define_property :max_code_lines_in_class @@ -53,6 +54,7 @@ def initialize allow_hard_tabs(false, level: :error) allow_screaming_snake_case_classes(false, level: :error) allow_trailing_line_spaces(false, level: :error) + allow_unnecessary_double_quotes(false, level: :warn) allow_invalid_ruby(false, level: :warn) indentation_spaces(2, level: :error) max_code_lines_in_class(300, level: :error) diff --git a/lib/tailor/rulers/allow_unnecessary_double_quotes_ruler.rb b/lib/tailor/rulers/allow_unnecessary_double_quotes_ruler.rb new file mode 100644 index 0000000..fd0b0c8 --- /dev/null +++ b/lib/tailor/rulers/allow_unnecessary_double_quotes_ruler.rb @@ -0,0 +1,63 @@ +require_relative '../ruler' + +class Tailor + module Rulers + class AllowUnnecessaryDoubleQuotesRuler < Tailor::Ruler + + def initialize(config, options) + super(config, options) + add_lexer_observers :nl + end + + def nl_update(lexed_line, lineno, column) + quotes(lexed_line).each do |quote| + unless contains_embedded_expression?(quote) || + contains_escape_sequence?(quote) + measure(lineno, column(quote.first)) + end + end + end + + # Checks to see if the double_quotes are unnecessary. + # + # @param [Fixnum] lineno Line the problem was found on. + # @param [Fixnum] column Column the problem was found on. + def measure(lineno, column) + @problems << Problem.new('unnecessary_double_quotes', lineno, column, + "Unnecessary double quotes at column #{column}, " + + 'expected single quotes.', @options[:level]) + end + + private + + def contains_embedded_expression?(tokens) + tokens.any? { |t| t[1] == :on_embexpr_beg } + end + + def contains_escape_sequence?(tokens) + tokens.any? do |t| + t[1] == :on_tstring_content and t[2].match(/\\[a-z]+/) + end + end + + def quotes(tokens) + tokens.select do |t| + true if (double_quote_start?(t))..(double_quote_end?(t)) + end.slice_before { |t| double_quote_start?(t) }.reject { |q| q.empty? } + end + + def column(token) + token[0][1] + end + + def double_quote_start?(token) + token[1] == :on_tstring_beg and token[2] == '"' + end + + def double_quote_end?(token) + token[1] == :on_tstring_end and token[2] == '"' + end + + end + end +end diff --git a/spec/functional/horizontal_spacing/hard_tabs_spec.rb b/spec/functional/horizontal_spacing/hard_tabs_spec.rb index 2b80347..40347fa 100644 --- a/spec/functional/horizontal_spacing/hard_tabs_spec.rb +++ b/spec/functional/horizontal_spacing/hard_tabs_spec.rb @@ -9,13 +9,13 @@ HARD_TABS['hard_tab'] = %Q{def something -\tputs "something" +\tputs 'something' end} HARD_TABS['hard_tab_with_spaces'] = %Q{class Thing def something -\t puts "something" +\t puts 'something' end end} @@ -27,14 +27,14 @@ def something HARD_TABS['hard_tab_with_1_indented_space'] = %Q{class Thing def something -\t puts "something" +\t puts 'something' end end} HARD_TABS['hard_tab_with_2_indented_spaces'] = %Q{class Thing def something -\t puts "something" +\t puts 'something' end end} diff --git a/spec/functional/string_quoting_spec.rb b/spec/functional/string_quoting_spec.rb new file mode 100644 index 0000000..ccaee46 --- /dev/null +++ b/spec/functional/string_quoting_spec.rb @@ -0,0 +1,97 @@ +require 'spec_helper' +require_relative '../support/string_quoting_cases' +require 'tailor/critic' +require 'tailor/configuration/style' + +describe 'String Quoting' do + + def file_name + self.class.description + end + + def contents + QUOTING[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 :single_quotes_no_interpolation do + it 'does not warn' do + critic.check_file(file_name, style.to_hash) + expect(critic.problems[file_name]).to be_empty + end + end + + context :double_quotes_with_interpolation do + it 'does not warn' do + critic.check_file(file_name, style.to_hash) + expect(critic.problems[file_name]).to be_empty + end + end + + context :double_quotes_no_interpolation do + it 'warns that double quotes are unnecessary' do + critic.check_file(file_name, style.to_hash) + expect(critic.problems[file_name]).to eql [{ + :type => 'unnecessary_double_quotes', + :line => 1, + :column => 6, + :message => 'Unnecessary double quotes at column 6, expected single quotes.', + :level => :warn + }] + end + end + + context :double_quotes_no_interpolation_twice do + it 'warns that double quotes are unnecessary' do + critic.check_file(file_name, style.to_hash) + expect(critic.problems[file_name]).to eql [ + { + :type => 'unnecessary_double_quotes', + :line => 1, + :column => 6, + :message => 'Unnecessary double quotes at column 6, expected single quotes.', + :level => :warn + }, + { + :type => 'unnecessary_double_quotes', + :line => 1, + :column => 14, + :message => 'Unnecessary double quotes at column 14, expected single quotes.', + :level => :warn + } + ] + end + end + + context :nested_quotes do + it 'does not warn' do + critic.check_file(file_name, style.to_hash) + expect(critic.problems[file_name]).to be_empty + end + end + + context :escape_sequence do + it 'does not warn when a double quoted string contains a newline' do + critic.check_file(file_name, style.to_hash) + expect(critic.problems[file_name]).to be_empty + end + end + +end diff --git a/spec/functional/vertical_spacing/class_length_spec.rb b/spec/functional/vertical_spacing/class_length_spec.rb index 6fc6204..964cfa8 100644 --- a/spec/functional/vertical_spacing/class_length_spec.rb +++ b/spec/functional/vertical_spacing/class_length_spec.rb @@ -10,7 +10,7 @@ include Pizza def barrel_roll - puts "DOABARRELROLL!" + puts 'DOABARRELROLL!' end end} diff --git a/spec/support/bad_indentation_cases.rb b/spec/support/bad_indentation_cases.rb index 8681e80..efb7cfa 100644 --- a/spec/support/bad_indentation_cases.rb +++ b/spec/support/bad_indentation_cases.rb @@ -28,20 +28,20 @@ INDENT_1['def_content_indented_end'] = %Q{def a - puts "stuff" + puts 'stuff' end} INDENT_1['class_def_content_outdented_end'] = %Q{class A def a - puts "stuff" + puts 'stuff' end end} INDENT_1['class_def_outdented_content'] = %Q{class A def a - puts "stuff" + puts 'stuff' end end} @@ -56,9 +56,9 @@ def self.my_method %Q{def my_method case true when true - puts "stuff" + puts 'stuff' when false - puts "blah blah" + puts 'blah blah' end end} @@ -66,9 +66,9 @@ def self.my_method %Q{def my_method case true # comment when true - puts "stuff" + puts 'stuff' when false - puts "blah blah" + puts 'blah blah' end end} @@ -76,9 +76,9 @@ def self.my_method %Q{def my_method case true when true - puts "stuff" + puts 'stuff' when false - puts "blah blah" + puts 'blah blah' end end} @@ -86,9 +86,9 @@ def self.my_method %Q{def my_method case true when true - puts "stuff" + puts 'stuff' when false - puts "blah blah" + puts 'blah blah' end end} @@ -96,9 +96,9 @@ def self.my_method %Q{def my_method case true when true - puts "stuff" + puts 'stuff' when false - puts "blah blah" + puts 'blah blah' end end} @@ -106,35 +106,35 @@ def self.my_method %Q{def my_method case true when true - puts "stuff" + puts 'stuff' when false - puts "blah blah" + puts 'blah blah' end end} INDENT_1['while_do_indented'] = %Q{ while true do - puts "something" + puts 'something' end} INDENT_1['while_do_outdented'] = %Q{def my_method while true do - puts "something" + puts 'something' end end} INDENT_1['while_do_content_outdented'] = %Q{def my_method while true do - puts "something" + puts 'something' end end} INDENT_1['while_do_content_indented'] = %Q{def my_method while true do - puts "something" + puts 'something' end end} @@ -240,7 +240,7 @@ def self.my_method '/profiles/DVR5000/ssdp_notification.erb'} INDENT_1['multi_line_method_call_end_in'] = - %Q{def initialize(raw_response) + %q{def initialize(raw_response) if raw_response.nil? || raw_response.empty? raise RTSP::Error, "#{self.class} received nil string--this shouldn't happen." diff --git a/spec/support/good_indentation_cases.rb b/spec/support/good_indentation_cases.rb index e973b86..0720780 100644 --- a/spec/support/good_indentation_cases.rb +++ b/spec/support/good_indentation_cases.rb @@ -101,7 +101,7 @@ class MyError < RuntimeError; end INDENT_OK['assignment_twolevel_paren_multistatement'] = %Q{result = Integer( String.new( - "1" + '1' ).to_i, 16 )} @@ -161,7 +161,7 @@ class MyError < RuntimeError; end %Q{def some_method do_something(one, two) rescue => e - log "It didn't work." + log 'It didn't work.' raise e end} @@ -189,7 +189,7 @@ class MyError < RuntimeError; end INDENT_OK['nested_def'] = %Q{def first_method def second_method - puts "hi" + puts 'hi' end end} @@ -233,12 +233,12 @@ class MyClass include Stuff def a_method - puts "hello" + puts 'hello' end end} INDENT_OK['if_modifier'] = - %Q{puts "hi" if nil.nil?} + %Q{puts 'hi' if nil.nil?} INDENT_OK['if_modifier2'] = %Q{start_key_registration_server if @profiles.values.include? :SM5000} @@ -249,7 +249,7 @@ def a_method end} INDENT_OK['unless_modifier'] = - %Q{puts "hi" unless nil.nil?} + %Q{puts 'hi' unless nil.nil?} INDENT_OK['def_return_unless_modifier'] = %Q{def a_method @@ -269,9 +269,9 @@ def a_method %Q{def my_method case true when true - puts "stuff" + puts 'stuff' when false - puts "blah blah" + puts 'blah blah' end end} @@ -297,7 +297,7 @@ def a_method INDENT_OK['while_do_loop'] = %Q{while true do - puts "something" + puts 'something' end} INDENT_OK['while_do_loop2'] = @@ -311,7 +311,7 @@ def a_method INDENT_OK['until_do_loop'] = %Q{until true do - puts "something" + puts 'something' end} INDENT_OK['until_do_loop2'] = @@ -430,22 +430,22 @@ def a_method INDENT_OK['deep_hash_with_rockets'] = %Q[im_deep = - { "one" => - { "1" => - { "a" => "A", - "b" => "B", - "c" => "C" }, - "2" => - { "d" => "D", - "e" => "E", - "f" => "F" } } }] + { 'one' => + { '1' => + { 'a' => 'A', + 'b' => 'B', + 'c' => 'C' }, + '2' => + { 'd' => 'D', + 'e' => 'E', + 'f' => 'F' } } }] INDENT_OK['embedded_strings_in_embedded_strings'] = %q[def friendly_time(time) if hours < 24 "#{(hours > 0) ? "#{hours} hour" : '' }#{(hours > 1) ? 's' : ''}" + " #{(mins > 0) ? "#{mins} minute" : '' }#{(mins > 1) ? 's' : ''}" + - " #{seconds} second#{(seconds > 1) ? "s" : ''} ago" + " #{seconds} second#{(seconds > 1) ? 's' : ''} ago" else time.to_s end @@ -483,8 +483,8 @@ def a_method ]} INDENT_OK['multi_line_brackets_embedded_hashes'] = - %Q{summary_table.rows << [{ value: "File", align: :center }, - { value: "Total Problems", align: :center }]} + %Q{summary_table.rows << [{ value: 'File', align: :center }, + { value: 'Total Problems', align: :center }]} INDENT_OK['brackets_combo'] = %Q{[2] @@ -634,7 +634,7 @@ def print_iteration_start iteration_number %Q{def your_thing( one ) - puts "stuff" + puts 'stuff' end} #------------------------------------------------------------------------------ @@ -688,10 +688,10 @@ def print_iteration_start iteration_number baz }.each do |thing| function thing do - puts "stuff" + puts 'stuff' end end -puts "post ends"} +puts 'post ends'} =begin INDENT_OK['rparen_and_do_same_line'] = @@ -706,8 +706,8 @@ def print_iteration_start iteration_number #------------------------------------------------------------------------------- INDENT_OK['single_line_begin_rescue_end'] = %Q{def log - l = begin; lineno; rescue; ""; end - c = begin; column; rescue; ""; end + l = begin; lineno; rescue; ''; end + c = begin; column; rescue; ''; end subclass_name = self.class.to_s.sub(/^Tailor::/, '') args.first.insert(0, "<\#{subclass_name}> \#{l}[\#{c}]: ") Tailor::Logger.log(*args) @@ -737,7 +737,7 @@ def foo end} INDENT_OK['combo3'] = - %Q{def report_turducken(results, performance_results) + %q{def report_turducken(results, performance_results) stuffing[:log_files] = { "\#{File.basename @logger.log_file_location}" => File.read(@logger.log_file_location).gsub(/(?<)(?\\/)?(?\\w)/, '\\k!\\k\\k') }.merge remote_logs @@ -747,7 +747,7 @@ def foo @config[:turducken_password]) suite_result_url = Stuffer.stuff(stuffing) rescue Errno::ECONNREFUSED - @logger.error "Unable to connect to Turducken server!" + @logger.error 'Unable to connect to Turducken server!' end suite_result_url diff --git a/spec/support/string_quoting_cases.rb b/spec/support/string_quoting_cases.rb new file mode 100644 index 0000000..c71b0b4 --- /dev/null +++ b/spec/support/string_quoting_cases.rb @@ -0,0 +1,25 @@ +QUOTING = {} + +QUOTING['single_quotes_no_interpolation'] = + %q{foo = 'bar' +} + +QUOTING['double_quotes_with_interpolation'] = + %q{foo = "bar#{baz}" +} + +QUOTING['double_quotes_no_interpolation'] = + %q{foo = "bar" +} + +QUOTING['double_quotes_no_interpolation_twice'] = + %q{foo = "bar" + "baz" +} + +QUOTING['escape_sequence'] = + %q{foo = "bar\n" +} + +QUOTING['nested_quotes'] = + %q{foo = "foo#{bar('baz')}" +} diff --git a/spec/unit/tailor/configuration/style_spec.rb b/spec/unit/tailor/configuration/style_spec.rb index 1f49c24..022ffc1 100644 --- a/spec/unit/tailor/configuration/style_spec.rb +++ b/spec/unit/tailor/configuration/style_spec.rb @@ -161,6 +161,7 @@ :allow_screaming_snake_case_classes => [false, { :level => :error }], :allow_trailing_line_spaces => [false, { :level => :error }], :allow_invalid_ruby => [false, { :level => :warn }], + :allow_unnecessary_double_quotes => [false, { :level => :warn }], :indentation_spaces => [2, { :level => :error }], :max_code_lines_in_class => [300, { :level => :error }], :max_code_lines_in_method => [30, { :level => :error }], diff --git a/spec/unit/tailor/configuration_spec.rb b/spec/unit/tailor/configuration_spec.rb index 3cffc9e..45c23bd 100644 --- a/spec/unit/tailor/configuration_spec.rb +++ b/spec/unit/tailor/configuration_spec.rb @@ -44,6 +44,7 @@ allow_screaming_snake_case_classes: [false, { level: :error }], allow_trailing_line_spaces: [false, { level: :error }], allow_invalid_ruby: [false, { level: :warn }], + allow_unnecessary_double_quotes: [false, { :level => :warn }], indentation_spaces: [2, { level: :error }], max_code_lines_in_class: [300, { level: :error }], max_code_lines_in_method: [30, { level: :error }],