From da450d4f12bcff3e0ac363d8ef94e89dbef16f5d Mon Sep 17 00:00:00 2001 From: Scott Leberknight <174812+sleberknight@users.noreply.github.com> Date: Mon, 3 Jun 2024 14:08:41 -0400 Subject: [PATCH] Sonar cleanup (#1150) * Local variables should not shadow class fields ( java:S1117 ) * Methods should not have identical implementations ( java:S4144 ) * Java features should be preferred to Guava ( java:S4738 ) * Restricted Identifiers should not be used as Identifiers ( java:S6213 ) --- .../VaultEncryptionHelperIntegrationTest.java | 4 +- .../kiwiproject/beans/BeanConverterTest.java | 6 +-- .../kiwiproject/collect/KiwiArraysTest.java | 4 +- .../collect/KiwiCollectionsTest.java | 3 +- .../kiwiproject/collect/KiwiListsTest.java | 4 +- .../concurrent/StripedLockTest.java | 32 ++++++------- .../json/JsonHelperBasicsTest.java | 45 +++---------------- .../util/function/KiwiBiConsumersTest.java | 6 +-- .../util/function/KiwiConsumersTest.java | 2 +- 9 files changed, 37 insertions(+), 69 deletions(-) diff --git a/src/test/java/org/kiwiproject/ansible/vault/VaultEncryptionHelperIntegrationTest.java b/src/test/java/org/kiwiproject/ansible/vault/VaultEncryptionHelperIntegrationTest.java index 94d392ca..f11ed78a 100644 --- a/src/test/java/org/kiwiproject/ansible/vault/VaultEncryptionHelperIntegrationTest.java +++ b/src/test/java/org/kiwiproject/ansible/vault/VaultEncryptionHelperIntegrationTest.java @@ -266,9 +266,9 @@ void shouldThrowWhenGivenAnUnencryptedFile() throws IOException { @Test void shouldThrowWhenGivenFileThatDoesNotExist() { - var encryptedFile = Path.of("/does/not/exist.txt"); + var missingEncryptedFile = Path.of("/does/not/exist.txt"); - assertThatThrownBy(() -> helper.decryptFile(encryptedFile, outputFile)) + assertThatThrownBy(() -> helper.decryptFile(missingEncryptedFile, outputFile)) .isExactlyInstanceOf(VaultEncryptionException.class) .hasMessageStartingWith("ansible-vault returned non-zero exit code 1. Stderr: "); } diff --git a/src/test/java/org/kiwiproject/beans/BeanConverterTest.java b/src/test/java/org/kiwiproject/beans/BeanConverterTest.java index dae45fd2..51203762 100644 --- a/src/test/java/org/kiwiproject/beans/BeanConverterTest.java +++ b/src/test/java/org/kiwiproject/beans/BeanConverterTest.java @@ -152,8 +152,8 @@ class GetPropertyMapper { @BeforeEach void setUp() { converter = new BeanConverter<>(); - converter.addPropertyMapper("stringField", testData -> testData.getStringField() + "!!!"); - converter.addPropertyMapper("numberField", testData -> testData.getNumberField() * 5); + converter.addPropertyMapper("stringField", data -> data.getStringField() + "!!!"); + converter.addPropertyMapper("numberField", data -> data.getNumberField() * 5); testData = constructTestData(); } @@ -178,7 +178,7 @@ void shouldThrowClassCastException_WhenGivenInvalidResultType() { // The following MUST declare a variable else the exception is not thrown. // It does not, however, matter whether it is declared with an explicit type. assertThatThrownBy(() -> { - //noinspection unused + @SuppressWarnings("unused") var aDouble = badResultTypeMapper.apply(testData); }).describedAs("should throw when try to assign result whether explicit type or using LVTI (var)") .isExactlyInstanceOf(ClassCastException.class); diff --git a/src/test/java/org/kiwiproject/collect/KiwiArraysTest.java b/src/test/java/org/kiwiproject/collect/KiwiArraysTest.java index 66de211d..2eb53449 100644 --- a/src/test/java/org/kiwiproject/collect/KiwiArraysTest.java +++ b/src/test/java/org/kiwiproject/collect/KiwiArraysTest.java @@ -129,10 +129,10 @@ class SortedWithComparator { @Test void shouldBeEmpty_WhenEmptyArray() { - Integer[] items = new Integer[0]; + Integer[] emptyItems = new Integer[0]; Comparator comparator = Comparator.reverseOrder(); - assertThat(KiwiArrays.sorted(items, comparator, Integer.class)).isEmpty(); + assertThat(KiwiArrays.sorted(emptyItems, comparator, Integer.class)).isEmpty(); } @Test diff --git a/src/test/java/org/kiwiproject/collect/KiwiCollectionsTest.java b/src/test/java/org/kiwiproject/collect/KiwiCollectionsTest.java index 4ba56a63..a538bf3f 100644 --- a/src/test/java/org/kiwiproject/collect/KiwiCollectionsTest.java +++ b/src/test/java/org/kiwiproject/collect/KiwiCollectionsTest.java @@ -6,7 +6,6 @@ import static org.assertj.core.api.Assertions.assertThatNullPointerException; import com.google.common.collect.HashMultiset; -import com.google.common.collect.ImmutableSet; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; @@ -326,7 +325,7 @@ static Stream> someNonSequencedCollections() { return Stream.of( new HashSet<>(), new CopyOnWriteArraySet<>(), - ImmutableSet.of(), + Set.of(), new ArrayBlockingQueue<>(5), HashMultiset.create() ); diff --git a/src/test/java/org/kiwiproject/collect/KiwiListsTest.java b/src/test/java/org/kiwiproject/collect/KiwiListsTest.java index d5b6a526..567eeca0 100644 --- a/src/test/java/org/kiwiproject/collect/KiwiListsTest.java +++ b/src/test/java/org/kiwiproject/collect/KiwiListsTest.java @@ -136,10 +136,10 @@ class SortedWithComparator { @Test void shouldBeEmpty_WhenEmptyList() { - List items = newArrayList(); + List emptyItems = newArrayList(); Comparator comparator = Comparator.reverseOrder(); - assertThat(KiwiLists.sorted(items, comparator)).isEmpty(); + assertThat(KiwiLists.sorted(emptyItems, comparator)).isEmpty(); } @Test diff --git a/src/test/java/org/kiwiproject/concurrent/StripedLockTest.java b/src/test/java/org/kiwiproject/concurrent/StripedLockTest.java index 1da81d23..d2b740b2 100644 --- a/src/test/java/org/kiwiproject/concurrent/StripedLockTest.java +++ b/src/test/java/org/kiwiproject/concurrent/StripedLockTest.java @@ -77,8 +77,8 @@ void testRunWithLock_WhenBlocking_ForWriteAndWriteLocks() { var recorder2 = new TaskRecorder("task2"); var completableFutures = List.of( - Async.doAsync(() -> lock.runWithWriteLock(recorder1.id, () -> recorder1.record(task))), - Async.doAsync(() -> lock.runWithWriteLock(recorder2.id, () -> recorder2.record(task))) + Async.doAsync(() -> lock.runWithWriteLock(recorder1.id, () -> recorder1.runTask(task))), + Async.doAsync(() -> lock.runWithWriteLock(recorder2.id, () -> recorder2.runTask(task))) ); waitForAll(completableFutures); @@ -94,8 +94,8 @@ void testRunWithLock_WhenNonBlocking_ForWriteAndWriteLocks() { var recorder2 = new TaskRecorder("task2"); var completableFutures = List.of( - Async.doAsync(() -> lock.runWithWriteLock(recorder1.id, () -> recorder1.record(task))), - Async.doAsync(() -> lock.runWithWriteLock(recorder2.id, () -> recorder2.record(task))) + Async.doAsync(() -> lock.runWithWriteLock(recorder1.id, () -> recorder1.runTask(task))), + Async.doAsync(() -> lock.runWithWriteLock(recorder2.id, () -> recorder2.runTask(task))) ); waitForAll(completableFutures); @@ -111,8 +111,8 @@ void testRunWithLock_WhenBlocking_ForReadAndWriteLocks() { var recorder2 = new TaskRecorder("task2"); var completableFutures = List.of( - Async.doAsync(() -> lock.runWithWriteLock(recorder1.id, () -> recorder1.record(task))), - Async.doAsync(() -> lock.runWithReadLock(recorder2.id, () -> recorder2.record(task))) + Async.doAsync(() -> lock.runWithWriteLock(recorder1.id, () -> recorder1.runTask(task))), + Async.doAsync(() -> lock.runWithReadLock(recorder2.id, () -> recorder2.runTask(task))) ); waitForAll(completableFutures); @@ -128,8 +128,8 @@ void testRunWithLock_WhenNonBlocking_ForReadAndReadLocks() { var recorder2 = new TaskRecorder("task2"); var completableFutures = List.of( - Async.doAsync(() -> lock.runWithReadLock(recorder1.id, () -> recorder1.record(task))), - Async.doAsync(() -> lock.runWithReadLock(recorder2.id, () -> recorder2.record(task))) + Async.doAsync(() -> lock.runWithReadLock(recorder1.id, () -> recorder1.runTask(task))), + Async.doAsync(() -> lock.runWithReadLock(recorder2.id, () -> recorder2.runTask(task))) ); waitForAll(completableFutures); @@ -146,8 +146,8 @@ void testSupplyWithLock_WhenNonBlocking_ForReadAndReadLocks() { var recorder2 = new TaskRecorder("task2"); var completableFutures = List.of( - Async.doAsync(() -> lock.supplyWithReadLock(recorder1.id, () -> recorder1.recordAndSupply(task1))), - Async.doAsync(() -> lock.supplyWithReadLock(recorder2.id, () -> recorder2.recordAndSupply(task2))) + Async.doAsync(() -> lock.supplyWithReadLock(recorder1.id, () -> recorder1.runTaskAndSupply(task1))), + Async.doAsync(() -> lock.supplyWithReadLock(recorder2.id, () -> recorder2.runTaskAndSupply(task2))) ); waitForAll(completableFutures); @@ -167,8 +167,8 @@ void testSupplyWithLock_WhenBlocking_ForWriteAndReadLocks() { var recorder2 = new TaskRecorder("task2"); var completableFutures = List.of( - Async.doAsync(() -> lock.supplyWithWriteLock(recorder1.id, () -> recorder1.recordAndSupply(task1))), - Async.doAsync(() -> lock.supplyWithReadLock(recorder2.id, () -> recorder2.recordAndSupply(task2))) + Async.doAsync(() -> lock.supplyWithWriteLock(recorder1.id, () -> recorder1.runTaskAndSupply(task1))), + Async.doAsync(() -> lock.supplyWithReadLock(recorder2.id, () -> recorder2.runTaskAndSupply(task2))) ); waitForAll(completableFutures); @@ -188,8 +188,8 @@ void testSupplyWithLock_WhenNonBlocking_ForReadAndWriteLocks() { var recorder2 = new TaskRecorder("task2"); var completableFutures = List.of( - Async.doAsync(() -> lock.supplyWithReadLock(recorder1.id, () -> recorder1.recordAndSupply(task1))), - Async.doAsync(() -> lock.supplyWithWriteLock(recorder2.id, () -> recorder2.recordAndSupply(task2))) + Async.doAsync(() -> lock.supplyWithReadLock(recorder1.id, () -> recorder1.runTaskAndSupply(task1))), + Async.doAsync(() -> lock.supplyWithWriteLock(recorder2.id, () -> recorder2.runTaskAndSupply(task2))) ); waitForAll(completableFutures); @@ -241,13 +241,13 @@ static class TaskRecorder { this.id = id; } - void record(Runnable task) { + void runTask(Runnable task) { start = Instant.now(); task.run(); stop = Instant.now(); } - T recordAndSupply(Supplier supplier) { + T runTaskAndSupply(Supplier supplier) { start = Instant.now(); var result = supplier.get(); stop = Instant.now(); diff --git a/src/test/java/org/kiwiproject/json/JsonHelperBasicsTest.java b/src/test/java/org/kiwiproject/json/JsonHelperBasicsTest.java index 46b12fc4..098f3592 100644 --- a/src/test/java/org/kiwiproject/json/JsonHelperBasicsTest.java +++ b/src/test/java/org/kiwiproject/json/JsonHelperBasicsTest.java @@ -106,8 +106,8 @@ void shouldConfigureObjectMapper_ToReadMillis() { var nowMillis = now.toEpochMilli(); var json = "{ \"timestamp\": " + nowMillis + "}"; - var jsonHelper = new JsonHelper(); - var holder = jsonHelper.toObject(json, TimestampHolder.class); + var plainJsonHelper = new JsonHelper(); + var holder = plainJsonHelper.toObject(json, TimestampHolder.class); assertThat(holder.getTimestamp()).isEqualTo(now.truncatedTo(ChronoUnit.MILLIS)); } @@ -117,8 +117,8 @@ void shouldConfigureObjectMapper_ToWriteMillis() { var now = Instant.now(); var holder = new TimestampHolder(now); - var jsonHelper = new JsonHelper(); - var json = jsonHelper.toJson(holder); + var plainJsonHelper = new JsonHelper(); + var json = plainJsonHelper.toJson(holder); assertThat(json).isEqualTo("{\"timestamp\":" + now.toEpochMilli() + "}"); } @@ -163,9 +163,9 @@ static class TimestampHolder { @Test void shouldGetObjectMapper() { var mapper = new ObjectMapper(); - var jsonHelper = new JsonHelper(mapper); + var customJsonHelper = new JsonHelper(mapper); - assertThat(jsonHelper.getObjectMapper()).isSameAs(mapper); + assertThat(customJsonHelper.getObjectMapper()).isSameAs(mapper); } @Test @@ -410,6 +410,7 @@ void shouldDeserializeJson_GivenTargetTypeReference() { } @ParameterizedTest + @NullAndEmptySource @ValueSource(strings = {" ", " ", "\t", " \n "}) void shouldReturnNull_GivenTargetTypeReference_AndBlankInput(String value) { var user = jsonHelper.toObject(value, new TypeReference() { @@ -418,15 +419,6 @@ void shouldReturnNull_GivenTargetTypeReference_AndBlankInput(String value) { assertThat(user).isNull(); } - @ParameterizedTest - @NullAndEmptySource - void shouldReturnNull_GivenTargetTypeReference_AndNullOrEmptyInput(String value) { - var user = jsonHelper.toObject(value, new TypeReference() { - }); - - assertThat(user).isNull(); - } - @Test void shouldReturnSameString_WhenGivenJson_AndTargetClass_IsString() { var json = "{ \"foo\": true, \"bar\": 42, \"baz\": \"quux\" }"; @@ -497,14 +489,6 @@ void shouldDeserializeJson_WhenGivenValidInput() { @ParameterizedTest @NullAndEmptySource - void shouldReturnSuppliedValue_WhenGivenNullAndEmptyInput(String value) { - var foo = newFoo(); - Supplier fooSupplier = () -> foo; - - assertThat(jsonHelper.toObjectOrSupply(value, Foo.class, fooSupplier)).isSameAs(foo); - } - - @ParameterizedTest @ValueSource(strings = {" ", " ", "\t", " \n "}) void shouldReturnSuppliedValue_WhenGivenBlankInput(String value) { var foo = newFoo(); @@ -585,14 +569,6 @@ void shouldReturnEmptyList_WhenGivenEmptyJsonArray() { @ParameterizedTest @NullAndEmptySource - void shouldReturnNull_WhenGivenNullOrEmptyInput(String value) { - var people = jsonHelper.toObjectList(value, new TypeReference>() { - }); - - assertThat(people).isNull(); - } - - @ParameterizedTest @ValueSource(strings = {" ", " ", "\t", " \n "}) void shouldReturnNull_WhenGivenBlankInput(String value) { var people = jsonHelper.toObjectList(value, new TypeReference>() { @@ -620,13 +596,6 @@ void shouldDeserializeJson_WhenGivenValidInput() { @ParameterizedTest @NullAndEmptySource - void shouldReturnNull_WhenGivenNullOrEmptyInput(String value) { - var people = jsonHelper.toMap(value); - - assertThat(people).isNull(); - } - - @ParameterizedTest @ValueSource(strings = {" ", " ", "\t", " \n "}) void shouldReturnNull_WhenGivenBlankInput(String value) { var people = jsonHelper.toMap(value); diff --git a/src/test/java/org/kiwiproject/util/function/KiwiBiConsumersTest.java b/src/test/java/org/kiwiproject/util/function/KiwiBiConsumersTest.java index e02dfe9e..f7e2a4f6 100644 --- a/src/test/java/org/kiwiproject/util/function/KiwiBiConsumersTest.java +++ b/src/test/java/org/kiwiproject/util/function/KiwiBiConsumersTest.java @@ -42,9 +42,9 @@ public BiConsumerNoOpScaffold() { } void mutateValues() { - mutateValues((numbers, text) -> { - numbers.add(ThreadLocalRandom.current().nextInt() % 100); - text.add(UUIDs.randomUUIDString()); + mutateValues((theNumbers, theText) -> { + theNumbers.add(ThreadLocalRandom.current().nextInt() % 100); + theText.add(UUIDs.randomUUIDString()); }); } diff --git a/src/test/java/org/kiwiproject/util/function/KiwiConsumersTest.java b/src/test/java/org/kiwiproject/util/function/KiwiConsumersTest.java index 065e5f3b..1aeef526 100644 --- a/src/test/java/org/kiwiproject/util/function/KiwiConsumersTest.java +++ b/src/test/java/org/kiwiproject/util/function/KiwiConsumersTest.java @@ -37,7 +37,7 @@ static class ConsumerNoOpScaffold { } void mutateValue() { - mutateValue(text -> text.add(UUIDs.randomUUIDString())); + mutateValue(theText -> theText.add(UUIDs.randomUUIDString())); } void mutateValue(Consumer> block) {