Skip to content

Commit 03c3a10

Browse files
ShubhamChaturvedi7Shubham Chaturvedi
ShubhamChaturvedi7
and
Shubham Chaturvedi
authored
Revert pr 496 (#798)
*Issue #, if available:* *Description of changes:* This PR reverts the commit for the change: #496 since it's not released to mpl yet. By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice. --------- Co-authored-by: Shubham Chaturvedi <scchatur@amazon.com>
1 parent a75a452 commit 03c3a10

File tree

8 files changed

+51
-64
lines changed

8 files changed

+51
-64
lines changed

Diff for: ‎.gitmodules

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,4 @@
44
[submodule "TestModels/dafny-dependencies/dafny"]
55
path = TestModels/dafny-dependencies/dafny
66
url = https://github.com/dafny-lang/dafny.git
7-
branch = actions-and-streaming-stdlibs
7+
branch = actions-and-streaming-stdlibs

Diff for: ‎TestModels/Constraints/Model/Constraints.smithy

+6-11
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ structure SimpleConstraintsConfig {
2222

2323
// This is just a sanity check on the smokeTests support.
2424
// We need to actually convert all the tests in test/WrappedSimpleConstraintsImplTest.dfy
25-
// to smoke tests now that https://github.com/smithy-lang/smithy-dafny/issues/278
25+
// to smoke tests once https://github.com/smithy-lang/smithy-dafny/issues/278
2626
// is fixed.
2727
@smithy.test#smokeTests([
2828
{
@@ -44,22 +44,17 @@ structure SimpleConstraintsConfig {
4444
success: {}
4545
}
4646
},
47-
{
48-
id: "GetConstraintsSuccessNoParams"
49-
vendorParams: {
50-
RequiredString: "foobar",
51-
}
52-
params: {}
53-
expect: {
54-
success: {}
55-
}
56-
},
5747
{
5848
id: "GetConstraintsFailure"
5949
vendorParams: {
6050
RequiredString: "foobar",
6151
}
6252
params: {
53+
// These two always have to be present because of https://github.com/smithy-lang/smithy-dafny/issues/278,
54+
// because otherwise they are interpreted as 0.
55+
OneToTen: 5,
56+
GreaterThanOne: 2,
57+
// This is the member that's actually invalid
6358
NonEmptyBlob: ""
6459
}
6560
expect: {

Diff for: ‎codegen/smithy-dafny-codegen/src/main/java/software/amazon/polymorph/smithydotnet/AwsSdkDotNetNameResolver.java

-7
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,6 @@ private boolean isGeneratedInSdk(final ShapeId shapeId) {
4646
}
4747

4848
@Override
49-
protected String baseTypeForMember(final MemberShape memberShape) {
50-
// The AWS SDK uses primitive types for required members.
51-
return memberShapeIsOptional(memberShape)
52-
? super.baseTypeForMember(memberShape)
53-
: baseTypeForShape(memberShape.getTarget());
54-
}
55-
5649
protected String baseTypeForEnum(final EnumShape enumShape) {
5750
if (isGeneratedInSdk(enumShape.getId())) {
5851
return "%s.%s".formatted(

Diff for: ‎codegen/smithy-dafny-codegen/src/main/java/software/amazon/polymorph/smithydotnet/AwsSdkTypeConversionCodegen.java

+1-5
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,7 @@ public TokenTree generateExtractOptionalMember(MemberShape memberShape) {
8282
final String propertyName = nameResolver.classPropertyForStructureMember(
8383
memberShape
8484
);
85-
return TokenTree.of(
86-
type,
87-
varName,
88-
"= (%s)value.%s;".formatted(type, propertyName)
89-
);
85+
return TokenTree.of(type, varName, "= value.%s;".formatted(propertyName));
9086
}
9187

9288
@Override

Diff for: ‎codegen/smithy-dafny-codegen/src/main/java/software/amazon/polymorph/smithydotnet/DotNetNameResolver.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -412,8 +412,9 @@ protected String baseTypeForUnion(final UnionShape unionShape) {
412412
}
413413

414414
protected String baseTypeForMember(final MemberShape memberShape) {
415-
// We always use nullable types for safety.
416-
return baseTypeForOptionalMember(memberShape);
415+
final String baseType = baseTypeForShape(memberShape.getTarget());
416+
final boolean isOptional = memberShapeIsOptional(memberShape);
417+
return isOptional ? baseTypeForOptionalMember(memberShape) : baseType;
417418
}
418419

419420
protected String baseTypeForOptionalMember(final MemberShape memberShape) {

Diff for: ‎codegen/smithy-dafny-codegen/src/main/java/software/amazon/polymorph/smithydotnet/TypeConversionCodegen.java

+15-21
Original file line numberDiff line numberDiff line change
@@ -797,6 +797,20 @@ public TypeConverter generateMemberConverter(final MemberShape memberShape) {
797797
TO_DAFNY
798798
);
799799

800+
if (!nameResolver.memberShapeIsOptional(memberShape)) {
801+
final TokenTree fromDafnyBody = Token.of(
802+
"return %s(value);".formatted(targetFromDafnyConverterName)
803+
);
804+
final TokenTree toDafnyBody = Token.of(
805+
"return %s(value);".formatted(targetToDafnyConverterName)
806+
);
807+
return buildConverterFromMethodBodies(
808+
memberShape,
809+
fromDafnyBody,
810+
toDafnyBody
811+
);
812+
}
813+
800814
String cSharpTypeUnModified;
801815
if (
802816
StringUtils.equals(
@@ -814,23 +828,6 @@ public TypeConverter generateMemberConverter(final MemberShape memberShape) {
814828
cSharpTypeUnModified.substring(0, (cSharpTypeUnModified.length() - 1));
815829
}
816830

817-
if (!nameResolver.memberShapeIsOptional(memberShape)) {
818-
final TokenTree fromDafnyBody = Token.of(
819-
"return %s(value);".formatted(targetFromDafnyConverterName)
820-
);
821-
final TokenTree toDafnyBody = Token.of(
822-
"return %s((%s)value);".formatted(
823-
targetToDafnyConverterName,
824-
cSharpTypeUnModified
825-
)
826-
);
827-
return buildConverterFromMethodBodies(
828-
memberShape,
829-
fromDafnyBody,
830-
toDafnyBody
831-
);
832-
}
833-
834831
final String cSharpType = cSharpTypeUnModified;
835832
final String cSharpOptionType;
836833
if (
@@ -903,8 +900,6 @@ public TypeConverter generateUnionConverter(final UnionShape unionShape) {
903900
.map(memberShape -> {
904901
final String propertyName =
905902
nameResolver.classPropertyForStructureMember(memberShape);
906-
final String propertyType =
907-
nameResolver.classPropertyTypeForStructureMember(memberShape);
908903
final String memberFromDafnyConverterName = typeConverterForShape(
909904
memberShape.getId(),
910905
FROM_DAFNY
@@ -935,9 +930,8 @@ public TypeConverter generateUnionConverter(final UnionShape unionShape) {
935930
.append(
936931
TokenTree
937932
.of(
938-
"converted.%s = (%s)(%s(concrete.dtor_%s));".formatted(
933+
"converted.%s = %s(concrete.dtor_%s);".formatted(
939934
propertyName,
940-
propertyType,
941935
memberFromDafnyConverterName,
942936
destructorValue
943937
),

Diff for: ‎codegen/smithy-dafny-codegen/src/main/java/software/amazon/polymorph/smithyjava/nameresolver/Native.java

+23-15
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import software.amazon.smithy.model.shapes.Shape;
3333
import software.amazon.smithy.model.shapes.ShapeId;
3434
import software.amazon.smithy.model.shapes.ShapeType;
35+
import software.amazon.smithy.model.traits.BoxTrait;
3536
import software.amazon.smithy.model.traits.EnumTrait;
3637
import software.amazon.smithy.model.traits.StreamingTrait;
3738

@@ -75,13 +76,13 @@ public class Native extends NameResolver {
7576
NATIVE_TYPES_BY_SIMPLE_SHAPE_TYPE =
7677
Map.ofEntries(
7778
Map.entry(ShapeType.BLOB, ClassName.get(ByteBuffer.class)),
78-
Map.entry(ShapeType.BOOLEAN, TypeName.BOOLEAN.box()),
79-
Map.entry(ShapeType.BYTE, TypeName.BYTE.box()),
80-
Map.entry(ShapeType.SHORT, TypeName.SHORT.box()),
81-
Map.entry(ShapeType.INTEGER, TypeName.INT.box()),
82-
Map.entry(ShapeType.LONG, TypeName.LONG.box()),
83-
Map.entry(ShapeType.FLOAT, TypeName.FLOAT.box()),
84-
Map.entry(ShapeType.DOUBLE, TypeName.DOUBLE.box()),
79+
Map.entry(ShapeType.BOOLEAN, TypeName.BOOLEAN),
80+
Map.entry(ShapeType.BYTE, TypeName.BYTE),
81+
Map.entry(ShapeType.SHORT, TypeName.SHORT),
82+
Map.entry(ShapeType.INTEGER, TypeName.INT),
83+
Map.entry(ShapeType.LONG, TypeName.LONG),
84+
Map.entry(ShapeType.FLOAT, TypeName.FLOAT),
85+
Map.entry(ShapeType.DOUBLE, TypeName.DOUBLE),
8586
// TODO: AWS SDK V2 uses Instant, not Date
8687
Map.entry(ShapeType.TIMESTAMP, ClassName.get(Date.class)),
8788
Map.entry(ShapeType.BIG_DECIMAL, ClassName.get(BigDecimal.class)),
@@ -143,14 +144,21 @@ public TypeName typeForShape(final ShapeId shapeId) {
143144
}
144145

145146
return switch (shape.getType()) {
146-
// All primitives are boxed for safety, even if @required
147-
case BOOLEAN,
148-
BYTE,
149-
SHORT,
150-
INTEGER,
151-
LONG,
152-
FLOAT,
153-
DOUBLE -> NATIVE_TYPES_BY_SIMPLE_SHAPE_TYPE.get(shape.getType());
147+
case BOOLEAN, BYTE, SHORT, INTEGER, LONG, FLOAT, DOUBLE -> {
148+
/* From the Smithy Docs:
149+
* "Boolean, byte, short, integer, long, float, and double shapes
150+
* are only considered boxed if they are marked with the box trait.
151+
* All other shapes are always considered boxed."
152+
* https://smithy.io/1.0/spec/core/type-refinement-traits.html#box-trait
153+
* While Smithy Models SHOULD use Smithy Prelude shapes to avoid this confusion,
154+
* they do not have to.
155+
* Hence, the need to check if these shapes have the box trait
156+
*/
157+
final TypeName typeName = NATIVE_TYPES_BY_SIMPLE_SHAPE_TYPE.get(
158+
shape.getType()
159+
);
160+
yield shape.hasTrait(BoxTrait.class) ? typeName.box() : typeName;
161+
}
154162
// For supported simple shapes, just map to native types
155163
case BLOB,
156164
TIMESTAMP,

Diff for: ‎codegen/smithy-dafny-codegen/src/test/java/software/amazon/polymorph/smithyjava/generator/library/Constants.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,10 @@ class Constants {
1111
static String RANGE_TRAIT_INTEGER_BUILD_EXPECTED =
1212
"""
1313
%s {
14-
if (Objects.nonNull(this.zeroToTen()) && this.zeroToTen() < 0) {
14+
if (this._zeroToTenSet && this.zeroToTen() < 0) {
1515
throw new IllegalArgumentException("`zeroToTen` must be greater than or equal to 0");
1616
}
17-
if (Objects.nonNull(this.zeroToTen()) && this.zeroToTen() > 10) {
17+
if (this._zeroToTenSet && this.zeroToTen() > 10) {
1818
throw new IllegalArgumentException("`zeroToTen` must be less than or equal to 10.");
1919
}
2020
%s

0 commit comments

Comments
 (0)