Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

BUILD SUCCESS

Thanks. I'm probably not going to try to figure out what it takes to integrate things fully, but I will try to eventually have a look at the pom.xml changes to see if there's enough benefit there to change my mind.

Thanks for the response.

integrate things fully

You mean the actual runtime?

To make this opt in and still last, we could simply disable by default the execution via switch rewrite.skip=true.

Just discovered you are (of course) a committer to error-prone as well, so im wondering why are you hesistating?
Im sure you have good intend and reasoning to do so.

Despite the large time consumption, its generally nice to fixes instead of dealing with them bugs manually, right?

Hi,

might consider these changes suggested by error.prone. Then talk about possible CI integration depending on the actual time spend. Just a few minuets might be accepbtable as positive tradeoff for the flaws/issues being fixed.

Thanks.

• Apply UnnecessaryParentheses #7930

There are a lot of changes in this and your other PR, and I would like to manage to spend a bunch of time to think them all through and offer a good response. Unfortunately, I have other responsibilities that I'm falling down on, so I'm trying to focus my Guava time more on changes that are user-facing. So I'll speak a little more generally.

Guava has been, in our experience, a worst-case scenario for static analysis:

• It's a highly tested codebase with relatively few bugs. (To be completely clear, it has a handful of significant, known bugs, but they are things like subtle concurrency issues that aren't typically caught by static analysis.) This means that the suspicious-looking patterns that survive are frequently the edge cases in which those patterns are actually correct.
• It has a lot of older code. This means that it may follow practices of an earlier era. Modernizations to that can be welcome, but full modernization is a big project that is not necessarily worth the effort, even with significant automation.
• Some known problems can't be fixed for API-compatibility reasons.
• Guava supports a variety of environments (e.g., Java 8, Android, GWT, J2CL, J2ObjC), so it can't use all the libraries that you'd hope for. Even beyond that, it's just used widely in general, so there's an unusually high chance that a "safe" change will cause trouble for someone (example).

So, even as someone who loves static analysis, I am ambivalent about it for Guava. We do occasionally get nice improvements out of it, but when I fix static-analysis warnings, it's usually "so that those warnings will stop getting in our way," not "so that the code gets better." For Error Prone,that's been a long, ongoing process, one that I'm still not sure justifies the time spent (even, again, when much of the process is automated).

Given that, I expect for additional static analysis for Guava to often feel more like a source of interruptions than like a source of improvements that end up justifying the costs. That says less about the static analysis than it does about Guava. (Certainly OpenRewrite has a better reputation than a variety of other static analyzers that I've seen run on Guava over the years, and the results so far have looked better.)

I am likely to still pull some changes out of the PRs (like those for imports, @Override annotations, and the now-useless test.add.args property that you've mentioned). But I don't expect to turn on further static analysis for the project. If there are specific changes you'd like to see enough to go to further effort, I'd suggest trying separate PRs. I want to make clear now that there's still a decent chance that we'll turn down or indefinitely postpone any of them (for the reasons already discussed or because they cause merge conflicts with our slightly different internal copy of Guava or because there are different viewpoints on specific best practices).B ut if there are specific changes that you believe strongly in but that don't catch my eye, that would be the way to go.

scope adjusted to UnnecessaryParentheses

I am confusing myself a bit with the PRs here, sorry. I think I did end up merging the pom.xml changes that I intended, and then I had closed this in favor of #7930 and any other future PRs that we try to push through. So maybe that did makes sense.