gemini-code-assist[bot] commented on code in PR #38453:
URL: https://github.com/apache/beam/pull/38453#discussion_r3221840738
##########
buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy:
##########
@@ -1516,72 +1516,66 @@ class BeamModulePlugin implements Plugin<Project> {
project.tasks.analyzeDependencies.enabled = false
}
- // errorprone requires java9+ compiler. It can be used with Java8 but
then sets a java9+ errorproneJavac.
- // However, the redirect ignores any task that forks and defines either
a javaHome or an executable,
- // see https://github.com/tbroyer/gradle-errorprone-plugin#jdk-8-support
- // which means errorprone cannot run when gradle runs on Java11+ but
serve `-testJavaVersion=8 -Pjava8Home` options
- if (!(project.findProperty('testJavaVersion') == '8')) {
- // Enable errorprone static analysis
- project.apply plugin: 'net.ltgt.errorprone'
+ // Enable errorprone static analysis
+ project.apply plugin: 'net.ltgt.errorprone'
Review Comment:
The removal of the Java 8 guard for Error Prone (previously at line 1523)
and its associated explanatory comment may lead to build failures. As noted in
the removed comment, Error Prone has known issues when the Gradle runner is on
Java 11+ but the target is Java 8 (via `-testJavaVersion=8`). Unless the
underlying compatibility issue has been resolved, this guard should be
maintained to prevent regressions in Java 8 environments. Consider moving the
`testJavaVersion` detection logic (introduced at line 1621) higher up in the
method so it can be used here to safely exclude Java 8.
##########
buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy:
##########
@@ -1516,72 +1516,66 @@ class BeamModulePlugin implements Plugin<Project> {
project.tasks.analyzeDependencies.enabled = false
}
- // errorprone requires java9+ compiler. It can be used with Java8 but
then sets a java9+ errorproneJavac.
- // However, the redirect ignores any task that forks and defines either
a javaHome or an executable,
- // see https://github.com/tbroyer/gradle-errorprone-plugin#jdk-8-support
- // which means errorprone cannot run when gradle runs on Java11+ but
serve `-testJavaVersion=8 -Pjava8Home` options
- if (!(project.findProperty('testJavaVersion') == '8')) {
- // Enable errorprone static analysis
- project.apply plugin: 'net.ltgt.errorprone'
+ // Enable errorprone static analysis
+ project.apply plugin: 'net.ltgt.errorprone'
- project.dependencies {
-
errorprone("com.google.errorprone:error_prone_core:$errorprone_version")
- errorprone("jp.skypencil.errorprone.slf4j:errorprone-slf4j:0.1.28")
- }
-
- project.configurations.errorprone { resolutionStrategy.force
"com.google.errorprone:error_prone_core:$errorprone_version" }
-
- project.tasks.withType(JavaCompile) {
- options.errorprone.disableWarningsInGeneratedCode = true
- options.errorprone.excludedPaths =
'(.*/)?(build/generated-src|build/generated.*avro-java|build/generated)/.*'
-
- // Error Prone requires some packages to be exported/opened on Java
versions that support modules,
- // i.e. Java 9 and up. The flags became mandatory in Java 17 with
JEP-403.
- // The -J prefix is not needed if forkOptions.javaHome is unset,
- // see http://github.com/gradle/gradle/issues/22747
- if (options.forkOptions.javaHome == null) {
- options.fork = true
- options.forkOptions.jvmArgs += errorProneAddModuleOpts
- }
- def disabledChecks = [
- // TODO(https://github.com/apache/beam/issues/20955): Enable
errorprone checks
- "AutoValueImmutableFields",
- "ComparableType",
- "DoNotMockAutoValue",
- "EmptyBlockTag",
- "ExtendsAutoValue",
- "InlineMeSuggester",
- "InvalidBlockTag",
- "JodaConstructors",
- "MixedMutabilityReturnType",
- "PreferJavaTimeOverload",
- "Slf4jSignOnlyFormat",
- "UnrecognisedJavadocTag",
- // errorprone 3.2.0+ checks
- "DirectInvocationOnMock",
- "MockNotUsedInProduction",
- "NullableWildcard",
- "SuperCallToObjectMethod",
- // Intended suppressions with justifications
- // for encoding efficiency and backward compatibility
- "EnumOrdinal",
- // widely used in non-public methods
- "NotJavadoc",
- // return values used for assignments widely, and for backward
compatibility.
- "NonApiType",
- // Used to test self equal
- "SelfAssertion",
- // Sometimes a static logger is preferred, which is the convention
currently used in beam. See docs:
- //
https://github.com/KengoTODA/findbugs-slf4j#slf4j_logger_should_be_non_static
- "Slf4jLoggerShouldBeNonStatic",
- // allow implicit Locale.Default
- "StringCaseLocaleUsage",
- // DoFn methods are executed reflectively at pipeline runtime
- "UnusedMethod",
- ]
- disabledChecks.each {
- options.errorprone.errorproneArgs.add("-Xep:${it}:OFF")
- }
+ project.dependencies {
+
errorprone("com.google.errorprone:error_prone_core:$errorprone_version")
+ errorprone("jp.skypencil.errorprone.slf4j:errorprone-slf4j:0.1.28")
+ }
+
+ project.configurations.errorprone { resolutionStrategy.force
"com.google.errorprone:error_prone_core:$errorprone_version" }
+
+ project.tasks.withType(JavaCompile) {
+ options.errorprone.disableWarningsInGeneratedCode = true
+ options.errorprone.excludedPaths =
'(.*/)?(build/generated-src|build/generated.*avro-java|build/generated)/.*'
+
+ // Error Prone requires some packages to be exported/opened on Java
versions that support modules,
+ // i.e. Java 9 and up. The flags became mandatory in Java 17 with
JEP-403.
+ // The -J prefix is not needed if forkOptions.javaHome is unset,
+ // see http://github.com/gradle/gradle/issues/22747
+ if (options.forkOptions.javaHome == null) {
+ options.fork = true
+ options.forkOptions.jvmArgs += errorProneAddModuleOpts
+ }
+ def disabledChecks = [
+ // TODO(https://github.com/apache/beam/issues/20955): Enable
errorprone checks
+ "AutoValueImmutableFields",
+ "ComparableType",
+ "DoNotMockAutoValue",
+ "EmptyBlockTag",
+ "ExtendsAutoValue",
+ "InlineMeSuggester",
+ "InvalidBlockTag",
+ "JodaConstructors",
+ "MixedMutabilityReturnType",
+ "PreferJavaTimeOverload",
+ "Slf4jSignOnlyFormat",
+ "UnrecognisedJavadocTag",
Review Comment:
There is a typo in the Error Prone check name. It should be
`UnrecognizedJavadocTag` (with a 'z') instead of `UnrecognisedJavadocTag`.
Because of this typo, the check is likely not being disabled as intended.
```
"UnrecognizedJavadocTag",
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
