Apply UnnecessaryParentheses

[Pankraz76]

enabler for:

• Add rewrite support for UnnecessaryParentheses #7934

[Pankraz76]

if you consider this let me know i need to revert the maven suggestions then if these could not be applied as well.

[Pankraz76]

[INFO] Tests run: 0, Failures: 0, Errors: 0, Skipped: 0
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for Guava Maven Parent 999.0.0-HEAD-jre-SNAPSHOT:
[INFO]
[INFO] Guava Maven Parent ................................. SUCCESS [ 0.114 s]
[INFO] Guava: Google Core Libraries for Java .............. SUCCESS [ 3.942 s]
[INFO] Guava BOM .......................................... SUCCESS [ 0.000 s]
[INFO] Guava Testing Library .............................. SUCCESS [01:40 min]
[INFO] Guava Unit Tests ................................... SUCCESS [07:29 min]
[INFO] Guava GWT compatible libs .......................... SUCCESS [ 6.779 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 09:20 min
[INFO] Finished at: 2025-08-09T14:33:11+02:00
[INFO] ------------------------------------------------------------------------

[Stephan202]

The pom.xml changes seem unrelated to the application of the Refaster rules. Just some drive-by comments.

[Pankraz76]

The pom.xml changes seem unrelated

yes sorry, had dirty working copy including:

• https://docs.openrewrite.org/recipes/maven/bestpractices
• Add staticanalysis.UnnecessaryParentheses #7916

Will undo/assuming we gonna merge this dedicated:

• Add openrewrite.maven.BestPractices #7932

[Pankraz76]

of course this change should not be merges this way, considering simple replication of the UnnecessaryParentheses in dedicated branch, pulling this upfront.

[cpovirk]

I'd be happy to take the UnnecessaryParentheses changes if you're interested in creating a PR for just them. Some of the library changes look like things that we can't take for Android reasons, but length() == 0 -> isEmpty() would also be good, as would (I think) BigDecimal.valueOf.

[Pankraz76]

Can rebase at any time, but due to the code density, conflicts are likely to arise soon. This is another good example why the plugin is in need to avoid spending time on issues like this. As it will be happen very soon after its fixed once but not forever. Making the whole effort most efficient if fixed for the future as well.

It's either a one-time effort to fix it now, or dealing with it constantly—which is inefficient until it's properly resolved once.

[cpovirk]

The best way to avoid spending time on this level of issue is often just to ignore it, sadly. I do think that the mass fix here (which looks to be more aggressive than Error Prone was in #7696 and #7771) is worthwhile, and I have sent it for review internally. Still, I think the cost of the first breakage from a build-time check here would outweigh the benefits of the backsliding protection, and I am happy to leave the checking off. If we want to clean up such errors in the future, I think the most efficient way to do that will be another mass run of the check.

[Pankraz76]

sadly

What about tools like Checkstyle? Looking at large-scale projects like PMD and Checkstyle, they don’t have this issue. Maven also uses Spotless to automate these tedious tasks and discussions, no offence. Its about fixing issues like you mentioning not about ignoring stuff. Its not an improvement leaving the flaws open considering this hindering to an easy access of the code.

Investing all the time spent on manual formatting fixes would be better spent on a single commit with a plugin.

We’re past the era where formatting errors require:

• Worst case: Being ignored
• Bad case: Manual fixes
• Best case: Automation via tools like Spotless

Major projects like Maven, Quarkus, and Spring all rely on automation—because at scale, there’s no other practical way.

[Pankraz76]

This could be made simply by merging and activating the failOnDryRunResults param afterwards making the commit easy and reproducible for everyone.

[cpovirk]

There is a lot that can be said about both the importance of code simplifications/improvements and the way to make those improvements at the lowest cost (e.g., build errors/warnings vs. automated code-review comments vs. human action vs. periodic cleanups vs. presubmits). (I think that there may be a presentation of one perspective on this in https://storage.googleapis.com/gweb-research2023-media/pubtools/4365.pdf.) Our situation is more complicated still because our Maven build is sort of "secondary" in our normal workflow. Breakages that are delayed to that phase need to provide a lot of value before they pay off.

[cpovirk]

As I should probably have anticipated, and as I hinted at in #7934 (comment), there's some disagreement over the value of parentheses on ternary conditions. (Opinions range from "always" to "never," with space for intermediate options like "only if the conditional is 'complex' (e.g., foo == bar || foo instanceof Baz)" or "not if there's a lot of parenthesization already on the line.") It seems to fall into the range of "not controversial enough that we need to force a rule to prevent discussions but controversial enough that a rule would annoy people." I'm making an attempt to revert those changes and take what's left, but I may end up erring in one direction or the other.

[Pankraz76]

As I should probably have anticipated, and as I hinted at in #7934 (comment), there's some disagreement over the value of parentheses on ternary conditions. (Opinions range from "always" to "never," with space for intermediate options like "only if the conditional is 'complex' (e.g., foo == bar || foo instanceof Baz)" or "not if there's a lot of parenthesization already on the line.") It seems to fall into the range of "not controversial enough that we need to force a rule to prevent discussions but controversial enough that a rule would annoy people." I'm making an attempt to revert those changes and take what's left, but I may end up erring in one direction or the other.

yes "unfortunately" thats true, theres always the need for some exception, using suppression serving both parties.

Having a little freedom, while not having to strive in chaos.

[Pankraz76]

Assuming something much broader like RemoveUnusedPrivateMethods is someone most ppl. would agree upon to avoid. Even tho still some like to preserve making software archeology.

• Add rewrite support for staticanalysis.RemoveUnusedPrivateMethods #7938

[cpovirk]

I forgot to also post: A teammate referred me to https://google.github.io/styleguide/javaguide.html#s4.7-grouping-parentheses, which is the standard we're up against.

The Java changes in #7938 look good except for the ones that appear to be removing serialization methods that Java calls automatically.

[Pankraz76]

serialization methods that Java calls automatically

Thanks very interesting. Need to ignore these, in the recipe, if this is actually true/good.

[Pankraz76]

but again this effort is considered waste, as its not done, it will happen again.

We should try to stop starting and start finishing. As this obsolete stuff will occur anytime again if there is no quality gate imposed. Currently its bug/issue/flaw eldorado. I have fixed bugs in maven due to this kind of issue, so please dont argue its not important.

• [S1161] Add missing @Override to overriding and implementing methods apache/maven#2402
• [MNG-8729] Use correct outputStream destination; request instead of path in DefaultPluginXmlFactory#write apache/maven#2312
• Remove unused private methods apache/maven#2310

[Pankraz76]

serialization methods that Java calls automatically

Thanks very interesting. Need to ignore these, in the recipe, if this is actually true/good.

Is this something real or just some random google stuff? @timtebeek

https://github.com/search?q=repo%3Agoogle%2Fguava%20writeReplace&type=code

Inconsistent again. Its private. Is it called via reflection then or how should this work in runtime ?

---

Not even the code format is working, its really sad to see this important project in such conditions.

[Pankraz76]

• Add rewrite support for RemoveUnusedPrivateMethods #7944

[timtebeek]

serialization methods that Java calls automatically

Thanks very interesting. Need to ignore these, in the recipe, if this is actually true/good.

Is this something real or just some random google stuff? @timtebeek

This case was missed; fixed now and likely released later today: openrewrite/rewrite-static-analysis@976d366