Fixes #3757 Some cases of assignment in conditional do not warn as in…

… MRI
jruby · Apr 1, 2016 · 1a829a7 · 1a829a7
1 parent 2d88cd8
commit 1a829a7
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 3 deletions.
diff --git a/core/src/main/java/org/jruby/ast/HashNode.java b/core/src/main/java/org/jruby/ast/HashNode.java
@@ -34,6 +34,7 @@
 import java.util.ArrayList;
 import java.util.List;
 
+import org.jruby.ast.types.ILiteralNode;
 import org.jruby.ast.visitor.NodeVisitor;
 import org.jruby.lexer.yacc.ISourcePosition;
 import org.jruby.util.KeyValuePair;
@@ -42,7 +43,7 @@
  * A Literal Hash that can represent either a {a=&amp;b, c=&amp;d} type expression or the list 
  * of default values in a method call.
  */
-public class HashNode extends Node {
+public class HashNode extends Node implements ILiteralNode {
     private final List<KeyValuePair<Node,Node>> pairs;
 
     public HashNode(ISourcePosition position) {

diff --git a/core/src/main/java/org/jruby/parser/ParserSupport.java b/core/src/main/java/org/jruby/parser/ParserSupport.java
@@ -548,14 +548,37 @@ private boolean checkAssignmentInCondition(Node node) {
             lexer.compile_error(PID.MULTIPLE_ASSIGNMENT_IN_CONDITIONAL, "multiple assignment in conditional");
         } else if (node instanceof LocalAsgnNode || node instanceof DAsgnNode || node instanceof GlobalAsgnNode || node instanceof InstAsgnNode) {
             Node valueNode = ((AssignableNode) node).getValueNode();
-            if (valueNode instanceof ILiteralNode || valueNode instanceof NilNode || valueNode instanceof TrueNode || valueNode instanceof FalseNode) {
+            if (isStaticContent(valueNode)) {
                 warnings.warn(ID.ASSIGNMENT_IN_CONDITIONAL, node.getPosition(), "found = in conditional, should be ==");
             }
             return true;
         } 
 
         return false;
     }
+
+    // Only literals or does it contain something more dynamic like variables?
+    private boolean isStaticContent(Node node) {
+        if (node instanceof HashNode) {
+            HashNode hash = (HashNode) node;
+            for (KeyValuePair<Node, Node> pair : hash.getPairs()) {
+                if (!isStaticContent(pair.getKey()) || !isStaticContent(pair.getValue())) return false;
+            }
+            return true;
+        } else if (node instanceof ArrayNode) {
+            ArrayNode array = (ArrayNode) node;
+            int size = array.size();
+
+            for (int i = 0; i < size; i++) {
+                if (!isStaticContent(array.get(i))) return false;
+            }
+            return true;
+        } else if (node instanceof ILiteralNode || node instanceof NilNode || node instanceof TrueNode || node instanceof FalseNode) {
+            return true;
+        }
+
+        return false;
+    }
 
     protected Node makeNullNil(Node node) {
         return node == null ? NilImplicitNode.NIL : node;

diff --git a/test/mri/excludes/TestRubyOptions.rb b/test/mri/excludes/TestRubyOptions.rb
@@ -1,4 +1,3 @@
-exclude :test_assignment_in_conditional, "needs investigation"
 exclude :test_chdir, "needs investigation"
 exclude :test_copyright, "needs investigation"
 exclude :test_debug, "needs investigation"