Skip to content

Commit

Permalink
Avoid namespace collisions when collapsing graph to custom node (#9330)…
Browse files Browse the repository at this point in the history 
… (#9360)

* this works - but I am hesitant to modify this replacer.
Not sure how this effects node2code. - may create rewriter directly.

* add a new visitor method for typedIdentifierNode
use it in custmNodeManager collapse method

* move shortQualifiedNameReplacer
update references
add comments to node2code mutation tests

* add more tests
add extra logic to deserialization for symbol node element resolver cache

* revert public API break, keep rewriter in dynamoCore but in new file

* remove extra usings added previously.

* remove debugging code
  • Loading branch information
@mjkkirschner @QilongTang
mjkkirschner authored and QilongTang committed Jan 3, 2019
1 parent b121ab2 commit 2351da5
Show file tree
Hide file tree
Showing 12 changed files with 476 additions and 207 deletions.
58 changes: 47 additions & 11 deletions src/DynamoCore/Core/CustomNodeManager.cs
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Xml;
using Dynamo.Engine;
using Dynamo.Engine.NodeToCode;
using Dynamo.Graph;
using Dynamo.Graph.Annotations;
using Dynamo.Graph.Connectors;
Expand All @@ -22,6 +17,13 @@
using Dynamo.Properties;
using Dynamo.Selection;
using Dynamo.Utilities;
using ProtoCore.AST.AssociativeAST;
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Xml;
using Symbol = Dynamo.Graph.Nodes.CustomNodes.Symbol;

namespace Dynamo.Core
Expand Down Expand Up @@ -674,7 +676,8 @@ private bool InitializeCustomNode(
nodeGraph.ElementResolver,
workspaceInfo);
}
else {
else
{
Exception ex;
if (DynamoUtilities.PathHelper.isValidJson(workspaceInfo.FileName, out jsonDoc, out ex))
{
Expand Down Expand Up @@ -1031,6 +1034,7 @@ internal CustomNodeWorkspaceModel Collapse(

var inConnectors = new List<Tuple<NodeModel, int>>();
var uniqueInputSenders = new Dictionary<Tuple<NodeModel, int>, Symbol>();
var classTable = this.libraryServices.LibraryManagementCore.ClassTable;

//Step 3: insert variables (reference step 1)
foreach (var input in Enumerable.Range(0, inputs.Count).Zip(inputs, Tuple.Create))
Expand Down Expand Up @@ -1077,10 +1081,17 @@ internal CustomNodeWorkspaceModel Collapse(
var funcDesc = dsFunc.Controller.Definition;
parameters = funcDesc.Parameters.ToList();

// if the node is an instance member the function won't contain a
// parameter for this type so we need to generate a new typedParameter.
if (funcDesc.Type == Engine.FunctionType.InstanceMethod ||
funcDesc.Type == Engine.FunctionType.InstanceProperty)
{
var dummyType = new ProtoCore.Type() { Name = funcDesc.ClassName };
var dummyType = new ProtoCore.Type
{
Name = funcDesc.ClassName,
UID = classTable.IndexOf(funcDesc.ClassName)
};

var instanceParam = new TypedParameter(funcDesc.ClassName, dummyType);
parameters.Insert(0, instanceParam);
}
Expand All @@ -1090,10 +1101,35 @@ internal CustomNodeWorkspaceModel Collapse(
// input_var_name : type
if (parameters != null && parameters.Count() > inputReceiverData)
{
var typeName = parameters[inputReceiverData].DisplayTypeName;
if (!string.IsNullOrEmpty(typeName))
var port = inputReceiverNode.InPorts[inputReceiverData];
var typedParameter = parameters[inputReceiverData];
// initially set the type name to the full type name
// then try to shorten it.
if (!string.IsNullOrEmpty(typedParameter.Type.Name))
{
node.InputSymbol += " : " + typeName;
try
{

var typedNode = new TypedIdentifierNode
{
Name = port.Name,
Value = port.Name,
datatype = typedParameter.Type,
TypeAlias = typedParameter.Type.Name
};

NodeToCodeCompiler.ReplaceWithShortestQualifiedName(
classTable,
new List<AssociativeNode> { typedNode },
currentWorkspace.ElementResolver);

node.InputSymbol = $"{typedNode.Value} :{typedNode.TypeAlias}";
}
catch(Exception e)
{
node.InputSymbol += ":" + typedParameter.Type.Name;
this.AsLogger().LogError($"{e.Message}: could not generate a short type name for {typedParameter.Type.Name}");
}
}
}

Expand Down
1 change: 1 addition & 0 deletions src/DynamoCore/DynamoCore.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ limitations under the License.
<Compile Include="Configuration\DebugSettings.cs" />
<Compile Include="Configuration\Context.cs" />
<Compile Include="Core\DynamoMigrator.cs" />
<Compile Include="Engine\ShortestQualifiedNameReplacer.cs" />
<Compile Include="Exceptions\LibraryLoadFailedException.cs" />
<Compile Include="Graph\Nodes\NodeOutputData.cs" />
<Compile Include="Graph\Nodes\NodeInputData.cs" />
Expand Down
146 changes: 5 additions & 141 deletions src/DynamoCore/Engine/NodeToCode/NodeToCode.cs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
using System;
using System.Collections.Generic;
using System.Linq;
using Dynamo.Core;
using Dynamo.Core;
using Dynamo.Engine.CodeGeneration;
using Dynamo.Graph;
using Dynamo.Graph.Nodes;
Expand All @@ -10,7 +7,9 @@
using ProtoCore.DSASM;
using ProtoCore.Namespace;
using ProtoCore.SyntaxAnalysis;
using ProtoCore.Utils;
using System;
using System.Collections.Generic;
using System.Linq;

namespace Dynamo.Engine.NodeToCode
{
Expand Down Expand Up @@ -302,142 +301,6 @@ public override AssociativeNode VisitBinaryExpressionNode(BinaryExpressionNode n
}
}

/// <summary>
/// Replace a fully qualified function call with short name.
/// </summary>
internal class ShortestQualifiedNameReplacer : AstReplacer
{
private readonly ClassTable classTable;
private readonly ElementResolver resolver;

public ShortestQualifiedNameReplacer(ClassTable classTable, ElementResolver resolver)
{
this.classTable = classTable;
this.resolver = resolver;
}

public override AssociativeNode VisitIdentifierListNode(IdentifierListNode node)
{
if (node == null)
return null;

// First pass attempt to resolve the node class name
// and shorten it before traversing it deeper
AssociativeNode shortNameNode;
if (TryShortenClassName(node, out shortNameNode))
return shortNameNode;

var rightNode = node.RightNode;
var leftNode = node.LeftNode;

rightNode = rightNode.Accept(this);
var newLeftNode = leftNode.Accept(this);

node = new IdentifierListNode
{
LeftNode = newLeftNode,
RightNode = rightNode,
Optr = Operator.dot
};
return leftNode != newLeftNode ? node : RewriteNodeWithShortName(node);
}

private bool TryShortenClassName(IdentifierListNode node, out AssociativeNode shortNameNode)
{
shortNameNode = null;

string qualifiedName = CoreUtils.GetIdentifierExceptMethodName(node);

// if it is a global method with no class
if (string.IsNullOrEmpty(qualifiedName))
return false;

// Make sure qualifiedName is not a property
var matchingClasses = classTable.GetAllMatchingClasses(qualifiedName);
if (matchingClasses.Length == 0)
return false;

string className = qualifiedName.Split('.').Last();

var symbol = new ProtoCore.Namespace.Symbol(qualifiedName);
if (!symbol.Matches(node.ToString()))
return false;

shortNameNode = CreateNodeFromShortName(className, qualifiedName);
return shortNameNode != null;
}

private IdentifierListNode RewriteNodeWithShortName(IdentifierListNode node)
{
// Get class name from AST
string qualifiedName = CoreUtils.GetIdentifierExceptMethodName(node);

// if it is a global method
if (string.IsNullOrEmpty(qualifiedName))
return node;

// Make sure qualifiedName is not a property
var lNode = node.LeftNode;
var matchingClasses = classTable.GetAllMatchingClasses(qualifiedName);
while (matchingClasses.Length == 0 && lNode is IdentifierListNode)
{
qualifiedName = lNode.ToString();
matchingClasses = classTable.GetAllMatchingClasses(qualifiedName);
lNode = ((IdentifierListNode)lNode).LeftNode;
}
qualifiedName = lNode.ToString();
string className = qualifiedName.Split('.').Last();

var newIdentList = CreateNodeFromShortName(className, qualifiedName);
if (newIdentList == null)
return node;

// Replace class name in input node with short name (newIdentList)
node = new IdentifierListNode
{
LeftNode = newIdentList,
RightNode = node.RightNode,
Optr = Operator.dot
};
return node;
}

private AssociativeNode CreateNodeFromShortName(string className, string qualifiedName)
{
// Get the list of conflicting namespaces that contain the same class name
var matchingClasses = CoreUtils.GetResolvedClassName(classTable, AstFactory.BuildIdentifier(className));
if (matchingClasses.Length == 0)
return null;

string shortName;
// if there is no class conflict simply use the class name as the shortest name
if (matchingClasses.Length == 1)
{
shortName = className;
}
else
{
shortName = resolver != null ? resolver.LookupShortName(qualifiedName) : null;

if (string.IsNullOrEmpty(shortName))
{
// Use the namespace list as input to derive the list of shortest unique names
var symbolList =
matchingClasses.Select(matchingClass => new ProtoCore.Namespace.Symbol(matchingClass))
.ToList();
var shortNames = ProtoCore.Namespace.Symbol.GetShortestUniqueNames(symbolList);

// Get the shortest name corresponding to the fully qualified name
shortName = shortNames[new ProtoCore.Namespace.Symbol(qualifiedName)];
}
}
// Rewrite the AST using the shortest name
var newIdentList = CoreUtils.CreateNodeFromString(shortName);
return newIdentList;

}
}

/// <summary>
/// Check if an identifier is used.
/// </summary>
Expand Down Expand Up @@ -1195,6 +1058,7 @@ public static void ReplaceWithShortestQualifiedName(ClassTable classTable, IEnum
}
}


/// <summary>
/// Compile a set of nodes to ASTs.
///
Expand Down
Loading

0 comments on commit 2351da5

Please sign in to comment.