fix(dotnet): Use nested classes for proxies to avoid name collision (#…

…2368)

Fixes #2367 

C# proxy classes now nested in associated type using the name `Jsii_Proxy`. Using non-standard `_` in naming to further reduce chance of collision.

I would like to do some more testing since I'm not familiar with the details of how the proxies work.

---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0

packages/@jsii/dotnet-runtime/src/Amazon.JSII.Runtime/Deputy/DeputyBase.cs

@@ -120,7 +120,7 @@ protected DeputyBase(ByRefValue reference)
             referenceMap.AddNativeReference(Reference, this);
         }
 
-        public ByRefValue Reference { get; }
+        internal ByRefValue Reference { get; }
 
         #region GetProperty
 
@@ -476,7 +476,7 @@ private static JsiiMethodAttribute GetMethodAttributeCore(System.Type type, stri
         /// <summary>
         /// Unsafely obtains a proxy of a given type for this instance. This method allows obtaining a proxy instance
         /// that is not known to be supported by the backing object instance; in which case the behavior of any
-        /// operation that is not supported by the backing instance is undefined. 
+        /// operation that is not supported by the backing instance is undefined.
         /// </summary>
         /// <typeparam name="T">
         /// A jsii-managed interface to obtain a proxy for.
@@ -535,7 +535,7 @@ object IConvertible.ToType(System.Type conversionType, IFormatProvider? provider
             bool ToTypeCore(out object? result)
             {
                 if (!conversionType.IsInstanceOfType(this)) return MakeProxy(conversionType, false, out result);
-                
+
                 result = this;
                 return true;
 
@@ -549,7 +549,7 @@ private bool MakeProxy(Type interfaceType, bool force, [NotNullWhen(true)] out o
                 result = null;
                 return false;
             }
-            
+
             var interfaceAttribute = interfaceType.GetCustomAttribute<JsiiInterfaceAttribute>();
             if (interfaceAttribute == null)
             {
@@ -581,7 +581,7 @@ private bool MakeProxy(Type interfaceType, bool force, [NotNullWhen(true)] out o
 
             result = constructorInfo.Invoke(new object[]{ Reference.ForProxy() });
             return true;
-            
+
             bool TryFindSupportedInterface(string declaredFqn, string[] availableFqns, ITypeCache types, out string? foundFqn)
             {
                 var declaredType = types.GetInterfaceType(declaredFqn);

packages/jsii-pacmak/lib/targets/dotnet/dotnetgenerator.ts

@@ -163,14 +163,14 @@ export class DotNetGenerator extends Generator {
   }
 
   protected onEndInterface(ifc: spec.InterfaceType) {
+    // emit interface proxy class
+    this.emitInterfaceProxy(ifc);
+
     const interfaceName = this.nameutils.convertInterfaceName(ifc);
     this.code.closeBlock();
     const namespace = this.namespaceFor(this.assembly, ifc);
     this.closeFileIfNeeded(interfaceName, namespace, this.isNested(ifc));
 
-    // emit interface proxy class
-    this.emitInterfaceProxy(ifc);
-
     // emit implementation class
     // TODO: If datatype then we may not need the interface proxy to be created, We could do with just the interface impl?
     if (ifc.datatype) {
@@ -361,14 +361,14 @@ export class DotNetGenerator extends Generator {
   }
 
   protected onEndClass(cls: spec.ClassType) {
+    if (cls.abstract) {
+      this.emitInterfaceProxy(cls);
+    }
+
     this.code.closeBlock();
     const className = this.nameutils.convertClassName(cls);
     const namespace = this.namespaceFor(this.assembly, cls);
     this.closeFileIfNeeded(className, namespace, this.isNested(cls));
-
-    if (cls.abstract) {
-      this.emitInterfaceProxy(cls);
-    }
   }
 
   protected onField(
@@ -511,8 +511,10 @@ export class DotNetGenerator extends Generator {
       // Methods should always be virtual when possible
       virtualKeyWord = 'virtual ';
     }
+
     const access = this.renderAccessLevel(method);
     const methodName = this.nameutils.convertMethodName(method.name);
+
     const isOptional = method.returns && method.returns.optional ? '?' : '';
     const signature = `${returnType}${isOptional} ${methodName}(${this.renderMethodParameters(
       method,
@@ -637,19 +639,24 @@ export class DotNetGenerator extends Generator {
    * Emits an interface proxy for an interface or an abstract class.
    */
   private emitInterfaceProxy(ifc: spec.InterfaceType | spec.ClassType): void {
-    // No need to slugify for a proxy
-    const name = `${this.nameutils.convertTypeName(ifc.name)}Proxy`;
+    const name = '_Proxy';
     const namespace = this.namespaceFor(this.assembly, ifc);
-    const isNested = this.isNested(ifc);
+    const isNested = true;
     this.openFileIfNeeded(name, namespace, isNested);
 
+    this.code.line();
     this.dotnetDocGenerator.emitDocs(ifc);
     this.dotnetRuntimeGenerator.emitAttributesForInterfaceProxy(ifc);
+
     const interfaceFqn = this.typeresolver.toNativeFqn(ifc.fqn);
     const suffix = spec.isInterfaceType(ifc)
       ? `: DeputyBase, ${interfaceFqn}`
       : `: ${interfaceFqn}`;
-    this.code.openBlock(`internal sealed class ${name} ${suffix}`);
+    const newModifier = this.proxyMustUseNewModifier(ifc) ? 'new ' : '';
+
+    this.code.openBlock(
+      `${newModifier}internal sealed class ${name} ${suffix}`,
+    );
 
     // Create the private constructor
     this.code.openBlock(
@@ -668,6 +675,46 @@ export class DotNetGenerator extends Generator {
     this.closeFileIfNeeded(name, namespace, isNested);
   }
 
+  /**
+   * Determines whether any ancestor of the given type must use the `new`
+   * modifier when introducing it's own proxy.
+   *
+   * If the type is a `class`, then it must use `new` if it extends another
+   * abstract class defined in the same assembly (since proxies are internal,
+   * external types' proxies are not visible in that context).
+   *
+   * If the type is an `interface`, then it must use `new` if it extends another
+   * interface from the same assembly.
+   *
+   * @param type the tested proxy-able type (an abstract class or an interface).
+   *
+   * @returns true if any ancestor of this type has a visible proxy.
+   */
+  private proxyMustUseNewModifier(
+    type: spec.ClassType | spec.InterfaceType,
+  ): boolean {
+    if (spec.isClassType(type)) {
+      if (type.base == null) {
+        return false;
+      }
+
+      const base = this.findType(type.base) as spec.ClassType;
+      return (
+        base.assembly === type.assembly &&
+        (base.abstract
+          ? true
+          : // An abstract class could extend a concrete class... We must walk up the inheritance tree in this case...
+            this.proxyMustUseNewModifier(base))
+      );
+    }
+
+    return (
+      type.interfaces?.find(
+        (fqn) => this.findType(fqn).assembly === type.assembly,
+      ) != null
+    );
+  }
+
   /**
    * Emits an Interface Datatype class
    *

packages/jsii-pacmak/lib/targets/dotnet/filegenerator.ts

@@ -135,9 +135,21 @@ export class FileGenerator {
       }
     });
 
+    const warnings = rootNode.ele('PropertyGroup');
     // Suppress warnings about [Obsolete] members, this is the author's choice!
-    rootNode.ele('PropertyGroup').ele('NoWarn').text('0612,0618');
-
+    warnings.comment('Silence [Obsolete] warnings');
+    warnings.ele('NoWarn').text('0612,0618');
+    // Treat select warnings as errors, as these are likely codegen bugs:
+    warnings.comment(
+      'Treat warnings symptomatic of code generation bugs as errors',
+    );
+    warnings.ele(
+      'WarningsAsErrors',
+      [
+        '0108', // 'member1' hides inherited member 'member2'. Use the new keyword if hiding was intended.
+        '0109', // The member 'member' does not hide an inherited member. The new keyword is not required.
+      ].join(','),
+    );
     const xml = rootNode.end({ pretty: true, spaceBeforeSlash: true });
 
     // Sending the xml content to the codemaker to ensure the file is written