ReplaceCollectToListWithToList

[Pankraz76]

• Modernize codebase with Java improvements apache/maven#2277
• [POC] rewrite-maven-plugin: Introduce OpenRewrite by Moderne apache/maven#2322
• [MNG-5913] Allow defining aliases for existing server configurations in settings.xml apache/maven#2333
• [MNG-5913] Allow defining aliases for existing server configurations in settings.xml apache/maven#2333 (comment)

FYI: @gnodet @elharo

need suppression support for old API then we can fix it once. @timtebeek

[github-actions]

Some suggestions could not be made:

• src/main/resources/META-INF/rewrite/examples.yml
  • lines 660-659
  • lines 705-721
  • lines 1254-1273
  • lines 1306-1305
  • lines 2337-2336
  • lines 2352-2364
  • lines 2801-2800
  • lines 2820-2831

[gnodet]

The problem is that this code change is not semantically the same. The stream.collect(Collectors.toList()) returns a mutable list, while stream.toList() returns an immutable collection.

So this cannot be applied blindly using a recipe imho.

[Pankraz76]

So this cannot be applied blindly using a recipe imho.

on greenfield yes, to ensure https://adabeat.com/fp/immutability-in-functional-programming/

for legacy, or valid exceptions, like maven we need suppression.

[iddeepak]

LGTM!

[Pankraz76]

The problem is that this code change is not semantically the same. The stream.collect(Collectors.toList()) returns a mutable list, while stream.toList() returns an immutable collection.

So this cannot be applied blindly using a recipe imho.

yes we can: make change and wrap in new ArrayList(). Then it will return new operational list. Other side effects would be sus relying on pass-by-value, manipulation SSOT violating getter/setter convention.

Then we have same API, but no new violations following: https://kyleshevlin.com/just-enough-fp-immutability/

[timtebeek]

This would fail to pick up static imports of toList. Wherever possible prefer to use MethodMatchers as opposed to looking at simple names, as that avoids these pitfalls.

[Pankraz76]

recent discovery seems fishy - the king is dead.

• https://medium.com/@jonathan.leitschuh/when-open-source-isnt-how-openrewrite-lost-its-way-642053be287d
• https://news.ycombinator.com/item?id=43987976

then checkstyle-stack using PMD/spotbugs seems only option - long live the king:

• [POC-FIX-PMD] maven-pmd-plugin: reactivate PMD - Best practices apache/maven#2331
• support legacy projects, auto-generation of excludeFromFailureFile pmd/pmd#5735
• [POC-Issue#5735]: support legacy automate excludeFromFailureFile apache/maven-pmd-plugin#202

[timtebeek]

I've spoken at length with the author linked above at Code Remix Summit last week, helped him understand and analyze the changes, and got him set up to continue his work. Despite this he came out with a deeply flawed analysis and false statements, which unfortunately are being taken at face value.

Figured I'd provide some more context about the changes made from our perspective:

• https://medium.com/@timtebeek/the-openrewrite-community-potluck

You're all welcome to use and contribute to OpenRewrite for as much as you'd like; we see no reason to be hesitant at all, but understand opinions may differ.