From 8013efd7fa4755b43e59349d1cbed18fff10ec83 Mon Sep 17 00:00:00 2001 From: Steven Yuan Date: Thu, 26 Jan 2023 17:09:49 -0800 Subject: [PATCH] Use GoModuleInfo for both ManifestWriter and GoModGenerator Currently, `ManifestWriter` and `GoModGenerator` use different ways to calculate non-standard library dependencies and the go directive (minimum go version), which could produce inconsistencies. This commit updates both writers to use the same source of information (`GoModuleInfo`) for writing `generated.json` and `go.mod`. --- .../smithy/go/codegen/CodegenVisitor.java | 2 +- .../smithy/go/codegen/GoModGenerator.java | 25 ++-- .../smithy/go/codegen/GoModuleInfo.java | 125 ++++++++++++++++++ .../amazon/smithy/go/codegen/GoSettings.java | 6 +- .../smithy/go/codegen/ManifestWriter.java | 72 ++++------ 5 files changed, 162 insertions(+), 68 deletions(-) create mode 100644 codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/GoModuleInfo.java diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/CodegenVisitor.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/CodegenVisitor.java index 4ef351e47..c850b94f2 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/CodegenVisitor.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/CodegenVisitor.java @@ -255,7 +255,7 @@ void execute() { List dependencies = writers.getDependencies(); writers.flushWriters(); - GoModGenerator.writeGoMod(settings, fileManifest, SymbolDependency.gatherDependencies(dependencies.stream())); + GoModGenerator.writeGoMod(settings, fileManifest, dependencies); LOGGER.fine("Generating build manifest file"); ManifestWriter.writeManifest(settings, model, fileManifest, dependencies); diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/GoModGenerator.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/GoModGenerator.java index bdc460f23..73b49f588 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/GoModGenerator.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/GoModGenerator.java @@ -18,10 +18,9 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.util.List; import java.util.Map; -import java.util.TreeMap; import java.util.logging.Logger; -import java.util.stream.Collectors; import software.amazon.smithy.build.FileManifest; import software.amazon.smithy.codegen.core.CodegenException; import software.amazon.smithy.codegen.core.SymbolDependency; @@ -40,7 +39,7 @@ private GoModGenerator() {} static void writeGoMod( GoSettings settings, FileManifest manifest, - Map> dependencies + List dependencies ) { Boolean generateGoMod = settings.getGenerateGoMod(); if (!generateGoMod) { @@ -63,25 +62,19 @@ static void writeGoMod( manifest.addFile(goModFile); CodegenUtils.runCommand("go mod init " + settings.getModuleName(), manifest.getBaseDir()); - Map externalDependencies = getExternalDependencies(dependencies); - for (Map.Entry dependency : externalDependencies.entrySet()) { + GoModuleInfo goModuleInfo = new GoModuleInfo.Builder() + .goDirective(settings.getGoDirective()) + .dependencies(dependencies) + .build(); + + for (Map.Entry dependency : goModuleInfo.getMinimumNonStdLibDependencies().entrySet()) { CodegenUtils.runCommand( String.format("go mod edit -require=%s@%s", dependency.getKey(), dependency.getValue()), manifest.getBaseDir()); } CodegenUtils.runCommand( - String.format("go mod edit -go=%s", settings.getGoDirective()), + String.format("go mod edit -go=%s", goModuleInfo.getGoDirective()), manifest.getBaseDir()); } - - private static Map getExternalDependencies( - Map> dependencies - ) { - return dependencies.entrySet().stream() - .filter(entry -> !entry.getKey().equals("stdlib")) - .flatMap(entry -> entry.getValue().entrySet().stream()) - .collect(Collectors.toMap( - Map.Entry::getKey, entry -> entry.getValue().getVersion(), (a, b) -> b, TreeMap::new)); - } } diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/GoModuleInfo.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/GoModuleInfo.java new file mode 100644 index 000000000..cb2e0da57 --- /dev/null +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/GoModuleInfo.java @@ -0,0 +1,125 @@ +/* + * Copyright 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.smithy.go.codegen; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.TreeMap; +import java.util.function.Predicate; +import java.util.stream.Collectors; +import software.amazon.smithy.codegen.core.SymbolDependency; +import software.amazon.smithy.utils.BuilderRef; +import software.amazon.smithy.utils.SmithyBuilder; + +public final class GoModuleInfo { + + public static final String DEFAULT_GO_DIRECTIVE = "1.15"; + + private List dependencies; + + private List stdLibDependencies; + private List nonStdLibDependencies; + private String goDirective; + private Map minimumNonStdLibDependencies; + + private GoModuleInfo(Builder builder) { + goDirective = SmithyBuilder.requiredState("goDirective", builder.goDirective); + dependencies = builder.dependencies.copy(); + + // Intermediate dependency information + stdLibDependencies = gatherStdLibDependencies(); + nonStdLibDependencies = gatherNonStdLibDependencies(); + + // Module information used by ManifestWriter and GoModGenerator + goDirective = calculateMinimumGoDirective(); + minimumNonStdLibDependencies = gatherMinimumNonStdDependencies(); + } + + public String getGoDirective() { + return goDirective; + } + + public Map getMinimumNonStdLibDependencies() { + return minimumNonStdLibDependencies; + } + + private String calculateMinimumGoDirective() { + String minimumGoDirective = goDirective; + if (minimumGoDirective.compareTo(DEFAULT_GO_DIRECTIVE) < 0) { + throw new IllegalArgumentException( + "`goDirective` must be greater than or equal to the default go directive (" + + DEFAULT_GO_DIRECTIVE + "): " + minimumGoDirective); + } + for (SymbolDependency dependency : stdLibDependencies) { + var otherVersion = dependency.getVersion(); + if (minimumGoDirective.compareTo(otherVersion) < 0) { + minimumGoDirective = otherVersion; + } + } + return minimumGoDirective; + } + + private List gatherStdLibDependencies() { + return filterDependencies(dependency -> + dependency.getDependencyType().equals(GoDependency.Type.STANDARD_LIBRARY.toString())); + } + + private List gatherNonStdLibDependencies() { + return filterDependencies(dependency -> + !dependency.getDependencyType().equals(GoDependency.Type.STANDARD_LIBRARY.toString())); + } + + private List filterDependencies( + Predicate predicate + ) { + List filteredDependencies = new ArrayList<>(); + for (SymbolDependency dependency : dependencies) { + if (predicate.test(dependency)) { + filteredDependencies.add(dependency); + } + } + return filteredDependencies; + } + + private Map gatherMinimumNonStdDependencies() { + return SymbolDependency.gatherDependencies(nonStdLibDependencies.stream(), + GoDependency::mergeByMinimumVersionSelection).entrySet().stream().flatMap( + entry -> entry.getValue().entrySet().stream()).collect( + Collectors.toMap(Map.Entry::getKey, entry -> entry.getValue().getVersion(), (a, b) -> b, TreeMap::new)); + } + + public static class Builder implements SmithyBuilder { + private String goDirective; + private final BuilderRef> dependencies = BuilderRef.forList(); + + public Builder goDirective(String goDirective) { + this.goDirective = goDirective; + return this; + } + + public Builder dependencies(List dependencies) { + this.dependencies.clear(); + this.dependencies.get().addAll(dependencies); + return this; + } + + @Override + public GoModuleInfo build() { + return new GoModuleInfo(this); + } + } +} diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/GoSettings.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/GoSettings.java index ea6e348b0..c9f44ac48 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/GoSettings.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/GoSettings.java @@ -31,8 +31,6 @@ */ public final class GoSettings { - private static final String DEFAULT_GO_DIRECTIVE = "1.15"; - private static final String SERVICE = "service"; private static final String MODULE_NAME = "module"; private static final String MODULE_DESCRIPTION = "moduleDescription"; @@ -45,7 +43,7 @@ public final class GoSettings { private String moduleDescription = ""; private String moduleVersion; private Boolean generateGoMod = false; - private String goDirective = DEFAULT_GO_DIRECTIVE; + private String goDirective = GoModuleInfo.DEFAULT_GO_DIRECTIVE; private ShapeId protocol; /** @@ -65,7 +63,7 @@ public static GoSettings from(ObjectNode config) { MODULE_DESCRIPTION, settings.getModuleName() + " client")); settings.setModuleVersion(config.getStringMemberOrDefault(MODULE_VERSION, null)); settings.setGenerateGoMod(config.getBooleanMemberOrDefault(GENERATE_GO_MOD, false)); - settings.setGoDirective(config.getStringMemberOrDefault(GO_DIRECTIVE, DEFAULT_GO_DIRECTIVE)); + settings.setGoDirective(config.getStringMemberOrDefault(GO_DIRECTIVE, GoModuleInfo.DEFAULT_GO_DIRECTIVE)); return settings; } diff --git a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/ManifestWriter.java b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/ManifestWriter.java index 69d3679c8..6b60d1a29 100644 --- a/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/ManifestWriter.java +++ b/codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/ManifestWriter.java @@ -23,11 +23,8 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Optional; -import java.util.TreeMap; import java.util.logging.Logger; import java.util.stream.Collectors; -import java.util.stream.Stream; import software.amazon.smithy.build.FileManifest; import software.amazon.smithy.codegen.core.CodegenException; import software.amazon.smithy.codegen.core.SymbolDependency; @@ -61,7 +58,7 @@ private ManifestWriter(Builder builder) { moduleName = SmithyBuilder.requiredState("moduleName", builder.moduleName); fileManifest = SmithyBuilder.requiredState("fileManifest", builder.fileManifest); dependencies = builder.dependencies.copy(); - goDirective = builder.goDirective; + goDirective = SmithyBuilder.requiredState("goDirective", builder.goDirective); isUnstable = builder.isUnstable; } @@ -113,57 +110,38 @@ public void writeManifest() { } private Node buildManifestFile() { - List nonStdLib = new ArrayList<>(); - Optional minimumGoVersion = Optional.empty(); - - for (SymbolDependency dependency : dependencies) { - if (!dependency.getDependencyType().equals(GoDependency.Type.STANDARD_LIBRARY.toString())) { - nonStdLib.add(dependency); - continue; - } - - var otherVersion = dependency.getVersion(); - if (minimumGoVersion.isPresent()) { - if (minimumGoVersion.get().compareTo(otherVersion) < 0) { - minimumGoVersion = Optional.of(otherVersion); - } - } else { - minimumGoVersion = Optional.of(otherVersion); - } - } - - Map manifestNodes = new HashMap<>(); - - Map minimumDependencies = gatherMinimumDependencies(nonStdLib.stream()); + GoModuleInfo goModuleInfo = new GoModuleInfo.Builder() + .goDirective(goDirective) + .dependencies(dependencies) + .build(); + + Map dependencyNodes = gatherDependencyNodes(goModuleInfo.getMinimumNonStdLibDependencies()); + Collection generatedFiles = gatherGeneratedFiles(fileManifest); + return ObjectNode.objectNode(Map.of( + StringNode.from("module"), StringNode.from(moduleName), + StringNode.from("go"), StringNode.from(goModuleInfo.getGoDirective()), + StringNode.from("dependencies"), ObjectNode.objectNode(dependencyNodes), + StringNode.from("files"), ArrayNode.fromStrings(generatedFiles), + StringNode.from("unstable"), BooleanNode.from(isUnstable) + )).withDeepSortedKeys(); + } + private Map gatherDependencyNodes(Map dependencies) { Map dependencyNodes = new HashMap<>(); - for (Map.Entry entry : minimumDependencies.entrySet()) { + for (Map.Entry entry : dependencies.entrySet()) { dependencyNodes.put(StringNode.from(entry.getKey()), StringNode.from(entry.getValue())); } + return dependencyNodes; + } + private static Collection gatherGeneratedFiles(FileManifest fileManifest) { Collection generatedFiles = new ArrayList<>(); Path baseDir = fileManifest.getBaseDir(); for (Path filePath : fileManifest.getFiles()) { generatedFiles.add(baseDir.relativize(filePath).toString()); } generatedFiles = generatedFiles.stream().sorted().collect(Collectors.toList()); - - manifestNodes.put(StringNode.from("module"), StringNode.from(moduleName)); - manifestNodes.put(StringNode.from("go"), StringNode.from(minimumGoVersion.orElse(this.goDirective))); - manifestNodes.put(StringNode.from("dependencies"), ObjectNode.objectNode(dependencyNodes)); - manifestNodes.put(StringNode.from("files"), ArrayNode.fromStrings(generatedFiles)); - manifestNodes.put(StringNode.from("unstable"), BooleanNode.from(isUnstable)); - - return ObjectNode.objectNode(manifestNodes).withDeepSortedKeys(); - } - - private static Map gatherMinimumDependencies( - Stream symbolStream - ) { - return SymbolDependency.gatherDependencies(symbolStream, - GoDependency::mergeByMinimumVersionSelection).entrySet().stream().flatMap( - entry -> entry.getValue().entrySet().stream()).collect( - Collectors.toMap(Map.Entry::getKey, entry -> entry.getValue().getVersion(), (a, b) -> b, TreeMap::new)); + return generatedFiles; } public static Builder builder() { @@ -174,7 +152,7 @@ public static class Builder implements SmithyBuilder { private String moduleName; private FileManifest fileManifest; private final BuilderRef> dependencies = BuilderRef.forList(); - private String goDirective; + private String goDirective = GoModuleInfo.DEFAULT_GO_DIRECTIVE; private boolean isUnstable; public Builder moduleName(String moduleName) { @@ -193,8 +171,8 @@ public Builder dependencies(List dependencies) { return this; } - public Builder goDirective(String minimumGoVersion) { - this.goDirective = minimumGoVersion; + public Builder goDirective(String goDirective) { + this.goDirective = goDirective; return this; }