Skip to content

Fix trait codegen for List or Map traits with annotations or Javadocs #2729

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

Merged
merged 3 commits into from
Aug 5, 2025
Merged
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 @@ -4,7 +4,9 @@
*/
package software.amazon.smithy.traitcodegen.integrations.annotations;

import software.amazon.smithy.model.shapes.Shape;
import software.amazon.smithy.model.traits.DeprecatedTrait;
import software.amazon.smithy.model.traits.TraitDefinition;
import software.amazon.smithy.traitcodegen.sections.ClassSection;
import software.amazon.smithy.traitcodegen.sections.EnumVariantSection;
import software.amazon.smithy.traitcodegen.sections.GetterSection;
Expand All @@ -27,7 +29,8 @@ public boolean isIntercepted(CodeSection section) {
if (section instanceof ClassSection) {
return ((ClassSection) section).shape().hasTrait(DeprecatedTrait.ID);
} else if (section instanceof GetterSection) {
return ((GetterSection) section).shape().hasTrait(DeprecatedTrait.ID);
Shape shape = ((GetterSection) section).shape();
return shape.hasTrait(DeprecatedTrait.ID) && !shape.hasTrait(TraitDefinition.ID);
} else if (section instanceof EnumVariantSection) {
return ((EnumVariantSection) section).memberShape().hasTrait(DeprecatedTrait.ID);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package software.amazon.smithy.traitcodegen.integrations.javadoc;

import software.amazon.smithy.model.shapes.Shape;
import software.amazon.smithy.model.traits.TraitDefinition;
import software.amazon.smithy.traitcodegen.sections.ClassSection;
import software.amazon.smithy.traitcodegen.sections.EnumVariantSection;
import software.amazon.smithy.traitcodegen.sections.GetterSection;
Expand Down Expand Up @@ -40,6 +41,9 @@ public void prepend(TraitCodegenWriter writer, CodeSection section) {
shape = ((ClassSection) section).shape();
} else if (section instanceof GetterSection) {
shape = ((GetterSection) section).shape();
if (shape.hasTrait(TraitDefinition.ID)) {
return;
}
} else if (section instanceof EnumVariantSection) {
shape = ((EnumVariantSection) section).memberShape();
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,48 @@ void unstableAnnotationOnEnumVariant() {
assertTrue(fileContents.contains(expected));
}

@Test
void deprecatedAnnotationOnListTrait() {
String fileContents = getFileContentsFromShapeName("DeprecatedList", true);
String expected = "@Deprecated\n" +
"@SmithyGenerated\n" +
"public final class DeprecatedListTrait";
assertTrue(fileContents.contains(expected));
}

@Test
void noDeprecatedAnnotationOnListGetValues() {
String fileContents = getFileContentsFromShapeName("DeprecatedList", true);
String expected = " }\n\n" +
" public List<Integer> getValues() {";
assertTrue(fileContents.contains(expected));
}

@Test
void deprecatedAnnotationOnMapTrait() {
String fileContents = getFileContentsFromShapeName("DeprecatedMap", true);
String expected = "@Deprecated\n" +
"@SmithyGenerated\n" +
"public final class DeprecatedMapTrait";
assertTrue(fileContents.contains(expected));
}

@Test
void noDeprecatedAnnotationOnMapGetValues() {
String fileContents = getFileContentsFromShapeName("DeprecatedMap", true);
String expected = " }\n\n" +
" public Map<String, Integer> getValues() {";
assertTrue(fileContents.contains(expected));
}

@Test
void deprecatedAnnotationOnListMember() {
String fileContents = getFileContentsFromShapeName("DeprecatedStructure", true);
String expected = " @Deprecated\n" +
" public Optional<Map<String, Integer>> getDeprecatedMap() {";
assertTrue(fileContents.contains(expected));
}

private String getFileContentsFromShapeName(String className, boolean isTrait) {
String suffix = isTrait ? "Trait" : "";
String path = String.format("com/example/traits/%s%s.java", className, suffix);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,57 @@ void allDocumentationIncludedTogetherEnumVariant() {
assertTrue(fileContents.contains(expected));
}

@Test
void deprecatedAnnotationAndNoteForListTrait() {
String fileContents = getFileContentsFromShapeName("DeprecatedList", true);
String expectedForClass = "/**\n" +
" * A deprecated list trait\n" +
" *\n" +
" * @see <a href=\"https://example.com\">Example</a>\n" +
" * @since 4.5\n" +
" * @deprecated As of yesterday. A message\n" +
" */\n" +
"@Deprecated\n" +
"@SmithyGenerated\n" +
"public final class DeprecatedListTrait";
String expectedForGetter = " }\n\n" +
" public List<Integer> getValues() {";
assertTrue(fileContents.contains(expectedForClass));
assertTrue(fileContents.contains(expectedForGetter));
}

@Test
void deprecatedAnnotationAndNoteForMapTrait() {
String fileContents = getFileContentsFromShapeName("DeprecatedMap", true);
String expectedForClass = "/**\n" +
" * A deprecated map trait\n" +
" *\n" +
" * @see <a href=\"https://example.com\">Example</a>\n" +
" * @since 4.5\n" +
" * @deprecated As of yesterday. A message\n" +
" */\n" +
"@Deprecated\n" +
"@SmithyGenerated\n" +
"public final class DeprecatedMapTrait";
String expectedForGetter = " }\n\n" +
" public Map<String, Integer> getValues() {";
assertTrue(fileContents.contains(expectedForClass));
assertTrue(fileContents.contains(expectedForGetter));
}

@Test
void deprecatedAnnotationAndNoteForListMember() {
String fileContents = getFileContentsFromShapeName("DeprecatedStructure", true);
String expected = " /**\n" +
" * @deprecated As of yesterday. A message\n" +
" */\n" +
" @Deprecated\n" +
" public Optional<List<Integer>> getDeprecatedList() {\n" +
" return Optional.ofNullable(deprecatedList);\n" +
" }";
assertTrue(fileContents.contains(expected));
}

private String getFileContentsFromShapeName(String className, boolean isTrait) {
String suffix = isTrait ? "Trait" : "";
String path = String.format("com/example/traits/%s%s.java", className, suffix);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ structure DeprecatedStructure {
/// Has docs in addition to deprecated
@deprecated
deprecatedWithDocs: String

@deprecated
deprecatedMap: MemberMap
}

map MemberMap {
key: String
value: Integer
}

@trait
Expand All @@ -41,4 +49,15 @@ enum EnumWithAnnotations {
UNSTABLE
}

@trait
@deprecated
list DeprecatedList {
member: Integer
}

@trait
@deprecated
map DeprecatedMap {
key: String
value: Integer
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@ structure DeprecatedStructure {
/// Has docs in addition to deprecated
@deprecated(message: "A message", since: "yesterday")
deprecatedWithDocs: String

@deprecated(message: "A message", since: "yesterday")
deprecatedList: MemberList
}

list MemberList {
member: Integer
}

@trait
Expand Down Expand Up @@ -78,3 +85,21 @@ enum EnumVariantsTest {
B
}

/// A deprecated list trait
@trait
@deprecated(message: "A message", since: "yesterday")
@externalDocumentation(Example: "https://example.com")
@since("4.5")
list DeprecatedList {
member: Integer
}

/// A deprecated map trait
@trait
@deprecated(message: "A message", since: "yesterday")
@externalDocumentation(Example: "https://example.com")
@since("4.5")
map DeprecatedMap {
key: String
value: Integer
}
Loading