Skip to content

Commit

Permalink
fix(dotnet): missing ? on nullable interface members
Browse files Browse the repository at this point in the history
When interfaces declare nullable members, the code generator erroneously omitted the `?` that denotes it, while generated implementations of the interface would have it, causing compilation failures.

Fixes #1285
  • Loading branch information
RomainMuller committed Feb 24, 2020
1 parent ad98f66 commit 658d2be
Show file tree
Hide file tree
Showing 73 changed files with 319 additions and 124 deletions.
7 changes: 7 additions & 0 deletions packages/jsii-calc/lib/compliance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2458,3 +2458,10 @@ export class InterfaceCollections {

private constructor(){ }
}

/**
* Checks that optional result from interface method code generates correctly
*/
export interface IOptionalMethod {
optional(): string | undefined;
}
35 changes: 34 additions & 1 deletion packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -5507,6 +5507,39 @@
}
]
},
"jsii-calc.IOptionalMethod": {
"assembly": "jsii-calc",
"docs": {
"stability": "experimental",
"summary": "Checks that optional result from interface method code generates correctly."
},
"fqn": "jsii-calc.IOptionalMethod",
"kind": "interface",
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 2465
},
"methods": [
{
"abstract": true,
"docs": {
"stability": "experimental"
},
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 2466
},
"name": "optional",
"returns": {
"optional": true,
"type": {
"primitive": "string"
}
}
}
],
"name": "IOptionalMethod"
},
"jsii-calc.IPrivatelyImplemented": {
"assembly": "jsii-calc",
"docs": {
Expand Down Expand Up @@ -12123,5 +12156,5 @@
}
},
"version": "1.0.0",
"fingerprint": "rtlqfLVk+YuqJRqeFCR5A3mesEPpfJgo5m1gRM323+U="
"fingerprint": "oTbIPjVGH+NSJCWHMqRYC7fJ3XjjZSr8TvvRkANEonA="
}
3 changes: 2 additions & 1 deletion packages/jsii-pacmak/lib/targets/dotnet/dotnetgenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,8 @@ export class DotNetGenerator extends Generator {
this.dotnetDocGenerator.emitDocs(method);
this.dotnetRuntimeGenerator.emitAttributesForMethod(ifc, method);
const returnType = method.returns ? this.typeresolver.toDotNetType(method.returns.type) : 'void';
this.code.line(`${returnType} ${this.nameutils.convertMethodName(method.name)}(${this.renderMethodParameters(method)});`);
const nullable = method.returns?.optional ? '?' : '';
this.code.line(`${returnType}${nullable} ${this.nameutils.convertMethodName(method.name)}(${this.renderMethodParameters(method)});`);
}

