refactoring/fix-even-more-code-smells

api/src/main/java/org/itsallcode/openfasttrace/api/core/DeepCoverageResolver.java

@@ -0,0 +1,77 @@
+package org.itsallcode.openfasttrace.api.core;
+
+/**
+ * Resolves the recursive deep coverage status of linked specification items.
+ */
+final class DeepCoverageResolver
+{
+    private DeepCoverageResolver()
+    {
+        // utility class
+    }
+
+    /**
+     * Resolve the deep coverage status of a linked specification item.
+     * 
+     * @param item
+     *            item from which to start the resolution
+     * @param onlyAcceptApprovedItemStatus
+     *            if true, only accept items with status "approved" as coverage
+     * @return "covered" if the item is covered, "uncovered" if it is not
+     *         covered, "cycle" if the item is part of a cycle
+     */
+    // [impl->dsn~tracing.deep-coverage~1]
+    static DeepCoverageStatus resolve(final LinkedSpecificationItem item,
+            final boolean onlyAcceptApprovedItemStatus)
+    {
+        return getDeepCoverageStatusEndRecursionStartingAt(item, item.getId(),
+                DeepCoverageStatus.COVERED, onlyAcceptApprovedItemStatus);
+    }
+
+    // [impl->dsn~tracing.link-cycle~1]
+    private static DeepCoverageStatus getDeepCoverageStatusEndRecursionStartingAt(
+            final LinkedSpecificationItem item, final SpecificationItemId startId,
+            final DeepCoverageStatus worstStatusSeen,
+            final boolean onlyAcceptApprovedItemStatus)
+    {
+        DeepCoverageStatus status = worstStatusSeen;
+        status = adjustDeepCoverageStatusIfApprovedRequired(item, onlyAcceptApprovedItemStatus,
+                status);
+
+        for (final LinkedSpecificationItem incomingItem : item.getIncomingItems())
+        {
+            if (incomingItem.getId().equals(startId))
+            {
+                return DeepCoverageStatus.CYCLE;
+            }
+            else
+            {
+                final DeepCoverageStatus otherStatus = getDeepCoverageStatusEndRecursionStartingAt(
+                        incomingItem, startId, status, onlyAcceptApprovedItemStatus);
+                if (otherStatus == DeepCoverageStatus.CYCLE)
+                {
+                    return DeepCoverageStatus.CYCLE;
+                }
+                status = DeepCoverageStatus.getWorst(status, otherStatus);
+            }
+        }
+        if (status == DeepCoverageStatus.COVERED && !item.isCoveredShallow())
+        {
+            return DeepCoverageStatus.UNCOVERED;
+        }
+        else
+        {
+            return status;
+        }
+    }
+
+    private static DeepCoverageStatus adjustDeepCoverageStatusIfApprovedRequired(
+            final LinkedSpecificationItem item, final boolean onlyAcceptApprovedItemStatus,
+            final DeepCoverageStatus deepCoveredStatus)
+    {
+        return (onlyAcceptApprovedItemStatus && deepCoveredStatus == DeepCoverageStatus.COVERED
+                && item.getStatus() != ItemStatus.APPROVED)
+                        ? DeepCoverageStatus.UNCOVERED
+                        : deepCoveredStatus;
+    }
+}

api/src/main/java/org/itsallcode/openfasttrace/api/core/LinkedSpecificationItem.java

@@ -8,6 +8,7 @@
  * Specification items with links that can be followed.
  */
 // [impl->dsn~linked-specification-item~1]
