Skip to content

fix(sonar): use dedicated File assertions (S5838) - #376

Merged
dfa1 merged 1 commit into
mainfrom
sonar/assertj-file-assertions
Sep 6, 2026
Merged

fix(sonar): use dedicated File assertions (S5838)#376
dfa1 merged 1 commit into
mainfrom
sonar/assertj-file-assertions

Conversation

@dfa1

@dfa1 dfa1 commented Sep 6, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes 2 of the 13 open java:S5838 findings: assertThat(file.canRead()).isTrue()assertThat(file).canRead() (same for canWrite()), using AssertJ's dedicated FileAssert.

Triage note on the other 11 S5838 findings + all 10 S3415 findings

I checked every remaining S5838/S3415 finding before touching anything, since the last PR (#368) taught me not to trust Sonar's suggested rewrite blindly. Result: all of them are false positives, and I've left them untouched:

  • 11 S5838 findings ("Use isZero() instead") are all on values statically typed Number (FilteredAggregate.sum(), Compute.filteredSum(...)) — AssertJ has no assertThat(Number) overload and ObjectAssert/ AbstractObjectAssert have no isZero() method (verified via javap against assertj-core-3.27.3.jar). Applying the suggested fix does not compile. .isEqualTo(0L) is the correct form for a Number-typed value.
  • 10 S3415 findings ("swap actual/expected") are all on AssertJ's fluent assertThat(actual).isEqualTo(expected) / .isInstanceOf(...) chains, already in the correct, idiomatic order. The rule (tagged junit upstream, designed for two-arg assertEquals(expected, actual)) doesn't understand AssertJ's fluent single-argument form — one flagged line is even assertThat(x).isInstanceOf(Key.class), which has no "expected/actual" concept for the rule to check.

Both categories are unfixable-as-suggested without breaking the build (S5838) or actively making the code worse (S3415), so this PR only contains the 2 genuine fixes.

Test plan

  • Full reactor compile succeeds with the change (confirms AssertJ's Assertions.assertThat(File) / AbstractFileAssert#canRead()/canWrite() are valid).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KW6yskY3pE759XUg2UUtf8
@dfa1
dfa1 merged commit 93931e2 into main Sep 6, 2026
6 checks passed
@dfa1
dfa1 deleted the sonar/assertj-file-assertions branch September 6, 2026 06:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant