[DYN-1682]: Improve node warning, autocomplete and Node2Code behavior with class name conflicts

src/DynamoCore/Engine/CodeCompletion/CodeCompletionServices.cs

@@ -191,7 +191,7 @@ private void AddTypesToCompletionData(string stringToComplete, List<CompletionDa
             var partialName = stringToComplete.ToLower();
             // Add matching Classes
             var classMirrorGroups = StaticMirror.GetAllTypes(core).
-                Where(x => !x.IsHiddenInLibrary && x.Alias.ToLower().Contains(partialName)).
+                Where(x => x.Alias.ToLower().Contains(partialName)).
                     GroupBy(x => x.Alias);
 
             foreach (var classMirrorGroup in classMirrorGroups)
@@ -214,7 +214,7 @@ private void AddTypesToCompletionData(string stringToComplete, List<CompletionDa
                 }
                 // Filter out empty types
                 completions.AddRange(classMirrorGroup.
-                    Where(x => !x.IsEmpty).
+                    Where(x => !x.IsEmpty && !x.IsHiddenInLibrary).
                         Select(x =>
                                 CompletionData.ConvertMirrorToCompletionData(x, useShorterName,
                                     resolver: resolver)));

src/DynamoCore/Engine/ShortestQualifiedNameReplacer.cs

@@ -1,4 +1,5 @@
 using System.Linq;
+
 using ProtoCore;
 using ProtoCore.AST.AssociativeAST;
 using ProtoCore.DSASM;
@@ -90,7 +91,40 @@ public override AssociativeNode VisitIdentifierListNode(IdentifierListNode node)
                 RightNode = rightNode,
                 Optr = Operator.dot
             };
-            return leftNode != newLeftNode ? node : RewriteNodeWithShortName(node);
+            return node;
+        }
+
+        public override AssociativeNode VisitIdentifierNode(IdentifierNode node)
+        {
+            var name = node.ToString();
+            string[] strIdentList = name.Split('.');
+
+            // return IdentifierNode if identifier string is not separated by '.'
+            if (strIdentList.Length == 1)
+            {
+                return new IdentifierNode(strIdentList[0]);
+            }
+
+            // create IdentifierListNode from string such that
+            // RightNode is IdentifierNode representing classname
+            // and LeftNode is IdentifierNode representing one or more namespaces.
+            var rightNode = new IdentifierNode(strIdentList.Last());
+            string ident = "";
+            for (int i = 0; i < strIdentList.Length - 1; i++)
+            {
+                if (!string.IsNullOrEmpty(ident))
+                    ident += ".";
+                ident += strIdentList[i];
+            }
+            var leftNode = new IdentifierNode(ident);
+
+            var identListNode = new IdentifierListNode
+            {
+                LeftNode = leftNode,
+                RightNode = rightNode,
+                Optr = Operator.dot
+            };
+            return identListNode.Accept(this);
         }
 
         private bool TryShortenClassName(IdentifierListNode node, out AssociativeNode shortNameNode)
@@ -108,84 +142,78 @@ private bool TryShortenClassName(IdentifierListNode node, out AssociativeNode sh
             if (matchingClasses.Length == 0)
                 return false;
 
-            string className = qualifiedName.Split('.').Last();
-
-            var symbol = new ProtoCore.Namespace.Symbol(qualifiedName);
-            if (!symbol.Matches(node.ToString()))
+            if(qualifiedName != node.ToString())
                 return false;
 
+            string className = qualifiedName.Split('.').Last();
+
             shortNameNode = CreateNodeFromShortName(className, qualifiedName);
             return shortNameNode != null;
         }
 
