Identical static methods in parent/child classes are mistakenly recognized as invalid in JSII

[Chriscbr]

🐛 Bug Report

Related issues: #3407, #2358

Affected Languages

• TypeScript or Javascript
• Python
• Java
• .NET (C#, F#, ...)
• Go

General Information

• JSII Version: 1.54.0
• Platform: macOS 12.2.1

What is the problem?

When compiling with JSII, overriding static methods on child classes with different return types results in an error, even when the types are technically compatible. For example:

class BaseValue {
    public readonly prop1: string = "hi";
}

class DerivedValue extends BaseValue {
    public readonly prop2: boolean = true;
}

class BaseClass {
    public static hello(): BaseValue {
        return new BaseValue();
    }
}

class DerivedClass extends BaseClass {
    public static hello(): DerivedValue {
        return new DerivedValue();
    }
}

produces an error like:

error JSII5003: "DerivedClass#hello" changes the return type to "DerivedValue" when overriding BaseClass. Change it to "BaseValue"

Since DerivedValue extends BaseValue, this compiles in TypeScript and is valid ES6 code. This kind of structure is also valid in Java:

class BaseValue {
  private int prop1;
  public BaseValue(int prop1) {
    this.prop1 = prop1;
  }
}

class DerivedValue extends BaseValue {
  private int prop2;
  public DerivedValue(int prop1, int prop2) {
    super(prop1);
    this.prop2 = prop2;
  }
}

class Base {
  public static BaseValue hello() {
    return new BaseValue(5);
  }
}

class Derived {
  public static DerivedValue hello() {
    return new DerivedValue(10, 5);
  }
}

In C# I think this is also valid, but this needs validation. I suspect it's valid based on this comment in our docs:

!!! danger Properties and methods that are static can feature the overrides attribute, as static members are inherited with the prototype in JavaScript (as part of the ES6 specification). Not all target languages have this capability (most, like C# and Java, only support hiding static declarations), and consequently code generators may ignore this (or explicitly hide parent declarations) instead.

In Python, I think @ classmethod's can be directly overriden so I don't think this causes any issues. (can anyone confirm?)

In Go there's no real "static" methods, so the transpiled code from jsii should also be valid.

Verbose Log

N/A

[RomainMuller]

Allowing this might result in signatures that are impossible to accurately represent in future languages we might want to add, such as Rust and Swift, where there may be an interest in representing static members as class-implemented traits (would be useful in user-land, but AFAIK does not allow specialization).

That means if we allow this and represent it in languages where possible (that is - all supported languages at this point, AFAIK), we might end up forced to represent a different (less specific) API in some future languages... Making it easy for API developers to overlook the developer experience implications to this...

---

I'm not entirely against implementing this change, but then we should also consider whether we should extend the same courtesy to C# (i.e: generating the override declaration it supports, with a less specific signature than in the original code).

I would encourage writing a basic RFC for this to ensure we have evaluated the current & future customer implications of this change.

[echeung-amzn]

I seem to be hitting a similar issue as this, except the types are actually identical.

In module A:

export type MetricWithAlarmSupport = Metric | MathExpression;

export class MetricFactory {
  adaptMetric(metric: MetricWithAlarmSupport): MetricWithAlarmSupport {
    ...
  }
}

In module B:

import { MetricWithAlarmSupport, MetricFactory } from "module-a";

export class MyMetricFactory extends MetricFactory {
  adaptMetric(metric: MetricWithAlarmSupport): MetricWithAlarmSupport {
    ...
  }
}

Results in errors like:

error JSII5003: "module-b.MyMetricFactory#adaptMetric" changes the return type to "monocdk.aws_cloudwatch.Metric | monocdk.aws_cloudwatch.MathExpression" when overriding module-a.MetricFactory. Change it to "monocdk.aws_cloudwatch.Metric | monocdk.aws_cloudwatch.MathExpression"

Replacing the MetricWithAlarmSupport with Metric | MathExpression results in the same thing.

Is this related to this issue, or should I file another one?

[Chriscbr]

@echeung-amzn Hmm, that sounds like it might be another bug. If you could file a separate issue for it that would be great! It would be especially helpful if you can write a minimal repro. (You can use projen to bootstrap a new jsii project if needed).

[echeung-amzn]

@Chriscbr Thanks, I've logged it as #3495 including a link to a minimal repro project.