- builders/default: fix treatment of masgn in &&/|| (refs #69).

whitequark · Jun 15, 2013 · 75c512b · 75c512b
1 parent 6675fc7
commit 75c512b
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 11 deletions.
diff --git a/lib/parser/builders/default.rb b/lib/parser/builders/default.rb
@@ -650,7 +650,7 @@ def not_op(not_t, receiver=nil)
     # Logical operations: and, or
 
     def logical_op(type, lhs, op_t, rhs)
-      n(type, [ check_condition(lhs), check_condition(rhs) ],
+      n(type, [ lhs, rhs ],
         binary_op_map(lhs, op_t, rhs))
     end
 
@@ -801,11 +801,25 @@ def begin(begin_t, body, end_t)
     #
 
     def check_condition(cond)
-      if cond.type == :masgn
+      case cond.type
+      when :masgn
         diagnostic :error, ERRORS[:masgn_as_condition],
                    cond.loc.expression
-      elsif cond.type == :begin
-        check_condition(cond.children.last)
+
+      when :begin
+        if cond.children.count == 1
+          check_condition(cond.children.last)
+        else
+          cond
+        end
+
+      when :and, :or
+        lhs, rhs = *cond
+
+        cond.updated(nil, [
+          check_condition(lhs),
+          check_condition(rhs)
+        ])
       end
 
       cond

diff --git a/test/test_parser.rb b/test/test_parser.rb
@@ -3410,13 +3410,23 @@ def test_or
         |~~~~~~~~~~ expression})
   end
 
-  def test_and_or_masgn_invalid
-    assert_diagnoses(
-      [:error, :masgn_as_condition],
+  def test_and_or_masgn
+    assert_parses(
+      s(:and,
+        s(:lvar, :foo),
+        s(:begin,
+          s(:masgn,
+            s(:mlhs, s(:lvasgn, :a), s(:lvasgn, :b)),
+            s(:lvar, :bar)))),
       %q{foo && (a, b = bar)})
 
-    assert_diagnoses(
-      [:error, :masgn_as_condition],
+    assert_parses(
+      s(:or,
+        s(:lvar, :foo),
+        s(:begin,
+          s(:masgn,
+            s(:mlhs, s(:lvasgn, :a), s(:lvasgn, :b)),
+            s(:lvar, :bar)))),
       %q{foo || (a, b = bar)})
   end
 
@@ -3561,10 +3571,28 @@ def test_tern_masgn
   end
 
   def test_cond_begin_masgn
+    assert_parses(
+      s(:if,
+        s(:begin,
+          s(:lvar, :bar),
+          s(:masgn,
+            s(:mlhs, s(:lvasgn, :a), s(:lvasgn, :b)),
+            s(:lvar, :foo))),
+        s(:nil),
+        nil),
+      %q{if (bar; a, b = foo); end})
+  end
+
+  def test_cond_begin_and_or_masgn
     assert_diagnoses(
       [:error, :masgn_as_condition],
-      %q{if (bar; a, b = foo); end},
-      %q{         ~~~~~~~~~~ location})
+      %q{if foo && (a, b = bar); end},
+      %q{           ~~~~~~~~~~ location})
+
+    assert_diagnoses(
+      [:error, :masgn_as_condition],
+      %q{if foo || (a, b = bar); end},
+      %q{           ~~~~~~~~~~ location})
   end
 
   # Case matching