Identify unbounded ids in binary expressions (#11125)

Unbounded identifiers were not recognized in some binary expressions. This was a parser issue, which was failing to set the line and column properly for some expressions. The problem was detected when negative floats or characters (not strings) were used in the right hand side. The issue was also fixed for declarative blocks for completeness but it should not affect variable creation for unbounded identifiers.
DynamoDS · Sep 17, 2020 · aeeebce · aeeebce
1 parent e60306d
commit aeeebce
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 32 deletions.
diff --git a/src/Engine/ProtoCore/Parser/Parser.cs b/src/Engine/ProtoCore/Parser/Parser.cs
@@ -2325,6 +2325,7 @@ void Associative_Number(out ProtoCore.AST.AssociativeAST.AssociativeNode node) {
 			else
 			{
 			   node.line = line; node.col = col;
+			   node.endLine = t.line; node.endCol = t.col;
 			}
 
 		} else SynErr(93);
@@ -2339,10 +2340,9 @@ void Associative_Char(out ProtoCore.AST.AssociativeAST.AssociativeNode node) {
 
 		node = new ProtoCore.AST.AssociativeAST.CharNode() 
 		{ 
-		   Value = t.val.Substring(1, t.val.Length - 2),
-		   line = t.line,
-		   col = t.col
+		   Value = t.val.Substring(1, t.val.Length - 2)
 		}; 
+		NodeUtils.SetNodeLocation(node, t);
 
 	}
 
@@ -2351,7 +2351,7 @@ void Associative_String(out ProtoCore.AST.AssociativeAST.AssociativeNode node) {
 		Expect(4);
 		node = new ProtoCore.AST.AssociativeAST.StringNode() 
 		{ 
-		   Value = GetEscapedString(t.val.Length <= 2 ? "" : t.val.Substring(1, t.val.Length - 2)),
+		   Value = GetEscapedString(t.val.Length <= 2 ? "" : t.val.Substring(1, t.val.Length - 2))
 		}; 
 		NodeUtils.SetNodeLocation(node, t);
 
@@ -3710,10 +3710,9 @@ void Imperative_Char(out ProtoCore.AST.ImperativeAST.ImperativeNode node) {
 
 		node = new ProtoCore.AST.ImperativeAST.CharNode() 
 		{ 
-		   Value = t.val.Substring(1, t.val.Length - 2),
-		   line = t.line,
-		   col = t.col
-		}; 
+		   Value = t.val.Substring(1, t.val.Length - 2)
+		};
+		NodeUtils.SetNodeLocation(node, t);
 
 	}
 
@@ -3722,10 +3721,9 @@ void Imperative_String(out ProtoCore.AST.ImperativeAST.ImperativeNode node) {
 		Expect(4);
 		node = new ProtoCore.AST.ImperativeAST.StringNode() 
 		{ 
-		   Value = GetEscapedString(t.val.Length <= 2 ? "" : t.val.Substring(1, t.val.Length - 2)), 
-		   line = t.line,
-		   col = t.col
+		   Value = GetEscapedString(t.val.Length <= 2 ? "" : t.val.Substring(1, t.val.Length - 2))
 		}; 
+		NodeUtils.SetNodeLocation(node, t);
 
 	}
 
@@ -3756,7 +3754,8 @@ void Imperative_num(out ProtoCore.AST.ImperativeAST.ImperativeNode node) {
 
 			if (ProtoCore.DSASM.Constants.kInvalidIndex != line)
 			{
-			   node.line = line; node.col = col; 
+			   node.line = line; node.col = col;
+			   node.endLine = t.line; node.endCol = t.col;
 			}
 			else
 			{
@@ -3775,10 +3774,15 @@ void Imperative_num(out ProtoCore.AST.ImperativeAST.ImperativeNode node) {
 			   node = new ProtoCore.AST.ImperativeAST.NullNode();
 			}
 
-			if (ProtoCore.DSASM.Constants.kInvalidIndex != line){
-			   node.line = line; node.col = col; }
-			else{
-			   NodeUtils.SetNodeLocation(node, t); }
+			if (ProtoCore.DSASM.Constants.kInvalidIndex != line)
+			{
+			   node.line = line; node.col = col;
+			   node.endLine = t.line; node.endCol = t.col;
+			}
+			else
+			{
+			   NodeUtils.SetNodeLocation(node, t);
+			}
 
 		} else SynErr(118);
 	}

diff --git a/src/Engine/ProtoCore/Parser/atg/Associative.atg b/src/Engine/ProtoCore/Parser/atg/Associative.atg
@@ -1438,6 +1438,7 @@ Associative_Number<out ProtoCore.AST.AssociativeAST.AssociativeNode node>
             else
             {
                 node.line = line; node.col = col;
+                node.endLine = t.line; node.endCol = t.col;
             }
         .)
     )
