Omit unrecognized models from sources

Smithy-Build is often invoked from the Smithy CLI, and developers
often provide directories that contain model files. If a directory
contains files other than recognized Smithy models, those unknown
files are treated as "sources" and ultimately wind up in the sources
plugin manifest file. Files that are not Smithy model files should
not show up in the manifest as this can cause invalid JARs to get
created by tooling like the Smithy Gradle plugin.

Sources are now eagerly filtered when they are added to SmithyBuild.
If someone, somehow, manually configures the sources plugin to use
unrecognized Smithy model files, the plugin will skip those files
and log a warning rather than create an invalid manifest.

smithy-build/src/main/java/software/amazon/smithy/build/SmithyBuild.java

@@ -15,7 +15,12 @@
 
 package software.amazon.smithy.build;
 
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.FileSystems;
+import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.PathMatcher;
 import java.nio.file.Paths;
 import java.util.Collections;
 import java.util.HashSet;
@@ -29,6 +34,7 @@
 import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.function.Supplier;
+import java.util.logging.Logger;
 import software.amazon.smithy.build.model.SmithyBuildConfig;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.loader.ModelAssembler;
@@ -42,6 +48,9 @@ public final class SmithyBuild {
     /** The version of Smithy build. */
     public static final String VERSION = "1.0";
 
+    private static final Logger LOGGER = Logger.getLogger(SmithyBuild.class.getName());
+    private static final PathMatcher VALID_MODELS = FileSystems.getDefault().getPathMatcher("glob:*.{json,jar,smithy}");
+
     SmithyBuildConfig config;
     Path outputDirectory;
     Function<String, Optional<ProjectionTransformer>> transformFactory;
@@ -187,8 +196,27 @@ public SmithyBuild config(SmithyBuildConfig config) {
     // Add a source path using absolute paths to better de-conflict source files. ModelAssembler also
     // de-conflicts imports with absolute paths, but this ensures the same file doesn't appear twice in
     // the build plugin output (though it does not use realpath to de-conflict based on symlinks).
+    //
+    // Ignores and logs when an unsupported model file is encountered.
     private void addSource(Path path) {
-        sources.add(path.toAbsolutePath());
+        try {
+            if (Files.isDirectory(path)) {
+                // Pre-emptively crawl the given models to filter them up front into a flat, file-only, list.
+                Files.list(path).filter(Files::isRegularFile).forEach(this::addSourceFile);
+            } else {
+                addSourceFile(path);
+            }
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);
+        }
+    }
+
+    private void addSourceFile(Path file) {
+        if (!VALID_MODELS.matches(file.getFileName())) {
+            LOGGER.warning("Omitting unsupported Smithy model file from model sources: " + file);
+        } else {
+            sources.add(file.toAbsolutePath());
+        }
     }
 
     /**
@@ -380,6 +408,12 @@ public SmithyBuild pluginClassLoader(ClassLoader pluginClassLoader) {
      * unique across the entire set of files. The sources directories are
      * essentially flattened into a single directory.
      *
+     * <p>Unsupported model files are ignored and not treated as sources.
+     * This can happen when adding model files from a directory that contains
+     * a mix of model files and non-model files. Filtering models here prevents
+     * unsupported files from appearing in places like JAR manifest files where
+     * they are not allowed.
+     *
      * @param pathToSources Path to source directories to mark.
      * @return Returns the builder.
      */

smithy-build/src/main/java/software/amazon/smithy/build/plugins/SourcesPlugin.java

@@ -145,8 +145,16 @@ private static void copyFile(List<String> names, FileManifest manifest, Path tar
                     + ValidationUtils.tickedList(manifest.getFiles()));
         }
 
-        manifest.writeFile(target, contents);
-        names.add(target.toString());
+        String filename = target.toString();
+
+        // Even though sources are filtered in SmithyBuild, it's theoretically possible that someone could call this
+        // plugin manually. In that case, refuse to write unsupported files to the manifest.
+        if (filename.endsWith(".smithy") || filename.endsWith(".json")) {
+            manifest.writeFile(target, contents);
+            names.add(target.toString());
+        } else {
+            LOGGER.warning("Omitting unrecognized file from Smithy model manifest: " + filename);
+        }
     }
 
     private static void projectSources(PluginContext context) {

smithy-build/src/test/java/software/amazon/smithy/build/SmithyBuildTest.java

@@ -48,12 +48,16 @@
 import org.junit.jupiter.api.Assumptions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 import software.amazon.smithy.build.model.ProjectionConfig;
 import software.amazon.smithy.build.model.SmithyBuildConfig;
 import software.amazon.smithy.build.model.TransformConfig;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.SourceException;
 import software.amazon.smithy.model.SourceLocation;
+import software.amazon.smithy.model.loader.ModelAssembler;
 import software.amazon.smithy.model.node.Node;
 import software.amazon.smithy.model.node.ObjectNode;
 import software.amazon.smithy.model.shapes.ShapeId;
@@ -898,5 +902,51 @@ public void detectsConflictingArtifactNames() throws Exception {
         assertThat(e.getMessage(), containsString("Multiple plugins use the same artifact name 'foo' in "
                                                   + "the 'source' projection"));
     }
-}
 