-        private IdentifierListNode RewriteNodeWithShortName(IdentifierListNode node)
+        private AssociativeNode CreateNodeFromShortName(string className, string qualifiedName)
         {
-            // Get class name from AST
-            string qualifiedName = CoreUtils.GetIdentifierExceptMethodName(node);
+            // Get the list of conflicting namespaces that contain the same class name
+            var matchingClasses = classTable.ClassNodes.Where(x =>
+            {
+                //var isHiddenInLibrary = x.ClassAttributes?.HiddenInLibrary ?? false;
+                return x.Name.Split('.').Last().Equals(className) /*&& !isHiddenInLibrary*/;
+            }).ToList();
 
-            // if it is a global method
-            if (string.IsNullOrEmpty(qualifiedName))
-                return node;
+            //var matchingClasses = CoreUtils.GetResolvedClassName(classTable, AstFactory.BuildIdentifier(className));
+            if (matchingClasses.Count == 0)
+                return null;
 
-            // Make sure qualifiedName is not a property
-            var lNode = node.LeftNode;
-            var matchingClasses = classTable.GetAllMatchingClasses(qualifiedName);
-            while (matchingClasses.Length == 0 && lNode is IdentifierListNode)
+            if (matchingClasses.Count == 1)
             {
-                qualifiedName = lNode.ToString();
-                matchingClasses = classTable.GetAllMatchingClasses(qualifiedName);
-                lNode = ((IdentifierListNode)lNode).LeftNode;
+                // If there is no class conflict simply use the class name as the shortest name.
+                return CoreUtils.CreateNodeFromString(className);
             }
-            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
+            var shortName = resolver?.LookupShortName(qualifiedName);
+            if (!string.IsNullOrEmpty(shortName))
             {
-                LeftNode = newIdentList,
-                RightNode = node.RightNode,
-                Optr = Operator.dot
-            };
-            return node;
-        }
+                return CoreUtils.CreateNodeFromString(shortName);
+            }
 
-        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;
+            // Use the namespace list to derive the list of shortest unique names
+            var symbolList =
+                matchingClasses.Select(matchingClass => new Symbol(matchingClass.Name));
+            var shortNames = Symbol.GetShortestUniqueNames(symbolList);
 
-            string shortName;
-            // if there is no class conflict simply use the class name as the shortest name
-            if (matchingClasses.Length == 1)
+            // remove hidden class if any from shortNames before proceeding
+            var hiddenClassNode = matchingClasses.FirstOrDefault(x => x.ClassAttributes?.HiddenInLibrary ?? false);
+            if(hiddenClassNode != null)
             {
-                shortName = className;
+                var keyToRemove = new Symbol(hiddenClassNode.Name);
+                shortNames.Remove(keyToRemove);
             }
-            else
+
+            // Get the shortest name corresponding to the fully qualified name
+            if(shortNames.TryGetValue(new Symbol(qualifiedName), out shortName))
             {
-                shortName = resolver != null ? resolver.LookupShortName(qualifiedName) : null;
+                return CoreUtils.CreateNodeFromString(shortName);
+            }
 
-                if (string.IsNullOrEmpty(shortName))
+            // If shortName for fully qualified classname is not found, it could be a base class
+            // present in class hierarchy of any of the other matchingClasses, in which case
+            // set shortName to the one for the derived class.
+            var qualifiedClassNode = classTable.ClassNodes.FirstOrDefault(x => x.Name == qualifiedName);
+            var classHierarchies = matchingClasses.Select(y => classTable.GetClassHierarchy(y));
+            foreach (var hierarchy in classHierarchies)
+            {
+                // If A derives from B, which derives from C, the hierarchy for A 
+                // is listed in that order: [A, B, C], so we start searching in reverse order.
+                for (int i = hierarchy.Count - 1; i >= 0; i--)
                 {
-                    // 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)];
+                    if (hierarchy[i] == qualifiedClassNode)
+                    {
+                        shortName = shortNames[new Symbol(hierarchy[0].Name)];
+                        return CoreUtils.CreateNodeFromString(shortName);
+                    }
                 }
             }
-            // Rewrite the AST using the shortest name
-            var newIdentList = CoreUtils.CreateNodeFromString(shortName);
-            return newIdentList;
-
+            return null;
         }
     }
 }

src/Engine/ProtoCore/ClassTable.cs

@@ -393,7 +393,7 @@ public class ClassTable
         //Symbol table to manage symbols with namespace
         private Namespace.SymbolTable symbolTable = new Namespace.SymbolTable();
 
