From 14b514dbb2d96e2715fac8e0c6051e9fa0817889 Mon Sep 17 00:00:00 2001 From: Owen Stephens Date: Fri, 22 Apr 2016 22:48:00 +0100 Subject: [PATCH] [Fix #3019] Add Style/EmptyCaseCondition cop (#3044) --- CHANGELOG.md | 2 + config/enabled.yml | 4 + lib/rubocop.rb | 1 + lib/rubocop/cop/style/empty_case_condition.rb | 89 ++++++++++++ .../cop/style/empty_case_condition_spec.rb | 132 ++++++++++++++++++ 5 files changed, 228 insertions(+) create mode 100644 lib/rubocop/cop/style/empty_case_condition.rb create mode 100644 spec/rubocop/cop/style/empty_case_condition_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a900c3937f8..867b78a7b426 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,8 +17,10 @@ * [#3052](https://github.com/bbatsov/rubocop/pull/3052): `Style/MultilineHashBraceLayout` enforced style supports `same_line` option. ([@panthomakos][]) * [#3052](https://github.com/bbatsov/rubocop/pull/3052): `Style/MultilineMethodCallBraceLayout` enforced style supports `same_line` option. ([@panthomakos][]) * [#3052](https://github.com/bbatsov/rubocop/pull/3052): `Style/MultilineMethodDefinitionBraceLayout` enforced style supports `same_line` option. ([@panthomakos][]) +* [#3019](https://github.com/bbatsov/rubocop/issues/3019): Add new `Style/EmptyCaseCondition` cop. ([@owst][]) ### Bug fixes + * [#3032](https://github.com/bbatsov/rubocop/issues/3032): Fix autocorrecting parentheses for predicate methods without space before args. ([@graemeboy][]) * [#3000](https://github.com/bbatsov/rubocop/pull/3000): Fix encoding crash on HTML output. ([@gerrywastaken][]) * [#2983](https://github.com/bbatsov/rubocop/pull/2983): `Style/AlignParameters` message was clarified for `with_fixed_indentation` style. ([@dylanahsmith][]) diff --git a/config/enabled.yml b/config/enabled.yml index 9b62691cbf45..85a25bb65e13 100644 --- a/config/enabled.yml +++ b/config/enabled.yml @@ -204,6 +204,10 @@ Style/EmptyElse: Description: 'Avoid empty else-clauses.' Enabled: true +Style/EmptyCaseCondition: + Description: 'Avoid empty condition in case statements.' + Enabled: true + Style/EmptyLineBetweenDefs: Description: 'Use empty lines between defs.' StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#empty-lines-between-methods' diff --git a/lib/rubocop.rb b/lib/rubocop.rb index 36a19efff073..42de172abf96 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -208,6 +208,7 @@ require 'rubocop/cop/style/double_negation' require 'rubocop/cop/style/each_with_object' require 'rubocop/cop/style/else_alignment' +require 'rubocop/cop/style/empty_case_condition' require 'rubocop/cop/style/empty_else' require 'rubocop/cop/style/empty_line_between_defs' require 'rubocop/cop/style/empty_lines' diff --git a/lib/rubocop/cop/style/empty_case_condition.rb b/lib/rubocop/cop/style/empty_case_condition.rb new file mode 100644 index 000000000000..75b4fbbb9beb --- /dev/null +++ b/lib/rubocop/cop/style/empty_case_condition.rb @@ -0,0 +1,89 @@ +# encoding: utf-8 +# frozen_string_literal: true + +module RuboCop + module Cop + module Style + # This cop checks for case statements with an empty condition. + # + # @example + # + # # bad: + # case + # when x == 0 + # puts 'x is 0' + # when y == 0 + # puts 'y is 0' + # else + # puts 'neither is 0' + # end + # + # # good: + # if x == 0 + # puts 'x is 0' + # elsif y == 0 + # puts 'y is 0' + # else + # puts 'neither is 0' + # end + # + # # good: (the case condition node is not empty) + # case n + # when 0 + # puts 'zero' + # when 1 + # puts 'one' + # else + # puts 'more' + # end + class EmptyCaseCondition < Cop + MSG = 'Do not use empty `case` condition, instead use an `if` '\ + 'expression.'.freeze + + def on_case(case_node) + condition_node = case_node.children.first + + add_offense(case_node, :keyword, MSG) if condition_node.nil? + end + + private + + def autocorrect(case_node) + lambda do |corrector| + _cond_node, *when_nodes, _else_node = *case_node + + correct_case_whens(corrector, case_node, when_nodes) + + correct_multiple_alternative_whens(corrector, when_nodes) + end + end + + def correct_case_whens(corrector, case_node, when_nodes) + case_to_first_when = + case_node.loc.keyword.join(when_nodes.first.loc.keyword) + + corrector.replace(case_to_first_when, 'if') + + when_nodes.drop(1).each do |when_node| + corrector.replace(when_node.loc.keyword, 'elsif') + end + end + + # Since an if condition containing commas is not syntactically valid, we + # correct `when x,y` to `if [x,y].any?`. + def correct_multiple_alternative_whens(corrector, when_nodes) + when_nodes.each do |when_node| + children = when_node.children + + # In `when a; r` we have two children: [a, r]. + # In `when a, b, c; r` we have 4. + next unless children.count > 2 + + corrector.insert_before(children.first.loc.expression, '[') + corrector.insert_after(children[-2].loc.expression, '].any?') + end + end + end + end + end +end diff --git a/spec/rubocop/cop/style/empty_case_condition_spec.rb b/spec/rubocop/cop/style/empty_case_condition_spec.rb new file mode 100644 index 000000000000..32458fd28708 --- /dev/null +++ b/spec/rubocop/cop/style/empty_case_condition_spec.rb @@ -0,0 +1,132 @@ +# encoding: utf-8 +# frozen_string_literal: true + +require 'spec_helper' + +describe RuboCop::Cop::Style::EmptyCaseCondition do + subject(:cop) { described_class.new } + + shared_examples 'detect/correct empty case, accept non-empty case' do + it 'registers an offense' do + inspect_source(cop, source) + expect(cop.messages).to eq [described_class::MSG] + end + + it 'correctly autocorrects' do + expect(autocorrect_source(cop, source)).to eq corrected_source.join("\n") + end + + let(:source_with_case) { source.map { |s| s.gsub(/case/, 'case :a') } } + + it 'accepts the source with case' do + inspect_source(cop, source_with_case) + expect(cop.messages).to be_empty + end + end + + context 'given a case statement with an empty case' do + context 'with multiple when branches and an else' do + let(:source) do + ['case', + 'when 1 == 2', + ' foo', + 'when 1 == 1', + ' bar', + 'else', + ' baz', + 'end'] + end + let(:corrected_source) do + ['if 1 == 2', + ' foo', + 'elsif 1 == 1', + ' bar', + 'else', + ' baz', + 'end'] + end + + it_behaves_like 'detect/correct empty case, accept non-empty case' + end + + context 'with multiple when branches and no else' do + let(:source) do + ['case', + 'when 1 == 2', + ' foo', + 'when 1 == 1', + ' bar', + 'end'] + end + let(:corrected_source) do + ['if 1 == 2', + ' foo', + 'elsif 1 == 1', + ' bar', + 'end'] + end + + it_behaves_like 'detect/correct empty case, accept non-empty case' + end + + context 'with a single when branch and an else' do + let(:source) do + ['case', + 'when 1 == 2', + ' foo', + 'else', + ' bar', + 'end'] + end + let(:corrected_source) do + ['if 1 == 2', + ' foo', + 'else', + ' bar', + 'end'] + end + + it_behaves_like 'detect/correct empty case, accept non-empty case' + end + + context 'with a single when branch and no else' do + let(:source) do + ['case', + 'when 1 == 2', + ' foo', + 'end'] + end + let(:corrected_source) do + ['if 1 == 2', + ' foo', + 'end'] + end + + it_behaves_like 'detect/correct empty case, accept non-empty case' + end + + context 'with a when branch including comma-delimited alternatives' do + let(:source) do + ['case', + 'when false', + ' foo', + 'when nil, false, 1', + ' bar', + 'when false, 1', + ' baz', + 'end'] + end + let(:corrected_source) do + ['if false', + ' foo', + 'elsif [nil, false, 1].any?', + ' bar', + 'elsif [false, 1].any?', + ' baz', + 'end'] + end + + it_behaves_like 'detect/correct empty case, accept non-empty case' + end + end +end