Add warning for enums without names

This adds a warning-level validation event for enums that do not use the name property. It additionally updates all built in enums and enums used for tests where the absense of the name isn't deliberate.
smithy-lang · Oct 21, 2020 · 10df05c · 10df05c
1 parent d44c147
commit 10df05c
Show file tree

Hide file tree

Showing 6 changed files with 121 additions and 32 deletions.
diff --git a/smithy-aws-apigateway-traits/src/main/resources/META-INF/smithy/aws.apigateway.json b/smithy-aws-apigateway-traits/src/main/resources/META-INF/smithy/aws.apigateway.json
@@ -287,8 +287,14 @@
             "traits": {
                 "smithy.api#private": {},
                 "smithy.api#enum": [
-                    {"value": "INTERNET"},
-                    {"value": "VPC_LINK"}
+                    {
+                        "name": "INTERNET",
+                        "value": "INTERNET"
+                    },
+                    {
+                        "name": "VPC_LINK",
+                        "value": "VPC_LINK"
+                    }
                 ]
             }
         },

diff --git a/smithy-aws-protocol-tests/model/shared-types.smithy b/smithy-aws-protocol-tests/model/shared-types.smithy
@@ -58,11 +58,26 @@ list TimestampList {
 }
 
 @enum([
-    {value: "Foo"},
-    {value: "Baz"},
-    {value: "Bar"},
-    {value: "1"},
-    {value: "0"},
+    {
+        name: "FOO",
+        value: "Foo",
+    },
+    {
+        name: "BAZ",
+        value: "Baz",
+    },
+    {
+        name: "BAR",
+        value: "Bar",
+    },
+    {
+        name: "ONE",
+        value: "1",
+    },
+    {
+        name: "ZERO",
+        value: "0",
+    },
 ])
 string FooEnum
 

