﻿[generator] Warn if type name matches enclosing namespace name.

jpobst · jpobst · commit 2af18489b650 · 2022-07-14T13:16:06.000-05:00
diff --git a/src/Java.Interop.Localization/Resources.Designer.cs b/src/Java.Interop.Localization/Resources.Designer.cs
diff --git a/src/Java.Interop.Localization/Resources.resx b/src/Java.Interop.Localization/Resources.resx
@@ -201,6 +201,10 @@ In this message, the term "constants" refers to class or interface members that
 {1} - .NET field name.
 {2} - Java type.</comment>
   </data>
+  <data name="Generator_BG8403" xml:space="preserve">
+    <value>Type '{0}' has a type name which matches the enclosing namespace name. See https://aka.ms/BG8403 for more information.</value>
+    <comment>{0} - Java type.</comment>
+  </data>
   <data name="Generator_BG8500" xml:space="preserve">
     <value>Unexpected child element of '&lt;interface&gt;': '{0}'.</value>
     <comment>{0} - XML element name.
diff --git a/src/Java.Interop.Localization/xlf/Resources.cs.xlf b/src/Java.Interop.Localization/xlf/Resources.cs.xlf
@@ -104,6 +104,11 @@ In this message, the term "constants" refers to class or interface members that
 {1} - .NET field name.
 {2} - Java type.</note>
       </trans-unit>
+      <trans-unit id="Generator_BG8403">
+        <source>Type '{0}' has a type name which matches the enclosing namespace name. See https://aka.ms/BG8403 for more information.</source>
+        <target state="new">Type '{0}' has a type name which matches the enclosing namespace name. See https://aka.ms/BG8403 for more information.</target>
+        <note>{0} - Java type.</note>
+      </trans-unit>
       <trans-unit id="Generator_BG8500">
         <source>Unexpected child element of '&lt;interface&gt;': '{0}'.</source>
         <target state="new">Unexpected child element of '&lt;interface&gt;': '{0}'.</target>
diff --git a/src/Java.Interop.Localization/xlf/Resources.de.xlf b/src/Java.Interop.Localization/xlf/Resources.de.xlf
@@ -104,6 +104,11 @@ In this message, the term "constants" refers to class or interface members that
 {1} - .NET field name.
 {2} - Java type.</note>
       </trans-unit>
+      <trans-unit id="Generator_BG8403">
+        <source>Type '{0}' has a type name which matches the enclosing namespace name. See https://aka.ms/BG8403 for more information.</source>
+        <target state="new">Type '{0}' has a type name which matches the enclosing namespace name. See https://aka.ms/BG8403 for more information.</target>
+        <note>{0} - Java type.</note>
+      </trans-unit>
       <trans-unit id="Generator_BG8500">
         <source>Unexpected child element of '&lt;interface&gt;': '{0}'.</source>
         <target state="new">Unexpected child element of '&lt;interface&gt;': '{0}'.</target>
diff --git a/src/Java.Interop.Localization/xlf/Resources.es.xlf b/src/Java.Interop.Localization/xlf/Resources.es.xlf
@@ -104,6 +104,11 @@ In this message, the term "constants" refers to class or interface members that
 {1} - .NET field name.
 {2} - Java type.</note>
       </trans-unit>
+      <trans-unit id="Generator_BG8403">
+        <source>Type '{0}' has a type name which matches the enclosing namespace name. See https://aka.ms/BG8403 for more information.</source>
+        <target state="new">Type '{0}' has a type name which matches the enclosing namespace name. See https://aka.ms/BG8403 for more information.</target>
+        <note>{0} - Java type.</note>
+      </trans-unit>
       <trans-unit id="Generator_BG8500">
         <source>Unexpected child element of '&lt;interface&gt;': '{0}'.</source>
         <target state="new">Unexpected child element of '&lt;interface&gt;': '{0}'.</target>
diff --git a/src/Java.Interop.Localization/xlf/Resources.fr.xlf b/src/Java.Interop.Localization/xlf/Resources.fr.xlf
@@ -104,6 +104,11 @@ In this message, the term "constants" refers to class or interface members that
 {1} - .NET field name.
 {2} - Java type.</note>
       </trans-unit>
+      <trans-unit id="Generator_BG8403">
+        <source>Type '{0}' has a type name which matches the enclosing namespace name. See https://aka.ms/BG8403 for more information.</source>
+        <target state="new">Type '{0}' has a type name which matches the enclosing namespace name. See https://aka.ms/BG8403 for more information.</target>
+        <note>{0} - Java type.</note>
+      </trans-unit>
       <trans-unit id="Generator_BG8500">
         <source>Unexpected child element of '&lt;interface&gt;': '{0}'.</source>
         <target state="new">Unexpected child element of '&lt;interface&gt;': '{0}'.</target>
diff --git a/src/Java.Interop.Localization/xlf/Resources.it.xlf b/src/Java.Interop.Localization/xlf/Resources.it.xlf
@@ -104,6 +104,11 @@ In this message, the term "constants" refers to class or interface members that
 {1} - .NET field name.
 {2} - Java type.</note>
       </trans-unit>
+      <trans-unit id="Generator_BG8403">
+        <source>Type '{0}' has a type name which matches the enclosing namespace name. See https://aka.ms/BG8403 for more information.</source>
+        <target state="new">Type '{0}' has a type name which matches the enclosing namespace name. See https://aka.ms/BG8403 for more information.</target>
+        <note>{0} - Java type.</note>
+      </trans-unit>
       <trans-unit id="Generator_BG8500">
         <source>Unexpected child element of '&lt;interface&gt;': '{0}'.</source>
         <target state="new">Unexpected child element of '&lt;interface&gt;': '{0}'.</target>
diff --git a/src/Java.Interop.Localization/xlf/Resources.ja.xlf b/src/Java.Interop.Localization/xlf/Resources.ja.xlf
@@ -104,6 +104,11 @@ In this message, the term "constants" refers to class or interface members that
 {1} - .NET field name.
 {2} - Java type.</note>
       </trans-unit>
+      <trans-unit id="Generator_BG8403">
+        <source>Type '{0}' has a type name which matches the enclosing namespace name. See https://aka.ms/BG8403 for more information.</source>
+        <target state="new">Type '{0}' has a type name which matches the enclosing namespace name. See https://aka.ms/BG8403 for more information.</target>
+        <note>{0} - Java type.</note>
+      </trans-unit>
       <trans-unit id="Generator_BG8500">
         <source>Unexpected child element of '&lt;interface&gt;': '{0}'.</source>
         <target state="new">Unexpected child element of '&lt;interface&gt;': '{0}'.</target>
diff --git a/src/Java.Interop.Localization/xlf/Resources.ko.xlf b/src/Java.Interop.Localization/xlf/Resources.ko.xlf
@@ -104,6 +104,11 @@ In this message, the term "constants" refers to class or interface members that
 {1} - .NET field name.
 {2} - Java type.</note>
       </trans-unit>
+      <trans-unit id="Generator_BG8403">
+        <source>Type '{0}' has a type name which matches the enclosing namespace name. See https://aka.ms/BG8403 for more information.</source>
+        <target state="new">Type '{0}' has a type name which matches the enclosing namespace name. See https://aka.ms/BG8403 for more information.</target>
+        <note>{0} - Java type.</note>
+      </trans-unit>
       <trans-unit id="Generator_BG8500">
         <source>Unexpected child element of '&lt;interface&gt;': '{0}'.</source>
         <target state="new">Unexpected child element of '&lt;interface&gt;': '{0}'.</target>
diff --git a/src/Java.Interop.Localization/xlf/Resources.pl.xlf b/src/Java.Interop.Localization/xlf/Resources.pl.xlf
@@ -104,6 +104,11 @@ In this message, the term "constants" refers to class or interface members that
 {1} - .NET field name.
 {2} - Java type.</note>
       </trans-unit>
+      <trans-unit id="Generator_BG8403">
+        <source>Type '{0}' has a type name which matches the enclosing namespace name. See https://aka.ms/BG8403 for more information.</source>
+        <target state="new">Type '{0}' has a type name which matches the enclosing namespace name. See https://aka.ms/BG8403 for more information.</target>
+        <note>{0} - Java type.</note>
+      </trans-unit>
       <trans-unit id="Generator_BG8500">
         <source>Unexpected child element of '&lt;interface&gt;': '{0}'.</source>
         <target state="new">Unexpected child element of '&lt;interface&gt;': '{0}'.</target>
diff --git a/src/Java.Interop.Localization/xlf/Resources.pt-BR.xlf b/src/Java.Interop.Localization/xlf/Resources.pt-BR.xlf
@@ -104,6 +104,11 @@ In this message, the term "constants" refers to class or interface members that
 {1} - .NET field name.
 {2} - Java type.</note>
       </trans-unit>
