Merge pull request #6756 from Drenmi/bugfix/retry-node

[Fix #6751] Prevent Style/OneLineConditional from breaking on retry and break keywords

CHANGELOG.md

@@ -11,6 +11,7 @@
 * [#6765](https://github.com/rubocop-hq/rubocop/pull/6765): Fix false positives in keyword arguments for `Style/MethodCallWithArgsParentheses` `omit_parentheses`. ([@gsamokovarov]][])
 * [#6763](https://github.com/rubocop-hq/rubocop/pull/6763): Fix false positives in range literals for `Style/MethodCallWithArgsParentheses` `omit_parentheses`. ([@gsamokovarov]][])
 * [#6748](https://github.com/rubocop-hq/rubocop/issues/6748): Fix `Style/RaiseArgs` auto-correction breaking in contexts that require parentheses. ([@drenmi][])
+* [#6751](https://github.com/rubocop-hq/rubocop/issues/6751): Prevent `Style/OneLineConditional` from breaking on `retry` and `break` keywords. ([@drenmi][])
 
 ### Changes
 

lib/rubocop.rb

@@ -38,6 +38,7 @@
 require_relative 'rubocop/ast/node/args_node'
 require_relative 'rubocop/ast/node/array_node'
 require_relative 'rubocop/ast/node/block_node'
+require_relative 'rubocop/ast/node/break_node'
 require_relative 'rubocop/ast/node/case_node'
 require_relative 'rubocop/ast/node/def_node'
 require_relative 'rubocop/ast/node/defined_node'
@@ -51,6 +52,7 @@
 require_relative 'rubocop/ast/node/range_node'
 require_relative 'rubocop/ast/node/regexp_node'
 require_relative 'rubocop/ast/node/resbody_node'
+require_relative 'rubocop/ast/node/retry_node'
 require_relative 'rubocop/ast/node/send_node'
 require_relative 'rubocop/ast/node/str_node'
 require_relative 'rubocop/ast/node/super_node'

lib/rubocop/ast/builder.rb

@@ -19,6 +19,7 @@ class Builder < Parser::Builders::Default
         args: ArgsNode,
         array: ArrayNode,
         block: BlockNode,
+        break: BreakNode,
         case: CaseNode,
         def: DefNode,
         defined?: DefinedNode,
@@ -34,6 +35,7 @@ class Builder < Parser::Builders::Default
         pair: PairNode,
         regexp: RegexpNode,
         resbody: ResbodyNode,
+        retry: RetryNode,
         csend: SendNode,
         send: SendNode,
         str: StrNode,

lib/rubocop/ast/node/break_node.rb

@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module AST
+    # A node extension for `break` nodes. This will be used in place of a
+    # plain node when the builder constructs the AST, making its methods
+    # available to all `break` nodes within RuboCop.
+    class BreakNode < Node
+      include MethodDispatchNode
+      include ParameterizedNode
+
+      def arguments
+        []
+      end
+    end
+  end
+end

lib/rubocop/ast/node/defined_node.rb

@@ -8,6 +8,10 @@ module AST
     class DefinedNode < Node
       include ParameterizedNode
       include MethodDispatchNode
+
+      def node_parts
+        [nil, :defined?, *to_a]
+      end
     end
   end
 end

lib/rubocop/ast/node/retry_node.rb

@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module AST
+    # A node extension for `retry` nodes. This will be used in place of a
+    # plain node when the builder constructs the AST, making its methods
+    # available to all `retry` nodes within RuboCop.
+    class RetryNode < Node
+      include MethodDispatchNode
+      include ParameterizedNode
+
+      def arguments
+        []
+      end
+    end
+  end
+end

lib/rubocop/cop/style/one_line_conditional.rb

@@ -91,7 +91,7 @@ def keyword_with_changed_precedence?(node)
           return false unless node.keyword?
           return true if node.prefix_not?
 
-          !node.parenthesized_call?
+          node.arguments? && !node.parenthesized_call?
         end
       end
     end

spec/rubocop/ast/break_node_spec.rb

@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::AST::BreakNode do
+  let(:break_node) { parse_source(source).ast }
+
+  describe '.new' do
+    context 'with a break node' do
+      let(:source) { 'break' }
+
+      it { expect(break_node.is_a?(described_class)).to be(true) }
+    end
+  end
+end

spec/rubocop/ast/defined_node_spec.rb

@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::AST::DefinedNode do
+  let(:defined_node) { parse_source(source).ast }
+
+  describe '.new' do
+    context 'with a defined? node' do
+      let(:source) { 'defined? :foo' }
+
+      it { expect(defined_node.is_a?(described_class)).to be(true) }
+    end
+  end
+
+  describe '#receiver' do
+    let(:source) { 'defined? :foo' }
+
+    it { expect(defined_node.receiver).to eq(nil) }
+  end
+
+  describe '#method_name' do
+    let(:source) { 'defined? :foo' }
+
+    it { expect(defined_node.method_name).to eq(:defined?) }
+  end
+
+  describe '#arguments' do
+    let(:source) { 'defined? :foo' }
+
+    it { expect(defined_node.arguments.size).to eq(1) }
+    it { expect(defined_node.arguments).to all(be_sym_type) }
+  end
+end

spec/rubocop/ast/retry_node_spec.rb

@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::AST::RetryNode do
+  let(:retry_node) { parse_source(source).ast }
+
+  describe '.new' do
+    context 'with a retry node' do
+      let(:source) { 'retry' }
+
+      it { expect(retry_node.is_a?(described_class)).to be(true) }
+    end
+  end
+end

spec/rubocop/cop/style/one_line_conditional_spec.rb

@@ -108,4 +108,26 @@
     corrected = autocorrect_source('if 0 + 0 then 1 + 1 else 2 + 2 end')
     expect(corrected).to eq('0 + 0 ? 1 + 1 : 2 + 2')
   end
+
+  it 'does not break when one of the branches contains a retry keyword' do
+    expect_offense(<<-RUBY.strip_indent)
+      if true then retry else 7 end
+      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Favor the ternary operator (`?:`) over `if/then/else/end` constructs.
+    RUBY
+
+    expect_correction(<<-RUBY.strip_indent)
+      true ? retry : 7
+    RUBY
+  end
+
+  it 'does not break when one of the branches contains a break keyword' do
+    expect_offense(<<-RUBY.strip_indent)
+      if true then break else 7 end
+      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Favor the ternary operator (`?:`) over `if/then/else/end` constructs.
+    RUBY
+
+    expect_correction(<<-RUBY.strip_indent)
+      true ? break : 7
+    RUBY
+  end
 end