From a342ff2a4800deb3da82032ced7380ce6aeda9e0 Mon Sep 17 00:00:00 2001 From: Andrew Parmet Date: Fri, 15 Nov 2024 16:45:58 -0500 Subject: [PATCH] Allow lint task to be configured after Buf v1.31.0 while using protobuf-gradle-plugin (#256) Gets custom linting working again with new versions of Buf. --- .../DirectorySpecificBufExecutionSupport.kt | 21 +++------------ .../build/buf/gradle/FormatApplyTask.kt | 2 +- .../build/buf/gradle/FormatCheckTask.kt | 2 +- .../build/buf/gradle/LintConfiguration.kt | 1 + src/main/kotlin/build/buf/gradle/LintTask.kt | 14 +++++++++- .../buf/gradle/LintWithProtobufGradleTest.kt | 12 ++++++++- .../buf.yaml | 0 .../build.gradle | 0 .../src/main/proto/buf/test/v1/test.proto | 0 .../buf.yaml | 9 +++++++ .../build.gradle | 22 +++++++++++++++ .../src/main/proto/buf/test/v1/test.proto | 27 +++++++++++++++++++ .../buf.yaml | 9 +++++++ .../build.gradle | 22 +++++++++++++++ .../src/main/proto/buf/test/v1/test.proto | 27 +++++++++++++++++++ 15 files changed, 146 insertions(+), 22 deletions(-) rename src/test/resources/LintWithProtobufGradleTest/{lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin => lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_pre_1_32_0}/buf.yaml (100%) rename src/test/resources/LintWithProtobufGradleTest/{lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin => lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_pre_1_32_0}/build.gradle (100%) rename src/test/resources/LintWithProtobufGradleTest/{lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin => lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_pre_1_32_0}/src/main/proto/buf/test/v1/test.proto (100%) create mode 100644 src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_v1/buf.yaml create mode 100644 src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_v1/build.gradle create mode 100644 src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_v1/src/main/proto/buf/test/v1/test.proto create mode 100644 src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_v2/buf.yaml create mode 100644 src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_v2/build.gradle create mode 100644 src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_v2/src/main/proto/buf/test/v1/test.proto diff --git a/src/main/kotlin/build/buf/gradle/DirectorySpecificBufExecutionSupport.kt b/src/main/kotlin/build/buf/gradle/DirectorySpecificBufExecutionSupport.kt index d3709f46..b0337df6 100644 --- a/src/main/kotlin/build/buf/gradle/DirectorySpecificBufExecutionSupport.kt +++ b/src/main/kotlin/build/buf/gradle/DirectorySpecificBufExecutionSupport.kt @@ -16,27 +16,12 @@ package build.buf.gradle import java.io.File -internal fun AbstractBufExecTask.execBufInSpecificDirectory( - vararg bufCommand: String, - customErrorMessage: ((String) -> String)? = null, -) { - execBufInSpecificDirectory(bufCommand.asList(), emptyList(), customErrorMessage) -} - internal fun AbstractBufExecTask.execBufInSpecificDirectory( bufCommand: String, - extraArgs: Iterable, - customErrorMessage: (String) -> String, -) { - execBufInSpecificDirectory(listOf(bufCommand), extraArgs, customErrorMessage) -} - -private fun AbstractBufExecTask.execBufInSpecificDirectory( - bufCommand: Iterable, - extraArgs: Iterable, + args: Iterable, customErrorMessage: ((String) -> String)? = null, ) { - fun runWithArgs(file: File? = null) = bufCommand + listOfNotNull(file?.let { makeMangledRelativizedPathStr(it) }) + extraArgs + fun runWithArgs(file: File? = null) = listOf(bufCommand) + listOfNotNull(file?.let { makeMangledRelativizedPathStr(it) }) + args when { hasProtobufGradlePlugin.get() -> @@ -44,7 +29,7 @@ private fun AbstractBufExecTask.execBufInSpecificDirectory( .filter { anyProtos(it) } .forEach { execBuf(runWithArgs(it), customErrorMessage) } hasWorkspace.get() -> - execBuf(bufCommand, customErrorMessage) + execBuf(listOf(bufCommand) + args, customErrorMessage) else -> execBuf(runWithArgs(), customErrorMessage) } diff --git a/src/main/kotlin/build/buf/gradle/FormatApplyTask.kt b/src/main/kotlin/build/buf/gradle/FormatApplyTask.kt index 9a753e63..7e7d63e9 100644 --- a/src/main/kotlin/build/buf/gradle/FormatApplyTask.kt +++ b/src/main/kotlin/build/buf/gradle/FormatApplyTask.kt @@ -30,6 +30,6 @@ abstract class FormatApplyTask : AbstractBufExecTask() { @TaskAction fun bufFormatApply() { - execBufInSpecificDirectory("format", "-w") + execBufInSpecificDirectory("format", listOf("-w")) } } diff --git a/src/main/kotlin/build/buf/gradle/FormatCheckTask.kt b/src/main/kotlin/build/buf/gradle/FormatCheckTask.kt index 04c5b8ae..d01f8577 100644 --- a/src/main/kotlin/build/buf/gradle/FormatCheckTask.kt +++ b/src/main/kotlin/build/buf/gradle/FormatCheckTask.kt @@ -25,7 +25,7 @@ abstract class FormatCheckTask : AbstractBufExecTask() { @TaskAction fun bufFormatCheck() { - execBufInSpecificDirectory("format", "-d", "--exit-code") { + execBufInSpecificDirectory("format", listOf("-d", "--exit-code")) { """ |Some Protobuf files had format violations: |$it diff --git a/src/main/kotlin/build/buf/gradle/LintConfiguration.kt b/src/main/kotlin/build/buf/gradle/LintConfiguration.kt index 1a56e6ec..bdf8a852 100644 --- a/src/main/kotlin/build/buf/gradle/LintConfiguration.kt +++ b/src/main/kotlin/build/buf/gradle/LintConfiguration.kt @@ -27,6 +27,7 @@ internal fun Project.configureLint() { bufConfigFile.set(project.bufConfigFile()) inputFiles.setFrom(obtainDefaultProtoFileSet()) + v1SyntaxOnly.set(bufV1SyntaxOnly()) } tasks.named(CHECK_TASK_NAME).dependsOn(BUF_LINT_TASK_NAME) diff --git a/src/main/kotlin/build/buf/gradle/LintTask.kt b/src/main/kotlin/build/buf/gradle/LintTask.kt index 8e3232ef..e6c3617c 100644 --- a/src/main/kotlin/build/buf/gradle/LintTask.kt +++ b/src/main/kotlin/build/buf/gradle/LintTask.kt @@ -16,6 +16,7 @@ package build.buf.gradle import org.gradle.api.file.ConfigurableFileCollection import org.gradle.api.provider.Property +import org.gradle.api.tasks.Input import org.gradle.api.tasks.InputFile import org.gradle.api.tasks.InputFiles import org.gradle.api.tasks.Optional @@ -34,11 +35,18 @@ abstract class LintTask : AbstractBufExecTask() { @get:Optional internal abstract val bufConfigFile: Property + @get:Input + internal abstract val v1SyntaxOnly: Property + @TaskAction fun bufLint() { execBufInSpecificDirectory( "lint", - bufConfigFile.orNull?.let { listOf("--config", it.readAndStripComments()) }.orEmpty(), + if (noWorkspaceAndV1Syntax() || noWorkspaceAndNoProtobufGradlePlugin()) { + bufConfigFile.orNull?.let { listOf("--config", it.readAndStripComments()) } + } else { + null + }.orEmpty(), ) { """ |Some Protobuf files had lint violations: @@ -47,6 +55,10 @@ abstract class LintTask : AbstractBufExecTask() { } } + private fun noWorkspaceAndV1Syntax() = !hasWorkspace.get() && v1SyntaxOnly.get() + + private fun noWorkspaceAndNoProtobufGradlePlugin() = !hasWorkspace.get() && !hasProtobufGradlePlugin.get() + private fun File.readAndStripComments() = lines(toPath()).use { lines -> lines.asSequence() diff --git a/src/test/kotlin/build/buf/gradle/LintWithProtobufGradleTest.kt b/src/test/kotlin/build/buf/gradle/LintWithProtobufGradleTest.kt index efb18244..e927bdb4 100644 --- a/src/test/kotlin/build/buf/gradle/LintWithProtobufGradleTest.kt +++ b/src/test/kotlin/build/buf/gradle/LintWithProtobufGradleTest.kt @@ -43,7 +43,17 @@ class LintWithProtobufGradleTest : ConfigOverrideableLintTests, AbstractLintTest } @Test - fun `lint a file with an implementation dependency and a lint config with the protobuf-gradle-plugin`() { + fun `lint a file with an implementation dependency and a lint config with the protobuf-gradle-plugin pre 1_32_0`() { + assertSuccess() + } + + @Test + fun `lint a file with an implementation dependency and a lint config with the protobuf-gradle-plugin v1`() { + assertSuccess() + } + + @Test + fun `lint a file with an implementation dependency and a lint config with the protobuf-gradle-plugin v2`() { assertSuccess() } } diff --git a/src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin/buf.yaml b/src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_pre_1_32_0/buf.yaml similarity index 100% rename from src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin/buf.yaml rename to src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_pre_1_32_0/buf.yaml diff --git a/src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin/build.gradle b/src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_pre_1_32_0/build.gradle similarity index 100% rename from src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin/build.gradle rename to src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_pre_1_32_0/build.gradle diff --git a/src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin/src/main/proto/buf/test/v1/test.proto b/src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_pre_1_32_0/src/main/proto/buf/test/v1/test.proto similarity index 100% rename from src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin/src/main/proto/buf/test/v1/test.proto rename to src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_pre_1_32_0/src/main/proto/buf/test/v1/test.proto diff --git a/src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_v1/buf.yaml b/src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_v1/buf.yaml new file mode 100644 index 00000000..683cfed5 --- /dev/null +++ b/src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_v1/buf.yaml @@ -0,0 +1,9 @@ +version: v1 +lint: + ignore: + - google + - protokt + use: + - STANDARD + except: + - ENUM_ZERO_VALUE_SUFFIX diff --git a/src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_v1/build.gradle b/src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_v1/build.gradle new file mode 100644 index 00000000..69b6e50e --- /dev/null +++ b/src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_v1/build.gradle @@ -0,0 +1,22 @@ +plugins { + id 'java' + id 'com.google.protobuf' version "$protobufGradleVersion" + id 'build.buf' +} + +repositories { + mavenCentral() +} + +protobuf { + protoc { + artifact = "com.google.protobuf:protoc:$protobufVersion" + } +} + +compileJava.enabled = false + +dependencies { + implementation "com.google.protobuf:protobuf-java:$protobufVersion" + implementation "com.toasttab.protokt:protokt-runtime:0.6.5" +} diff --git a/src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_v1/src/main/proto/buf/test/v1/test.proto b/src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_v1/src/main/proto/buf/test/v1/test.proto new file mode 100644 index 00000000..10cfa395 --- /dev/null +++ b/src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_v1/src/main/proto/buf/test/v1/test.proto @@ -0,0 +1,27 @@ +// Copyright 2024 Buf Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License 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. + +syntax = "proto3"; + +package buf.test.v1; + +import "protokt/protokt.proto"; + +message BasicMessage { + protokt.ProtoktFileOptions protokt_file_options = 1; + + enum BrokenEnum { + BROKEN_ENUM_NONSPECIFIED = 0; // should be _UNSPECIFIED + } +} diff --git a/src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_v2/buf.yaml b/src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_v2/buf.yaml new file mode 100644 index 00000000..05d2f99b --- /dev/null +++ b/src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_v2/buf.yaml @@ -0,0 +1,9 @@ +version: v2 +lint: + ignore: + - google + - protokt + use: + - STANDARD + except: + - ENUM_ZERO_VALUE_SUFFIX diff --git a/src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_v2/build.gradle b/src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_v2/build.gradle new file mode 100644 index 00000000..69b6e50e --- /dev/null +++ b/src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_v2/build.gradle @@ -0,0 +1,22 @@ +plugins { + id 'java' + id 'com.google.protobuf' version "$protobufGradleVersion" + id 'build.buf' +} + +repositories { + mavenCentral() +} + +protobuf { + protoc { + artifact = "com.google.protobuf:protoc:$protobufVersion" + } +} + +compileJava.enabled = false + +dependencies { + implementation "com.google.protobuf:protobuf-java:$protobufVersion" + implementation "com.toasttab.protokt:protokt-runtime:0.6.5" +} diff --git a/src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_v2/src/main/proto/buf/test/v1/test.proto b/src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_v2/src/main/proto/buf/test/v1/test.proto new file mode 100644 index 00000000..10cfa395 --- /dev/null +++ b/src/test/resources/LintWithProtobufGradleTest/lint_a_file_with_an_implementation_dependency_and_a_lint_config_with_the_protobuf_gradle_plugin_v2/src/main/proto/buf/test/v1/test.proto @@ -0,0 +1,27 @@ +// Copyright 2024 Buf Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License 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. + +syntax = "proto3"; + +package buf.test.v1; + +import "protokt/protokt.proto"; + +message BasicMessage { + protokt.ProtoktFileOptions protokt_file_options = 1; + + enum BrokenEnum { + BROKEN_ENUM_NONSPECIFIED = 0; // should be _UNSPECIFIED + } +}