Some sonar cleanup (picking the most prevalent or highest priority) #2502
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Mockito provides argument matchers for flexibly stubbing or verifying method calls. `Mockito.verify()`, `Mockito.when()`, `Stubber.when()` and `BDDMockito.given()` each have overloads with and without argument matchers. However, the default matching behavior (i.e. without argument matchers) uses `equals()`. If only the matcher `org.mockito.ArgumentMatchers.eq()` is used, the call is equivalent to the call without matchers, i.e. the `eq()` is not necessary and can be omitted. The resulting code is shorter and easier to read.
These aren't required, and add visual noise
timyates
force-pushed
the
bugfix-sonar-issues
branch
from
9cbc060 to
3ef25cb
Compare
vhscom
approved these changes
Jun 14, 2024
Contributor
vhscom
left a comment
vhscom
left a comment
There was a problem hiding this comment.
Nice clean-up. Re: CSS cascading order, the spec says:
Finally, sort by order specified: if two declarations have the same weight, origin and specificity, the latter specified wins. Declarations in imported style sheets are considered to be before any declarations in the style sheet itself.
mkimberlin
approved these changes
Jun 18, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Best reviewed commit by commit as I've tried to be strong and just fix one thing per commit...
Apologies for when I may have strayed 😢
This clears up a coupld of hundred Sonar errors and warnings
JUnit5 class and method visibility
In JUnit 5 it is recommendend to have package-private classes and methods for tests, Sonar flags this as an issue
Logger should be a private static field
Sonar flags this as a naming vilation (as it's all in caps), I think it just needed to be
private final staticReplace
Collectors.toList()as appropriateInstead of
stream.collect(Collectors.toList()), it is (since Java 16) recommended to usestream.toList().Care must be taken though, as the returned list is then unmodifiable (which is different to the old
Collectorsversion)assertEquals argument order
AssertEquals in tests expects the first argument to be the expected value.
Having it the wrong way round causes the output to be wrong, and can cause delays in debugging issues.
accidental self assignement
In 1 constructor, we were just setting the id to itself (as it wasn't passed as an argument)
In the second (due to capitalization) we weren't using the passed value for
Id.This wasn't an issue however as I don't believe the constructor is used 🤔
CSS:
deginstead ofegSonar caught a typo
CSS: duplicated padding member
I don't believe the spec says what to do when it's declared twice (hence the warning in Sonar).
I kept the second-most padding (which I believe is what most browsers do)
CSS: invalid
box-sizeparameterI believe this should be
box-sizingand I've update it as suchRemove unrequired Mockito
eq()usageThis is not required and makes the tests harder to read
Remove parentheses round single lambda parameters