Apply errorprone suggestions using `PatchLocation:IN_PLACE` for KillLeakingJavaProcesses by Pankraz76 · Pull Request #35019 · gradle/gradle

Pankraz76 · 2025-09-17T09:16:20Z

Context

Contributor Checklist

Review Contribution Guidelines.
Make sure that all commits are signed off to indicate that you agree to the terms of Developer Certificate of Origin.
Make sure all contributed code can be distributed under the terms of the Apache License 2.0, e.g. the code was written by yourself or the original code is licensed under a license compatible to Apache License 2.0.
Check "Allow edit from maintainers" option in pull request so that additional changes can be pushed by Gradle team.
Provide integration tests (under <subproject>/src/integTest) to verify changes from a user perspective.
Provide unit tests (under <subproject>/src/test) to verify logic.
Update User Guide, DSL Reference, and Javadoc for public-facing changes.
Ensure that tests pass sanity check: ./gradlew sanityCheck.
Ensure that tests pass locally: ./gradlew <changed-subproject>:quickTest.

Reviewing cheatsheet

Before merging the PR, comments starting with

❌ ❓must be fixed
🤔 💅 should be fixed
💭 may be fixed
🎉 celebrate happy things

Pankraz76

finally have found the way to apply this not wasting so much time on manual labor.

Pankraz76 · 2025-09-19T11:02:02Z

assumption: need to move config to root then its going to work out.

ov7a

Thank you for the contribution.

I'm a bit skeptical about this change.

First of all, it's experimental.

Second, the value is questionable. It introduces changes that's hard to understand (unless it provides a detailed log, if that's the case, please share it).

Also, a human needs to double-check the output. Take, for example, ZoneId change - it's fine in the context of this particular build logic, but it would be wrong in the context of a build (because the timestamp would not be reproducible). With auto-fix, it would be tempting to ignore dismiss that.

Pankraz76 · 2025-09-21T13:00:13Z

requested report:

Type-safe project accessors is an incubating feature.nvironment > Resolve dependencies of classpath > gradle-kotlin-dsl-plugins-6.4.0.pom

> Task :build-logic:documentation:compileGroovy
Groovy compilation avoidance is an incubating feature.
Note: /Users/vincent.potucek/IdeaProjects/gradle/build-logic/documentation/src/main/groovy/gradlebuild/docs/model/SimpleClassMetaDataRepository.java uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.

