[objc] Add ProcessedAssembly and uniqueness validation (#186)

* Detect any duplication based on the assembly (internal) name and also
  it's sanitized (safe) name. Otherwise generated code won't work;

* Avoid recomputing the name (or safe name) whenever possible;

* Adjust unit tests for some code movement that ease reuse;

docs/error.md

@@ -66,6 +66,12 @@ This might indicate a bug in the Embeddinator-4000; please file a bug report at
 
 The tool could not find the assembly `X` specified in the arguments.
 
+<h3><a name="EM0012"/>EM0012: The assembly name `X` is not unique</h3>
+
+More than one assembly supplied have the same, internal name and it would not be possible to distinguish between them at runtime.
+
+The most likely cause is that an assembly is specified more than once on the command-line arguments. However a renamed assembly still keeps it's original name and multiple copies cannot coexists.
+
 <h3><a name="EM0099"/>EM0099: Internal error *. Please file a bug report with a test case (https://github.com/mono/Embeddinator-4000/issues).</h3>
 
 This error message is reported when an internal consistency check in the Embeddinator-4000 fails.

objcgen/driver.cs

@@ -229,7 +229,7 @@ namespace Embeddinator {
 			g.Process (Assemblies);
 
 			Console.WriteLine ("Generating binding code...");
-			g.Generate (Assemblies);
+			g.Generate ();
 			g.Write (OutputDirectory);
 
 			var exe = typeof (Driver).Assembly;

objcgen/extensions.cs

@@ -1,10 +1,68 @@
 using System;
+using System.Text;
 
 using IKVM.Reflection;
 using Type = IKVM.Reflection.Type;
 
 namespace Embeddinator {
 
+	public static class StringExtensions {
+
+		public static string CamelCase (this string self)
+		{
+			if (self == null)
+				return null;
+			if (self.Length == 0)
+				return String.Empty;
+			return Char.ToLowerInvariant (self [0]) + self.Substring (1, self.Length - 1);
+		}
+
+		public static string PascalCase (this string self)
+		{
+			if (self == null)
+				return null;
+			if (self.Length == 0)
+				return String.Empty;
+			return Char.ToUpperInvariant (self [0]) + self.Substring (1, self.Length - 1);
+		}
+
+		public static string Sanitize (this string self)
+		{
+			if (self == null)
+				return null;
+
+			StringBuilder sb = null;
+
+			for (int i = 0; i < self.Length; i++) {
+				var ch = self [i];
+				switch (ch) {
+				case '.':
+				case '+':
+				case '/':
+				case '`':
+				case '@':
+				case '<':
+				case '>':
+				case '$':
+				case '-':
+				case ' ':
+					if (sb == null)
+						sb = new StringBuilder (self, 0, i, self.Length);
+					sb.Append ('_');
+					break;
+				default:
+					if (sb != null)
+						sb.Append (ch);
+					break;
+				}
+			}
+
+			if (sb != null)
+				return sb.ToString ();
+			return self;
+		}
+	}
+
 	public static class TypeExtensions {
 		
 		public static bool Is (this Type self, string @namespace, string name)

objcgen/generator.cs

@@ -8,20 +8,39 @@ namespace Embeddinator {
 
 	public class Generator {
 
-		public virtual void Process (IEnumerable<Assembly> assemblies)
+		protected List<ProcessedAssembly> assemblies = new List<ProcessedAssembly> ();
+
+		// uniqueness checks
+		HashSet<string> assembly_name = new HashSet<string> ();
+		HashSet<string> assembly_safename = new HashSet<string> ();
+
+		public virtual void Process (IEnumerable<Assembly> input)
 		{
 		}
 
-		public virtual void Generate (IEnumerable<Assembly> assemblies)
+		public bool AddIfUnique (ProcessedAssembly assembly)
+		{
+			if (assembly_name.Contains (assembly.Name))
+				return false;
+			if (assembly_safename.Contains (assembly.SafeName))
+				return false;
+
+			assemblies.Add (assembly);
+			assembly_name.Add (assembly.Name);
+			assembly_safename.Add (assembly.SafeName);
+			return true;
+		}
+
+		public virtual void Generate ()
 		{
 			foreach (var a in assemblies) {
 				Generate (a);
 			}
 		}
 
-		protected virtual void Generate (Assembly a)
+		protected virtual void Generate (ProcessedAssembly a)
 		{
-			foreach (var t in a.GetTypes ()) {
+			foreach (var t in a.Assembly.GetTypes ()) {
 				if (!t.IsPublic)
 					continue;
 				Generate (t);

objcgen/methodhelper.cs

@@ -16,7 +16,7 @@ namespace ObjC {
 			this.implementation = implementation;
 		}
 
-		public string AssemblyName { get; set; }
+		public string AssemblySafeName { get; set; }
 
 		public bool IsConstructor { get; set; }
 		public bool IsExtension { get; set; }
@@ -62,7 +62,7 @@ namespace ObjC {
 			implementation.WriteLine ("if (!__method) {");
 			implementation.WriteLineUnindented ("#if TOKENLOOKUP");
 			implementation.Indent++;
-			implementation.WriteLine ($"__method = mono_get_method (__{ObjCGenerator.SanitizeName (AssemblyName)}_image, 0x{MetadataToken:X8}, {ObjCTypeName}_class);");
+			implementation.WriteLine ($"__method = mono_get_method (__{AssemblySafeName}_image, 0x{MetadataToken:X8}, {ObjCTypeName}_class);");
 			implementation.WriteLineUnindented ("#else");
 			implementation.WriteLine ($"const char __method_name [] = \"{ManagedTypeName}:{MonoSignature}\";");
 			implementation.WriteLine ($"__method = mono_embeddinator_lookup_method (__method_name, {ObjCTypeName}_class);");

objcgen/objcgenerator-helpers.cs

@@ -36,7 +36,7 @@ namespace ObjC {
 					if (n == 0) {
 						bool isPropertyMethod = method.IsSpecialName && (method.Name.StartsWith ("get") || method.Name.StartsWith ("set"));
 						if (method.IsConstructor || !method.IsSpecialName || (useTypeNames && isPropertyMethod))
-							objc.Append (PascalCase (paramName));
+							objc.Append (paramName.PascalCase ());
 					} else
 						objc.Append (paramName.ToLowerInvariant ());
 				}

objcgen/objcgenerator-processor.cs

@@ -203,6 +203,7 @@ namespace ObjC {
 
 		List<Type> enums = new List<Type> ();
 		List<Type> types = new List<Type> ();
+
 		Dictionary<Type, List<ProcessedConstructor>> ctors = new Dictionary<Type, List<ProcessedConstructor>> ();
 		Dictionary<Type, List<ProcessedMethod>> methods = new Dictionary<Type, List<ProcessedMethod>> ();
 		Dictionary<Type, List<ProcessedProperty>> properties = new Dictionary<Type, List<ProcessedProperty>> ();
@@ -214,9 +215,14 @@ namespace ObjC {
 		bool implement_system_icomparable_t;
 		bool extension_type;
 
-		public override void Process (IEnumerable<Assembly> assemblies)
+		public override void Process (IEnumerable<Assembly> input)
 		{
-			foreach (var a in assemblies) {
+			foreach (var a in input) {
+				var pa = new ProcessedAssembly (a);
+				// ignoring/warning one is not an option as they could be different (e.g. different builds/versions)
+				if (!AddIfUnique (pa))
+					throw ErrorHelper.CreateError (12, $"The assembly name `{pa.Name}` is not unique");
+
 				foreach (var t in GetTypes (a)) {
 					if (t.IsEnum) {
 						enums.Add (t);

objcgen/objcgenerator.cs

@@ -16,7 +16,7 @@ namespace ObjC {
 		SourceWriter headers = new SourceWriter ();
 		SourceWriter implementation = new SourceWriter ();
 
-		public override void Generate (IEnumerable<Assembly> assemblies)
+		public override void Generate ()
 		{
 			headers.WriteLine ("#include \"embeddinator.h\"");
 			headers.WriteLine ("#import <Foundation/Foundation.h>");
@@ -45,7 +45,7 @@ namespace ObjC {
 			implementation.WriteLine ();
 
 			foreach (var a in assemblies)
-				implementation.WriteLine ($"MonoImage* __{SanitizeName (a.GetName ().Name)}_image;");
+				implementation.WriteLine ($"MonoImage* __{a.SafeName}_image;");
 			implementation.WriteLine ();
 
 			foreach (var t in types)
@@ -64,16 +64,16 @@ namespace ObjC {
 			implementation.WriteLine ("}");
 			implementation.WriteLine ();
 
-			base.Generate (assemblies);
+			base.Generate ();
 
 			headers.WriteLine ("NS_ASSUME_NONNULL_END");
 			headers.WriteLine ();
 		}
 
-		protected override void Generate (Assembly a)
+		protected override void Generate (ProcessedAssembly a)
 		{
-			var originalName = a.GetName ().Name;
+			var originalName = a.Name;
-			var name = SanitizeName (originalName);
+			var name = a.SafeName;
 			implementation.WriteLine ($"static void __lookup_assembly_{name} ()");
 			implementation.WriteLine ("{");
 			implementation.Indent++;
@@ -130,7 +130,7 @@ namespace ObjC {
 			implementation.WriteLine ();
 
 			foreach (var mi in methods) {
-				ImplementMethod (mi, CamelCase (mi.Name), true);
+				ImplementMethod (mi, mi.Name.CamelCase (), true);
 			}
 
 			headers.WriteLine ("@end");
@@ -181,7 +181,7 @@ namespace ObjC {
 
 		protected override void Generate (Type t)
 		{
-			var aname = SanitizeName (t.Assembly.GetName ().Name);
+			var aname = t.Assembly.GetName ().Name.Sanitize ();
 			var static_type = t.IsSealed && t.IsAbstract;
 
 			var managed_name = GetObjCName (t);
@@ -233,7 +233,7 @@ namespace ObjC {
 						GetSignatures ("initWith", ctor.Constructor.Name, ctor.Constructor, parameters, ctor.FallBackToTypeName, false, out name, out signature);
 
 					var builder = new MethodHelper (headers, implementation) {
-						AssemblyName = aname,
+						AssemblySafeName = aname,
 						ReturnType = "nullable instancetype",
 						ManagedTypeName = t.FullName,
 						MetadataToken = ctor.Constructor.MetadataToken,
@@ -315,7 +315,7 @@ namespace ObjC {
 				var pt = m.GetParameters () [0].ParameterType;
 				var builder = new ComparableHelper (headers, implementation) {
 					ObjCSignature = $"compare:({managed_name} * _Nullable)other",
-					AssemblyName = aname,
+					AssemblySafeName = aname,
 					MetadataToken = m.MetadataToken,
 					ObjCTypeName = managed_name,
 					ManagedTypeName = t.FullName,
@@ -328,7 +328,7 @@ namespace ObjC {
 			if (equals.TryGetValue (t, out m)) {
 				var builder = new EqualsHelper (headers, implementation) {
 					ObjCSignature = "isEqual:(id)other",
-					AssemblyName = aname,
+					AssemblySafeName = aname,
 					MetadataToken = m.MetadataToken,
 					ObjCTypeName = managed_name,
 					ManagedTypeName = t.FullName,
@@ -344,7 +344,7 @@ namespace ObjC {
 			if (hashes.TryGetValue (t, out m)) {
 				var builder = new HashHelper (headers, implementation) {
 					ObjCSignature = "hash",
-					AssemblyName = aname,
+					AssemblySafeName = aname,
 					MetadataToken = m.MetadataToken,
 					ObjCTypeName = managed_name,
 					ManagedTypeName = t.FullName,
@@ -424,7 +424,7 @@ namespace ObjC {
 			if (getter == null && setter != null)
 				throw new EmbeddinatorException (99, "Internal error `setter only`. Please file a bug report with a test case (https://github.com/mono/Embeddinator-4000/issues");
 
-			var name = CamelCase (pi.Name);
+			var name = pi.Name.CamelCase ();
 
 			headers.Write ("@property (nonatomic");
 			if (getter.IsStatic)
@@ -463,7 +463,7 @@ namespace ObjC {
 			if (bound)
 				field_type += " *";
 
-			var name = CamelCase (fi.Name);
+			var name = fi.Name.CamelCase ();
 			headers.WriteLine ($") {field_type} {name};");
 
 			// it's similar, but different from implementing a method
@@ -473,14 +473,13 @@ namespace ObjC {
 			var return_type = GetReturnType (type, fi.FieldType);
 
 			implementation.Write (fi.IsStatic ? '+' : '-');
-			implementation.WriteLine ($" ({return_type}) {CamelCase (fi.Name)}");
+			implementation.WriteLine ($" ({return_type}) {name}");
 			implementation.WriteLine ("{");
 			implementation.Indent++;
 			implementation.WriteLine ("static MonoClassField* __field = nil;");
 			implementation.WriteLine ("if (!__field) {");
 			implementation.Indent++;
 			implementation.WriteLineUnindented ("#if TOKENLOOKUP");
-			var aname = type.Assembly.GetName ().Name;
 			implementation.WriteLine ($"__field = mono_class_get_field ({managed_type_name}_class, 0x{fi.MetadataToken:X8});");
 			implementation.WriteLineUnindented ("#else");
 			implementation.WriteLine ($"const char __field_name [] = \"{fi.Name}\";");
@@ -514,7 +513,6 @@ namespace ObjC {
 			implementation.WriteLine ("if (!__field) {");
 			implementation.Indent++;
 			implementation.WriteLineUnindented ("#if TOKENLOOKUP");
-			aname = type.Assembly.GetName ().Name;
 			implementation.WriteLine ($"__field = mono_class_get_field ({managed_type_name}_class, 0x{fi.MetadataToken:X8});");
 			implementation.WriteLineUnindented ("#else");
 			implementation.WriteLine ($"const char __field_name [] = \"{fi.Name}\";");
@@ -562,7 +560,7 @@ namespace ObjC {
 			GetSignatures (name, managed_name, (MemberInfo)pi ?? info, parametersInfo, useTypeNames, isExtension, out objcsig, out monosig);
 
 			var builder = new MethodHelper (headers, implementation) {
-				AssemblyName = type.Assembly.GetName ().Name,
+				AssemblySafeName = type.Assembly.GetName ().Name.Sanitize (),
 				IsStatic = info.IsStatic,
 				IsExtension = isExtension,
 				ReturnType = GetReturnType (type, info.ReturnType),
@@ -619,11 +617,9 @@ namespace ObjC {
 			headers.WriteLine ("/** This is an helper method that inlines the following default values:");
 			foreach (var p in parameters) {
 				if (arguments.Length == 0) {
-					//if (mi == null)
+					arguments.Append (p.Name.PascalCase ()).Append (':');
-						//arguments.Append ("With");
-					arguments.Append (PascalCase (p.Name)).Append (':');
 				} else
-					arguments.Append (' ').Append (CamelCase (p.Name)).Append (':');
+					arguments.Append (' ').Append (p.Name.CamelCase ()).Append (':');
 				if (p.Position >= start && p.HasDefaultValue) {
 					var raw = FormatRawValue (p.ParameterType, p.RawDefaultValue);
 					headers.WriteLine ($" *     ({GetTypeName (p.ParameterType)}) {p.Name} = {raw};");
@@ -640,7 +636,7 @@ namespace ObjC {
 			if (mi == null)
 				name = start == 0 ? "init" : "initWith";
 			else
-				name = CamelCase (mb.Name);
+				name = mb.Name.CamelCase ();
 			
 			GetSignatures (name, mb.Name, mb, plist.ToArray (), false, false, out objcsig, out monosig);
 			var type = mb.DeclaringType;
@@ -851,57 +847,6 @@ namespace ObjC {
 			}
 		}
 
-		public static string CamelCase (string s)
-		{
-			if (s == null)
-				return null;
-			if (s.Length == 0)
-				return String.Empty;
-			return Char.ToLowerInvariant (s [0]) + s.Substring (1, s.Length - 1);
-		}
-
-		public static string PascalCase (string s)
-		{
-			if (s == null)
-				return null;
-			if (s.Length == 0)
-				return String.Empty;
-			return Char.ToUpperInvariant (s [0]) + s.Substring (1, s.Length - 1);
-		}
-
-		public static string SanitizeName (string name)
-		{
-			StringBuilder sb = null;
-
-			for (int i = 0; i < name.Length; i++) {
-				var ch = name [i];
-				switch (ch) {
-				case '.':
-				case '+':
-				case '/':
-				case '`':
-				case '@':
-				case '<':
-				case '>':
-				case '$':
-				case '-':
-				case ' ':
-					if (sb == null)
-						sb = new StringBuilder (name, 0, i, name.Length);
-					sb.Append ('_');
-					break;
-				default:
-					if (sb != null)
-						sb.Append (ch);
-					break;
-				}
-			}
-
-			if (sb != null)
-				return sb.ToString ();
-			return name;
-		}
-
 		public static string FormatRawValue (Type t, object o)
 		{
 			if (o == null)

objcgen/processedtypes.cs

@@ -5,20 +5,35 @@ using System.Linq;
 using IKVM.Reflection;
 using Type = IKVM.Reflection.Type;
 
-namespace ObjC {
+namespace Embeddinator {
 
 	// While processing user assemblies, we may come across conditions that will affect
 	// final code generation that we need to pass to the generation pass
 
-	public abstract class ProcessedBase {
+	public abstract class ProcessedMemberBase {
 		public bool FallBackToTypeName { get; set; }
 	}
 
-	public class ProcessedMethod : ProcessedBase {
+	public class ProcessedAssembly {
+
+		public Assembly Assembly { get; private set; }
+
+		public string Name { get; private set; }
+		public string SafeName { get; private set; }
+
+		public ProcessedAssembly (Assembly assembly)
+		{
+			Assembly = assembly;
+			Name = assembly.GetName ().Name;
+			SafeName = Name.Sanitize ();
+		}
+	}
+
+	public class ProcessedMethod : ProcessedMemberBase {
 		public MethodInfo Method { get; private set; }
 		public bool IsOperator { get; set; }
 
-		public string BaseName => IsOperator ? ObjCGenerator.CamelCase (Method.Name.Substring (3)) : ObjCGenerator.CamelCase (Method.Name);
+		public string BaseName => IsOperator ? Method.Name.Substring (3).CamelCase () : Method.Name.CamelCase ();
 
 		public ProcessedMethod (MethodInfo method)
 		{
@@ -26,7 +41,7 @@ namespace ObjC {
 		}
 	}
 
-	public class ProcessedProperty: ProcessedBase {
+	public class ProcessedProperty: ProcessedMemberBase {
 		public PropertyInfo Property { get; private set; }
 
 		public ProcessedProperty (PropertyInfo property)
@@ -35,7 +50,7 @@ namespace ObjC {
 		}
 	}
 
-	public class ProcessedConstructor : ProcessedBase {
+	public class ProcessedConstructor : ProcessedMemberBase {
 		public ConstructorInfo Constructor { get; private set; }
 
 		public ProcessedConstructor (ConstructorInfo constructor)
@@ -44,7 +59,7 @@ namespace ObjC {
 		}
 	}
 
-	public class ProcessedFieldInfo : ProcessedBase {
+	public class ProcessedFieldInfo : ProcessedMemberBase {
 		public FieldInfo Field { get; private set; }
 
 		public ProcessedFieldInfo (FieldInfo field)

tests/objcgentest/EmbedderTest.cs

@@ -62,6 +62,21 @@ namespace DriverTest
 			Assert.AreEqual (0, Driver.Main2 ("--platform", platform.ToString (), "--abi", "x86_64", "-c", dll, "-o", tmpdir), "build");
 		}
 
+		[Test]
+		public void DuplicateAssemblyName ()
+		{
+			var platform = Platform.macOS;
+			var dll = CompileLibrary (platform, libraryName: "dupe");
+			var tmpdir = Xamarin.Cache.CreateTemporaryDirectory ();
+			try {
+				Driver.Main2 ("--platform", platform.ToString (), "--abi", "x86_64", dll, dll, "-o", tmpdir);
+			}
+			catch (EmbeddinatorException ee) {
+				Assert.True (ee.Error, "Error");
+				Assert.That (ee.Code, Is.EqualTo (12), "Code");
+			}
+		}
+
 		string CompileLibrary (Platform platform, string code = null, string libraryName = null)
 		{
 			int exitCode;

tests/objcgentest/ObjCGeneratorTest.cs

@@ -14,24 +14,6 @@ namespace ObjCGeneratorTest {
 
 		public static Assembly mscorlib { get; } = Universe.Load ("mscorlib.dll");
 
-		[Test]
-		public void CamelCase ()
-		{
-			Assert.Null (ObjCGenerator.CamelCase (null), "null");
-			Assert.That (ObjCGenerator.CamelCase (String.Empty), Is.EqualTo (""), "length == 0");
-			Assert.That (ObjCGenerator.CamelCase ("S"), Is.EqualTo ("s"), "length == 1");
-			Assert.That (ObjCGenerator.CamelCase ("TU"), Is.EqualTo ("tU"), "length == 2");
-		}
-
-		[Test]
-		public void PascalCase ()
-		{
-			Assert.Null (ObjCGenerator.PascalCase (null), "null");
-			Assert.That (ObjCGenerator.PascalCase (String.Empty), Is.EqualTo (""), "length == 0");
-			Assert.That (ObjCGenerator.PascalCase ("s"), Is.EqualTo ("S"), "length == 1");
-			Assert.That (ObjCGenerator.PascalCase ("tu"), Is.EqualTo ("Tu"), "length == 2");
-		}
-
 		[Test]
 		public void TypeMatch ()
 		{

tests/objcgentest/StringExtensionsTest.cs

@@ -0,0 +1,29 @@
+using NUnit.Framework;
+using System;
+
+using Embeddinator;
+
+namespace ObjCGeneratorTest {
+
+	[TestFixture]
+	public class StringExtensionsTest {
+
+		[Test]
+		public void CamelCase ()
+		{
+			Assert.Null ((null as string).CamelCase (), "null");
+			Assert.That (String.Empty.CamelCase (), Is.EqualTo (""), "length == 0");
+			Assert.That ("S".CamelCase (), Is.EqualTo ("s"), "length == 1");
+			Assert.That ("TU".CamelCase (), Is.EqualTo ("tU"), "length == 2");
+		}
+
+		[Test]
+		public void PascalCase ()
+		{
+			Assert.Null ((null as string).PascalCase (), "null");
+			Assert.That (String.Empty.PascalCase (), Is.EqualTo (""), "length == 0");
+			Assert.That ("s".PascalCase (), Is.EqualTo ("S"), "length == 1");
+			Assert.That ("tu".PascalCase (), Is.EqualTo ("Tu"), "length == 2");
+		}
+	}
+}

tests/objcgentest/TypeExtensionsTest.cs

@@ -9,7 +9,7 @@ using Type = IKVM.Reflection.Type;
 namespace ObjCGeneratorTest {
 
 	[TestFixture]
-	public class ExtensionsTest {
+	public class TypeExtensionsTest {
 
 		public static Universe Universe { get; } = new Universe (UniverseOptions.None);
 

tests/objcgentest/objcgentest.csproj

@@ -36,7 +36,8 @@
     <Compile Include="Asserts.cs" />
     <Compile Include="EmbedderTest.cs" />
     <Compile Include="Cache.cs" />
-    <Compile Include="ExtensionsTest.cs" />
+    <Compile Include="TypeExtensionsTest.cs" />
+    <Compile Include="StringExtensionsTest.cs" />
   </ItemGroup>
   <ItemGroup>
     <None Include="packages.config" />