Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for regression in Node2Code. #11373

Merged
merged 3 commits into from
Dec 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/DynamoCore/Engine/ShortestQualifiedNameReplacer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public override AssociativeNode VisitIdentifierNode(IdentifierNode node)
// return IdentifierNode if identifier string is not separated by '.'
if (strIdentList.Length == 1)
{
return new IdentifierNode(strIdentList[0]);
return node;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If my memory serves correctly, then the AST walkers are a bit inconsistent when it comes to returning new AST nodes or the same node - maybe this should return a copy of the node instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The AST walkers are meant to return new nodes only if they are rewritten and return the original node if there is no change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah! Is that documented somewhere? Would it make sense to add a note about that to the base class or something? Not that it needs to be part of this pr.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can do.

}

// create IdentifierListNode from string such that
Expand Down
18 changes: 18 additions & 0 deletions test/DynamoCoreTests/NodeToCodeTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1398,6 +1398,24 @@ public void TestNodeWithVarArgument()
AssertPreviewValue(guid, new[] { "foo", "bar", "qux" });
}

[Test]
public void TestNodeToCodeWithBuiltInAddMethod()
{
OpenModel(@"core\node2code\builtinAddNode.dyn");
var nodes = CurrentDynamoModel.CurrentWorkspace.Nodes;
SelectAll(nodes);

var command = new DynamoModel.ConvertNodesToCodeCommand();
CurrentDynamoModel.ExecuteCommand(command);
CurrentDynamoModel.ForceRun();

var cbn = CurrentDynamoModel.CurrentWorkspace.Nodes.OfType<CodeBlockNodeModel>().FirstOrDefault();
Assert.IsNotNull(cbn);

var guid = cbn.GUID.ToString();
AssertPreviewValue(guid, 49);
}

[Test]
[Category("RegressionTests")]
public void TestMultioutputNode()
Expand Down
144 changes: 144 additions & 0 deletions test/core/node2code/builtinAddNode.dyn
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
{
"Uuid": "2c2cfb4e-70f7-4462-bca2-facc02603823",
"IsCustomNode": false,
"Description": null,
"Name": "builtinAddNode",
"ElementResolver": {
"ResolutionMap": {}
},
"Inputs": [],
"Outputs": [],
"Nodes": [
{
"ConcreteType": "Dynamo.Graph.Nodes.ZeroTouch.DSFunction, DynamoCore",
"NodeType": "FunctionNode",
"FunctionSignature": "+@var[]..[],var[]..[]",
"Id": "b9ce5822dce94a8bbd13d11b36c2c5b2",
"Inputs": [
{
"Id": "44f8e12394444e529e6dff105400cea8",
"Name": "x",
"Description": "x value.\n\nvar[]..[]",
"UsingDefaultValue": false,
"Level": 2,
"UseLevels": false,
"KeepListStructure": false
},
{
"Id": "95db0f3d6dd74a7298ecbcc325a9e530",
"Name": "y",
"Description": "y value.\n\nvar[]..[]",
"UsingDefaultValue": false,
"Level": 2,
"UseLevels": false,
"KeepListStructure": false
}
],
"Outputs": [
{
"Id": "452c6cbeaf7a4e0faa31d9336ad6a288",
"Name": "var[]..[]",
"Description": "var[]..[]",
"UsingDefaultValue": false,
"Level": 2,
"UseLevels": false,
"KeepListStructure": false
}
],
"Replication": "Auto",
"Description": "Adds x to y.\n\n+ (x: var[]..[], y: var[]..[]): var[]..[]"
},
{
"ConcreteType": "Dynamo.Graph.Nodes.CodeBlockNodeModel, DynamoCore",
"NodeType": "CodeBlockNode",
"Code": "32;\n17;",
"Id": "969d2265352c4e1ca335049dca655ad4",
"Inputs": [],
"Outputs": [
{
"Id": "b5754723e63f4537bb1a0e632f96716a",
"Name": "",
"Description": "Value of expression at line 1",
"UsingDefaultValue": false,
"Level": 2,
"UseLevels": false,
"KeepListStructure": false
},
{
"Id": "7ce41a3ce4864602b533dc5112c29b32",
"Name": "",
"Description": "Value of expression at line 2",
"UsingDefaultValue": false,
"Level": 2,
"UseLevels": false,
"KeepListStructure": false
}
],
"Replication": "Disabled",
"Description": "Allows for DesignScript code to be authored directly"
}
],
"Connectors": [
{
"Start": "b5754723e63f4537bb1a0e632f96716a",
"End": "44f8e12394444e529e6dff105400cea8",
"Id": "abb969fc755a4d9d8b8230a33b95d4ec"
},
{
"Start": "7ce41a3ce4864602b533dc5112c29b32",
"End": "95db0f3d6dd74a7298ecbcc325a9e530",
"Id": "81f9f996548d48fb9d686bca6ce3548c"
}
],
"Dependencies": [],
"NodeLibraryDependencies": [],
"Bindings": [],
"View": {
"Dynamo": {
"ScaleFactor": 1.0,
"HasRunWithoutCrash": true,
"IsVisibleInDynamoLibrary": true,
"Version": "2.11.0.3451",
"RunType": "Automatic",
"RunPeriod": "1000"
},
"Camera": {
"Name": "Background Preview",
"EyeX": -17.0,
"EyeY": 24.0,
"EyeZ": 50.0,
"LookX": 12.0,
"LookY": -13.0,
"LookZ": -58.0,
"UpX": 0.0,
"UpY": 1.0,
"UpZ": 0.0
},
"NodeViews": [
{
"Id": "b9ce5822dce94a8bbd13d11b36c2c5b2",
"IsSetAsInput": false,
"IsSetAsOutput": false,
"Name": "+",
"ShowGeometry": true,
"Excluded": false,
"X": 333.5,
"Y": 175.0
},
{
"Id": "969d2265352c4e1ca335049dca655ad4",
"IsSetAsInput": false,
"IsSetAsOutput": false,
"Name": "Code Block",
"ShowGeometry": true,
"Excluded": false,
"X": 167.0,
"Y": 181.0
}
],
"Annotations": [],
"X": 0.0,
"Y": 0.0,
"Zoom": 1.0
}
}