From 45eda40a09ba00f06f97901af0a5c48dab4609e0 Mon Sep 17 00:00:00 2001 From: Hunter Mellema <124718352+hpmellema@users.noreply.github.com> Date: Wed, 20 Mar 2024 07:58:57 -0600 Subject: [PATCH] Remove unecessary indirection added by SmithyBaseTask class (#134) Removes the SmithyBaseTask abstract class and merges it's functionality into the only extending class AbstractSmithyCliTask to reduce indirection. --- .../gradle/tasks/AbstractSmithyCliTask.java | 169 ++++++++++++++- .../smithy/gradle/tasks/BaseSmithyTask.java | 193 ------------------ .../smithy/gradle/tasks/SmithyBuildTask.java | 29 ++- .../smithy/gradle/tasks/SmithyFormatTask.java | 5 +- .../gradle/tasks/SmithyValidateTask.java | 8 +- 5 files changed, 186 insertions(+), 218 deletions(-) delete mode 100644 smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/BaseSmithyTask.java diff --git a/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/AbstractSmithyCliTask.java b/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/AbstractSmithyCliTask.java index 68456e6..bbd2332 100644 --- a/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/AbstractSmithyCliTask.java +++ b/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/AbstractSmithyCliTask.java @@ -7,12 +7,23 @@ import java.util.ArrayList; import java.util.List; +import java.util.logging.Level; +import org.gradle.StartParameter; +import org.gradle.api.DefaultTask; import org.gradle.api.file.FileCollection; +import org.gradle.api.logging.configuration.ShowStacktrace; import org.gradle.api.model.ObjectFactory; import org.gradle.api.provider.Property; +import org.gradle.api.provider.Provider; +import org.gradle.api.tasks.Classpath; import org.gradle.api.tasks.Input; +import org.gradle.api.tasks.InputFiles; +import org.gradle.api.tasks.Internal; import org.gradle.api.tasks.Optional; +import org.gradle.internal.logging.text.StyledTextOutput; +import org.gradle.internal.logging.text.StyledTextOutputFactory; import org.gradle.work.DisableCachingByDefault; +import org.gradle.workers.WorkerExecutor; import software.amazon.smithy.gradle.SmithyUtils; /** @@ -20,19 +31,43 @@ * (that is, tasks that are meant to be run ad-hoc). */ @DisableCachingByDefault(because = "Abstract super-class, not to be instantiated directly") -abstract class AbstractSmithyCliTask extends BaseSmithyTask { +abstract class AbstractSmithyCliTask extends DefaultTask { /** * Object factory used to create new gradle domain objects such as {@code FileCollection}s. */ protected final ObjectFactory objectFactory; + private final StartParameter startParameter; - AbstractSmithyCliTask(ObjectFactory objectFactory) { + AbstractSmithyCliTask(ObjectFactory objectFactory, StartParameter startParameter) { this.objectFactory = objectFactory; + this.startParameter = startParameter; + + getFork().convention(false); + getShowStackTrace().convention(ShowStacktrace.INTERNAL_EXCEPTIONS); getAllowUnknownTraits().convention(false); + + // By default, the build classpath and model discovery classpaths are empty file collections. + getBuildClasspath().set(getProject().files()); + getModelDiscoveryClasspath().set(getProject().files()); + + // if the smithyCli configuration exists use it by default + if (getProject().getConfigurations().findByName(SmithyUtils.SMITHY_CLI_CONFIGURATION_NAME) != null) { + getCliClasspath().convention(getProject().getConfigurations() + .getByName(SmithyUtils.SMITHY_CLI_CONFIGURATION_NAME)); + } } - /** Sets whether to fail a Smithy CLI task if an unknown trait is encountered. + /** + * Base classpath used for executing the smithy cli. + * + *

Note: this classpath must contain the smithy-cli jar. + */ + @Classpath + public abstract Property getCliClasspath(); + + /** + * Sets whether to fail a Smithy CLI task if an unknown trait is encountered. * *

Defaults to {@code false} * @@ -42,6 +77,85 @@ abstract class AbstractSmithyCliTask extends BaseSmithyTask { @Optional public abstract Property getAllowUnknownTraits(); + /** + * Classpath used for discovery of additional Smithy models during cli execution. + * + *

Defaults to an empty collection. + */ + @Classpath + @Optional + public abstract Property getModelDiscoveryClasspath(); + + /** + * Classpath to use for build dependencies. + */ + @Classpath + @Optional + public abstract Property getBuildClasspath(); + + /** + * Gets the list of models to execute a CLI command on. + * + *

These models are also considered "sources" when building a JAR + * for a project. A source model is a model that appears in the + * {@code META-INF/smithy} directory of a JAR. + * + *

This method will return an empty {@code FileCollection} and + * never {@code null}. + * + * @return Returns the models to validate. + */ + @InputFiles + @Optional + public abstract Property getModels(); + + /** + * Whether to fork a new process for executing the smithy cli. + * + *

If false, the smithy cli will be executed in a new thread instead of a + * new process + * + *

Defaults to {@code false} + * + * @return flag indicating fork setting. + */ + @Input + @Optional + public abstract Property getFork(); + + /** + * Sets the detail to include in stack traces. + * + *

Defaults to {@code ShowStacktrace.INTERNAL_EXCEPTIONS} + * + * @return stack trace setting. + */ + @Input + @Optional + public abstract Property getShowStackTrace(); + + /** + * Read-only property that returns the classpath used to determine the + * classpath used when executing the cli. + * + * @return classpath to use when executing cli command. + */ + @Internal + Provider getCliExecutionClasspath() { + return getCliClasspath().zip(getBuildClasspath(), FileCollection::plus); + } + + /** + * Read-only property that returns a worker executor to use for executing a CLI + * command. + * + * @return executor to use for CLI command. + */ + @Internal + WorkerExecutor getExecutor() { + return getServices().get(WorkerExecutor.class); + } + /** * Executes the given CLI command. * @@ -52,7 +166,7 @@ abstract class AbstractSmithyCliTask extends BaseSmithyTask { * @param additionalArgs Custom arguments that aren't one of the shared args. * @param sources Source files to execute the command on */ - final void executeCliProcess(String command, + protected void executeCliProcess(String command, List additionalArgs, FileCollection sources, boolean disableModelDiscovery @@ -81,4 +195,51 @@ final void executeCliProcess(String command, SmithyUtils.executeCli(getExecutor(), args, getCliExecutionClasspath().get(), getFork().get()); } + + /** + * Writes header-formatted text to the build output. + * + * @param text text to write as a header. + */ + protected void writeHeading(String text) { + StyledTextOutput output = getServices().get(StyledTextOutputFactory.class) + .create("smithy") + .style(StyledTextOutput.Style.Header); + output.println(text); + } + + /** + * Add --stacktrace and --logging settings based on Gradle's settings. + */ + protected void configureLoggingOptions(final List args) { + ShowStacktrace showStacktrace = getShowStackTrace().get(); + if (showStacktrace == ShowStacktrace.ALWAYS || showStacktrace == ShowStacktrace.ALWAYS_FULL) { + args.add("--stacktrace"); + } + + switch (startParameter.getLogLevel()) { + case DEBUG: + args.add("--debug"); + break; + case LIFECYCLE: // The default setting in Gradle, so use Smithy's default of WARNING. + case WARN: + args.add("--logging"); + args.add(Level.WARNING.toString()); + break; + case QUIET: + args.add("--logging"); + args.add(Level.OFF.toString()); + args.add("--quiet"); + break; + case ERROR: + args.add("--logging"); + args.add(Level.SEVERE.toString()); + break; + case INFO: + default: + args.add("--logging"); + args.add(Level.INFO.toString()); + break; + } + } } diff --git a/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/BaseSmithyTask.java b/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/BaseSmithyTask.java deleted file mode 100644 index 3a648af..0000000 --- a/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/BaseSmithyTask.java +++ /dev/null @@ -1,193 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -package software.amazon.smithy.gradle.tasks; - -import java.util.List; -import java.util.logging.Level; -import org.gradle.StartParameter; -import org.gradle.api.DefaultTask; -import org.gradle.api.file.FileCollection; -import org.gradle.api.logging.configuration.ShowStacktrace; -import org.gradle.api.provider.Property; -import org.gradle.api.provider.Provider; -import org.gradle.api.tasks.Classpath; -import org.gradle.api.tasks.Input; -import org.gradle.api.tasks.InputFiles; -import org.gradle.api.tasks.Internal; -import org.gradle.api.tasks.Optional; -import org.gradle.internal.logging.text.StyledTextOutput; -import org.gradle.internal.logging.text.StyledTextOutputFactory; -import org.gradle.work.DisableCachingByDefault; -import org.gradle.workers.WorkerExecutor; -import software.amazon.smithy.gradle.SmithyUtils; - -/** - * Base class for all Smithy tasks. - */ -@DisableCachingByDefault(because = "Abstract super-class, not to be instantiated directly") -abstract class BaseSmithyTask extends DefaultTask { - - private final StartParameter startParameter; - - BaseSmithyTask() { - getFork().convention(false); - getShowStackTrace().convention(ShowStacktrace.INTERNAL_EXCEPTIONS); - - // By default, the build classpath and model discovery classpaths are empty file collections. - getBuildClasspath().set(getProject().files()); - getModelDiscoveryClasspath().set(getProject().files()); - - // if the smithyCli configuration exists use it by default - if (getProject().getConfigurations().findByName(SmithyUtils.SMITHY_CLI_CONFIGURATION_NAME) != null) { - getCliClasspath().convention(getProject().getConfigurations() - .getByName(SmithyUtils.SMITHY_CLI_CONFIGURATION_NAME)); - } - - startParameter = getProject().getGradle().getStartParameter(); - } - - - /** - * Base classpath used for executing the smithy cli. - * - *

Note: this classpath must contain the smithy-cli jar. - */ - @Classpath - public abstract Property getCliClasspath(); - - /** - * Classpath used for discovery of additional Smithy models during cli execution. - * - *

Defaults to an empty collection. - */ - @Classpath - @Optional - public abstract Property getModelDiscoveryClasspath(); - - /** - * Classpath to use for build dependencies. - */ - @Classpath - @Optional - public abstract Property getBuildClasspath(); - - - /** - * Read-only property that returns the classpath used to determine the - * classpath used when executing the cli. - * - * @return classpath to use when executing cli command. - */ - @Internal - Provider getCliExecutionClasspath() { - return getCliClasspath().zip(getBuildClasspath(), FileCollection::plus); - } - - /** - * Gets the list of models to execute a CLI command on. - * - *

These models are also considered "sources" when building a JAR - * for a project. A source model is a model that appears in the - * {@code META-INF/smithy} directory of a JAR. - * - *

This method will return an empty {@code FileCollection} and - * never {@code null}. - * - * @return Returns the models to validate. - */ - @InputFiles - @Optional - public abstract Property getModels(); - - /** - * Whether to fork a new process for executing the smithy cli. - * - *

If false, the smithy cli will be executed in a new thread instead of a - * new process - * - *

Defaults to {@code false} - * - * @return flag indicating fork setting. - */ - @Input - @Optional - public abstract Property getFork(); - - - /** Sets the detail to include in stack traces. - * - *

Defaults to {@code ShowStacktrace.INTERNAL_EXCEPTIONS} - * - * @return stack trace setting. - */ - @Input - @Optional - public abstract Property getShowStackTrace(); - - /** - * Set the minimum reported validation severity. - * - *

This value should be one of: NOTE, WARNING [default], DANGER, ERROR. - * - * @return minimum validator severity - */ - @Input - @Optional - public abstract Property getSeverity(); - - @Internal - WorkerExecutor getExecutor() { - return getServices().get(WorkerExecutor.class); - } - - - /** - * Writes header-formatted text to the build output. - * - * @param text text to write as a header. - */ - protected void writeHeading(String text) { - StyledTextOutput output = getServices().get(StyledTextOutputFactory.class) - .create("smithy") - .style(StyledTextOutput.Style.Header); - output.println(text); - } - - /** - * Add --stacktrace and --logging settings based on Gradle's settings. - */ - void configureLoggingOptions(final List args) { - ShowStacktrace showStacktrace = getShowStackTrace().get(); - if (showStacktrace == ShowStacktrace.ALWAYS || showStacktrace == ShowStacktrace.ALWAYS_FULL) { - args.add("--stacktrace"); - } - - switch (startParameter.getLogLevel()) { - case DEBUG: - args.add("--debug"); - break; - case LIFECYCLE: // The default setting in Gradle, so use Smithy's default of WARNING. - case WARN: - args.add("--logging"); - args.add(Level.WARNING.toString()); - break; - case QUIET: - args.add("--logging"); - args.add(Level.OFF.toString()); - args.add("--quiet"); - break; - case ERROR: - args.add("--logging"); - args.add(Level.SEVERE.toString()); - break; - case INFO: - default: - args.add("--logging"); - args.add(Level.INFO.toString()); - break; - } - } -} diff --git a/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/SmithyBuildTask.java b/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/SmithyBuildTask.java index f04a6b5..3e6a475 100644 --- a/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/SmithyBuildTask.java +++ b/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/SmithyBuildTask.java @@ -10,6 +10,7 @@ import java.util.List; import java.util.stream.Collectors; import javax.inject.Inject; +import org.gradle.StartParameter; import org.gradle.api.GradleException; import org.gradle.api.file.DirectoryProperty; import org.gradle.api.file.FileCollection; @@ -36,8 +37,8 @@ */ public abstract class SmithyBuildTask extends AbstractSmithyCliTask { @Inject - public SmithyBuildTask(ObjectFactory objectFactory) { - super(objectFactory); + public SmithyBuildTask(ObjectFactory objectFactory, StartParameter startParameter) { + super(objectFactory, startParameter); getSourceProjection().convention("source"); getSeverity().convention(Severity.WARNING.toString()); @@ -69,18 +70,6 @@ public SmithyBuildTask(ObjectFactory objectFactory) { @InputFiles public abstract Property getSmithyBuildConfigs(); - /** - * Sets whether to fail a {@link SmithyBuildTask} if an unknown trait is encountered. - * - *

Defaults to {@code true}. - * - * @return flag indicating state of allowUnknownTraits setting. - */ - @Input - @Optional - public abstract Property getAllowUnknownTraits(); - - /** * Projection to treat as the "source" or primary projection. */ @@ -88,7 +77,6 @@ public SmithyBuildTask(ObjectFactory objectFactory) { @Optional public abstract Property getSourceProjection(); - /** * Output directory for Smithy build artifacts. */ @@ -96,6 +84,17 @@ public SmithyBuildTask(ObjectFactory objectFactory) { @Optional public abstract DirectoryProperty getOutputDir(); + /** + * Set the minimum reported validation severity. + * + *

This value should be one of: NOTE, WARNING [default], DANGER, ERROR. + * + * @return minimum validator severity + */ + @Input + @Optional + public abstract Property getSeverity(); + /** * Read-only property. * diff --git a/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/SmithyFormatTask.java b/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/SmithyFormatTask.java index f1bb941..eb5d5e1 100644 --- a/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/SmithyFormatTask.java +++ b/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/SmithyFormatTask.java @@ -7,6 +7,7 @@ import java.io.File; import javax.inject.Inject; +import org.gradle.StartParameter; import org.gradle.api.model.ObjectFactory; import org.gradle.api.tasks.TaskAction; import software.amazon.smithy.utils.ListUtils; @@ -28,8 +29,8 @@ public abstract class SmithyFormatTask extends AbstractSmithyCliTask { private static final String DESCRIPTION = "Formats smithy models."; @Inject - public SmithyFormatTask(ObjectFactory objectFactory) { - super(objectFactory); + public SmithyFormatTask(ObjectFactory objectFactory, StartParameter startParameter) { + super(objectFactory, startParameter); setDescription(DESCRIPTION); } diff --git a/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/SmithyValidateTask.java b/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/SmithyValidateTask.java index c89cbe0..2360c10 100644 --- a/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/SmithyValidateTask.java +++ b/smithy-base/src/main/java/software/amazon/smithy/gradle/tasks/SmithyValidateTask.java @@ -8,6 +8,7 @@ import java.util.ArrayList; import java.util.List; import javax.inject.Inject; +import org.gradle.StartParameter; import org.gradle.api.file.FileCollection; import org.gradle.api.model.ObjectFactory; import org.gradle.api.provider.Property; @@ -29,12 +30,11 @@ * */ public abstract class SmithyValidateTask extends AbstractSmithyCliTask { - private static final String DESCRIPTION = "Validates a jar containing smithy models."; - + private static final String DESCRIPTION = "Validates smithy models."; @Inject - public SmithyValidateTask(ObjectFactory objectFactory) { - super(objectFactory); + public SmithyValidateTask(ObjectFactory objectFactory, StartParameter startParameter) { + super(objectFactory, startParameter); getAllowUnknownTraits().convention(false); getDisableModelDiscovery().convention(false); getSeverity().convention(Severity.ERROR.toString());