-        private List<ClassNode> GetClassHierarchy(ClassNode node)
+        public List<ClassNode> GetClassHierarchy(ClassNode node)
         {
             var cNodes = new List<ClassNode> {node};
 
@@ -538,10 +538,17 @@ public bool TryGetFullyQualifiedName(string name, out string fullName)
         /// <returns>Array of fully qualified name of all matching symbols</returns>
         public string[] GetAllMatchingClasses(string name)
         {
+            // First check if there is an exact name match with fully qualified classname
+            Symbol symbol;
+            if (symbolTable.TryGetExactSymbol(name, out symbol))
+            {
+                if (Constants.kInvalidIndex != symbol.Id)
+                    return new[] {name};
+            }
+
             var symbols = symbolTable.TryGetSymbols(name, s => s.Matches(name));
 
             var classes = new List<string>();
-
             if (symbols.Length > 1)
             {
                 var baseClass = GetCommonBaseClass(symbols);

test/DynamoCoreTests/CodeBlockNodeTests.cs

@@ -2075,6 +2075,24 @@ public void TestCompletionWhenTyping()
             Assert.AreEqual(expected, actual);
         }
 
+        [Test]
+        [Category("UnitTests")]
+        public void TestCompletionWithGlobalClassWhenTyping()
+        {
+            var codeCompletionServices = new CodeCompletionServices(libraryServicesCore);
+            string code = "Dupt";
+            var completions = codeCompletionServices.SearchCompletions(code, Guid.Empty);
+
+            // Expected 4 completion items
+            Assert.AreEqual(3, completions.Count());
+
+            string[] expectedValues = {"DupTargetTest", "A.DupTargetTest", "FFITarget.B.DupTargetTest"};
+            var expected = expectedValues.OrderBy(x => x);
+            var actual = completions.Select(x => x.Text).OrderBy(x => x);
+
+            Assert.AreEqual(expected, actual);
+        }
+
         [Test]
         [Category("UnitTests")]
         public void TestMethodKeywordCompletionWhenTyping()
@@ -2146,7 +2164,7 @@ public void TestHiddenConflictingClassCompletionWhenTyping()
             // Expected 1 completion items
             Assert.AreEqual(1, completions.Count());
 
-            string[] expected = { "AnotherClassWithNameConflict" };
+            string[] expected = { "FirstNamespace.AnotherClassWithNameConflict" };
             var actual = completions.Select(x => x.Text).OrderBy(x => x);
 
             Assert.AreEqual(expected, actual);

test/DynamoCoreTests/NodeToCodeTest.cs

@@ -766,12 +766,75 @@ public void TestShortestQualifiedNameReplacerWithStaticProperty()
 
             Assert.IsNotNull(expr1);
 
-            // Since there is a conflict with FFITarget.DesignScript.Point and FFITarget.Dynamo.Point,
-            // node to code generates the shortest unique name, which in this case will be
-            // Autodesk.Point for Autodesk.DesignScript.Geometry.Point
             Assert.AreEqual("ElementResolverTarget.StaticProperty", expr1.RightNode.ToString());
         }
 
+        [Test]
+        public void TestShortestQualifiedNameReplacerWithGlobalClass()
+        {
+            string libraryPath = "FFITarget.dll";
+            if (!CurrentDynamoModel.EngineController.LibraryServices.IsLibraryLoaded(libraryPath))
+            {
+                CurrentDynamoModel.EngineController.LibraryServices.ImportLibrary(libraryPath);
+            }
+
+            OpenModel(@"core\node2code\ShortenNodeNameWithGlobalClass.dyn");
+            var nodes = CurrentDynamoModel.CurrentWorkspace.Nodes;
+            var engine = CurrentDynamoModel.EngineController;
+
+            var result = engine.ConvertNodesToCode(nodes, nodes);
+            result = NodeToCodeCompiler.ConstantPropagationForTemp(result, Enumerable.Empty<string>());
+            NodeToCodeCompiler.ReplaceWithShortestQualifiedName(engine.LibraryServices.LibraryManagementCore.ClassTable, result.AstNodes);
+            Assert.IsNotNull(result);
+            Assert.IsNotNull(result.AstNodes);
+
+            var asts = result.AstNodes.ToList();
+            Assert.AreEqual(2, asts.Count);
+
+            var expr1 = asts[0] as BinaryExpressionNode;
+            var expr2 = asts[1] as BinaryExpressionNode;
+
+            Assert.IsNotNull(expr1);
+            Assert.IsNotNull(expr2);
+
+            Assert.AreEqual("DupTargetTest.DupTargetTest()", expr1.RightNode.ToString());
+            Assert.AreEqual("DupTargetTest.Foo(t1)", expr2.RightNode.ToString());
+
+        }
+
+        [Test]
+        public void TestShortestQualifiedNameReplacerWithConflictingClass()
+        {
+            string libraryPath = "FFITarget.dll";
+            if (!CurrentDynamoModel.EngineController.LibraryServices.IsLibraryLoaded(libraryPath))
+            {
+                CurrentDynamoModel.EngineController.LibraryServices.ImportLibrary(libraryPath);
+            }
+
+            OpenModel(@"core\node2code\ShortenNodeNameWithConflictingClass.dyn");
+            var nodes = CurrentDynamoModel.CurrentWorkspace.Nodes;
+            var engine = CurrentDynamoModel.EngineController;
+
+            var result = engine.ConvertNodesToCode(nodes, nodes);
+            result = NodeToCodeCompiler.ConstantPropagationForTemp(result, Enumerable.Empty<string>());
+            NodeToCodeCompiler.ReplaceWithShortestQualifiedName(engine.LibraryServices.LibraryManagementCore.ClassTable, result.AstNodes);
+            Assert.IsNotNull(result);
+            Assert.IsNotNull(result.AstNodes);
+
+            var asts = result.AstNodes.ToList();
+            Assert.AreEqual(2, asts.Count);
+
+            var expr1 = asts[0] as BinaryExpressionNode;
+            var expr2 = asts[1] as BinaryExpressionNode;
+
+            Assert.IsNotNull(expr1);
+            Assert.IsNotNull(expr2);
+
+            Assert.AreEqual("DupTargetTest.DupTargetTest()", expr1.RightNode.ToString());
+            Assert.AreEqual("DupTargetTest.Prop(t1)", expr2.RightNode.ToString());
+
+        }
+
         [Test]
         public void TestBasicNode2CodeWorkFlow1()
         {
@@ -957,16 +1020,16 @@ public void TestPropertyToStaticPropertyInvocation()
         }
 
         [Test]
-        public void NonUniqueNamespaceConflict_on_node2code_doesNotCrash()
+        public void UniqueNamespace_on_node2code_noConflictWarning()
         {
             string libraryPath = "FFITarget.dll";
             if (!CurrentDynamoModel.EngineController.LibraryServices.IsLibraryLoaded(libraryPath))
             {
                 CurrentDynamoModel.EngineController.LibraryServices.ImportLibrary(libraryPath);
             }
 
-            // this graph contains a single "FFITarget.B.DupTargetTest" node that conflicts non-uniquely with namespace "FFITarget.C.B.DupTargetTest"
-            OpenModel(@"core\node2code\NonUniqueNamespaceConflict_throwsNodeWarning.dyn");
+            // this graph contains a single "FFITarget.B.DupTargetTest" node that should not conflict with namespace "FFITarget.C.B.DupTargetTest"
+            OpenModel(@"core\node2code\UniqueNamespace_on_node2code_noConflictWarning.dyn");
             var nodes = CurrentDynamoModel.CurrentWorkspace.Nodes;
             SelectAll(nodes);
             var command = new DynamoModel.ConvertNodesToCodeCommand();
@@ -976,8 +1039,7 @@ public void NonUniqueNamespaceConflict_on_node2code_doesNotCrash()
             var cbn = nodes.OfType<CodeBlockNodeModel>().FirstOrDefault();
             Assert.IsNotNull(cbn);
 
-            var error = "Multiple definitions for 'FFITarget.B.DupTargetTest' are found as FFITarget.C.B.DupTargetTest, FFITarget.B.DupTargetTest";
-            Assert.IsTrue(cbn.ToolTipText.Contains(error));
+            Assert.False(cbn.IsInErrorState);
         }