Make UnreferencedShape an opt-in linter instead

docs/source-2.0/guides/model-linters.rst

@@ -33,7 +33,67 @@ use in Smithy models.
 Linters in ``smithy-linters``
 -----------------------------
 
-The ``smithy-linters`` package defines the following linters.
+For Java developers using Smithy's reference implementation, the following
+linters (except for UnreferencedShape) are defined in the  ``software.amazon.smithy:smithy-linters``
+Maven package.
+
+
+.. _UnreferencedShape:
+
+UnreferencedShape
+=================
+
+Emits when a shape is not connected to the rest of the model. If no
+configuration is provided, the linter will check if a shape is connected to
+the closure of any service shape. A selector can be provided to define a
+custom set of "root" shapes to customize how the linter determines if a shape
+is unreferenced. Shapes that are connected through the :ref:`idref-trait`
+are considered connected.
+
+Rationale
+    Just like unused variables in code, removing unused shapes from a model
+    makes the model easier to maintain.
+
+Default severity
+    ``NOTE``
+
+Configuration
+    .. list-table::
+       :header-rows: 1
+       :widths: 20 20 60
+
+       * - Property
+         - Type
+         - Description
+       * - rootShapeSelector
+         - ``string``
+         - A :ref:`selector <selectors>` that specifies the root shape(s)
+           from which to detect if other shapes are connected. Defaults
+           to "service", meaning any shape connected to any service shape
+           in the model is considered referenced.
+
+Example:
+
+.. code-block:: smithy
+
+    $version: "2"
+
+    metadata validators = [
+        {
+            // Find shapes that aren't connected to service shapes or shapes
+            // marked with the trait, smithy.example#myCustomTrait.
+            name: "UnreferencedShape"
+            configuration: {
+                rootShapeSelector: ":is(service, [trait|smithy.example#myCustomTrait])"
+            }
+        }
+    ]
+
+.. note::
+
+    For backward compatibility reasons, the ``UnreferencedShape`` validator is available
+    in ``software.amazon.smithy:smithy-model`` Maven package and does not require
+    additional dependencies.
 
 
 .. _AbbreviationName:

smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/camel-case-validator-defaults-test.errors

@@ -38,6 +38,3 @@
 [DANGER] ns.baz#lowerStructureTrait$UpperCamelCase: Member shape member name, `UpperCamelCase`, is not lower camel case; members in the ns.baz namespace must all use lower camel case. | DefaultCamelCase
 [DANGER] ns.foo#Structure$lowerCamelCase: Member shape member name, `lowerCamelCase`, is not upper camel case; members in the MyService must all use upper camel case. | DefaultCamelCase
 [DANGER] ns.foo#Structure$snake_case: Member shape member name, `snake_case`, is not upper camel case; members in the MyService must all use upper camel case. | DefaultCamelCase
-
-[NOTE] ns.foo#Enum: The enum shape is not connected to from any service shape. | UnreferencedShape
-[NOTE] ns.foo#IntEnum: The intEnum shape is not connected to from any service shape. | UnreferencedShape

smithy-model/src/main/java/software/amazon/smithy/model/neighbor/UnreferencedShapes.java

@@ -21,31 +21,49 @@
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.knowledge.NeighborProviderIndex;
 import software.amazon.smithy.model.loader.Prelude;
+import software.amazon.smithy.model.selector.Selector;
 import software.amazon.smithy.model.shapes.Shape;
