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 broken behavior of IsVisibleInDynamoLibrary attribute with interfaces and Enums #9242

Merged
merged 7 commits into from
Nov 14, 2018
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
73 changes: 49 additions & 24 deletions src/Engine/ProtoCore/FFI/CLRDLLModule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ static CLRModuleType()
/// given type is not found, it creates a new one. If CLRDLLModule is
/// passed as null, it creates empty CLRModuleType.
/// </summary>
/// <param name="module">CLRDLLModule which imports this type</param>
/// <param name="type">System.Type to be imported in DesignScript</param>
/// <param name="module">CLRDLLModule which imports this type</param>
/// <param name="alias">Alias name, if any. For now its not supported.</param>
public static CLRModuleType GetInstance(Type type, CLRDLLModule module, string alias)
{
Expand Down Expand Up @@ -359,10 +359,10 @@ private ClassDeclNode ParseSystemType(Type type, string alias)
// marked as sealed and abstract.
classnode.IsStatic = type.IsAbstract && type.IsSealed;

// If all methods are static, it doesn't make sense to expose
// constructor.
if (!classnode.IsStatic)
{
// If all methods are static, it doesn't make sense to expose
// constructor.
ConstructorInfo[] ctors = type.GetConstructors();
foreach (var c in ctors)
{
Expand Down Expand Up @@ -663,23 +663,24 @@ private ProtoCore.AST.AssociativeAST.VarDeclNode ParseFieldDeclaration(FieldInfo
return varDeclNode;
}

private ProtoCore.AST.AssociativeAST.FunctionDefinitionNode ParseFieldAccessor(FieldInfo f)
private FunctionDefinitionNode ParseFieldAccessor(FieldInfo f)
{
if (null == f || SupressesImport(f))
return null;

ProtoCore.AST.AssociativeAST.FunctionDefinitionNode func = new ProtoCore.AST.AssociativeAST.FunctionDefinitionNode();
func.Name = string.Format("{0}{1}", Constants.kGetterPrefix, f.Name);
func.Signature = new ProtoCore.AST.AssociativeAST.ArgumentSignatureNode();
func.ReturnType = CLRModuleType.GetProtoCoreType(f.FieldType, Module);
func.FunctionBody = null;
func.Access = ProtoCore.CompilerDefinitions.AccessModifier.Public;
func.IsExternLib = true;
func.ExternLibName = Module.Name;
func.IsStatic = f.IsStatic;
//Set the method attribute for Enum properties.
func.MethodAttributes = new FFIMethodAttributes(f);

var func = new FunctionDefinitionNode
{
Name = string.Format("{0}{1}", Constants.kGetterPrefix, f.Name),
Signature = new ArgumentSignatureNode(),
ReturnType = CLRModuleType.GetProtoCoreType(f.FieldType, Module),
FunctionBody = null,
Access = ProtoCore.CompilerDefinitions.AccessModifier.Public,
IsExternLib = true,
ExternLibName = Module.Name,
IsStatic = f.IsStatic,
MethodAttributes = new FFIMethodAttributes(f),
};

return func;
}

Expand Down Expand Up @@ -1225,6 +1226,9 @@ public FFIClassAttributes(Type type)
if (type == null)
throw new ArgumentNullException("type");

// Hide all interfaces from library and search
if (type.IsInterface) HiddenInLibrary = true;

attributes = type.GetCustomAttributes(false).Cast<Attribute>().ToArray();
foreach (var attr in attributes)
{
Expand Down Expand Up @@ -1268,24 +1272,45 @@ public FFIMethodAttributes(FieldInfo f)
{
var atts = f.GetCustomAttributes(false).Cast<Attribute>();

var parentAtts = f.DeclaringType.GetCustomAttributes(false).Cast<Attribute>();
var isObsolete = false;
var hidden = false;
var message = "";
foreach(var attr in parentAtts)
{
if(attr is ObsoleteAttribute)
{
isObsolete = true;
message = (attr as ObsoleteAttribute).Message;
if (string.IsNullOrEmpty(message))
message = "Obsolete";
}

if (attr is IsVisibleInDynamoLibraryAttribute)
{
hidden = !((IsVisibleInDynamoLibraryAttribute)attr).Visible;
}
}

foreach (var attr in atts)
{
//Set the obsolete message for enum fields.
if (attr is IsObsoleteAttribute)
if (attr is ObsoleteAttribute)
{
HiddenInLibrary = true;
ObsoleteMessage = (attr as IsObsoleteAttribute).Message;
ObsoleteMessage = (attr as ObsoleteAttribute).Message;
Copy link
Member

Choose a reason for hiding this comment

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

@aparajit-pratap what is the difference between these attribute types?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IsObsolete is written by us and Obsolete is a standard attribute.

if (string.IsNullOrEmpty(ObsoleteMessage))
ObsoleteMessage = "Obsolete";
}
else if (attr is ObsoleteAttribute)
else if(attr is IsVisibleInDynamoLibraryAttribute)
{
HiddenInLibrary = true;
ObsoleteMessage = (attr as ObsoleteAttribute).Message;
if (string.IsNullOrEmpty(ObsoleteMessage))
ObsoleteMessage = "Obsolete";
HiddenInLibrary = !((IsVisibleInDynamoLibraryAttribute)attr).Visible;
}

}
if (isObsolete || hidden)
{
HiddenInLibrary = true;
if (isObsolete) ObsoleteMessage = message;
}
}

Expand Down
8 changes: 1 addition & 7 deletions src/Libraries/CoreNodes/DateTime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -233,21 +233,15 @@ public static System.TimeSpan TimeOfDay(System.DateTime dateTime)
/// Days of the Week
/// </summary>
[IsVisibleInDynamoLibrary(false)]
[Obsolete("This node is deprecated")]
public enum DayOfWeek
{
[Obsolete("This node is deprecated")]
[EnumDescription("EnumDateOfWeekSunday", typeof(Properties.Resources))]Sunday,
[Obsolete("This node is deprecated")]
[EnumDescription("EnumDateOfWeekMonday", typeof(Properties.Resources))]Monday,
[Obsolete("This node is deprecated")]
[EnumDescription("EnumDateOfWeekTuesday", typeof(Properties.Resources))]Tuesday,
[Obsolete("This node is deprecated")]
[EnumDescription("EnumDateOfWeekWednesday", typeof(Properties.Resources))]Wednesday,
[Obsolete("This node is deprecated")]
[EnumDescription("EnumDateOfWeekThursday", typeof(Properties.Resources))]Thursday,
[Obsolete("This node is deprecated")]
[EnumDescription("EnumDateOfWeekFriday", typeof(Properties.Resources))]Friday,
[Obsolete("This node is deprecated")]
[EnumDescription("EnumDateOfWeekSaturday", typeof(Properties.Resources))]Saturday
}

Expand Down
96 changes: 96 additions & 0 deletions test/DynamoCoreWpfTests/LibraryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using NUnit.Framework;
using ProtoCore;
using System;
using System.Linq;
using System.Xml;
using TestServices;

Expand Down Expand Up @@ -151,5 +152,100 @@ public void DumpLibraryToXmlZeroTouchTest()
}
}
}

[Test]
public void SearchHiddenInterfaceNodeTest()
{
var searchViewModel = new SearchViewModel(new NodeSearchModel());

LibraryLoaded = false;

string libraryPath = "FFITarget.dll";

// All we need to do here is to ensure that the target has been loaded
// at some point, so if it's already here, don't try and reload it
if (!libraryServices.IsLibraryLoaded(libraryPath))
{
libraryServices.ImportLibrary(libraryPath);
Assert.IsTrue(LibraryLoaded);
}

var fgToCompare = libraryServices.GetFunctionGroups(libraryPath);
foreach (var funcGroup in fgToCompare)
{
foreach (var functionDescriptor in funcGroup.Functions)
{
if (functionDescriptor.IsVisibleInLibrary && !functionDescriptor.DisplayName.Contains("GetType"))
{
searchViewModel.Model.Add(new ZeroTouchSearchElement(functionDescriptor));
}
}
}

var searchString = "InterfaceA";
var nodes = searchViewModel.Search(searchString);
var foundNodes = nodes.Where(n => n.Class.Equals(searchString));
Assert.IsFalse(foundNodes.Any());

searchString = "DerivedFromInterfaceA";
nodes = searchViewModel.Search(searchString);
foundNodes = nodes.Where(n => n.Class.Equals(searchString));
Assert.AreEqual(2, foundNodes.Count());

searchString = "TraceableId";
nodes = searchViewModel.Search(searchString);
foundNodes = nodes.Where(n => n.Class.Equals(searchString));
Assert.IsFalse(foundNodes.Any());

searchString = "ISerializable";
nodes = searchViewModel.Search(searchString);
foundNodes = nodes.Where(n => n.Class.Equals(searchString));
Assert.IsFalse(foundNodes.Any());
}

[Test]
public void SearchHiddenEnumTest()
{
var searchViewModel = new SearchViewModel(new NodeSearchModel());

LibraryLoaded = false;

string libraryPath = "FFITarget.dll";

// All we need to do here is to ensure that the target has been loaded
// at some point, so if it's already here, don't try and reload it
if (!libraryServices.IsLibraryLoaded(libraryPath))
{
libraryServices.ImportLibrary(libraryPath);
Assert.IsTrue(LibraryLoaded);
}

var fgToCompare = libraryServices.GetFunctionGroups(libraryPath);
foreach (var funcGroup in fgToCompare)
{
foreach (var functionDescriptor in funcGroup.Functions)
{
if (functionDescriptor.IsVisibleInLibrary && !functionDescriptor.DisplayName.Contains("GetType"))
{
searchViewModel.Model.Add(new ZeroTouchSearchElement(functionDescriptor));
}
}
}

var searchString = "Days";
var nodes = searchViewModel.Search(searchString);
var foundNodes = nodes.Where(n => n.Class.Equals(searchString));
Assert.IsFalse(foundNodes.Any());

searchString = "Sunday";
nodes = searchViewModel.Search(searchString);
foundNodes = nodes.Where(n => n.Class.Equals(searchString));
Assert.IsFalse(foundNodes.Any());

searchString = "Tuesday";
nodes = searchViewModel.Search(searchString);
foundNodes = nodes.Where(n => n.Class.Equals(searchString));
Assert.IsFalse(foundNodes.Any());
}
}
}
25 changes: 25 additions & 0 deletions test/Engine/FFITarget/CodeCompletionClass.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Runtime.Serialization;
using System.Text;

namespace FFITarget
Expand Down Expand Up @@ -125,4 +126,28 @@ public class AnotherClassWithNameConflict
public static string PropertyF { get; set; }
}
}

[IsVisibleInDynamoLibrary(false)]
[Serializable]
public class TraceableId : ISerializable
Copy link
Contributor

Choose a reason for hiding this comment

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

@aparajit-pratap Can you explain how this test works, I can't see how adding a class with the tag verify stuff from my knowledge.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@QilongTang I need to think how I can test for interfaces not appearing in search and library.

Copy link
Contributor

Choose a reason for hiding this comment

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

@aparajit-pratap What do you think of either screen comparison or search results comparison?

{
public int IntID { get; set; }

public void GetObjectData(SerializationInfo info, StreamingContext context)
{
info.AddValue("intID", IntID, typeof(int));
}
}

[IsVisibleInDynamoLibrary(false)]
public enum Days
{
Sunday,
Monday,
Tuesday,
Wednesday,
Thursday,
Friday,
Saturday
}
}