+    @ParameterizedTest
+    @MethodSource("unrecognizedModelPaths")
+    public void ignoreUnrecognizedModels(List<Path> models) throws URISyntaxException {
+        ModelAssembler assembler = Model.assembler();
+        for (Path path : models) {
+            assembler.addImport(path);
+        }
+        Model model = assembler.assemble().unwrap();
+
+        SmithyBuild builder = new SmithyBuild().model(model).fileManifestFactory(MockManifest::new);
+        models.forEach(builder::registerSources);
+
+        // Apply multiple projections to ensure that sources are filtered even in projections.
+        builder.config(SmithyBuildConfig.builder()
+                               .load(Paths.get(getClass().getResource("apply-multiple-projections.json").toURI()))
+                               .outputDirectory("/foo")
+                               .build());
+
+        SmithyBuildResult results = builder.build();
+        assertTrue(results.getProjectionResult("source").isPresent());
+        assertTrue(results.getProjectionResult("a").isPresent());
+
+        ProjectionResult sourceResult = results.getProjectionResult("source").get();
+        MockManifest sourceManifest = (MockManifest) sourceResult.getPluginManifest("sources").get();
+        String sourceSourcesManifestText = sourceManifest.getFileString("manifest").get();
+
+        assertThat(sourceSourcesManifestText, containsString("a.smithy"));
+        assertThat(sourceSourcesManifestText, not(containsString("foo.md")));
+
+        ProjectionResult aResult = results.getProjectionResult("source").get();
+        MockManifest aManifest = (MockManifest) aResult.getPluginManifest("sources").get();
+        String aSourcesManifestText = aManifest.getFileString("manifest").get();
+
+        assertThat(aSourcesManifestText, not(containsString("foo.md")));
+    }
+
+    public static List<Arguments> unrecognizedModelPaths() throws URISyntaxException {
+        Path rootPath = Paths.get(SmithyBuildTest.class.getResource("plugins/sources-ignores-unrecognized-files")
+                                          .toURI());
+        return ListUtils.of(
+            // Test that crawling the directory works.
+            Arguments.of(ListUtils.of(rootPath)),
+            // Test that passing explicit files works too.
+            Arguments.of(ListUtils.of(rootPath.resolve("a.smithy"), rootPath.resolve("foo.md")))
+        );
+    }
+}

smithy-build/src/test/java/software/amazon/smithy/build/plugins/SourcesPluginTest.java

@@ -208,4 +208,28 @@ public void copiesModelsDefinedInConfigAsSources() throws URISyntaxException {
         assertThat(manifest.hasFile("a.smithy"), is(true));
         assertThat(manifest.hasFile("d.smithy"), is(false));
     }
+
+    // When the sources plugin sees a file it does not support, it is ignored.
+    @Test
+    public void omitsUnsupportedFilesFromManifest() throws URISyntaxException {
+        Model model = Model.assembler()
+                .addImport(getClass().getResource("sources-ignores-unrecognized-files/a.smithy"))
+                .addImport(getClass().getResource("sources-ignores-unrecognized-files/foo.md"))
+                .assemble()
+                .unwrap();
+        MockManifest manifest = new MockManifest();
+        PluginContext context = PluginContext.builder()
+                .fileManifest(manifest)
+                .model(model)
+                .originalModel(model)
+                .sources(ListUtils.of(Paths.get(getClass().getResource("sources-ignores-unrecognized-files").toURI())))
+                .build();
+        new SourcesPlugin().execute(context);
+        String manifestString = manifest.getFileString("manifest").get();
+        // Normalize for Windows.
+        manifestString = manifestString.replace("\\", "/");
+
+        assertThat(manifestString, containsString("a.smithy\n"));
+        assertThat(manifestString, not(containsString("foo.md")));
+    }
 }

smithy-build/src/test/resources/software/amazon/smithy/build/plugins/sources-ignores-unrecognized-files/a.smithy

@@ -0,0 +1,5 @@
+$version: "2.0"
+
+namespace smithy.example
+
+string MyString

smithy-build/src/test/resources/software/amazon/smithy/build/plugins/sources-ignores-unrecognized-files/foo.md

@@ -0,0 +1,4 @@
+# Ignore me!
+
+The sources plugin should ignore unsupported files so that they don't show up
+in places like JARs.

smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelLoader.java

@@ -82,7 +82,7 @@ static boolean load(
                     return loadParsedNode(Node.parse(inputStream, filename), operationConsumer);
                 }
             } else {
-                LOGGER.warning(() -> "Ignoring unrecognized file: " + filename);
+                LOGGER.warning(() -> "Ignoring unrecognized Smithy model file: " + filename);
                 return false;
             }
         } catch (IOException e) {