solven-eu / cleanthat

Github App opening automatically cleaning PR
56 stars 16 forks source link

Draft of "AddAssign" mutator ignores argument order in String concatenation, possibly producing wrong results #803

Closed shrugalic closed 7 months ago

shrugalic commented 7 months ago

Setup

We just added cleanthat to our Gradle build of a Java 17 project, via spotless, with default settings plus Draft changes:

cleanthat().sourceCompatibility("17").includeDraft(true)

This fixed various little things, such as this one:

int seconds = Integer.parseInt(duration.substring(4));
int minutes = Integer.parseInt(duration.substring(0, 4));
- seconds = seconds + 60 * minutes;
+ seconds += 60 * minutes;

Issue

However, it also produced this:

String doStuffTo(String password, String salt) {
-     password = salt + password;
+     password += salt;

It appears that the order of arguments is ignored, which is fine when adding numbers, but possibly disastrous when concatenating Strings.

Expectation

The second example should be left alone as is, rather than producing a wrong result.

blacelle commented 7 months ago

This is fixed in Cleanthat 2.17 (https://github.com/solven-eu/cleanthat/blob/master/CHANGES.MD#217).

Latest is 2.18. Please updte manually cleanthat in your gradle config. (https://github.com/diffplug/spotless/blob/main/plugin-gradle/README.md#cleanthat)

blacelle commented 7 months ago

We may also to include less consensual rules, as described in https://github.com/solven-eu/cleanthat?tab=readme-ov-file#activate-all-mutators

shrugalic commented 7 months ago

This is fixed in Cleanthat 2.17 (https://github.com/solven-eu/cleanthat/blob/master/CHANGES.MD#217).

Latest is 2.18. Please updte manually cleanthat in your gradle config. (https://github.com/diffplug/spotless/blob/main/plugin-gradle/README.md#cleanthat)

I see, thank you. Before adding cleanthat to our config I just updated spotless to 6.25.0. It looks as if that includes cleanthat version 2.16.

My bad then, apologies. I'll close this issue.