+      <trans-unit id="Generator_BG8403">
+        <source>Type '{0}' has a type name which matches the enclosing namespace name. See https://aka.ms/BG8403 for more information.</source>
+        <target state="new">Type '{0}' has a type name which matches the enclosing namespace name. See https://aka.ms/BG8403 for more information.</target>
+        <note>{0} - Java type.</note>
+      </trans-unit>
       <trans-unit id="Generator_BG8500">
         <source>Unexpected child element of '&lt;interface&gt;': '{0}'.</source>
         <target state="new">Unexpected child element of '&lt;interface&gt;': '{0}'.</target>
diff --git a/src/Java.Interop.Localization/xlf/Resources.ru.xlf b/src/Java.Interop.Localization/xlf/Resources.ru.xlf
@@ -104,6 +104,11 @@ In this message, the term "constants" refers to class or interface members that
 {1} - .NET field name.
 {2} - Java type.</note>
       </trans-unit>
+      <trans-unit id="Generator_BG8403">
+        <source>Type '{0}' has a type name which matches the enclosing namespace name. See https://aka.ms/BG8403 for more information.</source>
+        <target state="new">Type '{0}' has a type name which matches the enclosing namespace name. See https://aka.ms/BG8403 for more information.</target>
+        <note>{0} - Java type.</note>
+      </trans-unit>
       <trans-unit id="Generator_BG8500">
         <source>Unexpected child element of '&lt;interface&gt;': '{0}'.</source>
         <target state="new">Unexpected child element of '&lt;interface&gt;': '{0}'.</target>
diff --git a/src/Java.Interop.Localization/xlf/Resources.tr.xlf b/src/Java.Interop.Localization/xlf/Resources.tr.xlf
@@ -104,6 +104,11 @@ In this message, the term "constants" refers to class or interface members that
 {1} - .NET field name.
 {2} - Java type.</note>
       </trans-unit>
+      <trans-unit id="Generator_BG8403">
+        <source>Type '{0}' has a type name which matches the enclosing namespace name. See https://aka.ms/BG8403 for more information.</source>
+        <target state="new">Type '{0}' has a type name which matches the enclosing namespace name. See https://aka.ms/BG8403 for more information.</target>
+        <note>{0} - Java type.</note>
+      </trans-unit>
       <trans-unit id="Generator_BG8500">
         <source>Unexpected child element of '&lt;interface&gt;': '{0}'.</source>
         <target state="new">Unexpected child element of '&lt;interface&gt;': '{0}'.</target>
diff --git a/src/Java.Interop.Localization/xlf/Resources.zh-Hans.xlf b/src/Java.Interop.Localization/xlf/Resources.zh-Hans.xlf
@@ -104,6 +104,11 @@ In this message, the term "constants" refers to class or interface members that
 {1} - .NET field name.
 {2} - Java type.</note>
       </trans-unit>
+      <trans-unit id="Generator_BG8403">
+        <source>Type '{0}' has a type name which matches the enclosing namespace name. See https://aka.ms/BG8403 for more information.</source>
+        <target state="new">Type '{0}' has a type name which matches the enclosing namespace name. See https://aka.ms/BG8403 for more information.</target>
+        <note>{0} - Java type.</note>
+      </trans-unit>
       <trans-unit id="Generator_BG8500">
         <source>Unexpected child element of '&lt;interface&gt;': '{0}'.</source>
         <target state="new">Unexpected child element of '&lt;interface&gt;': '{0}'.</target>
diff --git a/src/Java.Interop.Localization/xlf/Resources.zh-Hant.xlf b/src/Java.Interop.Localization/xlf/Resources.zh-Hant.xlf
@@ -104,6 +104,11 @@ In this message, the term "constants" refers to class or interface members that
 {1} - .NET field name.
 {2} - Java type.</note>
       </trans-unit>
+      <trans-unit id="Generator_BG8403">
+        <source>Type '{0}' has a type name which matches the enclosing namespace name. See https://aka.ms/BG8403 for more information.</source>
+        <target state="new">Type '{0}' has a type name which matches the enclosing namespace name. See https://aka.ms/BG8403 for more information.</target>
+        <note>{0} - Java type.</note>
+      </trans-unit>
       <trans-unit id="Generator_BG8500">
         <source>Unexpected child element of '&lt;interface&gt;': '{0}'.</source>
         <target state="new">Unexpected child element of '&lt;interface&gt;': '{0}'.</target>
