fix(jsii): class members named after the class result in illegal C# (#…

…1903)

It is illegal in C# to name a class' member (method, property) with the
same name as the class itself (even if the member is static). Since both
class and member names are pascal-cased in C#, this means a class'
members cannot share the same PascalCased representation.

This adds a compile-time validation for this condition, effectively
calling out member names to have the same PascalCased representation
as their declaring class' name.

This is currently only a warning, as this is "made to work" by altering
the type name by appending `_` at the end of it, which is ugly and
dangerous but works, and is currently done in several places).

As all warnings, this turns into an error when operating under `--strict`,
and future work (i.e: #1931) will allow more granular configuration
of these errors, so we can hopefully opt all new codebases into the
strict behavior and eventually drop the slugification.

Fixes #1880

docs/typescript-restrictions.md

@@ -75,6 +75,65 @@ users in languages with conflicts.
 
 [`jsii/lib/reserved-words.ts`]: ../packages/jsii/lib/reserved-words.ts
 
+## Type Members
+
+### Naming
+
+Methods and properties declared on *classes* cannot be named after the class (
+meaning they cannot share the same PascalCased representation), as this results
+in illegal **C#** code:
+
+> :memo: Due to existing usage (in such cases, an `_` is appended at the end of
+> the **type** name, effectively breaking existing .NET code if such a member
+> is introduced post-release), this restriction is only enforced when `jsii` is
+> invoked with the `--strict` parameter.
+>
+> It will be upgraded to *always* be an error in a future release.
+
+```ts
+export class Name {
+  // ⛔️ Illegal property name
+  public name: string;
+}
+
+export class Name {
+  // ⛔️ Illegal method name
+  public name(): void { /* ... */ }
+}
+```
+
+### Overriding
+
+The visibility of a type member cannot be changed when it is overridden, even if
+the change increases the visibility of said member, as this would result in
+illegal **C#** code:
+
+```ts
+export class Base {
+  protected method(): void { /* ... */ }
+}
+
+export class Child {
+  // ⛔️ Illegal change of visibility when overriding a member
+  public method(): void { /* ... */ }
+}
+```
+
+Additionally, **C#** does not allow changing the type signature of members while
+overriding them, even if the updated type signature is a strict specialization
+of the original one, and this is consequently also forbidden in `jsii`:
+
+```ts
+export class Base {
+  public method(): any { /* ... */ }
+}
+
+export class Child {
+  // ⛔️ Illegal change of signature when overriding a member
+  public method(): string { /* ... */ }
+}
+```
+
 ## Behavioral Interfaces & Structs
 
 `jsii` considers **TypeScript** interfaces in two distinct categories: *Structs*

packages/@jsii/dotnet-runtime-test/test/Amazon.JSII.Runtime.IntegrationTests/ComplianceTests.cs

@@ -29,9 +29,6 @@ public RuntimeException(string message)
             }
         }
 
-        // DateTime.UnixEpoch was added in .NET Core 2.1, but our build container only supports 2.0.
-        static readonly DateTime UnixEpoch = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc);
-
         const string Prefix = nameof(IntegrationTests) + ".Compliance.";
 
         private readonly IDisposable _serviceContainerFixture;
@@ -73,8 +70,8 @@ public void PrimitiveTypes()
             Assert.Equal(1234d, types.NumberProperty);
 
             // date
-            types.DateProperty = UnixEpoch.AddMilliseconds(123);
-            Assert.Equal(UnixEpoch.AddMilliseconds(123), types.DateProperty);
+            types.DateProperty = DateTime.UnixEpoch.AddMilliseconds(123);
+            Assert.Equal(DateTime.UnixEpoch.AddMilliseconds(123), types.DateProperty);
 
             // json
             types.JsonProperty = JObject.Parse(@"{ ""Foo"": { ""Bar"": 123 } }");
@@ -87,12 +84,12 @@ public void Dates()
             var types = new AllTypes();
 
             // strong type
-            types.DateProperty = UnixEpoch.AddMilliseconds(123);
-            Assert.Equal(UnixEpoch.AddMilliseconds(123), types.DateProperty);
+            types.DateProperty = DateTime.UnixEpoch.AddMilliseconds(123);
+            Assert.Equal(DateTime.UnixEpoch.AddMilliseconds(123), types.DateProperty);
 
             // weak type
-            types.AnyProperty = UnixEpoch.AddSeconds(999);
-            Assert.Equal(UnixEpoch.AddSeconds(999), types.AnyProperty);
+            types.AnyProperty = DateTime.UnixEpoch.AddSeconds(999);
+            Assert.Equal(DateTime.UnixEpoch.AddSeconds(999), types.AnyProperty);
         }
 
         [Fact(DisplayName = Prefix + nameof(CollectionTypes))]
@@ -142,8 +139,8 @@ public void DynamicTypes()
             Assert.Equal(12d, types.AnyProperty);
 
             // date
