Repository: smithy-lang/smithy (java)

Issue:
How to detect duplicate shape definitions?
Hi!

I would like to detect and prevent duplicate definitions of shapes within a codebase. Does smithy-model allow me to do that?

At the moment, it appears that duplicate definitions are only logged as a warning, and there's nothing in the model loading result that would tell me that this happened. The SourceLocation also gets deduplicated into a single definition.

To reiterate/rephrase, I want to stop people from doing

```smithy

@length(min: 1)
string NonEmptyString

@length(min: 1)
string NonEmptyString

```

in my repository. This has happened before (presumably as a result of a mishandled merge conflict) and it's only producing noise.

Pull request:
Emit NOTE on ignored duplicate shape
#### Background
Resolves #2194 in combination with [severity overrides](https://smithy.io/2.0/spec/model-validation.html#severity-overrides)

#### Testing
* Unit tests

---
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.


Patch:
diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/LoaderShapeMap.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/LoaderShapeMap.java
index 122bd5cd2..6b021d4c6 100644
--- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/LoaderShapeMap.java
+++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/LoaderShapeMap.java
@@ -348,9 +348,15 @@ final class LoaderShapeMap {
                                                        previous.getSourceLocation(), joiner.toString()));
                 return false;
             } else if (!LoaderUtils.isSameLocation(built, previous)) {
-                LOGGER.warning(() -> "Ignoring duplicate but equivalent shape definition: " + id
-                                     + " defined at " + built.getSourceLocation() + " and "
-                                     + previous.getSourceLocation());
+                events.add(ValidationEvent.builder()
+                        .id(Validator.MODEL_ERROR + ".IgnoredDuplicateDefinition")
+                        .severity(Severity.NOTE)
+                        .sourceLocation(previous.getSourceLocation())
+                        .shapeId(id)
+                        .message("Ignoring duplicate but equivalent shape definition: " + id
+                                + " defined at " + built.getSourceLocation() + " and "
+                                + previous.getSourceLocation())
+                        .build());
             }
         }
         return true;


Test patch:
diff --git a/smithy-aws-protocol-tests/model/restXml/document-maps.smithy b/smithy-aws-protocol-tests/model/restXml/document-maps.smithy
index fc2541a03..0062b98b9 100644
--- a/smithy-aws-protocol-tests/model/restXml/document-maps.smithy
+++ b/smithy-aws-protocol-tests/model/restXml/document-maps.smithy
@@ -650,6 +650,151 @@ apply NestedXmlMaps @httpResponseTests([
     },
 ])
 