diff --git a/src/Java.Interop.Tools.Generator/Utilities/Report.cs b/src/Java.Interop.Tools.Generator/Utilities/Report.cs
@@ -7,6 +7,7 @@ namespace Java.Interop.Tools.Generator
 	public class Report
 	{
 		public static int? Verbosity { get; set; }
+		public static Action<string>? OutputDelegate { get; set; }
 
 		public class LocalizedMessage
 		{
@@ -43,6 +44,7 @@ public LocalizedMessage (int code, string value)
 		public static LocalizedMessage WarningFieldNameCollision_Method => new LocalizedMessage (0x8401, Localization.Resources.Generator_BG8401_Method);
 		public static LocalizedMessage WarningFieldNameCollision_NestedType => new LocalizedMessage (0x8401, Localization.Resources.Generator_BG8401_NestedType);
 		public static LocalizedMessage WarningDuplicateField => new LocalizedMessage (0x8402, Localization.Resources.Generator_BG8402);
+		public static LocalizedMessage WarningTypeNameMatchesEnclosingNamespace => new LocalizedMessage (0x8403, Localization.Resources.Generator_BG8403);
 		public static LocalizedMessage WarningUnexpectedInterfaceChild => new LocalizedMessage (0x8500, Localization.Resources.Generator_BG8500);
 		public static LocalizedMessage WarningEmptyEventName => new LocalizedMessage (0x8501, Localization.Resources.Generator_BG8501);
 		public static LocalizedMessage WarningInvalidDueToInterfaces => new LocalizedMessage (0x8502, Localization.Resources.Generator_BG8502);
@@ -95,7 +97,7 @@ public static void LogCodedError (LocalizedMessage message, XNode? node, params
 
 		public static void LogCodedError (LocalizedMessage message, string? sourceFile, int line, int column, params string? [] args)
 		{
-			Console.Error.WriteLine (Format (true, message.Code, sourceFile, line, column, message.Value, args));
+			WriteOutput (Format (true, message.Code, sourceFile, line, column, message.Value, args));
 		}
 
 		public static void LogCodedWarning (int verbosity, LocalizedMessage message, params string? [] args)
@@ -120,17 +122,17 @@ public static void LogCodedWarning (int verbosity, LocalizedMessage message, Exc
 				return;
 
 			var supp = innerException != null ? "  For details, see verbose output." : null;
-			Console.Error.WriteLine (Format (false, message.Code, sourceFile, line, column, message.Value, args) + supp);
+			WriteOutput (Format (false, message.Code, sourceFile, line, column, message.Value, args) + supp);
 
 			if (innerException != null)
-				Console.Error.WriteLine (innerException);
+				WriteOutput (innerException.ToString ());
 		}
 		
 		public static void Verbose (int verbosity, string format, params object?[] args)
 		{
 			if (verbosity > (Verbosity ?? 0))
 				return;
-			Console.Error.WriteLine (format, args);
+			WriteOutput (format, args);
 		}
 
 		public static string FormatCodedMessage (bool error, LocalizedMessage message, params object? [] args)
@@ -169,6 +171,18 @@ public static string Format (bool error, int errorCode, string? sourceFile, int
 
 			return (file, pos?.LineNumber ?? -1, pos?.LinePosition ?? -1);
 		}
+
+		static void WriteOutput (string format, params object?[] args)
+		{
+			// Write to overridden output if requested
+			if (OutputDelegate != null) {
+				OutputDelegate (string.Format (format, args));
+				return;
+			}
+
+			// Write to Console.Error
+			Console.Error.WriteLine (format, args);
+		}
 	}
 
 	/// <summary>
diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs b/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs
@@ -1,7 +1,9 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Text;
 using generator.SourceWriters;
+using Java.Interop.Tools.Generator;
 using MonoDroid.Generation;
 using NUnit.Framework;
 using Xamarin.Android.Binder;
@@ -526,6 +528,46 @@ public void ObsoleteBoundMethodAbstractDeclaration ()
 
 			// Ensure [Obsolete] was written
 			Assert.True (writer.ToString ().Contains ("[Obsolete (@\"This is so old!\")]"), writer.ToString ());
+
+		[Test]
+		[NonParallelizable]	// We are setting a static property on Report
+		public void WarnIfTypeNameMatchesNamespace ()
+		{
+			var @class = new TestClass ("Object", "java.myclass.MyClass");
+			var sb = new StringBuilder ();
+
+			var write_output = new Action<string> ((s) => { sb.AppendLine (s); });
+			Report.OutputDelegate = write_output;
+
+			generator.Context.ContextTypes.Push (@class);
+			generator.WriteType (@class, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
+			generator.Context.ContextTypes.Pop ();
+
+			Report.OutputDelegate = null;
+
+			// Ensure the warning was raised
+			Assert.True (sb.ToString ().Contains ("warning BG8403"));
+		}
+
+		[Test]
+		[NonParallelizable]     // We are setting a static property on Report
+		public void DontWarnIfNestedTypeNameMatchesNamespace ()
+		{
+			var @class = new TestClass ("Object", "java.myclass.MyParentClass");
+			@class.NestedTypes.Add (new TestClass ("Object", "java.myclass.MyParentClass.MyClass"));
+			var sb = new StringBuilder ();
+
+			var write_output = new Action<string> ((s) => { sb.AppendLine (s); });
+			Report.OutputDelegate = write_output;
+
+			generator.Context.ContextTypes.Push (@class);
+			generator.WriteType (@class, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
+			generator.Context.ContextTypes.Pop ();
+
+			Report.OutputDelegate = null;
+
+			// The warning should not be raised if the nested type matches enclosing namespace
+			Assert.False (sb.ToString ().Contains ("warning BG8403"));
 		}
 	}
 
diff --git a/tools/generator/Extensions/StringExtensions.cs b/tools/generator/Extensions/StringExtensions.cs
@@ -0,0 +1,45 @@
+using System;
+using System.Diagnostics.CodeAnalysis;
+
+namespace generator
+{
+	static class StringExtensions
+	{
+		/// <summary>
+		/// Shortcut for !string.IsNullOrWhiteSpace (s)
+		/// </summary>
+		public static bool HasValue (this string s) => !string.IsNullOrWhiteSpace (s);
+
+		/// <summary>
+		/// Removes the final subset of a delimited string. ("127.0.0.1" -> "127.0.0")
+		/// </summary>
+		public static string ChompLast (this string s, char separator)
+		{
+			if (!s.HasValue ())
+				return s;
+
+			var index = s.LastIndexOf (separator);
+
+			if (index < 0)
+				return string.Empty;
+
+			return s.Substring (0, index);
+		}
+
+		/// <summary>
+		/// Returns the final subset of a delimited string. ("127.0.0.1" -> "1")
+		/// </summary>
+		public static string LastSubset (this string s, char separator)
+		{
+			if (!s.HasValue ())
+				return s;
+
+			var index = s.LastIndexOf (separator);
+
+			if (index < 0)
+				return s;
+
+			return s.Substring (index + 1);
+		}
+	}
+}
diff --git a/tools/generator/Java.Interop.Tools.Generator.CodeGeneration/JavaInteropCodeGenerator.cs b/tools/generator/Java.Interop.Tools.Generator.CodeGeneration/JavaInteropCodeGenerator.cs
@@ -44,6 +44,10 @@ public override void WriteType (GenBase gen, string indent, GenerationInfo gen_i
 			else
 				throw new InvalidOperationException ("Unknown GenBase type");
 
+			// We do this here because we only want to check for top-level types,
+			// we should not check types nested in other types.
+			SourceWriterExtensions.WarnIfTypeNameMatchesNamespace (type_writer, gen);
+
 			var cw = new CodeWriter (writer, indent);
 			type_writer.Write (cw);
 		}
diff --git a/tools/generator/SourceWriters/Extensions/SourceWriterExtensions.cs b/tools/generator/SourceWriters/Extensions/SourceWriterExtensions.cs
@@ -397,5 +397,18 @@ public static TypeWriter BuildManagedTypeModel (GenBase gen, CodeGenerationOptio
 
 			throw new InvalidOperationException ("Unknown GenBase type");
 		}
+
+		public static void WarnIfTypeNameMatchesNamespace (TypeWriter type, GenBase gen)
+		{
+			// We only care about the last part of a namespace
+			// eg: android.graphics.drawable -> drawable
+			var ns = gen.Namespace?.LastSubset ('.');
+
+			if (!ns.HasValue ())
+				return;
+
+			if (string.Equals (ns, type.Name, StringComparison.OrdinalIgnoreCase))
+				Report.LogCodedWarning (0, Report.WarningTypeNameMatchesEnclosingNamespace, $"{gen.Namespace}.{type.Name}");
+		}
 	}
 }
