From 8330aa3b38cf183241b3e4a44b696b3496617f42 Mon Sep 17 00:00:00 2001 From: Jonas Arvidsson Date: Sat, 5 Jul 2014 08:07:53 +0200 Subject: [PATCH 1/2] Add cop PercentQLiterals Checks `%Q` vs `%q`, and allow configuration to decide if `%q` should be used when possible or if `%Q` should be used always. For #835. --- CHANGELOG.md | 4 + config/default.yml | 6 + config/enabled.yml | 4 + lib/rubocop.rb | 1 + lib/rubocop/cop/style/percent_q_literals.rb | 54 ++++++++ .../cop/style/percent_q_literals_spec.rb | 122 ++++++++++++++++++ .../cop/style/unneeded_percent_q_spec.rb | 4 +- 7 files changed, 193 insertions(+), 2 deletions(-) create mode 100644 lib/rubocop/cop/style/percent_q_literals.rb create mode 100644 spec/rubocop/cop/style/percent_q_literals_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index ae763cd10974..da4f0ca4443c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Change log +### New features + +* [#835](https://github.com/bbatsov/rubocop/issues/835): New cop `PercentQLiterals` checks if use of `%Q` and `%q` matches configuration. ([@jonas054][]) + ## master (unreleased) ### Bugs fixed diff --git a/config/default.yml b/config/default.yml index 450c93e87137..8d1ccbe0e8ed 100644 --- a/config/default.yml +++ b/config/default.yml @@ -325,6 +325,12 @@ Style/PercentLiteralDelimiters: '%W': () '%x': () +Style/PercentQLiterals: + EnforcedStyle: lower_case_q + SupportedStyles: + - lower_case_q # Use %q when possible, %Q when necessary + - upper_case_q # Always use %Q + Style/PredicateName: NamePrefixBlacklist: - is_ diff --git a/config/enabled.yml b/config/enabled.yml index f9bae76569c8..84c010ad3058 100644 --- a/config/enabled.yml +++ b/config/enabled.yml @@ -371,6 +371,10 @@ Style/PercentLiteralDelimiters: Description: 'Use `%`-literal delimiters consistently' Enabled: true +Style/PercentQLiterals: + Description: 'Checks if uses of %Q/%q match the configured preference.' + Enabled: true + Style/PerlBackrefs: Description: 'Avoid Perl-style regex back references.' Enabled: true diff --git a/lib/rubocop.rb b/lib/rubocop.rb index 66e130cb4066..ce7add1ff6f5 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -186,6 +186,7 @@ require 'rubocop/cop/style/parameter_lists' require 'rubocop/cop/style/parentheses_around_condition' require 'rubocop/cop/style/percent_literal_delimiters' +require 'rubocop/cop/style/percent_q_literals' require 'rubocop/cop/style/perl_backrefs' require 'rubocop/cop/style/predicate_name' require 'rubocop/cop/style/proc' diff --git a/lib/rubocop/cop/style/percent_q_literals.rb b/lib/rubocop/cop/style/percent_q_literals.rb new file mode 100644 index 000000000000..22b4524e2371 --- /dev/null +++ b/lib/rubocop/cop/style/percent_q_literals.rb @@ -0,0 +1,54 @@ +# encoding: utf-8 + +module RuboCop + module Cop + module Style + # This cop checks for usage of the %Q() syntax when %q() would do. + class PercentQLiterals < Cop + include PercentLiteral + include ConfigurableEnforcedStyle + + def on_str(node) + process(node, '%Q', '%q') + end + + private + + def on_percent_literal(node, types) + return unless types.include?(type(node)) + if style == :lower_case_q + if type(node) == '%Q' + check(node, + 'Do not use `%Q` unless interpolation is needed. ' \ + 'Use `%q`.') + end + elsif type(node) == '%q' + check(node, 'Use `%Q` instead of `%q`.') + end + end + + def check(node, msg) + src = node.loc.expression.source + # Report offense only if changing case doesn't change semantics, + # i.e., if the string would become dynamic or has special characters. + return if node.children != + ProcessedSource.new(corrected(src)).ast.children + + add_offense(node, :begin, msg) + end + + def autocorrect(node) + src = node.loc.expression.source + + @corrections << lambda do |corrector| + corrector.replace(node.loc.expression, corrected(src)) + end + end + + def corrected(src) + src.sub(src[1], src[1].swapcase) + end + end + end + end +end diff --git a/spec/rubocop/cop/style/percent_q_literals_spec.rb b/spec/rubocop/cop/style/percent_q_literals_spec.rb new file mode 100644 index 000000000000..89f8f2fca464 --- /dev/null +++ b/spec/rubocop/cop/style/percent_q_literals_spec.rb @@ -0,0 +1,122 @@ +# encoding: utf-8 + +require 'spec_helper' + +describe RuboCop::Cop::Style::PercentQLiterals, :config do + subject(:cop) { described_class.new(config) } + + shared_examples 'accepts quote characters' do + it 'accepts single quotes' do + inspect_source(cop, ["'hi'"]) + expect(cop.offenses).to be_empty + end + + it 'accepts double quotes' do + inspect_source(cop, ['"hi"']) + expect(cop.offenses).to be_empty + end + end + + shared_examples 'accepts any q string with backslash t' do + context 'with special characters' do + it 'accepts %q' do + inspect_source(cop, ['%q(\t)']) + expect(cop.offenses).to be_empty + end + + it 'accepts %Q' do + inspect_source(cop, ['%Q(\t)']) + expect(cop.offenses).to be_empty + end + end + end + + context 'when EnforcedStyle is lower_case_q' do + let(:cop_config) { { 'EnforcedStyle' => 'lower_case_q' } } + + context 'without interpolation' do + it 'accepts %q' do + inspect_source(cop, ['%q(hi)']) + expect(cop.offenses).to be_empty + end + + it 'registers offense for %Q' do + inspect_source(cop, ['%Q(hi)']) + expect(cop.messages) + .to eq(['Do not use `%Q` unless interpolation is needed. Use `%q`.']) + expect(cop.highlights).to eq(['%Q(']) + end + + it 'auto-corrects' do + new_source = autocorrect_source(cop, '%Q(hi)') + expect(new_source).to eq('%q(hi)') + end + + include_examples 'accepts quote characters' + include_examples 'accepts any q string with backslash t' + end + + context 'with interpolation' do + it 'accepts %Q' do + inspect_source(cop, ['%Q(#{1 + 2})']) + expect(cop.offenses).to be_empty + end + + it 'accepts %q' do + # This is most probably a mistake, but not this cop's responisibility. + inspect_source(cop, ['%q(#{1 + 2})']) + expect(cop.offenses).to be_empty + end + + include_examples 'accepts quote characters' + end + end + + context 'when EnforcedStyle is upper_case_q' do + let(:cop_config) { { 'EnforcedStyle' => 'upper_case_q' } } + + context 'without interpolation' do + it 'registers offense for %q' do + inspect_source(cop, ['%q(hi)']) + expect(cop.messages).to eq(['Use `%Q` instead of `%q`.']) + expect(cop.highlights).to eq(['%q(']) + end + + it 'accepts %Q' do + inspect_source(cop, ['%Q(hi)']) + expect(cop.offenses).to be_empty + end + + it 'auto-corrects' do + new_source = autocorrect_source(cop, '%q[hi]') + expect(new_source).to eq('%Q[hi]') + end + + include_examples 'accepts quote characters' + include_examples 'accepts any q string with backslash t' + end + + context 'with interpolation' do + it 'accepts %Q' do + inspect_source(cop, ['%Q(#{1 + 2})']) + expect(cop.offenses).to be_empty + end + + it 'accepts %q' do + # It's strange if interpolation syntax appears inside a static string, + # but we can't be sure if it's a mistake or not. Changing it to %Q + # would alter semantics, so we leave it as it is. + inspect_source(cop, ['%q(#{1 + 2})']) + expect(cop.offenses).to be_empty + end + + it 'does not auto-correct' do + source = '%q(#{1 + 2})' + new_source = autocorrect_source(cop, source) + expect(new_source).to eq(source) + end + + include_examples 'accepts quote characters' + end + end +end diff --git a/spec/rubocop/cop/style/unneeded_percent_q_spec.rb b/spec/rubocop/cop/style/unneeded_percent_q_spec.rb index 613014426318..6597c28ea5be 100644 --- a/spec/rubocop/cop/style/unneeded_percent_q_spec.rb +++ b/spec/rubocop/cop/style/unneeded_percent_q_spec.rb @@ -18,7 +18,7 @@ end it 'accepts a string with single quotes and double quotes' do - inspect_source(cop, %Q(%q('"hi"'))) + inspect_source(cop, %q(%q('"hi"'))) expect(cop.offenses).to be_empty end @@ -43,7 +43,7 @@ end it 'accepts a string with single quotes and double quotes' do - inspect_source(cop, %Q(%Q('"hi"'))) + inspect_source(cop, %q(%Q('"hi"'))) expect(cop.offenses).to be_empty end From 6bf924a74119e89851b2bb3ee8ab07616e9d8920 Mon Sep 17 00:00:00 2001 From: Jonas Arvidsson Date: Mon, 7 Jul 2014 21:41:42 +0200 Subject: [PATCH 2/2] [Refactor] Move common logic from on_percent_literal to caller --- lib/rubocop/cop/mixin/percent_literal.rb | 3 ++- lib/rubocop/cop/style/percent_literal_delimiters.rb | 7 +++---- lib/rubocop/cop/style/percent_q_literals.rb | 3 +-- lib/rubocop/cop/style/unneeded_capital_w.rb | 6 ++---- 4 files changed, 8 insertions(+), 11 deletions(-) diff --git a/lib/rubocop/cop/mixin/percent_literal.rb b/lib/rubocop/cop/mixin/percent_literal.rb index e85ab777dc0d..d68b231cf90b 100644 --- a/lib/rubocop/cop/mixin/percent_literal.rb +++ b/lib/rubocop/cop/mixin/percent_literal.rb @@ -10,7 +10,8 @@ def percent_literal?(node) end def process(node, *types) - on_percent_literal(node, types) if percent_literal?(node) + return unless percent_literal?(node) && types.include?(type(node)) + on_percent_literal(node) end def begin_source(node) diff --git a/lib/rubocop/cop/style/percent_literal_delimiters.rb b/lib/rubocop/cop/style/percent_literal_delimiters.rb index 361f34045c30..25f35c95279d 100644 --- a/lib/rubocop/cop/style/percent_literal_delimiters.rb +++ b/lib/rubocop/cop/style/percent_literal_delimiters.rb @@ -59,11 +59,10 @@ def autocorrect(node) end end - def on_percent_literal(node, types) + def on_percent_literal(node) type = type(node) - return unless types.include?(type) && - !uses_preferred_delimiter?(node, type) && - !contains_preferred_delimiter?(node, type) + return if uses_preferred_delimiter?(node, type) || + contains_preferred_delimiter?(node, type) add_offense(node, :expression) end diff --git a/lib/rubocop/cop/style/percent_q_literals.rb b/lib/rubocop/cop/style/percent_q_literals.rb index 22b4524e2371..e94d8fe49656 100644 --- a/lib/rubocop/cop/style/percent_q_literals.rb +++ b/lib/rubocop/cop/style/percent_q_literals.rb @@ -14,8 +14,7 @@ def on_str(node) private - def on_percent_literal(node, types) - return unless types.include?(type(node)) + def on_percent_literal(node) if style == :lower_case_q if type(node) == '%Q' check(node, diff --git a/lib/rubocop/cop/style/unneeded_capital_w.rb b/lib/rubocop/cop/style/unneeded_capital_w.rb index 4d49571ec41a..b66f2b1c3146 100644 --- a/lib/rubocop/cop/style/unneeded_capital_w.rb +++ b/lib/rubocop/cop/style/unneeded_capital_w.rb @@ -16,10 +16,8 @@ def on_array(node) private - def on_percent_literal(node, types) - type = type(node) - return unless types.include?(type) && - node.children.none? { |x| x.type == :dstr } + def on_percent_literal(node) + return unless node.children.none? { |x| x.type == :dstr } add_offense(node, :expression) end