Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(.NET): align .NET w/ latest Java & Dafny #243

Draft
wants to merge 13 commits into
base: main-1.x
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,5 @@ module TestComAmazonawsSqs {
// Just asserting the request is successful.
// I could expect no queues but the test account might create some some day,
// and I don't want this to be brittle.
expect QueueUrls.Some?;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

import javax.annotation.Nonnull;

public class DafnyApiCodegen {
private final Model model;
private final ServiceShape serviceShape;
Expand Down Expand Up @@ -78,11 +80,7 @@ public Map<Path, TokenTree> generate() {
// so that I can include them.
final TokenTree generatedTypes = TokenTree
.of(
model
.shapes()
.filter(shape -> ModelUtils.isInServiceNamespace(shape.getId(), serviceShape))
// Sort by shape ID for deterministic generated code
.collect(Collectors.toCollection(TreeSet::new))
getShapes(model, serviceShape)
.stream()
.map(this::generateCodeForShape)
.flatMap(Optional::stream)
Expand Down Expand Up @@ -177,6 +175,15 @@ public Map<Path, TokenTree> generate() {
return Map.of(path, fullCode);
}

@Nonnull
public static TreeSet<Shape> getShapes(final Model model, final ServiceShape serviceShape) {
return model
.shapes()
.filter(shape -> ModelUtils.isInServiceNamespace(shape.getId(), serviceShape))
// Sort by shape ID for deterministic generated code
.collect(Collectors.toCollection(TreeSet::new));
}

public Map<Path, TokenTree> generateWrappedAbstractServiceModule(final Path outputDafny) {
if (serviceShape.hasTrait(ServiceTrait.class)) {
// TODO move the AWS SDK branch over here.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,10 @@

import java.util.Set;
import java.util.TreeSet;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import software.amazon.polymorph.utils.DafnyNameResolverHelpers;
import software.amazon.polymorph.utils.ModelUtils;
import software.amazon.polymorph.utils.SmithyConstants;
import software.amazon.polymorph.utils.Token;
import software.amazon.polymorph.utils.TokenTree;
import software.amazon.smithy.model.Model;
Expand All @@ -25,7 +24,6 @@
* for AWS SDK-specific types.
*/
public class AwsSdkTypeConversionCodegen extends TypeConversionCodegen {
private static final ShapeId SMITHY_STRING_SHAPE_ID = ShapeId.from("smithy.api#String");

public AwsSdkTypeConversionCodegen(final Model model, final ServiceShape serviceShape) {
super(model, serviceShape,
Expand All @@ -35,7 +33,10 @@ public AwsSdkTypeConversionCodegen(final Model model, final ServiceShape service
@Override
public Set<ShapeId> findShapeIdsToConvert() {
final Set<ShapeId> shapeIds = super.findShapeIdsToConvert();
shapeIds.add(SMITHY_STRING_SHAPE_ID); // needed for converting the message of an unknown error type
shapeIds.add(SmithyConstants.SMITHY_STRING_SHAPE_ID); // needed for converting the message of an unknown error type
// in .NET, there DDBv2 does not have entries for these Smithy Shapes.
shapeIds.remove(ShapeId.fromParts("com.amazonaws.dynamodb", "KinesisStreamingDestinationOutput"));
shapeIds.remove(ShapeId.fromParts("com.amazonaws.dynamodb", "KinesisStreamingDestinationInput"));
return shapeIds;
}

Expand All @@ -48,28 +49,6 @@ public TypeConverter generateStructureConverter(final StructureShape structureSh
return super.generateStructureConverter(structureShape);
}

/**
* We can't call the {@code IsSet} methods on AWS SDK classes' member properties because they're internal.
* The best we can do is to call the properties' getters, which calls {@code GetValueOrDefault}, which in turn may
* improperly coalesce absent optional values to 0 (for example).
*/
@Override
public TokenTree generateExtractOptionalMember(MemberShape memberShape) {
final String type;
if (StringUtils.equals(nameResolver.baseTypeForShape(memberShape.getId()),
AwsSdkDotNetNameResolver.DDB_ATTRIBUTE_VALUE_MODEL_NAMESPACE)) {
type = AwsSdkDotNetNameResolver.DDB_V2_ATTRIBUTE_VALUE;
} else {
type = nameResolver.baseTypeForShape(memberShape.getId());
}
final String varName = nameResolver.variableNameForClassProperty(memberShape);
final String propertyName = nameResolver.classPropertyForStructureMember(memberShape);
return TokenTree.of(
type,
varName,
"= value.%s;".formatted(propertyName));
}

@Override
protected boolean enumListAndMapMembersAreStringsInCSharp() {
return true;
Expand Down Expand Up @@ -156,7 +135,7 @@ protected TokenTree errorFromDanyBody(final TreeSet<StructureShape> errorShapes)
}
return Token.of("case %1$s dafnyVal:\nreturn %2$s(dafnyVal);".formatted(
nameResolver.dafnyTypeForShape(modeledErrorShapeId),
DotNetNameResolver.typeConverterForShape(modeledErrorShapeId, FROM_DAFNY)
DotNetNameResolver.qualifiedTypeConverter(modeledErrorShapeId, FROM_DAFNY)
));
})).lineSeparated();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import software.amazon.polymorph.utils.AwsSdkNameResolverHelpers;
import software.amazon.polymorph.utils.DafnyNameResolverHelpers;
import software.amazon.polymorph.utils.ModelUtils;
import software.amazon.polymorph.utils.SmithyConstants;
import software.amazon.smithy.model.Model;
import software.amazon.smithy.model.shapes.*;
import software.amazon.smithy.model.traits.EnumTrait;
Expand All @@ -33,6 +34,8 @@
import java.util.regex.Pattern;
import java.util.stream.Stream;

import static software.amazon.polymorph.utils.AwsSdkNameResolverHelpers.isInAwsSdkNamespace;

/**
* Provides a consistent mapping between names of model Shapes and generated identifiers.
*/
Expand Down Expand Up @@ -61,9 +64,9 @@ public DotNetNameResolver(Model model, ServiceShape serviceShape) {
/**
* Returns the C# namespace containing the C# implementation/interface for the given shape ID.
*/
public String namespaceForShapeId(final ShapeId shapeId) {
public static String namespaceForShapeId(final ShapeId shapeId) {
// TODO remove special AWS SDK special-case when https://github.com/awslabs/polymorph/issues/7 is resolved
final Function<String, String> segmentMapper = AwsSdkNameResolverHelpers.isInAwsSdkNamespace(shapeId)
final Function<String, String> segmentMapper = isInAwsSdkNamespace(shapeId)
? StringUtils::capitalize
: DotNetNameResolver::capitalizeNamespaceSegment;

Expand Down Expand Up @@ -346,13 +349,14 @@ protected String baseTypeForOptionalMember(final MemberShape memberShape) {
protected String baseTypeForService(final ServiceShape serviceShape) {
final ShapeId shapeId = serviceShape.getId();

if (AwsSdkNameResolverHelpers.isInAwsSdkNamespace(shapeId)) {
if (isInAwsSdkNamespace(shapeId)) {
return new AwsSdkDotNetNameResolver(model, serviceShape).baseTypeForService(serviceShape);
}
final LocalServiceTrait localServiceTrait = serviceShape.expectTrait(LocalServiceTrait.class);

// Base type for local service is defined here
return "%s.%s".formatted(
namespaceForShapeId(shapeId), shapeId.getName());
namespaceForShapeId(shapeId), localServiceTrait.getSdkId());
}

protected String baseTypeForResource(final ResourceShape resourceShape) {
Expand All @@ -377,7 +381,7 @@ public String baseTypeForShape(final ShapeId shapeId) {
() -> String.format("No native type for prelude shape %s", shapeId));
}

if (!AwsSdkNameResolverHelpers.isInAwsSdkNamespace(shapeId)) {
if (!isInAwsSdkNamespace(shapeId)) {
// The shape is not in an AWS Service,
// so use the base type switch
return baseTypeSwitch(shape);
Expand Down Expand Up @@ -429,7 +433,7 @@ private String nullableTypeForStructureMember(final MemberShape memberShape) {
* Returns the name of the (private) structure class field for the given member shape.
*/
public String classFieldForStructureMember(final MemberShape memberShape) {
return "_%s".formatted(StringUtils.uncapitalize(memberShape.getMemberName()));
return "%s".formatted(memberShape.getMemberName());
}

/**
Expand All @@ -440,10 +444,21 @@ public String classPropertyForStructureMember(final MemberShape memberShape) {
}

/**
* Returns the name of the given member shape's IsSet method.
* Returns the logic to determine if a StructureMember is set.
*/
public String isSetMethodForStructureMember(final MemberShape memberShape) {
return "IsSet%s".formatted(classPropertyForStructureMember(memberShape));
public String isSetForStructureMember(final MemberShape memberShape) {
final String property = classPropertyForStructureMember(memberShape);
final ShapeId containerId = memberShape.getContainer();
// When reading AWS-SDK objects,
if (isInAwsSdkNamespace(containerId)) {
// don't use IsSet<memberName> but use !null or !null and !empty check.
final Shape targetShape = model.expectShape(memberShape.getTarget());
if (SmithyConstants.LIST_MAP_SET_SHAPE_TYPES.contains(targetShape.getType())) {
return "%s is {Count: > 0}".formatted(property); // !null and !empty check
}
return "%s != null".formatted(property); // !null check
}
return "IsSet%s()".formatted(property);
}

/**
Expand Down Expand Up @@ -516,12 +531,12 @@ public static String typeConverterForShape(final ShapeId shapeId, final TypeConv
}

/**
* Returns the converter method name for the given shape and type conversion direction, qualified with the type
* conversion class name.
* Returns the converter method name for the given shape and type conversion direction,
* qualified with the .NET Namespace and type conversion class name.
*/
public static String qualifiedTypeConverter(final ShapeId shapeId, final TypeConversionDirection direction) {
final String methodName = DotNetNameResolver.typeConverterForShape(shapeId, direction);
return "%s.%s".formatted(DotNetNameResolver.TYPE_CONVERSION_CLASS_NAME, methodName);
return "%s.%s.%s".formatted(namespaceForShapeId(shapeId), DotNetNameResolver.TYPE_CONVERSION_CLASS_NAME, methodName);
}

/**
Expand Down Expand Up @@ -707,7 +722,7 @@ private String dafnyTypeForOptionalMember(final MemberShape memberShape, final b
private String dafnyTypeForService(final ServiceShape serviceShape) {
final ShapeId serviceShapeId = serviceShape.getId();

if (AwsSdkNameResolverHelpers.isInAwsSdkNamespace(serviceShapeId)) {
if (isInAwsSdkNamespace(serviceShapeId)) {
return "%s.%s"
.formatted(DafnyNameResolverHelpers.dafnyExternNamespaceForShapeId(serviceShapeId),
DafnyNameResolver.traitNameForServiceClient(serviceShape));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ public TokenTree generateStructureClassField(final MemberShape memberShape) {
return TokenTree.of(
Token.of("private"),
typeToken,
Token.of(nameResolver.classFieldForStructureMember(memberShape)),
Token.of("_%s".formatted(nameResolver.classFieldForStructureMember(memberShape))),
Token.of(';'));
}

Expand All @@ -300,8 +300,8 @@ public TokenTree generateStructureClassProperty(final MemberShape memberShape) {
final boolean isValueType = nameResolver.isValueType(memberShape.getTarget());
final String unwrapValue = isValueType ? ".GetValueOrDefault()" : "";

final TokenTree getter = Token.of("get { return this.%s%s; }".formatted(fieldName, unwrapValue));
final TokenTree setter = Token.of("set { this.%s = value; }".formatted(fieldName));
final TokenTree getter = Token.of("get { return this._%s%s; }".formatted(fieldName, unwrapValue));
final TokenTree setter = Token.of("set { this._%s = value; }".formatted(fieldName));

final String type = nameResolver.classPropertyTypeForStructureMember(memberShape);
final String propertyName = nameResolver.classPropertyForStructureMember(memberShape);
Expand All @@ -314,19 +314,19 @@ public TokenTree generateStructureClassProperty(final MemberShape memberShape) {
* @return IsSet method for either reference types or value types
*/
private TokenTree generateIsSetStructureClassProperty(final MemberShape memberShape) {
final String methodName = nameResolver.isSetMethodForStructureMember(memberShape);
final String methodName = nameResolver.isSetForStructureMember(memberShape);
TokenTree body;
if (nameResolver.isValueType(memberShape.getTarget())) {
body = TokenTree.of("return this.%s.HasValue;".formatted(
body = TokenTree.of("return this._%s.HasValue;".formatted(
nameResolver.classFieldForStructureMember(memberShape)));
} else {
body = TokenTree.of("return this.%s != null;".formatted(
body = TokenTree.of("return this._%s != null;".formatted(
nameResolver.classFieldForStructureMember(memberShape)));
}
// The correctness of these types are improved by making this public
// Then customers in native runtimes can use these methods to integrate
// the state of this structure
return TokenTree.of("public bool", methodName, "()").append(body.braced());
return TokenTree.of("public bool", methodName).append(body.braced());
}

/**
Expand All @@ -349,10 +349,10 @@ private TokenTree generateStructureValidateMethod(final StructureShape structure
final TokenTree checks = TokenTree.of(ModelUtils.streamStructureMembers(structureShape)
.filter(MemberShape::isRequired)
.map(memberShape -> {
final String isSetMethod = nameResolver.isSetMethodForStructureMember(memberShape);
final String isSetMethod = nameResolver.isSetForStructureMember(memberShape);
final String propertyName = nameResolver.classPropertyForStructureMember(memberShape);
return Token.of("""
if (!%s()) throw new System.ArgumentException("Missing value for required property '%s'");
if (!%s) throw new System.ArgumentException("Missing value for required property '%s'");
""".formatted(isSetMethod, propertyName));
})).braced();

Expand All @@ -366,8 +366,8 @@ private TokenTree generateUnionValidateMethod(final UnionShape unionShape) {
.of("var numberOfPropertiesSet =")
.append(TokenTree
.of( ModelUtils.streamUnionMembers(unionShape)
.map(memberShape -> nameResolver.isSetMethodForStructureMember(memberShape))
.map(isSet -> TokenTree.of("Convert.ToUInt16(%s())".formatted(isSet))))
.map(nameResolver::isSetForStructureMember)
.map(isSet -> TokenTree.of("Convert.ToUInt16(%s)".formatted(isSet))))
.separated(Token.of("+\n")))
.append(Token.of(";"));

Expand Down
Loading