-            types.AnyProperty = UnixEpoch.AddSeconds(1234);
-            Assert.Equal(UnixEpoch.AddSeconds(1234), types.AnyProperty);
+            types.AnyProperty = DateTime.UnixEpoch.AddSeconds(1234);
+            Assert.Equal(DateTime.UnixEpoch.AddSeconds(1234), types.AnyProperty);
 
             // json (notice that when deserialized, it is deserialized as a map).
             types.AnyProperty = new Dictionary<string, object>
@@ -327,7 +324,7 @@ public void Arrays()
         {
             var sum = new Sum
             {
-                Parts = new Value_[] {new Number(5), new Number(10), new Multiply(new Number(2), new Number(3))}
+                Parts = new NumericValue[] {new Number(5), new Number(10), new Multiply(new Number(2), new Number(3))}
             };
             Assert.Equal(10d + 5d + 2d * 3d, sum.Value);
             Assert.Equal(5d, sum.Parts[0].Value);
@@ -591,13 +588,6 @@ public void PropertyOverrides_Interfaces()
             Assert.Equal("Hello!?", interact.WriteAndRead("Hello"));
         }
 
-        [Fact(DisplayName = Prefix + nameof(InterfaceBuilder), Skip = "There is no fluent API for C#")]
-        public void InterfaceBuilder()
-        {
-            throw new NotImplementedException();
-        }
-
-
         [Fact(DisplayName = Prefix + nameof(SyncOverrides_SyncOverrides))]
         public void SyncOverrides_SyncOverrides()
         {
@@ -746,18 +736,6 @@ public void TestInterfaceParameter()
             Assert.Equal("I am literally friendly! Let me buy you a drink!", betterGreeting);
         }
 
-        [Fact(DisplayName = Prefix + nameof(Structs_StepBuilders), Skip = "There is no fluent API for C#")]
-        public void Structs_StepBuilders()
-        {
-            throw new NotImplementedException();
-        }
-
-        [Fact(DisplayName = Prefix + nameof(Structs_BuildersContainNullChecks), Skip = "There is no fluent API for C#")]
-        public void Structs_BuildersContainNullChecks()
-        {
-            throw new NotImplementedException();
-        }
-
         [Fact(DisplayName = Prefix + nameof(Structs_SerializeToJsii))]
         public void Structs_SerializeToJsii()
         {
@@ -944,10 +922,10 @@ public void OptionalAndVariadicArgumentsTest()
             variadicClassNoParams.AsArray(double.MinValue, list.ToArray());
         }
 
-        [Fact(DisplayName = Prefix + nameof(JsiiAgent))]
-        public void JsiiAgent()
+        [Fact(DisplayName = Prefix + nameof(JsiiAgentIsCorrect))]
+        public void JsiiAgentIsCorrect()
         {
-            Assert.Equal("DotNet/" + Environment.Version + "/.NETCoreApp,Version=v3.1/1.0.0.0", JsiiAgent_.JsiiAgent);
+            Assert.Equal("DotNet/" + Environment.Version + "/.NETCoreApp,Version=v3.1/1.0.0.0", JsiiAgent.Value);
         }
 
         [Fact(DisplayName = Prefix + nameof(ReceiveInstanceOfPrivateClass))]
@@ -980,7 +958,7 @@ public void EraseUnsetDataValues()
             Assert.Equal(new Dictionary<string, object> { [ "prop1"] = "value1" }, EraseUndefinedHashValues.Prop2IsUndefined());
         }
 
