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.

Sign up for GitHub

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

Avoid namespace collisions when collapsing graph to custom node #9330

Merged
merged 9 commits into from
Jan 2, 2019
Merged

Avoid namespace collisions when collapsing graph to custom node #9330

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
39236d2
this works - but I am hesitant to modify this replacer.
mjkkirschner Dec 19, 2018
7f08259
add a new visitor method for typedIdentifierNode
mjkkirschner Dec 20, 2018
e8324b7
Merge remote-tracking branch 'upstream/master' into elementResolverNull
mjkkirschner Dec 21, 2018
dfa9934
move shortQualifiedNameReplacer
mjkkirschner Dec 21, 2018
a59685b
add more tests
mjkkirschner Dec 22, 2018
eb00720
revert public API break, keep rewriter in dynamoCore but in new file
mjkkirschner Dec 22, 2018
466f588
Merge remote-tracking branch 'upstream/master' into elementResolverNull
mjkkirschner Jan 2, 2019
c1a123a
remove extra usings added previously.
mjkkirschner Jan 2, 2019
7372431
remove debugging code
mjkkirschner Jan 2, 2019
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
58 changes: 47 additions & 11 deletions src/DynamoCore/Core/CustomNodeManager.cs
View file
Open in desktop
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,
Copy link
Contributor

Choose a reason for hiding this comment

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

I see the nodeGraph is instantiated from an xml file? Is that why it doesn't get the correct element resolver as we are now using JSON?

Copy link
Member Author

Choose a reason for hiding this comment

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

here is where we use the deserialized element resolver when loading a JSON graph and pass it to the home workspace or customNode workspace that was just deserialized.

Dynamo/src/DynamoCore/Graph/Workspaces/SerializationConverters.cs

Line 532 in ec10f93

ws = new CustomNodeWorkspaceModel(factory, nodes, notes, annotations,

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}");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this set InputSymbol to the full name even though it is unable to find a short name? If not, it should.

Copy link
Member Author

Choose a reason for hiding this comment

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

yep, thats what it does.

}
}

Expand Down
1 change: 1 addition & 0 deletions src/DynamoCore/DynamoCore.csproj
View file
Open in desktop
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
Open in desktop
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