> Task :build-logic:cleanup:compileJava
/Users/vincent.potucek/IdeaProjects/gradle/build-logic/cleanup/src/main/java/gradlebuild/cleanup/services/KillLeakingJavaProcesses.java:141: warning: [JavaTimeDefaultTimeZone] LocalDateTime.now() is not allowed because it silently uses the system default time-zone. You must pass an explicit time-zone (e.g., ZoneId.of("America/Los_Angeles")) to this method.
        String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyy-MM-dd-HH-mm-ss"));
                                            ^
    (see https://errorprone.info/bugpattern/JavaTimeDefaultTimeZone)
  Did you mean 'String timestamp = LocalDateTime.now(ZoneId.systemDefault()).format(DateTimeFormatter.ofPattern("yyyy-MM-dd-HH-mm-ss"));'?
/Users/vincent.potucek/IdeaProjects/gradle/build-logic/cleanup/src/main/java/gradlebuild/cleanup/services/KillLeakingJavaProcesses.java:250: warning: [DefaultCharset] Implicit use of the platform default charset, which can result in differing behaviour between JVM executions or incorrect behavior if the encoding of the data source doesn't match expectations.
            return new ExecResult(args, process.exitValue(), stdout.toString(), stderr.toString());
                                                                            ^
    (see https://errorprone.info/bugpattern/DefaultCharset)
  Did you mean 'return new ExecResult(args, process.exitValue(), stdout.toString(UTF_8), stderr.toString());' or 'return new ExecResult(args, process.exitValue(), stdout.toString(Charset.defaultCharset()), stderr.toString());'?
/Users/vincent.potucek/IdeaProjects/gradle/build-logic/cleanup/src/main/java/gradlebuild/cleanup/services/KillLeakingJavaProcesses.java:250: warning: [DefaultCharset] Implicit use of the platform default charset, which can result in differing behaviour between JVM executions or incorrect behavior if the encoding of the data source doesn't match expectations.
            return new ExecResult(args, process.exitValue(), stdout.toString(), stderr.toString());
                                                                                               ^
    (see https://errorprone.info/bugpattern/DefaultCharset)
  Did you mean 'return new ExecResult(args, process.exitValue(), stdout.toString(), stderr.toString(UTF_8));' or 'return new ExecResult(args, process.exitValue(), stdout.toString(), stderr.toString(Charset.defaultCharset()));'?
/Users/vincent.potucek/IdeaProjects/gradle/build-logic/cleanup/src/main/java/gradlebuild/cleanup/services/KillLeakingJavaProcesses.java:261: warning: [DefaultCharset] Implicit use of the platform default charset, which can result in differing behaviour between JVM executions or incorrect behavior if the encoding of the data source doesn't match expectations.
                BufferedReader reader = new BufferedReader(new InputStreamReader(forkedProcessOutput));
                                                           ^
    (see https://errorprone.info/bugpattern/DefaultCharset)
  Did you mean 'BufferedReader reader = new BufferedReader(new InputStreamReader(forkedProcessOutput, UTF_8));' or 'BufferedReader reader = new BufferedReader(new InputStreamReader(forkedProcessOutput, Charset.defaultCharset()));'?
/Users/vincent.potucek/IdeaProjects/gradle/build-logic/cleanup/src/main/java/gradlebuild/cleanup/services/KillLeakingJavaProcesses.java:267: warning: [CatchAndPrintStackTrace] Logging or rethrowing exceptions should usually be preferred to catching and calling printStackTrace
                e.printStackTrace();
                ^
    (see https://errorprone.info/bugpattern/CatchAndPrintStackTrace)
Refactoring changes were successfully applied to file:///Users/vincent.potucek/IdeaProjects/gradle/build-logic/cleanup/src/main/java/gradlebuild/cleanup/services/KillLeakingJavaProcesses.java, please check the refactored code and recompile.
5 warnings
<=<=------------> 8% CONFIGURING [2m 30s]
> IDLE
> IDLE
> IDLE
> IDLE
> root project > :build-logic:profiling:generatePrecompiledScriptPluginAccessors
> IDLE
> IDLE
> root project > :build-logic:cleanup:compileJava
> IDLE
^C%                                                                                                                                                                                                                                      ➜  gradle git:(in-place-UnusedMethod) ✗

Pankraz76 · 2025-09-21T13:09:28Z

Notes on ErrorProne Usage

A human should always double-check the output.

ErrorProne is not about inventing or relying on custom opinions or arguments regarding standards.
Instead, it focuses on providing opinionated, enforced suggestions to automatically fix discovered issues.

The outcome is what ultimately matters. Copy-and-paste workarounds do not scale,
which is why it makes sense to activate the full capabilities of ErrorProne.
Considering it covers topics similar to PMD, the key value of ErrorProne is automation. Right, @rickie?

That’s why we have already integrated it into Checkstyle,
enforcing standards by automating them.

It might still be considered experimental,
but it has been working reliably — especially in light of the related PR.

However, it is not yet fully correct,
since the configuration only applies to specific projects and not globally, as might be expected.

Suggested References for a Perfectly Pitched Demo & Argumentation (by @rickie)

Signed-off-by: Vincent Potucek <vpotucek@me.com>

bot-gradle added from:contributor PR by an external contributor to-triage labels Sep 17, 2025

Pankraz76 commented Sep 17, 2025

View reviewed changes

Comment thread build-logic/cleanup/src/main/java/gradlebuild/cleanup/services/KillLeakingJavaProcesses.java

Comment thread build-logic-commons/gradle-plugin/src/main/kotlin/gradlebuild.code-quality.gradle.kts Outdated

This comment was marked as resolved.

Sign in to view

ov7a self-assigned this Sep 17, 2025

ov7a added in:building-gradle gradle/gradle build and removed to-triage labels Sep 17, 2025

ov7a reviewed Sep 19, 2025

View reviewed changes

Comment thread build-logic-commons/gradle-plugin/src/main/kotlin/gradlebuild.code-quality.gradle.kts Outdated

This comment was marked as resolved.

Sign in to view

cobexer requested changes Sep 23, 2025

View reviewed changes

Comment thread build-logic-commons/gradle-plugin/src/main/kotlin/gradlebuild.code-quality.gradle.kts Outdated

Apply errorprone suggestions using PatchLocation:IN_PLACE

65b9230

Signed-off-by: Vincent Potucek <vpotucek@me.com>

ov7a approved these changes Sep 23, 2025

View reviewed changes

ov7a changed the title ~~Apply errorprone suggestions using PatchLocation:IN_PLACE~~ Apply errorprone suggestions using PatchLocation:IN_PLACE for KillLeakingJavaProcesses Sep 23, 2025

ov7a enabled auto-merge September 23, 2025 15:08

cobexer approved these changes Sep 23, 2025

View reviewed changes

ov7a added this pull request to the merge queue Sep 23, 2025

bot-gradle added this to the 9.3.0 RC1 milestone Sep 23, 2025

Merged via the queue into gradle:master with commit 8b0b4d6 Sep 23, 2025
19 of 20 checks passed

hegyibalint mentioned this pull request Oct 3, 2025

Add rewrite support for errorprone.refasterrules #35203

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Apply errorprone suggestions using `PatchLocation:IN_PLACE` for KillLeakingJavaProcesses#35019

Apply errorprone suggestions using `PatchLocation:IN_PLACE` for KillLeakingJavaProcesses#35019
ov7a merged 1 commit into
gradle:masterfrom
Pankraz76:in-place-UnusedMethod-pr

Pankraz76 commented Sep 17, 2025

Uh oh!

Pankraz76 left a comment

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

Pankraz76 commented Sep 19, 2025

Uh oh!

ov7a left a comment •

edited

Loading

Uh oh!

Uh oh!

Pankraz76 commented Sep 21, 2025

Uh oh!

Pankraz76 commented Sep 21, 2025

Uh oh!

This comment was marked as resolved.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

Pankraz76 commented Sep 17, 2025

Context

Contributor Checklist

Reviewing cheatsheet

Uh oh!

Pankraz76 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

Pankraz76 commented Sep 19, 2025

Uh oh!

ov7a left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Pankraz76 commented Sep 21, 2025

Uh oh!

Pankraz76 commented Sep 21, 2025

Notes on ErrorProne Usage

Suggested References for a Perfectly Pitched Demo & Argumentation (by @rickie)

Uh oh!

This comment was marked as resolved.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ov7a left a comment •

edited

Loading