From 5922932642c796322839e5ef96d6394bce810182 Mon Sep 17 00:00:00 2001 From: Teemu Date: Mon, 7 Sep 2015 20:31:40 +0300 Subject: [PATCH] [Fix #2161] Explicit receiver in SignalException `Style/SignalException` doesn't register an offense when an explicit receiver is specified, unless it's `Kernel`. --- CHANGELOG.md | 1 + lib/rubocop/cop/style/signal_exception.rb | 35 +++++++++----- .../cop/style/signal_exception_spec.rb | 48 +++++++++++++++++++ 3 files changed, 73 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 960830f3f5cd..e170ed06f1f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ * [#2217](https://github.com/bbatsov/rubocop/issues/2217): Allow block arguments in `Style/SymbolProc`. ([@lumeet][]) * [#2213](https://github.com/bbatsov/rubocop/issues/2213): Write to cache with binary encoding to avoid transcoding exceptions in some locales. ([@jonas054][]) * [#2218](https://github.com/bbatsov/rubocop/issues/2218): Fix loading config error when safe yaml is only partially loaded. ([@maxjacobson][]) +* [#2161](https://github.com/bbatsov/rubocop/issues/2161): Allow an explicit receiver (except `Kernel`) in `Style/SignalException`. ([@lumeet][]) ## 0.34.0 (05/09/2015) diff --git a/lib/rubocop/cop/style/signal_exception.rb b/lib/rubocop/cop/style/signal_exception.rb index c2cf402c6f93..6b82afd30762 100644 --- a/lib/rubocop/cop/style/signal_exception.rb +++ b/lib/rubocop/cop/style/signal_exception.rb @@ -37,7 +37,8 @@ def autocorrect(node) lambda do |corrector| name = case style - when :semantic then command?(:raise, node) ? 'fail' : 'raise' + when :semantic + command_or_kernel_call?(:raise, node) ? 'fail' : 'raise' when :only_raise then 'raise' when :only_fail then 'fail' end @@ -63,30 +64,42 @@ def check_for(method_name, node) return unless node if style == :semantic - each_command(method_name, node) do |send_node| + each_command_or_kernel_call(method_name, node) do |send_node| next if ignored_node?(send_node) add_offense(send_node, :selector, message(method_name)) ignore_node(send_node) end - else - _receiver, selector, _args = *node - - if [:raise, :fail].include?(selector) && selector != method_name - add_offense(node, :selector, message(method_name)) - end + elsif command_or_kernel_call?(method_name == :raise ? :fail : :raise, + node) + add_offense(node, :selector, message(method_name)) end end + def command_or_kernel_call?(name, node) + command?(name, node) || kernel_call?(name, node) + end + + def kernel_call?(name, node) + return false unless node.type == :send + receiver, selector, _args = *node + + return false unless name == selector + return false unless receiver.const_type? + + _, constant = *receiver + constant == :Kernel + end + def allow(method_name, node) - each_command(method_name, node) do |send_node| + each_command_or_kernel_call(method_name, node) do |send_node| ignore_node(send_node) end end - def each_command(method_name, node) + def each_command_or_kernel_call(method_name, node) on_node(:send, node, :rescue) do |send_node| - yield send_node if command?(method_name, send_node) + yield send_node if command_or_kernel_call?(method_name, send_node) end end end diff --git a/spec/rubocop/cop/style/signal_exception_spec.rb b/spec/rubocop/cop/style/signal_exception_spec.rb index 3ceb23d05988..63549f1e4b74 100644 --- a/spec/rubocop/cop/style/signal_exception_spec.rb +++ b/spec/rubocop/cop/style/signal_exception_spec.rb @@ -111,6 +111,30 @@ expect(cop.offenses).to be_empty end + it 'accepts `raise` and `fail` with explicit receiver' do + inspect_source(cop, + ['def test', + ' test.raise', + 'rescue Exception', + ' test.fail', + 'end']) + expect(cop.offenses).to be_empty + end + + it 'registers an offense for `raise` and `fail` with `Kernel` as ' \ + 'explicit receiver' do + inspect_source(cop, + ['def test', + ' Kernel.raise', + 'rescue Exception', + ' Kernel.fail', + 'end']) + expect(cop.offenses.size).to eq(2) + expect(cop.messages) + .to eq(['Use `fail` instead of `raise` to signal exceptions.', + 'Use `raise` instead of `fail` to rethrow exceptions.']) + end + it 'registers an offense for raise not in a begin/rescue/end' do inspect_source(cop, ["case cop_config['EnforcedStyle']", @@ -218,6 +242,18 @@ .to eq(['Always use `raise` to signal exceptions.']) end + it 'accepts `fail` with explicit receiver' do + inspect_source(cop, 'test.fail') + expect(cop.offenses).to be_empty + end + + it 'registers an offense for `fail` with `Kernel` as explicit receiver' do + inspect_source(cop, 'Kernel.fail') + expect(cop.offenses.size).to eq(1) + expect(cop.messages) + .to eq(['Always use `raise` to signal exceptions.']) + end + it 'auto-corrects fail to raise always' do new_source = autocorrect_source(cop, ['begin', @@ -272,6 +308,18 @@ .to eq(['Always use `fail` to signal exceptions.']) end + it 'accepts `raise` with explicit receiver' do + inspect_source(cop, 'test.raise') + expect(cop.offenses).to be_empty + end + + it 'registers an offense for `raise` with `Kernel` as explicit receiver' do + inspect_source(cop, 'Kernel.raise') + expect(cop.offenses.size).to eq(1) + expect(cop.messages) + .to eq(['Always use `fail` to signal exceptions.']) + end + it 'auto-corrects raise to fail always' do new_source = autocorrect_source(cop, ['begin',