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 )
kiwiproject · Jun 3, 2024 · da450d4 · da450d4
1 parent 3c52b9b
commit da450d4
Show file tree

Hide file tree

Showing 9 changed files with 37 additions and 69 deletions.
diff --git 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
@@ -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
@@ -129,10 +129,10 @@ class SortedWithComparator {
 
         @Test
         void shouldBeEmpty_WhenEmptyArray() {
-            Integer[] items = new Integer[0];
+            Integer[] emptyItems = new Integer[0];
             Comparator<Integer> 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
@@ -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<Collection<Long>> 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
@@ -136,10 +136,10 @@ class SortedWithComparator {
 
         @Test
         void shouldBeEmpty_WhenEmptyList() {
-            List<Integer> items = newArrayList();
+            List<Integer> emptyItems = newArrayList();
             Comparator<Integer> 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
@@ -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> T recordAndSupply(Supplier<T> supplier) {
+        <T> T runTaskAndSupply(Supplier<T> 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
@@ -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<User>() {
@@ -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<User>() {
-            });
-
-            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<Foo> 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<List<Person>>() {
-            });
-
-            assertThat(people).isNull();
-        }
-
-        @ParameterizedTest
         @ValueSource(strings = {" ", "  ", "\t", " \n "})
         void shouldReturnNull_WhenGivenBlankInput(String value) {
             var people = jsonHelper.toObjectList(value, new TypeReference<List<Person>>() {
@@ -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
@@ -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
@@ -37,7 +37,7 @@ static class ConsumerNoOpScaffold {
         }
 
         void mutateValue() {
-            mutateValue(text -> text.add(UUIDs.randomUUIDString()));
+            mutateValue(theText -> theText.add(UUIDs.randomUUIDString()));
         }
 
         void mutateValue(Consumer<List<String>> block) {