+/// Nested Xml Maps with key/values with @xmlName
+@http(uri: "/NestedXmlMapWithXmlName", method: "POST")
+operation NestedXmlMapWithXmlName {
+    input: NestedXmlMapWithXmlNameInputOutput
+    output: NestedXmlMapWithXmlNameInputOutput
+}
+
+structure NestedXmlMapWithXmlNameInputOutput {
+    nestedXmlMapWithXmlNameMap: NestedXmlMapWithXmlNameMap
+}
+
+map NestedXmlMapWithXmlNameMap{
+    @xmlName("OuterKey")
+    key: String
+    
+    value: NestedXmlMapWithXmlNameInnerMap
+}
+
+map NestedXmlMapWithXmlNameInnerMap{
+    @xmlName("InnerKey")
+    key: String
+    
+    @xmlName("InnerValue")
+    value: String
+}
+
+apply NestedXmlMapWithXmlName @httpRequestTests([
+    {
+        id : "NestedXmlMapWithXmlNameSerializes",
+        documentation : "Serializes nested XML Maps in requests that have xmlName on members",
+        protocol: restXml,
+        method: "POST",
+        uri: "/NestedXmlMapWithXmlName",
+        body: """
+            <NestedXmlMapWithXmlNameRequest>
+                <nestedXmlMapWithXmlNameMap>
+                    <entry>
+                        <OuterKey>foo</OuterKey>
+                        <value>
+                            <entry>
+                                <InnerKey>bar</InnerKey>
+                                <InnerValue>Baz</InnerValue>
+                            </entry>
+                            <entry>
+                                <InnerKey>fizz</InnerKey>
+                                <InnerValue>Buzz</InnerValue>
+                            </entry>
+                        </value>
+                    </entry>
+                    <entry>
+                        <OuterKey>qux</OuterKey>
+                        <value>
+                            <entry>
+                                <InnerKey>foobar</InnerKey>
+                                <InnerValue>Bar</InnerValue>
+                            </entry>
+                            <entry>
+                                <InnerKey>fizzbuzz</InnerKey>
+                                <InnerValue>Buzz</InnerValue>
+                            </entry>
+                        </value>
+                    </entry>
+                </nestedXmlMapWithXmlNameMap>
+            </NestedXmlMapWithXmlNameRequest>
+        """,
+        bodyMediaType: "application/xml",
+        headers: {
+            "Content-Type": "application/xml",
+        },
+        params: {
+            nestedXmlMapWithXmlNameMap: {
+                foo: {
+                    bar: "Baz",
+                    fizz: "Buzz"
+                },
+                qux: {
+                    foobar: "Bar",
+                    fizzbuzz: "Buzz"
+                }
+            }
+        }
+    }
+
+
+
+
+])
+
+apply NestedXmlMapWithXmlName @httpResponseTests([
+    {
+        id: "NestedXmlMapWithXmlNameDeserializes",
+        documentation: "Serializes nested XML maps in responses that have xmlName on members",
+        protocol: restXml,
+        code: 200,
+        body: """
+            <NestedXmlMapWithXmlNameResponse>
+                <nestedXmlMapWithXmlNameMap>
+                    <entry>
+                        <OuterKey>foo</OuterKey>
+                        <value>
+                            <entry>
+                                <InnerKey>bar</InnerKey>
+                                <InnerValue>Baz</InnerValue>
+                            </entry>
+                            <entry>
+                                <InnerKey>fizz</InnerKey>
+                                <InnerValue>Buzz</InnerValue>
+                            </entry>
+                        </value>
+                    </entry>
+                    <entry>
+                        <OuterKey>qux</OuterKey>
+                        <value>
+                            <entry>
+                                <InnerKey>foobar</InnerKey>
+                                <InnerValue>Bar</InnerValue>
+                            </entry>
+                            <entry>
+                                <InnerKey>fizzbuzz</InnerKey>
+                                <InnerValue>Buzz</InnerValue>
+                            </entry>
+                        </value>
+                    </entry>
+                </nestedXmlMapWithXmlNameMap>
+            </NestedXmlMapWithXmlNameResponse>
+        """
+        bodyMediaType: "application/xml",
+        headers: {
+            "Content-Type": "application/xml"
+        }
+        params: {
+            nestedXmlMapWithXmlNameMap: {
+                foo: {
+                    bar: "Baz",
+                    fizz: "Buzz"
+                },
+                qux: {
+                    foobar: "Bar",
+                    fizzbuzz: "Buzz"
+                }
+            }
+        }
+    }
+])
+
 /// Maps with @xmlNamespace and @xmlName
 @http(uri: "/XmlMapWithXmlNamespace", method: "POST")
 operation XmlMapWithXmlNamespace {
diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/detects-ignorable-duplicates.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/detects-ignorable-duplicates.errors
new file mode 100644
index 000000000..3a4ae5585
--- /dev/null
+++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/detects-ignorable-duplicates.errors
@@ -0,0 +1,1 @@
+[NOTE] foo.baz#Foo: Ignoring duplicate but equivalent shape definition: foo.baz#Foo defined at | Model.IgnoredDuplicateDefinition
diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/detects-ignorable-duplicates.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/detects-ignorable-duplicates.smithy
new file mode 100644
index 000000000..a26d0e326
--- /dev/null
+++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/detects-ignorable-duplicates.smithy
@@ -0,0 +1,9 @@
+$version: "2.0"
+
+namespace foo.baz
+
+@length(min: 1)
+string Foo
+
+@length(min: 1)
+string Foo