-        [Fact(DisplayName = Prefix + nameof(ObjectIdDoesNotGetReallocatedWhenTheConstructorPassesThisOut), Skip = "Currently broken")]
+        [Fact(DisplayName = Prefix + nameof(ObjectIdDoesNotGetReallocatedWhenTheConstructorPassesThisOut))]
         public void ObjectIdDoesNotGetReallocatedWhenTheConstructorPassesThisOut()
         {
             var reflector = new PartiallyInitializedThisConsumerImpl();
@@ -1125,7 +1103,7 @@ class PartiallyInitializedThisConsumerImpl : PartiallyInitializedThisConsumer
             public override String ConsumePartiallyInitializedThis(ConstructorPassesThisOut obj, DateTime dt, AllTypesEnum ev)
             {
                 Assert.NotNull(obj);
-                Assert.Equal(new DateTime(0), dt);
+                Assert.Equal(DateTime.UnixEpoch, dt);
                 Assert.Equal(AllTypesEnum.THIS_IS_GREAT, ev);
 
                 return "OK";

packages/@jsii/java-runtime-test/project/src/test/java/software/amazon/jsii/testing/ComplianceTest.java

@@ -17,8 +17,8 @@
 import software.amazon.jsii.tests.calculator.lib.IFriendly;
 import software.amazon.jsii.tests.calculator.lib.MyFirstStruct;
 import software.amazon.jsii.tests.calculator.lib.Number;
+import software.amazon.jsii.tests.calculator.lib.NumericValue;
 import software.amazon.jsii.tests.calculator.lib.StructWithOnlyOptionals;
-import software.amazon.jsii.tests.calculator.lib.Value;
 import software.amazon.jsii.tests.calculator.submodule.child.OuterClass;
 
 import java.io.IOException;
@@ -216,7 +216,7 @@ public void callMethods() {
     public void unmarshallIntoAbstractType() {
         Calculator calc = new Calculator();
         calc.add(120);
-        Value value = calc.getCurr();
+        NumericValue value = calc.getCurr();
         assertEquals(120, value.getValue());
     }
 
@@ -1206,7 +1206,7 @@ public void nullShouldBeTreatedAsUndefined() {
 
     @Test
     public void testJsiiAgent() {
-        assertEquals("Java/" + System.getProperty("java.version"), JsiiAgent.getJsiiAgent());
+        assertEquals("Java/" + System.getProperty("java.version"), JsiiAgent.getValue());
     }
 
     /**

packages/@jsii/kernel/test/kernel.test.ts

@@ -1482,7 +1482,7 @@ defineTest('nulls are converted to undefined - properties', (sandbox) => {
 
 defineTest('JSII_AGENT is undefined in node.js', (sandbox) => {
   expect(
-    sandbox.sget({ fqn: 'jsii-calc.JsiiAgent', property: 'jsiiAgent' }).value,
+    sandbox.sget({ fqn: 'jsii-calc.JsiiAgent', property: 'value' }).value,
   ).toBe(undefined);
 });
 

packages/@jsii/python-runtime/tests/test_compliance.py

@@ -915,7 +915,7 @@ def test_nullShouldBeTreatedAsUndefined():
 
 
 def test_testJsiiAgent():
-    assert JsiiAgent.jsii_agent == f"Python/{platform.python_version()}"
+    assert JsiiAgent.value == f"Python/{platform.python_version()}"
 
 
 def test_receiveInstanceOfPrivateClass():

packages/@scope/jsii-calc-lib/lib/index.ts

@@ -3,100 +3,100 @@ import * as base from '@scope/jsii-calc-base';
 /**
  * Abstract class which represents a numeric value.
  */
-export abstract class Value extends base.Base {
-    /**
-     * The value.
-     */
-    public abstract readonly value: number;
+export abstract class NumericValue extends base.Base {
+  /**
+   * The value.
+   */
+  public abstract readonly value: number;
 
-    /**
-     * String representation of the value.
-     */
-    public toString() {
-        return this.value.toString();
-    }
+  /**
+   * String representation of the value.
+   */
+  public toString() {
+    return this.value.toString();
+  }
 }
 
 /**
  * The general contract for a concrete number.
  */
 export interface IDoublable {
-    readonly doubleValue: number;
+  readonly doubleValue: number;
 }
 
 /**
  * Represents a concrete number.
  */
-export class Number extends Value implements IDoublable {
-    /**
-     * Creates a Number object.
-     * @param value The number.
-     */
-    constructor(readonly value: number) {
-        super();
-    }
+export class Number extends NumericValue implements IDoublable {
+  /**
+   * Creates a Number object.
+   * @param value The number.
+   */
+  public constructor(public readonly value: number) {
+    super();
+  }
 
-    /**
-     * The number multiplied by 2.
-     */
-    get doubleValue() {
-        return 2 * this.value;
-    }
+  /**
+   * The number multiplied by 2.
+   */
+  public get doubleValue() {
+    return 2 * this.value;
+  }
 }
 
 /**
  * Represents an operation on values.
  */
-export abstract class Operation extends Value {
-    public abstract toString(): string;
+export abstract class Operation extends NumericValue {
+  public abstract toString(): string;
 }
 
 /**
  * Applies to classes that are considered friendly. These classes can be greeted with
  * a "hello" or "goodbye" blessing and they will respond back in a fun and friendly manner.
  */
 export interface IFriendly {
-    /**
-     * Say hello!
-     */
-    hello(): string
+  /**
+   * Say hello!
+   */
+  hello(): string;
 }
 
 /**
  * This is the first struct we have created in jsii
  */
 export interface MyFirstStruct {
-    /**
-     * A string value
-     */
-    readonly astring: string
+  /**
+   * A string value
+   */
+  readonly astring: string;
 
-    /**
-     * An awesome number value
-     */
-    readonly anumber: number
-    readonly firstOptional?: string[]
+  /**
+   * An awesome number value
+   */
+  readonly anumber: number;
+  readonly firstOptional?: string[];
 }
 
 /**
  * This is a struct with only optional properties.
  */
 export interface StructWithOnlyOptionals {
-    /**
-     * The first optional!
-     */
-    readonly optional1?: string
-    readonly optional2?: number
-    readonly optional3?: boolean
+  /**
+   * The first optional!
+   */
+  readonly optional1?: string;
+  readonly optional2?: number;
+  readonly optional3?: boolean;
 }
 
 /**
  * Check that enums from \@scoped packages can be references.
  * See awslabs/jsii#138
  */
 export enum EnumFromScopedModule {
-    VALUE1,
-    VALUE2
+  VALUE1,
+  VALUE2,
 }
 
 /**
@@ -106,7 +106,7 @@ export enum EnumFromScopedModule {
  * far enough up the tree.
  */
 export interface IThreeLevelsInterface extends base.IBaseInterface {
-    baz(): void;
+  baz(): void;
 }
 
 export * as submodule from './submodule';