@@ -1453,10 +1454,9 @@ Associative_Char<out ProtoCore.AST.AssociativeAST.AssociativeNode node>
 
                 node = new ProtoCore.AST.AssociativeAST.CharNode() 
                 { 
-                    Value = t.val.Substring(1, t.val.Length - 2),
-                    line = t.line,
-                    col = t.col
+                    Value = t.val.Substring(1, t.val.Length - 2)
                 }; 
+                NodeUtils.SetNodeLocation(node, t);
             .)
 .
 
@@ -1466,7 +1466,7 @@ Associative_String<out ProtoCore.AST.AssociativeAST.AssociativeNode node>
             (. 
                 node = new ProtoCore.AST.AssociativeAST.StringNode() 
                 { 
-                    Value = GetEscapedString(t.val.Length <= 2 ? "" : t.val.Substring(1, t.val.Length - 2)),
+                    Value = GetEscapedString(t.val.Length <= 2 ? "" : t.val.Substring(1, t.val.Length - 2))
                 }; 
                 NodeUtils.SetNodeLocation(node, t);
             .)

diff --git a/src/Engine/ProtoCore/Parser/atg/Imperative.atg b/src/Engine/ProtoCore/Parser/atg/Imperative.atg
@@ -750,10 +750,9 @@ Imperative_Char<out ProtoCore.AST.ImperativeAST.ImperativeNode node>
 
                         node = new ProtoCore.AST.ImperativeAST.CharNode() 
                         { 
-                            Value = t.val.Substring(1, t.val.Length - 2),
-                            line = t.line,
-                            col = t.col
-                        }; 
+                            Value = t.val.Substring(1, t.val.Length - 2)
+                        };
+                        NodeUtils.SetNodeLocation(node, t);
                     .)
 .
 
@@ -763,10 +762,9 @@ Imperative_String<out ProtoCore.AST.ImperativeAST.ImperativeNode node>
     textstring      (. 
                         node = new ProtoCore.AST.ImperativeAST.StringNode() 
                         { 
-                            Value = GetEscapedString(t.val.Length <= 2 ? "" : t.val.Substring(1, t.val.Length - 2)), 
-                            line = t.line,
-                            col = t.col
+                            Value = GetEscapedString(t.val.Length <= 2 ? "" : t.val.Substring(1, t.val.Length - 2))
                         }; 
+                        NodeUtils.SetNodeLocation(node, t);
                     .)
 .
 
@@ -1147,7 +1145,8 @@ Imperative_num<out ProtoCore.AST.ImperativeAST.ImperativeNode node>
 
             if (ProtoCore.DSASM.Constants.kInvalidIndex != line)
             {
-                node.line = line; node.col = col; 
+                node.line = line; node.col = col;
+                node.endLine = t.line; node.endCol = t.col;
             }
             else
             {
@@ -1167,10 +1166,15 @@ Imperative_num<out ProtoCore.AST.ImperativeAST.ImperativeNode node>
                 node = new ProtoCore.AST.ImperativeAST.NullNode();
             }
 
-            if (ProtoCore.DSASM.Constants.kInvalidIndex != line){
-                node.line = line; node.col = col; }
-            else{
-                NodeUtils.SetNodeLocation(node, t); }
+            if (ProtoCore.DSASM.Constants.kInvalidIndex != line)
+            {
+                node.line = line; node.col = col;
+                node.endLine = t.line; node.endCol = t.col;
+            }
+            else
+            {
+                NodeUtils.SetNodeLocation(node, t);
+            }
         .)
     )
 .

diff --git a/test/Engine/ProtoTest/GraphCompiler/NewFrontEndTests.cs b/test/Engine/ProtoTest/GraphCompiler/NewFrontEndTests.cs
@@ -79,5 +79,24 @@ public void TestUnboundIdentifierInUnnamedSignedExpression()
             Assert.AreEqual(1, inputIdentifier.Count);
             Assert.AreEqual("a", inputIdentifier.ElementAt(0).Value);
         }
+
+        [Test]
+        public void TestUnboundIdentifierInBinaryExpression()
+        {
+            var binaryExpressions = new[] { "x==-0.5;", "x>0.5;", "x<-1;", "x!=1;", "x<=\"a\";", "x>='a';", "x==true;", "x!=false;", "x==null;" };
+
+            foreach (var expression in binaryExpressions)
+            {
+                ElementResolver elementResolver = new ElementResolver();
+                ParseParam parseParam = new ParseParam(Guid.NewGuid(), expression, elementResolver);
+
+                Assert.IsTrue(CompilerUtils.PreCompileCodeBlock(thisTest.CreateTestCore(), ref parseParam));
+                Assert.IsTrue(parseParam.ParsedNodes != null && parseParam.ParsedNodes.Any());
+
+                var inputIdentifier = parseParam.UnboundIdentifiers;
+                Assert.AreEqual(1, inputIdentifier.Count);
+                Assert.AreEqual("x", inputIdentifier.ElementAt(0).Value);
+            }
+        }
     }
 }