Skip to content

Commit be2f5af

Browse files
joewyzjoewyz
committed
Update plugin tests and address comments
1 parent 561470d commit be2f5af

File tree

15 files changed

+1398
-44
lines changed

15 files changed

+1398
-44
lines changed

‎codegen/codegen-core/src/it/resources/META-INF/smithy/naming/naming-collision.smithy‎

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,6 @@ operation Naming {
2323
innerDeserializer: ShapeSerializer
2424

2525
abc: ABC
26-
27-
union: UnionWithTypeMember
28-
29-
map: Map
30-
31-
list: List
32-
33-
realList: RealList
3426
}
3527
errors: [
3628
IllegalArgumentException
@@ -75,23 +67,3 @@ structure IllegalArgumentException {}
7567
structure ABC {
7668

7769
}
78-
79-
@private
80-
union UnionWithTypeMember {
81-
type: Type
82-
}
83-
84-
@private
85-
structure Map {
86-
87-
}
88-
89-
@private
90-
structure List {
91-
92-
}
93-
94-
@private
95-
list RealList {
96-
member: String
97-
}

‎codegen/codegen-core/src/main/java/software/amazon/smithy/java/codegen/CodegenUtils.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,7 @@ public static String toUpperSnakeCase(String string) {
339339
public static Symbol getInnerTypeEnumSymbol(Symbol symbol) {
340340
return symbol.toBuilder()
341341
.namespace(symbol.getFullName(), ".")
342-
.putProperty("IS_INNER_CLASS", true)
342+
.putProperty(SymbolProperties.IS_LOCALLY_DEFINED, true)
343343
.name("Type")
344344
.build();
345345
}

‎codegen/codegen-core/src/main/java/software/amazon/smithy/java/codegen/SymbolProperties.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ public final class SymbolProperties {
3232
*/
3333
public static final Property<Boolean> IS_JAVA_ARRAY = Property.named("is-java-array");
3434

35+
/**
36+
* Indicates if a symbol represents an inner class.
37+
*/
38+
public static final Property<Boolean> IS_LOCALLY_DEFINED = Property.named("is-locally-defined");
39+
3540
/**
3641
* Method on {@link java.util.Collection} to use to create an immutable copy of the collection type
3742
* a symbol represents.

‎codegen/codegen-core/src/main/java/software/amazon/smithy/java/codegen/generators/UnionGenerator.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
import java.util.List;
1010
import java.util.Objects;
1111
import java.util.function.Consumer;
12-
import software.amazon.smithy.codegen.core.Symbol;
1312
import software.amazon.smithy.codegen.core.SymbolProvider;
1413
import software.amazon.smithy.codegen.core.directed.ContextualDirective;
1514
import software.amazon.smithy.codegen.core.directed.GenerateUnionDirective;
@@ -43,8 +42,8 @@ public void accept(GenerateUnionDirective<CodeGenerationContext, JavaCodegenSett
4342
}
4443
var shape = directive.shape();
4544
directive.context().writerDelegator().useShapeWriter(shape, writer -> {
46-
Symbol innerTypeEnumSymbol = CodegenUtils.getInnerTypeEnumSymbol(directive.symbol());
47-
writer.addLocalDefinedSymbol(innerTypeEnumSymbol);
45+
var innerTypeEnumSymbol = CodegenUtils.getInnerTypeEnumSymbol(directive.symbol());
46+
writer.addLocallyDefinedSymbol(innerTypeEnumSymbol);
4847
writer.pushState(new ClassSection(shape));
4948
var template = """
5049
public abstract class ${shape:T} implements ${serializableStruct:T} {

‎codegen/codegen-core/src/main/java/software/amazon/smithy/java/codegen/writer/JavaImportContainer.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public void importSymbol(Symbol symbol, String s) {
2929
// Do not import primitive types, java.lang standard library imports or inner classes.
3030
if (symbol.expectProperty(SymbolProperties.IS_PRIMITIVE)
3131
|| symbol.getNamespace().startsWith("java.lang")
32-
|| symbol.getProperty("IS_INNER_CLASS").isPresent()) {
32+
|| symbol.getProperty(SymbolProperties.IS_LOCALLY_DEFINED).isPresent()) {
3333
return;
3434
}
3535
Set<Symbol> duplicates = imports.computeIfAbsent(symbol.getName(), sn -> new HashSet<>());

‎codegen/codegen-core/src/main/java/software/amazon/smithy/java/codegen/writer/JavaWriter.java‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public final class JavaWriter extends DeferredSymbolWriter<JavaWriter, JavaImpor
3434
private final String packageNamespace;
3535
private final JavaCodegenSettings settings;
3636
private final String filename;
37-
private final Set<String> localDefinedSymbol = new HashSet<>();
37+
private final Set<String> locallyDefinedNames = new HashSet<>();
3838

3939
public JavaWriter(JavaCodegenSettings settings, String packageNamespace, String filename) {
4040
super(new JavaImportContainer(packageNamespace));
@@ -102,25 +102,27 @@ private void putNameContext() {
102102
Symbol symbol = duplicates.iterator().next();
103103
// Use fully qualified names for java.lang.* if there is duplicate names
104104
// defined under the same package.
105-
String symbolName = packageDefinedNames.contains(symbol.getName())
106-
&& symbol.getNamespace().equals("java.lang") ? symbol.getFullName() : symbol.getName();
107-
putContext(symbol.getFullName().replace("[]", "Array"), symbolName);
105+
String symbolName = symbol.getName();
106+
if (packageDefinedNames.contains(symbol.getName()) && symbol.getNamespace().equals("java.lang")) {
107+
symbolName = symbol.getFullName();
108+
}
109+
putContext(symbol.getFullName(), symbolName);
108110
}
109111
}
110112
}
111113

112114
private String deduplicate(Symbol dupe) {
113-
if (useShortName(dupe)) {
115+
if (useSimpleName(dupe)) {
114116
return dupe.getName();
115117
}
116118
return dupe.getFullName();
117119
}
118120

119-
private boolean useShortName(Symbol dupe) {
120-
// Only inner class symbol and those symbols with non-conflicting names
121+
private boolean useSimpleName(Symbol dupe) {
122+
// Only locally defined symbols and those symbols with non-conflicting names
121123
// under the same namespace can use short name.
122-
if (localDefinedSymbol.contains(dupe.getName())) {
123-
return dupe.getProperty("IS_INNER_CLASS").isPresent();
124+
if (locallyDefinedNames.contains(dupe.getName())) {
125+
return dupe.getProperty(SymbolProperties.IS_LOCALLY_DEFINED).isPresent();
124126
}
125127
return dupe.getNamespace().equals(packageNamespace);
126128
}
@@ -132,8 +134,8 @@ private boolean useShortName(Symbol dupe) {
132134
*
133135
* @param symbol the symbol reserved in this file.
134136
*/
135-
public void addLocalDefinedSymbol(Symbol symbol) {
136-
localDefinedSymbol.add(symbol.getName());
137+
public void addLocallyDefinedSymbol(Symbol symbol) {
138+
locallyDefinedNames.add(symbol.getName());
137139
symbolTable.computeIfAbsent(symbol.getName(), k -> new HashSet<>()).add(symbol);
138140
}
139141

‎codegen/plugins/types-codegen/src/test/resources/software/amazon/smithy/java/codegen/types/test-cases/naming-conflict/expected/BuilderShape.java‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
2+
package software.amazon.smithy.java.example.standalone.model;
3+
4+
import java.util.Objects;
5+
import software.amazon.smithy.java.core.schema.Schema;
6+
import software.amazon.smithy.java.core.schema.SerializableStruct;
7+
import software.amazon.smithy.java.core.schema.ShapeBuilder;
8+
import software.amazon.smithy.java.core.serde.ShapeDeserializer;
9+
import software.amazon.smithy.java.core.serde.ShapeSerializer;
10+
import software.amazon.smithy.java.core.serde.ToStringSerializer;
11+
import software.amazon.smithy.model.shapes.ShapeId;
12+
import software.amazon.smithy.utils.SmithyGenerated;
13+
14+
@SmithyGenerated
15+
public final class BuilderShape implements SerializableStruct {
16+
17+
public static final Schema $SCHEMA = Schemas.BUILDER;
18+
19+
public static final ShapeId $ID = $SCHEMA.id();
20+
21+
private BuilderShape(Builder builder) {
22+
}
23+
24+
@Override
25+
public String toString() {
26+
return ToStringSerializer.serialize(this);
27+
}
28+
29+
@Override
30+
public boolean equals(Object other) {
31+
if (other == this) {
32+
return true;
33+
}
34+
return other != null && getClass() == other.getClass();
35+
}
36+
37+
@Override
38+
public int hashCode() {
39+
return Objects.hash();
40+
}
41+
42+
@Override
43+
public Schema schema() {
44+
return $SCHEMA;
45+
}
46+
47+
@Override
48+
public void serializeMembers(ShapeSerializer serializer) {
49+
50+
}
51+
52+
@Override
53+
@SuppressWarnings("unchecked")
54+
public <T> T getMemberValue(Schema member) {
55+
throw new IllegalArgumentException("Attempted to get non-existent member: " + member.id());
56+
}
57+
58+
/**
59+
* Create a new builder containing all the current property values of this object.
60+
*
61+
* <p><strong>Note:</strong> This method performs only a shallow copy of the original properties.
62+
*
63+
* @return a builder for {@link BuilderShape}.
64+
*/
65+
public Builder toBuilder() {
66+
var builder = new Builder();
67+
return builder;
68+
}
69+
70+
/**
71+
* @return returns a new Builder.
72+
*/
73+
public static Builder builder() {
74+
return new Builder();
75+
}
76+
77+
/**
78+
* Builder for {@link BuilderShape}.
79+
*/
80+
public static final class Builder implements ShapeBuilder<BuilderShape> {
81+
82+
private Builder() {}
83+
84+
@Override
85+
public Schema schema() {
86+
return $SCHEMA;
87+
}
88+
89+
@Override
90+
public BuilderShape build() {
91+
return new BuilderShape(this);
92+
}
93+
94+
@Override
95+
public Builder deserialize(ShapeDeserializer decoder) {
96+
decoder.readStruct($SCHEMA, this, $InnerDeserializer.INSTANCE);
97+
return this;
98+
}
99+
100+
@Override
101+
public Builder deserializeMember(ShapeDeserializer decoder, Schema schema) {
102+
decoder.readStruct(schema.assertMemberTargetIs($SCHEMA), this, $InnerDeserializer.INSTANCE);
103+
return this;
104+
}
105+
106+
private static final class $InnerDeserializer implements ShapeDeserializer.StructMemberConsumer<Builder> {
107+
private static final $InnerDeserializer INSTANCE = new $InnerDeserializer();
108+
109+
@Override
110+
public void accept(Builder builder, Schema member, ShapeDeserializer de) {}
111+
}
112+
}
113+
}
114+

‎codegen/plugins/types-codegen/src/test/resources/software/amazon/smithy/java/codegen/types/test-cases/naming-conflict/expected/List.java‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
2+
package software.amazon.smithy.java.example.standalone.model;
3+
4+
import java.util.Objects;
5+
import software.amazon.smithy.java.core.schema.Schema;
6+
import software.amazon.smithy.java.core.schema.SerializableStruct;
7+
import software.amazon.smithy.java.core.schema.ShapeBuilder;
8+
import software.amazon.smithy.java.core.serde.ShapeDeserializer;
9+
import software.amazon.smithy.java.core.serde.ShapeSerializer;
10+
import software.amazon.smithy.java.core.serde.ToStringSerializer;
11+
import software.amazon.smithy.model.shapes.ShapeId;
12+
import software.amazon.smithy.utils.SmithyGenerated;
13+
14+
@SmithyGenerated
15+
public final class List implements SerializableStruct {
16+
17+
public static final Schema $SCHEMA = Schemas.LIST;
18+
19+
public static final ShapeId $ID = $SCHEMA.id();
20+
21+
private List(Builder builder) {
22+
}
23+
24+
@Override
25+
public String toString() {
26+
return ToStringSerializer.serialize(this);
27+
}
28+
29+
@Override
30+
public boolean equals(Object other) {
31+
if (other == this) {
32+
return true;
33+
}
34+
return other != null && getClass() == other.getClass();
35+
}
36+
37+
@Override
38+
public int hashCode() {
39+
return Objects.hash();
40+
}
41+
42+
@Override
43+
public Schema schema() {
44+
return $SCHEMA;
45+
}
46+
47+
@Override
48+
public void serializeMembers(ShapeSerializer serializer) {
49+
50+
}
51+
52+
@Override
53+
@SuppressWarnings("unchecked")
54+
public <T> T getMemberValue(Schema member) {
55+
throw new IllegalArgumentException("Attempted to get non-existent member: " + member.id());
56+
}
57+
58+
/**
59+
* Create a new builder containing all the current property values of this object.
60+
*
61+
* <p><strong>Note:</strong> This method performs only a shallow copy of the original properties.
62+
*
63+
* @return a builder for {@link List}.
64+
*/
65+
public Builder toBuilder() {
66+
var builder = new Builder();
67+
return builder;
68+
}
69+
70+
/**
71+
* @return returns a new Builder.
72+
*/
73+
public static Builder builder() {
74+
return new Builder();
75+
}
76+
77+
/**
78+
* Builder for {@link List}.
79+
*/
80+
public static final class Builder implements ShapeBuilder<List> {
81+
82+
private Builder() {}
83+
84+
@Override
85+
public Schema schema() {
86+
return $SCHEMA;
87+
}
88+
89+
@Override
90+
public List build() {
91+
return new List(this);
92+
}
93+
94+
@Override
95+
public Builder deserialize(ShapeDeserializer decoder) {
96+
decoder.readStruct($SCHEMA, this, $InnerDeserializer.INSTANCE);
97+
return this;
98+
}
99+
100+
@Override
101+
public Builder deserializeMember(ShapeDeserializer decoder, Schema schema) {
102+
decoder.readStruct(schema.assertMemberTargetIs($SCHEMA), this, $InnerDeserializer.INSTANCE);
103+
return this;
104+
}
105+
106+
private static final class $InnerDeserializer implements ShapeDeserializer.StructMemberConsumer<Builder> {
107+
private static final $InnerDeserializer INSTANCE = new $InnerDeserializer();
108+
109+
@Override
110+
public void accept(Builder builder, Schema member, ShapeDeserializer de) {}
111+
}
112+
}
113+
}
114+

0 commit comments

Comments
 (0)