+@SuppressWarnings("java:S1448") // This is a facade class. Reducing methods hurts expressiveness.
 public class LinkedSpecificationItem
 {
     private final SpecificationItem item;
@@ -325,11 +326,9 @@ public boolean isCoveredShallowWithApprovedItems()
      *
      * @return covered, uncovered or cycle.
      */
-    // [impl->dsn~tracing.deep-coverage~1]
     public DeepCoverageStatus getDeepCoverageStatus()
     {
-        return getDeepCoverageStatusEndRecursionStartingAt(this.getId(),
-                DeepCoverageStatus.COVERED, false);
+        return DeepCoverageResolver.resolve(this, false);
     }
 
     /**
@@ -340,54 +339,10 @@ public DeepCoverageStatus getDeepCoverageStatus()
      */
     public DeepCoverageStatus getDeepCoverageStatusOnlyAcceptApprovedItems()
     {
-        return getDeepCoverageStatusEndRecursionStartingAt(this.getId(),
-                DeepCoverageStatus.COVERED, true);
+        return DeepCoverageResolver.resolve(this, true);
     }
 
-    // [impl->dsn~tracing.link-cycle~1]
-    private DeepCoverageStatus getDeepCoverageStatusEndRecursionStartingAt(
-            final SpecificationItemId startId, final DeepCoverageStatus worstStatusSeen,
-            final boolean onlyAcceptApprovedItemStatus)
-    {
-        DeepCoverageStatus status = worstStatusSeen;
-        status = adjustDeepCoverageStatusIfApprovedRequired(onlyAcceptApprovedItemStatus, status);
-
-        for (final LinkedSpecificationItem incomingItem : getIncomingItems())
-        {
-            if (incomingItem.getId().equals(startId))
-            {
-                return DeepCoverageStatus.CYCLE;
-            }
-            else
-            {
-                final DeepCoverageStatus otherStatus = incomingItem
-                        .getDeepCoverageStatusEndRecursionStartingAt(startId, status, onlyAcceptApprovedItemStatus);
-                if (otherStatus == DeepCoverageStatus.CYCLE)
-                {
-                    return DeepCoverageStatus.CYCLE;
-                }
-                status = DeepCoverageStatus.getWorst(status, otherStatus);
-            }
-        }
-        if (status == DeepCoverageStatus.COVERED && !isCoveredShallow())
-        {
-            return DeepCoverageStatus.UNCOVERED;
-        }
-        else
-        {
-            return status;
-        }
-    }
-
-    private DeepCoverageStatus adjustDeepCoverageStatusIfApprovedRequired(final boolean onlyAcceptApprovedItemStatus,
-            final DeepCoverageStatus deepCoveredStatus)
-    {
-        return (onlyAcceptApprovedItemStatus && deepCoveredStatus == DeepCoverageStatus.COVERED && !isApproved())
-                ? DeepCoverageStatus.UNCOVERED
-                : deepCoveredStatus;
-    }
-
-    private List<LinkedSpecificationItem> getIncomingItems()
+    List<LinkedSpecificationItem> getIncomingItems()
     {
         return this.links.entrySet() //
                 .stream() //

api/src/main/java/org/itsallcode/openfasttrace/api/report/Reportable.java

@@ -6,6 +6,7 @@
  * Interface for coverage reports.
  */
 @FunctionalInterface
+@SuppressWarnings("java:S1711") // Replacing with Consumder would shade API intent.
 public interface Reportable
 {
     /**

api/src/test/java/org/itsallcode/openfasttrace/api/core/TestDeepCoverageResolver.java

@@ -0,0 +1,78 @@
+package org.itsallcode.openfasttrace.api.core;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.itsallcode.openfasttrace.api.core.SampleArtifactTypes.*;
+
+import org.junit.jupiter.api.Test;
+
+class TestDeepCoverageResolver
+{
+    // [utest->dsn~tracing.deep-coverage~1]
+    @Test
+    void testResolve_Covered()
+    {
+        final LinkedSpecificationItem item = item(REQ, "item", IMPL);
+        final LinkedSpecificationItem coveringItem = item(IMPL, "implementation");
+        item.addLinkToItemWithStatus(coveringItem, LinkStatus.COVERED_SHALLOW);
+        assertThat(DeepCoverageResolver.resolve(item, false), equalTo(DeepCoverageStatus.COVERED));
+    }
+
+    // [utest->dsn~tracing.deep-coverage~1]
+    @Test
+    void testResolve_MissingCoverage()
+    {
+        final LinkedSpecificationItem item = item(REQ, "item");
+        final LinkedSpecificationItem coveringItem = item(IMPL, "implementation", IMPL, UMAN);
+        item.addLinkToItemWithStatus(coveringItem, LinkStatus.COVERED_SHALLOW);
+        assertThat(DeepCoverageResolver.resolve(item, false), equalTo(DeepCoverageStatus.UNCOVERED));
+    }
+
+    // [utest->dsn~tracing.deep-coverage~1]
+    @Test
+    void testResolve_MissingApprovedStatusIfOnlyApprovedItemsAccepted()
+    {
+        final LinkedSpecificationItem item = itemWithStatus(REQ, "item", ItemStatus.PROPOSED);
+        assertThat(DeepCoverageResolver.resolve(item, false), equalTo(DeepCoverageStatus.COVERED));
+        assertThat(DeepCoverageResolver.resolve(item, true), equalTo(DeepCoverageStatus.UNCOVERED));
+    }
+
+    // [utest->dsn~tracing.link-cycle~1]
+    @Test
+    void testResolve_CycleIfSelfLinkExists()
+    {
+        final LinkedSpecificationItem item = item(REQ, "item");
+        item.addLinkToItemWithStatus(item, LinkStatus.COVERED_SHALLOW);
+        assertThat(DeepCoverageResolver.resolve(item, false), equalTo(DeepCoverageStatus.CYCLE));
+    }
+
+    // [utest->dsn~tracing.link-cycle~1]
+    @Test
+    void testResolve_DeepCycle()
+    {
+        final LinkedSpecificationItem item = item(REQ, "item");
+        final LinkedSpecificationItem coveringItem = item(IMPL, "implementation");
+        item.addLinkToItemWithStatus(coveringItem, LinkStatus.COVERED_SHALLOW);
+        coveringItem.addLinkToItemWithStatus(item, LinkStatus.COVERED_SHALLOW);
+        assertThat(DeepCoverageResolver.resolve(item, false), equalTo(DeepCoverageStatus.CYCLE));
+    }
+
+    private static LinkedSpecificationItem item(final String artifactType, final String name,
+            final String... needsArtifactTypes)
+    {
+        return itemWithStatus(artifactType, name, ItemStatus.APPROVED, needsArtifactTypes);
+    }
+
+    private static LinkedSpecificationItem itemWithStatus(final String artifactType,
+            final String name, final ItemStatus status, final String... needsArtifactTypes)
+    {
+        final SpecificationItem.Builder builder = SpecificationItem.builder()
+                .id(artifactType, name, 1)
+                .status(status);
+        for (final String needsArtifactType : needsArtifactTypes)
+        {
+            builder.addNeedsArtifactType(needsArtifactType);
+        }
+        return new LinkedSpecificationItem(builder.build());
+    }
+}

api/src/test/java/org/itsallcode/openfasttrace/api/core/TestLinkedSpecificationItem.java

@@ -9,7 +9,6 @@
 import static org.junit.jupiter.api.Assertions.assertAll;
 import static org.mockito.Mockito.*;
 
-import java.util.Arrays;
 import java.util.List;
 
 import org.junit.jupiter.api.BeforeEach;
@@ -68,7 +67,7 @@ void testGetCoveredArtifactTypes()
     @Test
     void testGetUncoveredArtifactTypes()
     {
-        when(this.itemMock.getNeedsArtifactTypes()).thenReturn(Arrays.asList(UMAN, REQ));
+        when(this.itemMock.getNeedsArtifactTypes()).thenReturn(List.of(UMAN, REQ));
         when(this.coveredItemMock.getArtifactType()).thenReturn(UMAN);
         this.linkedItem.addLinkToItemWithStatus(coveredLinkedItem, LinkStatus.COVERED_SHALLOW);
         assertItemHasUncoveredArtifactTypes(this.linkedItem, REQ);
@@ -85,7 +84,7 @@ void testGetOverCoveredArtifactTypes()
     @Test
     void testIsCoveredShallow_Ok()
     {
-        when(this.itemMock.getNeedsArtifactTypes()).thenReturn(Arrays.asList(UMAN, IMPL));
+        when(this.itemMock.getNeedsArtifactTypes()).thenReturn(List.of(UMAN, IMPL));
         when(this.coveredItemMock.getArtifactType()).thenReturn(UMAN);
         this.linkedItem.addLinkToItemWithStatus(coveredLinkedItem, LinkStatus.COVERED_SHALLOW);
         when(this.coveredItemMock.getArtifactType()).thenReturn(IMPL);
@@ -96,7 +95,7 @@ void testIsCoveredShallow_Ok()
     @Test
     void testIsCoveredShallow_NotOk_WrongCoverage()
     {
-        when(this.itemMock.getNeedsArtifactTypes()).thenReturn(Arrays.asList(UMAN, IMPL));
+        when(this.itemMock.getNeedsArtifactTypes()).thenReturn(List.of(UMAN, IMPL));
         when(this.coveredItemMock.getArtifactType()).thenReturn(UMAN);
         this.linkedItem.addLinkToItemWithStatus(coveredLinkedItem, LinkStatus.COVERED_SHALLOW);
         when(this.coveredItemMock.getArtifactType()).thenReturn(REQ);
@@ -107,36 +106,19 @@ void testIsCoveredShallow_NotOk_WrongCoverage()
     @Test
     void testIsCoveredShallow_NotOk_MissingCoverage()
     {
-        when(this.itemMock.getNeedsArtifactTypes()).thenReturn(Arrays.asList(UMAN, IMPL));
+        when(this.itemMock.getNeedsArtifactTypes()).thenReturn(List.of(UMAN, IMPL));
         when(this.coveredItemMock.getArtifactType()).thenReturn(UMAN);
         this.linkedItem.addLinkToItemWithStatus(coveredLinkedItem, LinkStatus.COVERED_SHALLOW);
         assertItemCoveredShallow(this.linkedItem, false);
     }
 
-    // [utest->dsn~tracing.deep-coverage~1]
-    @Test
-    void testGetDeepCoverageStatus_Covered()
-    {
-        prepareCoverThis();
-        assertItemDeepCoverageStatus(this.linkedItem, DeepCoverageStatus.COVERED);
-    }
-
     private void prepareCoverThis()
     {
-        when(this.itemMock.getNeedsArtifactTypes()).thenReturn(Arrays.asList(IMPL));
+        when(this.itemMock.getNeedsArtifactTypes()).thenReturn(List.of(IMPL));
         when(this.coveredItemMock.getArtifactType()).thenReturn(IMPL);
         this.linkedItem.addLinkToItemWithStatus(coveredLinkedItem, LinkStatus.COVERED_SHALLOW);
     }
 
-    // [utest->dsn~tracing.deep-coverage~1]
-    @Test
-    void testGetDeepCoverageStatus_MissingCoverage()
-    {
-        when(this.coveredItemMock.getNeedsArtifactTypes()).thenReturn(Arrays.asList(IMPL, UMAN));
-        this.linkedItem.addLinkToItemWithStatus(this.coveredLinkedItem, LinkStatus.COVERED_SHALLOW);
-        assertItemDeepCoverageStatus(this.linkedItem, DeepCoverageStatus.UNCOVERED);
-    }
-
     // [utest->dsn~tracing.defect-items~2]
     @Test
     void testIsDefect_False()
@@ -220,23 +202,6 @@ void testCountDuplicateLinks()
         assertThat(this.linkedItem.countDuplicateLinks(), equalTo(2));
     }
 
-    // [utest->dsn~tracing.link-cycle~1]
-    @Test
-    void testGetDeepCoverageStatus_CylceIfSelfLink()
-    {
-        this.linkedItem.addLinkToItemWithStatus(this.linkedItem, LinkStatus.COVERED_SHALLOW);
-        assertThat(this.linkedItem.getDeepCoverageStatus(), equalTo(DeepCoverageStatus.CYCLE));
-    }
-
-    // [utest->dsn~tracing.link-cycle~1]
-    @Test
-    void testGetDeepCoverageStatus_DeepCycle()
-    {
-        this.linkedItem.addLinkToItemWithStatus(this.otherLinkedItem, LinkStatus.COVERED_SHALLOW);
-        this.otherLinkedItem.addLinkToItemWithStatus(this.linkedItem, LinkStatus.COVERED_SHALLOW);
-        assertThat(this.linkedItem.getDeepCoverageStatus(), equalTo(DeepCoverageStatus.CYCLE));
-    }
-
     @Test
     void testGetTitleWithFallback_HasTitle()
     {
@@ -329,4 +294,4 @@ void testGetRevision()
         when(this.itemMock.getRevision()).thenReturn(expectedRevision);
         assertThat(this.linkedItem.getRevision(), equalTo(expectedRevision));
     }
-}
+}

core/src/main/java/org/itsallcode/openfasttrace/core/importer/MultiFileImporterImpl.java

@@ -67,15 +67,15 @@ public MultiFileImporter importAny(final List<Path> paths)
             }
             else
             {
-                LOG.warning(() -> "No such input file or directory \"" + path.toString()
-                        + "\". Skipping.");
+                LOG.warning(() -> "No such input file or directory \"" + path + "\". Skipping.");
             }
         }
         return this;
     }
 
     // [impl->dsn~input-directory-recursive-traversal~1]
     @Override
+    @SuppressWarnings("java:S1941") // Item count needs to be captured before object is modified.
     public MultiFileImporter importRecursiveDir(final Path dir, final String glob)
     {
         final PathMatcher matcher = dir.getFileSystem().getPathMatcher("glob:" + glob);