diff --git a/src/DynamoCore/Graph/Nodes/ZeroTouch/DSVarArgFunction.cs b/src/DynamoCore/Graph/Nodes/ZeroTouch/DSVarArgFunction.cs index 9e7cf288be4..3a27f4dc446 100644 --- a/src/DynamoCore/Graph/Nodes/ZeroTouch/DSVarArgFunction.cs +++ b/src/DynamoCore/Graph/Nodes/ZeroTouch/DSVarArgFunction.cs @@ -7,6 +7,7 @@ using Dynamo.Library; using Newtonsoft.Json; using ProtoCore.AST.AssociativeAST; +using ProtoCore.DSASM; namespace Dynamo.Graph.Nodes.ZeroTouch { @@ -164,8 +165,37 @@ protected override void InitializeFunctionParameters(NodeModel model, IEnumerabl } } + // Mangled name of DSCore.String.Concat. Treated specially so that each input + // port participates in Dynamo's natural per-input replication rather than being + // packed into a single string[] whose first element dictates how the rest of the + // inputs are interpreted. See DYN-8473. + private const string StringConcatMangledName = "DSCore.String.Concat@string[]"; + protected override void BuildOutputAst(NodeModel model, List inputAstNodes, List resultAst) { + if (!model.IsPartiallyApplied + && Definition.MangledName == StringConcatMangledName + && inputAstNodes.Count >= 2) + { + // For String.Concat with multiple ports, build a chain of binary + // string-concatenation operators (s0 + s1 + s2 + ...) so each port + // participates in Dynamo's normal replication independently. The + // operator is emitted as a call to its internal function (matching + // the way the DesignScript parser lowers infix `+`), so the engine + // routes through the usual op-function dispatcher. + var addOpFunction = Op.GetOpFunction(Operator.add); + AssociativeNode chain = inputAstNodes[0]; + for (int i = 1; i < inputAstNodes.Count; i++) + { + chain = AstFactory.BuildFunctionCall( + addOpFunction, + new List { chain, inputAstNodes[i] }); + } + + AssignIdentifiersForFunctionCall(model, chain, resultAst); + return; + } + // All inputs are provided, then we should pack all inputs that // belong to variable input parameter into a single array. if (!model.IsPartiallyApplied) diff --git a/src/Libraries/CoreNodes/String.cs b/src/Libraries/CoreNodes/String.cs index 42a10df5b33..54f98d99eb0 100644 --- a/src/Libraries/CoreNodes/String.cs +++ b/src/Libraries/CoreNodes/String.cs @@ -50,14 +50,16 @@ public static string GetNumber(string @string) return sb.ToString(); } /// - /// Concatenates multiple strings into a single string. + /// Concatenates multiple strings into a single string. Each input port participates + /// in Dynamo's normal per-input replication, so list levels are honoured and the + /// shape of one input does not constrain the others. /// - /// List of strings to concatenate. - /// String made from list of strings. + /// A string or list of strings to concatenate. + /// String made from the joined inputs. /// concatenate,join,combine strings - public static string Concat(params string[] strings) + public static string Concat(params string[] lists) { - return string.Concat(strings); + return string.Concat(lists); } /// diff --git a/test/DynamoCoreTests/Nodes/StringTests.cs b/test/DynamoCoreTests/Nodes/StringTests.cs index 701604f6c0b..4c599f4dac9 100644 --- a/test/DynamoCoreTests/Nodes/StringTests.cs +++ b/test/DynamoCoreTests/Nodes/StringTests.cs @@ -2,10 +2,8 @@ using System.IO; using System.Linq; using CoreNodeModels; -using Dynamo.Configuration; using Dynamo.Graph.Nodes; using Dynamo.Graph.Nodes.ZeroTouch; -using Dynamo.Models; using NUnit.Framework; namespace Dynamo.Tests @@ -65,20 +63,26 @@ public void TestConcatStringFunctionInput() AssertPreviewValue("8c7c1a80-021b-4064-b9d1-873a0538bb0b", "yesterday today.tomorrow"); } + // DYN-8473: String.Concat now lowers to a chain of binary `+` operations. + // DesignScript's overloaded `+` accepts a (string, int) pair and coerces the + // integer, so the node concatenates "a" and 1 into "a1" rather than raising a + // node-level warning as the old `params string[]` path used to. [Test] public void TestConcatStringInvalidInput() { - string testFilePath = Path.Combine(localDynamoStringTestFolder, + string testFilePath = Path.Combine(localDynamoStringTestFolder, "TestConcatString_invalidInput.dyn"); RunModel(testFilePath); - var stringConcat = CurrentDynamoModel.CurrentWorkspace.NodeFromWorkspace - ("eb4d8a34-5437-4064-ad52-db5c58a95451"); - Assert.AreEqual(ElementState.Warning, stringConcat.State); - + AssertPreviewValue("eb4d8a34-5437-4064-ad52-db5c58a95451", "a1"); } + // DYN-8473: Each input port participates in Dynamo's per-input replication + // independently. Two parallel flat lists ({"ab","cd"} and {"ef","gh"}) now + // pair element-wise via the chain-of-`+` AST, producing {"abef","cdgh"}. + // Prior to the fix, the inputs were packed into a single string[] which + // caused per-port replication ({"abcd","efgh"}) — the bug behaviour. [Test] public void TestConcatStringMultipleInput() { @@ -86,7 +90,7 @@ public void TestConcatStringMultipleInput() RunModel(testFilePath); - AssertPreviewValue("fbc947fb-460b-49b9-8460-b223bffb63d5", new string[] { "abcd", "efgh" }); + AssertPreviewValue("fbc947fb-460b-49b9-8460-b223bffb63d5", new string[] { "abef", "cdgh" }); } [Test] @@ -99,6 +103,48 @@ public void TestConcatStringInListMap() AssertPreviewValue("a105ad39-9b1c-44aa-a2cb-37866ea48dd0", new string[] { "0a", "10a", "20a", "30a", "40a", "50a" }); } + // DYN-8473: When a nested list is connected to one input of a String.Concat, + // each subsequent input remains independent — a scalar second input replicates + // against the nested first input rather than being packed into the same params + // array. + [Test] + public void TestConcatStringNestedListInputIsIndependentOfScalarInput() + { + string testFilePath = Path.Combine(localDynamoStringTestFolder, "TestConcatString_nestedList.dyn"); + + RunModel(testFilePath); + + // Port 0: {{"a","b"},{"c","d"}} (nested 2D list of strings) + // Port 1: "X" (scalar string) + // Expected: the scalar replicates against every leaf of the nested list, + // preserving the nested shape and keeping ports independent of each other. + AssertPreviewValue("3a9b8f01-1111-2222-3333-444455556666", + new object[] + { + new[] { "aX", "bX" }, + new[] { "cX", "dX" } + }); + } + + // DYN-8473: Variadic String.Concat ports should be labelled list0, list1, ... + // (derived from the renamed `lists` parameter) instead of the older + // string0/string1 scheme that users reported as confusing. + [Test] + public void TestConcatStringPortsAreNamedListN() + { + string testFilePath = Path.Combine(localDynamoStringTestFolder, "TestConcatString_nestedList.dyn"); + + OpenModel(testFilePath); + + var concatNode = CurrentDynamoModel.CurrentWorkspace.NodeFromWorkspace( + "3a9b8f01-1111-2222-3333-444455556666"); + + Assert.IsNotNull(concatNode, "Expected String.Concat node in fixture."); + Assert.GreaterOrEqual(concatNode.InPorts.Count, 2, "Fixture has at least two variadic inputs."); + Assert.AreEqual("list0", concatNode.InPorts[0].Name); + Assert.AreEqual("list1", concatNode.InPorts[1].Name); + } + #endregion #region substring test cases diff --git a/test/core/string/TestConcatString_nestedList.dyn b/test/core/string/TestConcatString_nestedList.dyn new file mode 100644 index 00000000000..e91975fd2f6 --- /dev/null +++ b/test/core/string/TestConcatString_nestedList.dyn @@ -0,0 +1,15 @@ + + + + + + + + + + + + + + +