protected onInterfaceMethodOverload(ifc: spec.InterfaceType, overload: spec.Method, _originalMethod: spec.Method) {
Expand Down
6 changes: 3 additions & 3 deletions packages/jsii-pacmak/lib/targets/java.ts
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@ class JavaGenerator extends Generator {

protected onInterfaceMethod(_ifc: spec.InterfaceType, method: spec.Method) {
this.code.line();
const returnType = method.returns ? this.toJavaType(method.returns.type) : 'void';
const returnType = method.returns ? this.toDecoratedJavaType(method.returns) : 'void';
this.addJavaDocs(method);
this.emitStabilityAnnotations(method);
this.code.line(`${returnType} ${method.name}(${this.renderMethodParameters(method)});`);
Expand All @@ -616,8 +616,8 @@ class JavaGenerator extends Generator {
}

protected onInterfaceProperty(_ifc: spec.InterfaceType, prop: spec.Property) {
const getterType = this.toJavaType(prop.type);
const setterTypes = this.toJavaTypes(prop.type);
const getterType = this.toDecoratedJavaType(prop);
const setterTypes = this.toDecoratedJavaTypes(prop);
const propName = this.code.toPascalCase(JavaGenerator.safeJavaPropertyName(prop.name));

// for unions we only generate overloads for setters, not getters.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
@software.amazon.jsii.Jsii.Proxy(VeryBaseProps.Jsii$Proxy.class)
public interface VeryBaseProps extends software.amazon.jsii.JsiiSerializable {

software.amazon.jsii.tests.calculator.baseofbase.Very getFoo();
@org.jetbrains.annotations.NotNull software.amazon.jsii.tests.calculator.baseofbase.Very getFoo();

/**
* @return a {@link Builder} of {@link VeryBaseProps}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
@software.amazon.jsii.Jsii.Proxy(BaseProps.Jsii$Proxy.class)
public interface BaseProps extends software.amazon.jsii.JsiiSerializable, software.amazon.jsii.tests.calculator.baseofbase.VeryBaseProps {

java.lang.String getBar();
@org.jetbrains.annotations.NotNull java.lang.String getBar();

/**
* @return a {@link Builder} of {@link BaseProps}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public interface IDoublable extends software.amazon.jsii.JsiiSerializable {
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Deprecated)
@Deprecated
java.lang.Number getDoubleValue();
@org.jetbrains.annotations.NotNull java.lang.Number getDoubleValue();

/**
* A proxy class which represents a concrete javascript instance of this type.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public interface IFriendly extends software.amazon.jsii.JsiiSerializable {
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Deprecated)
@Deprecated
java.lang.String hello();
@org.jetbrains.annotations.NotNull java.lang.String hello();

/**
* A proxy class which represents a concrete javascript instance of this type.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,20 @@ public interface MyFirstStruct extends software.amazon.jsii.JsiiSerializable {
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Deprecated)
@Deprecated
java.lang.Number getAnumber();
@org.jetbrains.annotations.NotNull java.lang.Number getAnumber();

/**
* A string value.
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Deprecated)
@Deprecated
java.lang.String getAstring();
@org.jetbrains.annotations.NotNull java.lang.String getAstring();

/**
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Deprecated)
@Deprecated
default java.util.List<java.lang.String> getFirstOptional() {
default @org.jetbrains.annotations.Nullable java.util.List<java.lang.String> getFirstOptional() {
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,23 @@ public interface StructWithOnlyOptionals extends software.amazon.jsii.JsiiSerial
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Deprecated)
@Deprecated
default java.lang.String getOptional1() {
default @org.jetbrains.annotations.Nullable java.lang.String getOptional1() {
return null;
}

/**
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Deprecated)
@Deprecated
default java.lang.Number getOptional2() {
default @org.jetbrains.annotations.Nullable java.lang.Number getOptional2() {
return null;
}

/**
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Deprecated)
@Deprecated
default java.lang.Boolean getOptional3() {
default @org.jetbrains.annotations.Nullable java.lang.Boolean getOptional3() {
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5507,6 +5507,39 @@
}
]
},
"jsii-calc.IOptionalMethod": {
"assembly": "jsii-calc",
"docs": {
"stability": "experimental",
"summary": "Checks that optional result from interface method code generates correctly."
},
"fqn": "jsii-calc.IOptionalMethod",
"kind": "interface",
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 2465
},
"methods": [
{
"abstract": true,
"docs": {
"stability": "experimental"
},
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 2466
},
"name": "optional",
"returns": {
"optional": true,
"type": {
"primitive": "string"
}
}
}
],
"name": "IOptionalMethod"
},
"jsii-calc.IPrivatelyImplemented": {
"assembly": "jsii-calc",
"docs": {
Expand Down Expand Up @@ -12123,5 +12156,5 @@
}
},
"version": "1.0.0",
"fingerprint": "rtlqfLVk+YuqJRqeFCR5A3mesEPpfJgo5m1gRM323+U="
"fingerprint": "oTbIPjVGH+NSJCWHMqRYC7fJ3XjjZSr8TvvRkANEonA="
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
using Amazon.JSII.Runtime.Deputy;

#pragma warning disable CS0672,CS0809,CS1591

namespace Amazon.JSII.Tests.CalculatorNamespace
{
/// <summary>Checks that optional result from interface method code generates correctly.</summary>
/// <remarks>
/// <strong>Stability</strong>: Experimental
/// </remarks>
[JsiiInterface(nativeType: typeof(IOptionalMethod), fullyQualifiedName: "jsii-calc.IOptionalMethod")]
public interface IOptionalMethod
{
/// <remarks>
/// <strong>Stability</strong>: Experimental
/// </remarks>
[JsiiMethod(name: "optional", returnsJson: "{\"optional\":true,\"type\":{\"primitive\":\"string\"}}")]
string? Optional();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
using Amazon.JSII.Runtime.Deputy;

#pragma warning disable CS0672,CS0809,CS1591

namespace Amazon.JSII.Tests.CalculatorNamespace
{
/// <summary>Checks that optional result from interface method code generates correctly.</summary>
/// <remarks>
/// <strong>Stability</strong>: Experimental
/// </remarks>
[JsiiTypeProxy(nativeType: typeof(IOptionalMethod), fullyQualifiedName: "jsii-calc.IOptionalMethod")]
internal sealed class IOptionalMethodProxy : DeputyBase, Amazon.JSII.Tests.CalculatorNamespace.IOptionalMethod
{
private IOptionalMethodProxy(ByRefValue reference): base(reference)
{
}

/// <remarks>
/// <strong>Stability</strong>: Experimental
/// </remarks>
[JsiiMethod(name: "optional", returnsJson: "{\"optional\":true,\"type\":{\"primitive\":\"string\"}}")]
public string? Optional()
{
return InvokeInstanceMethod<string?>(new System.Type[]{}, new object[]{});
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ protected Class<?> resolveClass(final String fqn) throws ClassNotFoundException
case "jsii-calc.IMutableObjectLiteral": return software.amazon.jsii.tests.calculator.IMutableObjectLiteral.class;
case "jsii-calc.INonInternalInterface": return software.amazon.jsii.tests.calculator.INonInternalInterface.class;
case "jsii-calc.IObjectWithProperty": return software.amazon.jsii.tests.calculator.IObjectWithProperty.class;
case "jsii-calc.IOptionalMethod": return software.amazon.jsii.tests.calculator.IOptionalMethod.class;
case "jsii-calc.IPrivatelyImplemented": return software.amazon.jsii.tests.calculator.IPrivatelyImplemented.class;
case "jsii-calc.IPublicInterface": return software.amazon.jsii.tests.calculator.IPublicInterface.class;
case "jsii-calc.IPublicInterface2": return software.amazon.jsii.tests.calculator.IPublicInterface2.class;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public interface CalculatorProps extends software.amazon.jsii.JsiiSerializable {
* EXPERIMENTAL
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Experimental)
default java.lang.Number getInitialValue() {
default @org.jetbrains.annotations.Nullable java.lang.Number getInitialValue() {
return null;
}

Expand All @@ -33,7 +33,7 @@ default java.lang.Number getInitialValue() {
* EXPERIMENTAL
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Experimental)
default java.lang.Number getMaximumValue() {
default @org.jetbrains.annotations.Nullable java.lang.Number getMaximumValue() {
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public interface ChildStruct982 extends software.amazon.jsii.JsiiSerializable, s
* EXPERIMENTAL
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Experimental)
java.lang.Number getBar();
@org.jetbrains.annotations.NotNull java.lang.Number getBar();

/**
* @return a {@link Builder} of {@link ChildStruct982}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public interface ConfusingToJacksonStruct extends software.amazon.jsii.JsiiSeria
* EXPERIMENTAL
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Experimental)
default java.lang.Object getUnionProperty() {
default @org.jetbrains.annotations.Nullable java.lang.Object getUnionProperty() {
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public interface DeprecatedStruct extends software.amazon.jsii.JsiiSerializable
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Deprecated)
@Deprecated
java.lang.String getReadonlyProperty();
@org.jetbrains.annotations.NotNull java.lang.String getReadonlyProperty();

/**
* @return a {@link Builder} of {@link DeprecatedStruct}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,45 +15,45 @@ public interface DerivedStruct extends software.amazon.jsii.JsiiSerializable, so
* EXPERIMENTAL
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Experimental)
java.time.Instant getAnotherRequired();
@org.jetbrains.annotations.NotNull java.time.Instant getAnotherRequired();

/**
* EXPERIMENTAL
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Experimental)
java.lang.Boolean getBool();
@org.jetbrains.annotations.NotNull java.lang.Boolean getBool();

/**
* An example of a non primitive property.
* <p>
* EXPERIMENTAL
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Experimental)
software.amazon.jsii.tests.calculator.DoubleTrouble getNonPrimitive();
@org.jetbrains.annotations.NotNull software.amazon.jsii.tests.calculator.DoubleTrouble getNonPrimitive();

/**
* This is optional.
* <p>
* EXPERIMENTAL
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Experimental)
default java.util.Map<java.lang.String, software.amazon.jsii.tests.calculator.lib.Value> getAnotherOptional() {
default @org.jetbrains.annotations.Nullable java.util.Map<java.lang.String, software.amazon.jsii.tests.calculator.lib.Value> getAnotherOptional() {
return null;
}

/**
* EXPERIMENTAL
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Experimental)
default java.lang.Object getOptionalAny() {
default @org.jetbrains.annotations.Nullable java.lang.Object getOptionalAny() {
return null;
}

/**
* EXPERIMENTAL
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Experimental)
default java.util.List<java.lang.String> getOptionalArray() {
default @org.jetbrains.annotations.Nullable java.util.List<java.lang.String> getOptionalArray() {
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public interface DiamondInheritanceBaseLevelStruct extends software.amazon.jsii.
* EXPERIMENTAL
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Experimental)
java.lang.String getBaseLevelProperty();
@org.jetbrains.annotations.NotNull java.lang.String getBaseLevelProperty();

/**
* @return a {@link Builder} of {@link DiamondInheritanceBaseLevelStruct}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public interface DiamondInheritanceFirstMidLevelStruct extends software.amazon.j
* EXPERIMENTAL
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Experimental)
java.lang.String getFirstMidLevelProperty();
@org.jetbrains.annotations.NotNull java.lang.String getFirstMidLevelProperty();

/**
* @return a {@link Builder} of {@link DiamondInheritanceFirstMidLevelStruct}
Expand Down
Loading

0 comments on commit 658d2be

Please sign in to comment.