Skip to content

Commit

Permalink
refactor: Address changes for pr 241
Browse files Browse the repository at this point in the history 
* Add .DS_Store to .gitignore
* Rename JsiiInterfaceProxyAttribute to JsiiTypeProxy
* Proxy implementations are sealed
* Removed redundant checks for proxy resolution in runtime.

Refactor of #241
  • Loading branch information
@costleya
costleya committed Sep 22, 2018
1 parent 795ce88 commit bdfe854
Show file tree
Hide file tree
Showing 41 changed files with 182 additions and 180 deletions.
3 changes: 3 additions & 0 deletions .gitignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -368,3 +368,6 @@ jspm_packages/
# that NuGet uses for external packages. We don't want to ignore the
# lerna packages.
!/packages/*

#MacOS metadata
.DS_Store
16 changes: 8 additions & 8 deletions ...t-generator/src/Amazon.JSII.Generator.UnitTests/Class/AbstractClassProxyGeneratorTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ public void IncludesAttribute()
var expected =
@"namespace MyNamespace
{
[JsiiInterfaceProxy(typeof(MyClass), ""myFqn"")]
internal class MyClassProxy : MyClass
[JsiiTypeProxy(typeof(MyClass), ""myFqn"")]
internal sealed class MyClassProxy : MyClass
{
private MyClassProxy(ByRefValue reference): base(reference)
{
Expand Down Expand Up @@ -66,8 +66,8 @@ public void IncludesDocs()
@"namespace MyNamespace
{
/// <remarks>foo: bar</remarks>
[JsiiInterfaceProxy(typeof(MyClass), ""myFqn"")]
internal class MyClassProxy : MyClass
[JsiiTypeProxy(typeof(MyClass), ""myFqn"")]
internal sealed class MyClassProxy : MyClass
{
private MyClassProxy(ByRefValue reference): base(reference)
{
Expand Down Expand Up @@ -115,8 +115,8 @@ public void IncludesProperties()
string expected =
@"namespace MyNamespace
{
[JsiiInterfaceProxy(typeof(MyClass), ""myFqn"")]
internal class MyClassProxy : MyClass
[JsiiTypeProxy(typeof(MyClass), ""myFqn"")]
internal sealed class MyClassProxy : MyClass
{
private MyClassProxy(ByRefValue reference): base(reference)
{
Expand Down Expand Up @@ -168,8 +168,8 @@ public void IncludesMethods()
string expected =
@"namespace MyNamespace
{
[JsiiInterfaceProxy(typeof(MyClass), ""myFqn"")]
internal class MyClassProxy : MyClass
[JsiiTypeProxy(typeof(MyClass), ""myFqn"")]
internal sealed class MyClassProxy : MyClass
{
private MyClassProxy(ByRefValue reference): base(reference)
{
Expand Down
71 changes: 37 additions & 34 deletions ...t-generator/src/Amazon.JSII.Generator.UnitTests/Interface/InterfaceProxyGeneratorTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,21 @@
using Microsoft.CodeAnalysis;
using Newtonsoft.Json;
using Xunit;
using TypeKind = Amazon.JSII.JsonModel.Spec.TypeKind;

namespace Amazon.JSII.Generator.UnitTests.Interface
{
public class InterfaceProxyGeneratorTests : GeneratorTestBase
{
const string Prefix = nameof(Generator) + "." + nameof(InterfaceProxyGenerator) + ".";

string Render(InterfaceType interfaceType, string package = "myPackage", string @namespace = "MyNamespace", string typeName = "MyInterface")
string Render(InterfaceType interfaceType, string package = "myPackage", string @namespace = "MyNamespace",
string typeName = "MyInterface")
{
Symbols.MapTypeToPackage(interfaceType.FullyQualifiedName, package);
string suffix = interfaceType.Namespace != null ? "." + interfaceType.Namespace : "";
Symbols.MapNamespace(interfaceType.Assembly + suffix, @namespace);
Symbols.MapTypeName(interfaceType.FullyQualifiedName, typeName, JsonModel.Spec.TypeKind.Interface);
Symbols.MapTypeName(interfaceType.FullyQualifiedName, typeName, TypeKind.Interface);

var generator = new InterfaceProxyGenerator(package, interfaceType, Symbols, Namespaces);

Expand All @@ -37,10 +39,10 @@ public void IncludesAttribute()

string actual = Render(interfaceType);
string expected =
@"namespace MyNamespace
@"namespace MyNamespace
{
[JsiiInterfaceProxy(typeof(IMyInterface), ""myInterfaceFqn"")]
internal class MyInterfaceProxy : DeputyBase, IMyInterface
[JsiiTypeProxy(typeof(IMyInterface), ""myInterfaceFqn"")]
internal sealed class MyInterfaceProxy : DeputyBase, IMyInterface
{
private MyInterfaceProxy(ByRefValue reference): base(reference)
{
Expand All @@ -59,24 +61,24 @@ public void IncludesMethods()
"myPackage",
"myInterface",
"myNamespace",
methods: new Method[] { new Method(false, false, true, name: "myMethod") }
methods: new Method[] {new Method(false, false, true, name: "myMethod")}
);

Symbols.MapMethodName("myInterfaceFqn", "myMethod", "MyMethod");

string actual = Render(interfaceType);
string expected =
@"namespace MyNamespace
@"namespace MyNamespace
{
[JsiiInterfaceProxy(typeof(IMyInterface), ""myInterfaceFqn"")]
internal class MyInterfaceProxy : DeputyBase, IMyInterface
[JsiiTypeProxy(typeof(IMyInterface), ""myInterfaceFqn"")]
internal sealed class MyInterfaceProxy : DeputyBase, IMyInterface
{
private MyInterfaceProxy(ByRefValue reference): base(reference)
{
}
[JsiiMethod(""myMethod"", null, ""[]"")]
public virtual void MyMethod()
public void MyMethod()
{
InvokeInstanceVoidMethod(new object[]{});
}
Expand All @@ -94,53 +96,53 @@ public void IncludesAncestorInterfaceMethods()
"myPackage",
"myAncestorInterface",
"myNamespace",
methods: new [] { new Method(false, false, true, name: "myAncestorMethod") }
methods: new[] {new Method(false, false, true, name: "myAncestorMethod")}
);
InterfaceType baseInterface = new InterfaceType
(
"myBaseInterfaceFqn",
"myPackage",
"myBaseInterface",
"myNamespace",
methods: new[] { new Method(false, false, true, name: "myBaseMethod") },
interfaces: new[] { new TypeReference("myAncestorInterfaceFqn") }
methods: new[] {new Method(false, false, true, name: "myBaseMethod")},
interfaces: new[] {new TypeReference("myAncestorInterfaceFqn")}
);
InterfaceType interfaceType = new InterfaceType
(
"myInterfaceFqn",
"myPackage",
"myInterface",
"myNamespace",
interfaces: new[] { new TypeReference("myBaseInterfaceFqn") }
interfaces: new[] {new TypeReference("myBaseInterfaceFqn")}
);

Symbols.MapTypeName("myAncestorInterfaceFqn", "MyAncestorInterface", JsonModel.Spec.TypeKind.Interface);
Symbols.MapTypeName("myAncestorInterfaceFqn", "MyAncestorInterface", TypeKind.Interface);
Symbols.MapFullyQualifiedNameToType("myAncestorInterfaceFqn", ancestorInterface);
Symbols.MapMethodName("myInterfaceFqn", "myAncestorMethod", "MyAncestorMethod");

Symbols.MapTypeName("myBaseInterfaceFqn", "MyBaseInterface", JsonModel.Spec.TypeKind.Interface);
Symbols.MapTypeName("myBaseInterfaceFqn", "MyBaseInterface", TypeKind.Interface);
Symbols.MapFullyQualifiedNameToType("myBaseInterfaceFqn", baseInterface);
Symbols.MapMethodName("myInterfaceFqn", "myBaseMethod", "MyBaseMethod");

string actual = Render(interfaceType);
string expected =
@"namespace MyNamespace
@"namespace MyNamespace
{
[JsiiInterfaceProxy(typeof(IMyInterface), ""myInterfaceFqn"")]
internal class MyInterfaceProxy : DeputyBase, IMyInterface
[JsiiTypeProxy(typeof(IMyInterface), ""myInterfaceFqn"")]
internal sealed class MyInterfaceProxy : DeputyBase, IMyInterface
{
private MyInterfaceProxy(ByRefValue reference): base(reference)
{
}
[JsiiMethod(""myBaseMethod"", null, ""[]"")]
public virtual void MyBaseMethod()
public void MyBaseMethod()
{
InvokeInstanceVoidMethod(new object[]{});
}
[JsiiMethod(""myAncestorMethod"", null, ""[]"")]
public virtual void MyAncestorMethod()
public void MyAncestorMethod()
{
InvokeInstanceVoidMethod(new object[]{});
}
Expand All @@ -158,16 +160,16 @@ public void IncludesDocs()
"myPackage",
"myInterface",
"myNamespace",
docs: new Docs { { "foo", "bar" } }
docs: new Docs {{"foo", "bar"}}
);

string actual = Render(interfaceType);
string expected =
@"namespace MyNamespace
@"namespace MyNamespace
{
/// <remarks>foo: bar</remarks>
[JsiiInterfaceProxy(typeof(IMyInterface), ""myInterfaceFqn"")]
internal class MyInterfaceProxy : DeputyBase, IMyInterface
[JsiiTypeProxy(typeof(IMyInterface), ""myInterfaceFqn"")]
internal sealed class MyInterfaceProxy : DeputyBase, IMyInterface
{
private MyInterfaceProxy(ByRefValue reference): base(reference)
{
Expand All @@ -181,7 +183,7 @@ private MyInterfaceProxy(ByRefValue reference): base(reference)
public void EnvironmentRegression()
{
const string json =
@" {
@" {
""docs"": {
""comment"": ""Models an AWS execution environment, for use within the CDK toolkit.""
},
Expand Down Expand Up @@ -231,37 +233,38 @@ public void EnvironmentRegression()
Symbols.MapPropertyName("jsii$aws_cdk_cx_api$.Environment", "account", "Account");
Symbols.MapPropertyName("jsii$aws_cdk_cx_api$.Environment", "region", "Region");

string actual = Render(interfaceType, package: "aws-cdk-cx-api", @namespace: "Aws.Cdk.CxApi", typeName: "Environment");
string actual = Render(interfaceType, package: "aws-cdk-cx-api", @namespace: "Aws.Cdk.CxApi",
typeName: "Environment");
string expected =
@"namespace Aws.Cdk.CxApi
@"namespace Aws.Cdk.CxApi
{
/// <summary>Models an AWS execution environment, for use within the CDK toolkit.</summary>
[JsiiInterfaceProxy(typeof(IEnvironment), ""jsii$aws_cdk_cx_api$.Environment"")]
internal class EnvironmentProxy : DeputyBase, IEnvironment
[JsiiTypeProxy(typeof(IEnvironment), ""jsii$aws_cdk_cx_api$.Environment"")]
internal sealed class EnvironmentProxy : DeputyBase, IEnvironment
{
private EnvironmentProxy(ByRefValue reference): base(reference)
{
}
/// <summary>The arbitrary name of this environment (user-set, or at least user-meaningful) </summary>
[JsiiProperty(""name"", ""{\""primitive\"":\""string\""}"")]
public virtual string Name
public string Name
{
get => GetInstanceProperty<string>();
set => SetInstanceProperty(value);
}
/// <summary>The 12-digit AWS account ID for the account this environment deploys into </summary>
[JsiiProperty(""account"", ""{\""primitive\"":\""string\""}"")]
public virtual string Account
public string Account
{
get => GetInstanceProperty<string>();
set => SetInstanceProperty(value);
}
/// <summary>The AWS region name where this environment deploys into </summary>
[JsiiProperty(""region"", ""{\""primitive\"":\""string\""}"")]
public virtual string Region
public string Region
{
get => GetInstanceProperty<string>();
set => SetInstanceProperty(value);
Expand All @@ -271,4 +274,4 @@ public virtual string Region
Assert.Equal(expected, actual, ignoreLineEndingDifferences: true);
}
}
}
}
15 changes: 8 additions & 7 deletions ...rator/src/Amazon.JSII.Generator.UnitTests/Interface/InterfaceProxyMethodGeneratorTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Xunit;
using TypeKind = Amazon.JSII.JsonModel.Spec.TypeKind;

namespace Amazon.JSII.Generator.UnitTests.Interface
{
Expand All @@ -23,7 +24,7 @@ string Render(Method method)

Symbols.MapTypeToPackage("myInterfaceFqn", "myPackage");
Symbols.MapNamespace("", "MyNamespace");
Symbols.MapTypeName("myInterfaceFqn", "MyInterface", JsonModel.Spec.TypeKind.Interface);
Symbols.MapTypeName("myInterfaceFqn", "MyInterface", TypeKind.Interface);

var generator = new InterfaceProxyMethodGenerator(interfaceType, method, Symbols, Namespaces);

Expand All @@ -41,7 +42,7 @@ public void IncludesAttribute()
string actual = Render(method);
string expected =
@"[JsiiMethod(""myMethod"", null, ""[]"")]
public virtual void MyMethod()
public void MyMethod()
{
InvokeInstanceVoidMethod(new object[]{});
}";
Expand All @@ -62,14 +63,14 @@ public void IncluesParameters()
);

Symbols.MapMethodName("myInterfaceFqn", "myMethod", "MyMethod");
Symbols.MapTypeName("myParamTypeFqn", "MyParamType", JsonModel.Spec.TypeKind.Class);
Symbols.MapTypeName("myParamTypeFqn", "MyParamType", TypeKind.Class);
Symbols.MapParameterName("myParam", "myParam");
Symbols.MapParameterName("event", "@event");

string actual = Render(method);
string expected =
@"[JsiiMethod(""myMethod"", null, ""[{\""name\"":\""myParam\"",\""type\"":{\""fqn\"":\""myParamTypeFqn\""}},{\""name\"":\""event\"",\""type\"":{\""primitive\"":\""string\""}}]"")]
public virtual void MyMethod(MyParamType myParam, string @event)
public void MyMethod(MyParamType myParam, string @event)
{
InvokeInstanceVoidMethod(new object[]{myParam, @event});
}";
Expand All @@ -90,7 +91,7 @@ public void DoesNotIncludeDocs()
string actual = Render(method);
string expected =
@"[JsiiMethod(""myMethod"", null, ""[]"")]
public virtual void MyMethod()
public void MyMethod()
{
InvokeInstanceVoidMethod(new object[]{});
}";
Expand All @@ -107,12 +108,12 @@ public void IncludesReturnTypeForNonVoid()
);

Symbols.MapMethodName("myInterfaceFqn", "myMethod", "MyMethod");
Symbols.MapTypeName("myReturnTypeFqn", "MyReturnType", JsonModel.Spec.TypeKind.Class);
Symbols.MapTypeName("myReturnTypeFqn", "MyReturnType", TypeKind.Class);

string actual = Render(method);
string expected =
@"[JsiiMethod(""myMethod"", ""{\""fqn\"":\""myReturnTypeFqn\""}"", ""[]"")]
public virtual MyReturnType MyMethod()
public MyReturnType MyMethod()
{
return InvokeInstanceMethod<MyReturnType>(new object[]{});
}";
Expand Down
14 changes: 12 additions & 2 deletions ...ages/jsii-dotnet-generator/src/Amazon.JSII.Generator/Class/AbstractClassProxyGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,12 @@ IEnumerable<Method> GetAllMethodsRecurse(Type currentType, IEnumerable<Method> m
return methods;
}

// Only get the first declaration encountered, and keep it if it is abstract.
/*
Only get the first declaration encountered, and keep it if it is abstract. The list contains ALL
methods and properties encountered, in the order encountered. An abstract class can have concrete
implementations. Therefore, we only generate methods/properties if the first member encountered
is unimplemented.
*/
return GetAllMethodsRecurse(type, Enumerable.Empty<Method>())
.GroupBy(m => (m.Name,
string.Join("",
Expand Down Expand Up @@ -154,7 +159,12 @@ IEnumerable<Property> GetAllPropertiesRecurse(Type currentType, IEnumerable<Prop
return properties;
}

// Only get the first declaration encountered, and keep it if it is abstract.
/*
Only get the first declaration encountered, and keep it if it is abstract. The list contains ALL
methods and properties encountered, in the order encountered. An abstract class can have concrete
implementations. Therefore, we only generate methods/properties if the first member encountered
is unimplemented.
*/
return GetAllPropertiesRecurse(type, Enumerable.Empty<Property>())
.GroupBy(p => p.Name)
.Select(g => g.First())
Expand Down
4 changes: 0 additions & 4 deletions ...sii-dotnet-generator/src/Amazon.JSII.Generator/Class/AbstractClassProxyMethodGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,6 @@ protected override IEnumerable<SyntaxKind> GetModifierKeywords()
{
yield return SyntaxKind.OverrideKeyword;
}
else
{
yield return SyntaxKind.VirtualKeyword;
}
}

protected override BlockSyntax GetBody()
Expand Down
4 changes: 0 additions & 4 deletions ...i-dotnet-generator/src/Amazon.JSII.Generator/Class/AbstractClassProxyPropertyGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,6 @@ protected override IEnumerable<SyntaxKind> GetModifierKeywords()
{
yield return SyntaxKind.OverrideKeyword;
}
else
{
yield return SyntaxKind.VirtualKeyword;
}
}
}
}
Loading

0 comments on commit bdfe854

Please sign in to comment.