+import software.amazon.smithy.model.shapes.ShapeId;
 import software.amazon.smithy.model.traits.TraitDefinition;
 import software.amazon.smithy.utils.FunctionalUtils;
 
 /**
- * Finds shapes that are not connected to a service shape, are not trait
- * definitions, are not referenced by trait definitions, and are not
- * referenced in trait values through
+ * Finds shapes that are not connected to a "root" shape, are not trait definitions, are not referenced by trait
+ * definitions, and are not referenced in trait values through
  * {@link software.amazon.smithy.model.traits.IdRefTrait}.
  *
+ * <p>The "root" shapes defaults to all service shapes in the model. You can customize this by providing a selector
+ * that considers every matching shape a root shape. For example, a model might consider all shapes marked with
+ * a trait called "root" to be a root shape.
+ *
  * <p>Prelude shapes are never considered unreferenced.
  */
 public final class UnreferencedShapes {
 
+    private static final Selector SERVICE_SHAPES = Selector.parse("service");
+
+    private final Selector rootShapeSelector;
     private final Predicate<Shape> keepFilter;
 
+    /**
+     * @param keepFilter        Predicate that if matched keeps a shape from being unreferenced.
+     * @param rootShapeSelector Selector that returns the root shapes to traverse from (defaults to all services).
+     */
+    public UnreferencedShapes(Predicate<Shape> keepFilter, Selector rootShapeSelector) {
+        this.keepFilter = keepFilter;
+        this.rootShapeSelector = rootShapeSelector;
+    }
+
     public UnreferencedShapes() {
         this(FunctionalUtils.alwaysTrue());
     }
 
-    /**
-     * @param keepFilter Predicate that if matched keeps a shape from being unreferenced.
-     */
+    public UnreferencedShapes(Selector selector) {
+        this(FunctionalUtils.alwaysTrue(), selector);
+    }
+
     public UnreferencedShapes(Predicate<Shape> keepFilter) {
-        this.keepFilter = keepFilter;
+        this(keepFilter, SERVICE_SHAPES);
     }
 
     /**
@@ -60,21 +78,26 @@ public Set<Shape> compute(Model model) {
         Walker shapeWalker = new Walker(providerWithIdRefRelationships);
 
         // Find all shapes connected to any service shape.
-        Set<Shape> connected = new HashSet<>();
-        for (Shape service : model.getServiceShapes()) {
-            connected.addAll(shapeWalker.walkShapes(service));
+        Set<ShapeId> connected = new HashSet<>();
+
+        // Stop traversing into trees that are already traversed.
+        Predicate<Relationship> traversed = rel -> !connected.contains(rel.getNeighborShapeId());
+
+        Set<Shape> rootShapes = rootShapeSelector.select(model);
+        for (Shape root : rootShapes) {
+            shapeWalker.iterateShapes(root, traversed).forEachRemaining(shape -> connected.add(shape.getId()));
         }
 
         // Don't remove shapes that are traits or connected to traits.
         for (Shape trait : model.getShapesWithTrait(TraitDefinition.class)) {
-            connected.addAll(shapeWalker.walkShapes(trait));
+            shapeWalker.iterateShapes(trait, traversed).forEachRemaining(shape -> connected.add(shape.getId()));
         }
 
-        // Any shape that wasn't identified as connected to a service is considered unreferenced.
+        // Any shape that wasn't identified as connected to a root is considered unreferenced.
         Set<Shape> result = new HashSet<>();
         for (Shape shape : model.toSet()) {
             if (!shape.isMemberShape()
-                    && !connected.contains(shape)
+                    && !connected.contains(shape.getId())
                     && !Prelude.isPreludeShape(shape)
                     && keepFilter.test(shape)) {
                 result.add(shape);

smithy-model/src/main/java/software/amazon/smithy/model/validation/linters/UnreferencedShapeValidator.java

@@ -0,0 +1,89 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package software.amazon.smithy.model.validation.linters;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+import software.amazon.smithy.model.Model;
+import software.amazon.smithy.model.neighbor.UnreferencedShapes;
+import software.amazon.smithy.model.node.ExpectationNotMetException;
+import software.amazon.smithy.model.node.ObjectNode;
+import software.amazon.smithy.model.selector.Selector;
+import software.amazon.smithy.model.selector.SelectorSyntaxException;
+import software.amazon.smithy.model.shapes.Shape;
+import software.amazon.smithy.model.validation.AbstractValidator;
+import software.amazon.smithy.model.validation.ValidationEvent;
+import software.amazon.smithy.model.validation.ValidatorService;
+
+/**
+ * Finds shapes that aren't connected to any shapes considered "roots" (defaults to service shapes).
+ *
+ * <p>This linter is in smithy-model rather than smithy-linters so that it's easier for developers relying on the
+ * deprecated UnreferencedShape validator that was always on to migrate to this one instead without needing another
+ * dependency.
+ */
+public final class UnreferencedShapeValidator extends AbstractValidator {
+
+    public static final class Config {
+
+        private Selector rootShapeSelector = Selector.parse("service");
+
+        /**
+         * The Selector used to find root shapes, and any shape that is not connected to any of the returned root
+         * shapes is considered unreferenced.
+         *
+         * <p>Defaults to "service" if not set.
+         *
+         * @return Returns the root selector;
+         */
+        public Selector getRootShapeSelector() {
+            return rootShapeSelector;
+        }
+
+        public void setRootShapeSelector(Selector rootShapeSelector) {
+            this.rootShapeSelector = Objects.requireNonNull(rootShapeSelector);
+        }
+    }
+
+    public static final class Provider extends ValidatorService.Provider {
+        public Provider() {
+            super(UnreferencedShapeValidator.class, configuration -> {
+                Config config = new Config();
+                ObjectNode node = configuration.expectObjectNode()
+                        .expectNoAdditionalProperties(Collections.singleton("rootShapeSelector"));
+                node.getStringMember("rootShapeSelector").ifPresent(rootShapeNode -> {
+                    try {
+                        config.setRootShapeSelector(Selector.parse(rootShapeNode.getValue()));
+                    } catch (SelectorSyntaxException e) {
+                        throw new ExpectationNotMetException("Error parsing `rootShapeSelector`: " + e.getMessage(),
+                                                             rootShapeNode);
+                    }
+                });
+                return new UnreferencedShapeValidator(config);
+            });
+        }
+    }
+
+    private final Config config;
+
+    private UnreferencedShapeValidator(Config config) {
+        this.config = config;
+    }
+
+    @Override
+    public List<ValidationEvent> validate(Model model) {
+        List<ValidationEvent> events = new ArrayList<>();
+
+        for (Shape shape : new UnreferencedShapes(config.rootShapeSelector).compute(model)) {
+            events.add(note(shape, "This shape is unreferenced. It has no modeled connections to shapes "
+                                   + "that match the following selector: `" + config.rootShapeSelector + "`"));
+        }
+
+        return events;
+    }
+}

smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/UnreferencedShapeValidator.java

@@ -27,7 +27,11 @@
 /**
  * Adds a validation note event for each shape in the model that is not
  * connected to a service shape.
+ *
+ * <p>This validator is deprecated and no longer applied by default.
+ * Use the UnreferencedShapeValidator from smithy-linters instead.
  */
+@Deprecated
 public final class UnreferencedShapeValidator extends AbstractValidator {
     @Override
     public List<ValidationEvent> validate(Model model) {

smithy-model/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.Validator

@@ -52,6 +52,5 @@ software.amazon.smithy.model.validation.validators.TraitTargetValidator
 software.amazon.smithy.model.validation.validators.TraitValueValidator
 software.amazon.smithy.model.validation.validators.UnionValidator
 software.amazon.smithy.model.validation.validators.UnitTypeValidator
-software.amazon.smithy.model.validation.validators.UnreferencedShapeValidator
 software.amazon.smithy.model.validation.validators.UnstableTraitValidator
 software.amazon.smithy.model.validation.validators.XmlNamespaceTraitValidator

smithy-model/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.ValidatorService

@@ -1,2 +1,3 @@
 software.amazon.smithy.model.validation.linters.EmitEachSelectorValidator$Provider
 software.amazon.smithy.model.validation.linters.EmitNoneSelectorValidator$Provider
+software.amazon.smithy.model.validation.linters.UnreferencedShapeValidator$Provider

smithy-model/src/test/java/software/amazon/smithy/model/loader/ValidationEventDecoratorTest.java

@@ -17,20 +17,16 @@
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.Matchers.hasSize;
-import static org.hamcrest.Matchers.notNullValue;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import java.util.Arrays;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
-import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.node.ObjectNode;
 import software.amazon.smithy.model.shapes.ShapeId;
-import software.amazon.smithy.model.validation.NodeValidationVisitorTest;
 import software.amazon.smithy.model.validation.ValidatedResult;
 import software.amazon.smithy.model.validation.ValidationEvent;
 import software.amazon.smithy.model.validation.ValidationEventDecorator;
@@ -40,23 +36,19 @@
 
 public class ValidationEventDecoratorTest {
 
-    static final String HINT = "Consider connecting this structure to a service";
-    static final String UNREFERENCED_SHAPE_EVENT_ID = "UnreferencedShape";
-    static final Set<ShapeId> STRUCT_SHAPE_IDS = SetUtils.of(ShapeId.from("ns.foo#Structure"),
-                                                             ShapeId.from("ns.foo#Structure2"),
-                                                             ShapeId.from("ns.foo#Structure3"),
-                                                             ShapeId.from("ns.foo#Structure4"));
+    static final String HINT = "We had to deprecate this";
+    static final String SHAPE_EVENT_ID = "DeprecatedShape";
+    static final Set<ShapeId> MATCHING_SHAPE_IDS = SetUtils.of(ShapeId.from("smithy.example#Foo$a"));
 
     @Test
     public void canDecorateValidationEvents() {
         ValidatedResult<Model> result = Model.assembler()
-                                             .addImport(NodeValidationVisitorTest.class.getResource("node-validator"
-                                                                                                    + ".json"))
-                                             .validatorFactory(testFactory(new DummyHintValidationEventDecorator()))
-                                             .assemble();
+                .addImport(getClass().getResource("validation-event-decorator-test.smithy"))
+                .validatorFactory(testFactory(new TestValidationEventDecorator()))
+                .assemble();
         for (ValidationEvent event : result.getValidationEvents()) {
             ShapeId eventShapeId = event.getShapeId().orElse(null);
-            if (STRUCT_SHAPE_IDS.contains(eventShapeId)) {
+            if (MATCHING_SHAPE_IDS.contains(eventShapeId)) {
                 assertThat(event.getHint().isPresent(), equalTo(true));
                 assertThat(event.getHint().get(), equalTo(HINT));
             } else {
@@ -69,10 +61,9 @@ public void canDecorateValidationEvents() {
     public void exceptionsAreNotCaughtWhenDecoratorsThrow() {
         assertThrows(RuntimeException.class, () -> {
             Model.assembler()
-                 .addImport(NodeValidationVisitorTest.class.getResource("node-validator"
-                                                                        + ".json"))
-                 .validatorFactory(testFactory(new ThrowingValidationEventDecorator()))
-                 .assemble();
+                    .addImport(getClass().getResource("validation-event-decorator-test.smithy"))
+                    .validatorFactory(testFactory(new ThrowingValidationEventDecorator()))
+                    .assemble();
         });
     }
 
@@ -96,11 +87,10 @@ public Optional<Validator> createValidator(String name, ObjectNode configuration
         };
     }
 
-    static class DummyHintValidationEventDecorator implements ValidationEventDecorator {
-
+    static class TestValidationEventDecorator implements ValidationEventDecorator {
         @Override
         public boolean canDecorate(ValidationEvent ev) {
-            return ev.containsId(UNREFERENCED_SHAPE_EVENT_ID) && ev.getMessage().contains("The structure ");
+            return ev.containsId(SHAPE_EVENT_ID);
         }
 
         @Override
@@ -110,15 +100,14 @@ public ValidationEvent decorate(ValidationEvent ev) {
     }
 
     static class ThrowingValidationEventDecorator implements ValidationEventDecorator {
-
         @Override
         public boolean canDecorate(ValidationEvent ev) {
-            return ev.containsId(UNREFERENCED_SHAPE_EVENT_ID) && ev.getMessage().contains("The structure ");
+            return ev.containsId(SHAPE_EVENT_ID);
         }
 
         @Override
         public ValidationEvent decorate(ValidationEvent ev) {
-            throw new RuntimeException("ups");
+            throw new RuntimeException("oops!");
         }
     }
 }

smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-request-response-validator.errors

@@ -22,8 +22,6 @@
 [ERROR] ns.foo#NInput$a: This `a` structure member is marked with the `httpLabel` trait, but no corresponding `http` URI label could be found when used as the input of the `ns.foo#N` operation. | HttpLabelTrait
 [ERROR] ns.foo#PInput$a: The `a` structure member corresponds to a greedy label when used as the input of the `ns.foo#P` operation. This member targets (integer: `ns.foo#Integer`), but greedy labels must target string shapes. | HttpLabelTrait
 [ERROR] ns.foo#RInput$b: `httpHeader` binding of `x-foo` conflicts with the `httpPrefixHeaders` binding of `ns.foo#RInput$a` to ``. `httpHeader` bindings must not case-insensitively start with any `httpPrefixHeaders` bindings. | HttpPrefixHeadersTrait
-[NOTE] ns.foo#BadError: The structure shape is not connected to from any service shape. | UnreferencedShape
-[NOTE] ns.foo#BadErrorMultipleBindings: The structure shape is not connected to from any service shape. | UnreferencedShape
 [ERROR] ns.foo#GInput$b: Trait `httpHeader` cannot be applied to `ns.foo#GInput$b`. This trait may only be applied to shapes that match the following selector: structure > :test(member > :test(boolean, number, string, timestamp, list > member > :test(boolean, number, string, timestamp))) | TraitTarget
 [ERROR] ns.foo#GOutput$b: Trait `httpHeader` cannot be applied to `ns.foo#GOutput$b`. This trait may only be applied to shapes that match the following selector: structure > :test(member > :test(boolean, number, string, timestamp, list > member > :test(boolean, number, string, timestamp))) | TraitTarget
 [ERROR] ns.foo#HInput$a: Trait `httpHeader` cannot be applied to `ns.foo#HInput$a`. This trait may only be applied to shapes that match the following selector: structure > :test(member > :test(boolean, number, string, timestamp, list > member > :test(boolean, number, string, timestamp))) | TraitTarget