diff --git a/.../src/main/java/software/amazon/smithy/model/validation/validators/EnumTraitValidator.java b/.../src/main/java/software/amazon/smithy/model/validation/validators/EnumTraitValidator.java
@@ -53,7 +53,7 @@ private List<ValidationEvent> validateEnumTrait(Shape shape, EnumTrait trait) {
         Set<String> names = new HashSet<>();
         Set<String> values = new HashSet<>();
 
-        // Ensure that names are unique.
+        // Ensure that values are unique.
         for (EnumDefinition definition : trait.getValues()) {
             if (!values.add(definition.getValue())) {
                 events.add(error(shape, trait, String.format(
@@ -79,8 +79,8 @@ private List<ValidationEvent> validateEnumTrait(Shape shape, EnumTrait trait) {
             }
         }
 
-        // If one enum definition has a name, then they all must have names.
         if (!names.isEmpty()) {
+            // If one enum definition has a name, then they all must have names.
             for (EnumDefinition definition : trait.getValues()) {
                 if (!definition.getName().isPresent()) {
                     events.add(error(shape, trait, String.format(
@@ -89,6 +89,12 @@ private List<ValidationEvent> validateEnumTrait(Shape shape, EnumTrait trait) {
                             definition.getValue())));
                 }
             }
+        } else {
+            // Enums SHOULD have names, so warn if there are none.
+            ValidationEvent event = warning(shape, trait, "Enums should define the `name` property to allow rich "
+                    + "types to be generated in code generators.");
+            // Change the id of the event so that it can be suppressed separately.
+            events.add(event.toBuilder().id("EnumNamesPresent").build());
         }
 
         return events;

diff --git a/...resources/software/amazon/smithy/model/errorfiles/validators/enum-trait-validation.errors b/...resources/software/amazon/smithy/model/errorfiles/validators/enum-trait-validation.errors
@@ -1,3 +1,4 @@
+[WARNING] ns.foo#ValidNoName: Enums should define the `name` property to allow rich types to be generated in code generators. | EnumNamesPresent
 [WARNING] ns.foo#Warn1: The name `_bar` does not match the recommended enum name format of beginning with an uppercase letter, followed by any number of uppercase letters, numbers, or underscores. | EnumTrait
 [WARNING] ns.foo#Warn1: The name `baz` does not match the recommended enum name format of beginning with an uppercase letter, followed by any number of uppercase letters, numbers, or underscores. | EnumTrait
 [WARNING] ns.foo#Invalid2: The name `invalid!` does not match the recommended enum name format of beginning with an uppercase letter, followed by any number of uppercase letters, numbers, or underscores. | EnumTrait

diff --git a/...t/resources/software/amazon/smithy/model/errorfiles/validators/enum-trait-validation.json b/...t/resources/software/amazon/smithy/model/errorfiles/validators/enum-trait-validation.json
@@ -1,16 +1,20 @@
 {
     "smithy": "1.0",
     "shapes": {
-        "ns.foo#Valid1": {
+        "ns.foo#ValidNoName": {
             "type": "string",
             "traits": {
                 "smithy.api#enum": [
-                    {"value": "foo"},
-                    {"value": "bar"}
+                    {
+                        "value": "foo"
+                    },
+                    {
+                        "value": "bar"
+                    }
                 ]
             }
         },
-        "ns.foo#Valid2": {
+        "ns.foo#ValidFullDefinition": {
             "type": "string",
             "traits": {
                 "smithy.api#enum": [
@@ -91,8 +95,14 @@
             "type": "string",
             "traits": {
                 "smithy.api#enum": [
-                    {"value": "a"},
-                    {"value": "a"}
+                    {
+                        "name": "A",
+                        "value": "a"
+                    },
+                    {
+                        "name": "B",
+                        "value": "a"
+                    }
                 ]
             }
         }

diff --git a/...raits/src/test/resources/software/amazon/smithy/mqtt/traits/errorfiles/job-service.smithy b/...raits/src/test/resources/software/amazon/smithy/mqtt/traits/errorfiles/job-service.smithy
@@ -50,15 +50,42 @@ structure RejectedError {
 }
 
 @enum([
-  {value: "InvalidTopic"},
-  {value: "InvalidJson"},
-  {value: "InvalidRequest"},
-  {value: "InvalidStateTransition"},
-  {value: "ResourceNotFound"},
-  {value: "VersionMismatch"},
-  {value: "InternalError"},
-  {value: "RequestThrottled"},
-  {value: "TerminalStateReached"},
+  {
+    name: "INVALID_TOPIC",
+    value: "InvalidTopic",
+  },
+  {
+    name: "INVALID_JSON",
+    value: "InvalidJson",
+  },
+  {
+    name: "INVALID_REQUEST",
+    value: "InvalidRequest",
+  },
+  {
+    name: "INVALID_STATE_TRANSITION",
+    value: "InvalidStateTransition",
+  },
+  {
+    name: "RESOURCE_NOT_FOUND",
+    value: "ResourceNotFound",
+  },
+  {
+    name: "VERSION_MISMATCH",
+    value: "VersionMismatch",
+  },
+  {
+    name: "INTERNAL_ERROR",
+    value: "InternalError",
+  },
+  {
+    name: "REQUEST_THROTTLED",
+    value: "RequestThrottled",
+  },
+  {
+    name: "TERMINAL_STATE_REACHED",
+    value: "TerminalStateReached",
+  },
 ])
 string RejectedErrorCode
 
@@ -203,14 +230,38 @@ structure JobExecutionData {
 }
 
 @enum([
-  {value: "QUEUED"},
-  {value: "IN_PROGRESS"},
-  {value: "TIMED_OUT"},
-  {value: "FAILED"},
-  {value: "SUCCEEDED"},
-  {value: "CANCELED"},
-  {value: "REJECTED"},
-  {value: "REMOVED"},
+  {
+    name: "QUEUED",
+    value: "QUEUED",
+  },
+  {
+    name: "IN_PROGRESS",
+    value: "IN_PROGRESS",
+  },
+  {
+    name: "TIMED_OUT",
+    value: "TIMED_OUT",
+  },
+  {
+    name: "FAILED",
+    value: "FAILED",
+  },
+  {
+    name: "SUCCEEDED",
+    value: "SUCCEEDED",
+  },
+  {
+    name: "CANCELED",
+    value: "CANCELED",
+  },
+  {
+    name: "REJECTED",
+    value: "REJECTED",
+  },
+  {
+    name: "REMOVED",
+    value: "REMOVED",
+  },
 ])
 string JobStatus