From a92c73d0caf8cc6af151508ea71b28b562967ec4 Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Mon, 31 Aug 2026 00:55:52 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=93=9D=20CodeRabbit=20Chat:=20Resolve=20K?= =?UTF-8?q?otlin=20Analysis=20Review=20Findings=20and=20Add=20Regression?= =?UTF-8?q?=20Tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../JavaSourceFileGraphBuilder.java | 8 +--- .../KotlinSourceFileGraphBuilder.java | 8 +--- .../graphbuilder/SourceFileGraphBuilder.java | 7 +++ .../graphbuilder/metrics/MethodMetrics.java | 17 +++++++ ...FileGraphBuilderTestPathExclusionTest.java | 8 ++++ ...ssMetricsFinalizationImmutabilityTest.java | 26 ++++++++++ ...dencyVisitorLogicJavaKotlinParityTest.java | 47 ++++++++----------- .../main/java/org/hjug/cbc/CycleRanker.java | 4 ++ .../org/hjug/cbc/CycleRankerKotlinTest.java | 5 ++ 9 files changed, 90 insertions(+), 40 deletions(-) diff --git a/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/graphbuilder/JavaSourceFileGraphBuilder.java b/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/graphbuilder/JavaSourceFileGraphBuilder.java index 06f6471d..88fb0733 100644 --- a/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/graphbuilder/JavaSourceFileGraphBuilder.java +++ b/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/graphbuilder/JavaSourceFileGraphBuilder.java @@ -62,18 +62,14 @@ public CodebaseGraphDTO buildGraph(String repositoryPath, String repositoryRoot, new GraphMetricsCollector(classReferencesGraph, packageReferencesGraph); MetricsCollectingVisitor metricsVisitor = new MetricsCollectingVisitor(metricsCollector); - String testDirPattern = config.getTestSourceDirectory() != null - ? config.getTestSourceDirectory().replace('\\', '/') - : ""; - try (Stream pathStream = Files.walk(Path.of(srcDirectory.getAbsolutePath()))) { Stream filteredStream = pathStream.filter(file -> file.toString().endsWith(".java")); if (config.isExcludeTests() && config.getTestSourceDirectory() != null && !config.getTestSourceDirectory().isEmpty()) { - filteredStream = filteredStream.filter( - file -> !file.toString().replace('\\', '/').contains(testDirPattern)); + filteredStream = filteredStream.filter(file -> !SourceFileGraphBuilder.isInConfiguredDirectory( + file, config.getTestSourceDirectory())); } List list = filteredStream.collect(Collectors.toList()); log.info("JavaSourceFileGraphBuilder: walking {} Java files under {}", list.size(), repositoryPath); diff --git a/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/graphbuilder/KotlinSourceFileGraphBuilder.java b/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/graphbuilder/KotlinSourceFileGraphBuilder.java index 70e29660..735cffaa 100644 --- a/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/graphbuilder/KotlinSourceFileGraphBuilder.java +++ b/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/graphbuilder/KotlinSourceFileGraphBuilder.java @@ -72,18 +72,14 @@ public CodebaseGraphDTO buildGraph(String repositoryPath, String repositoryRoot, new GraphMetricsCollector(classReferencesGraph, packageReferencesGraph); KotlinMetricsCollectingVisitor metricsVisitor = new KotlinMetricsCollectingVisitor(metricsCollector); - String testDirPattern = config.getTestSourceDirectory() != null - ? config.getTestSourceDirectory().replace('\\', '/') - : ""; - try (Stream pathStream = Files.walk(Path.of(srcDirectory.getAbsolutePath()))) { Stream filteredStream = pathStream.filter( file -> file.toString().endsWith(".kt") || file.toString().endsWith(".kts")); if (config.isExcludeTests() && config.getTestSourceDirectory() != null && !config.getTestSourceDirectory().isEmpty()) { - filteredStream = filteredStream.filter( - file -> !file.toString().replace('\\', '/').contains(testDirPattern)); + filteredStream = filteredStream.filter(file -> !SourceFileGraphBuilder.isInConfiguredDirectory( + file, config.getTestSourceDirectory())); } List list = filteredStream.collect(Collectors.toList()); diff --git a/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/graphbuilder/SourceFileGraphBuilder.java b/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/graphbuilder/SourceFileGraphBuilder.java index 881d1491..c045e30a 100644 --- a/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/graphbuilder/SourceFileGraphBuilder.java +++ b/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/graphbuilder/SourceFileGraphBuilder.java @@ -1,6 +1,7 @@ package org.hjug.graphbuilder.graphbuilder; import java.io.IOException; +import java.nio.file.Path; import org.hjug.graphbuilder.CodebaseGraphDTO; import org.hjug.graphbuilder.GraphBuilderConfig; @@ -10,6 +11,12 @@ */ public interface SourceFileGraphBuilder { + static boolean isInConfiguredDirectory(Path file, String directory) { + String normalizedFile = "/" + file.toString().replace('\\', '/') + "/"; + String normalizedDirectory = directory.replace('\\', '/').replaceAll("^/+|/+$", ""); + return normalizedFile.contains("/" + normalizedDirectory + "/"); + } + /** * Build a {@link CodebaseGraphDTO} representing class/package dependency * graphs and disharmony metrics for the given source repository. diff --git a/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/metrics/MethodMetrics.java b/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/metrics/MethodMetrics.java index ee738550..b335a755 100644 --- a/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/metrics/MethodMetrics.java +++ b/codebase-graph-builder/src/main/java/org/hjug/graphbuilder/metrics/MethodMetrics.java @@ -3,6 +3,7 @@ import java.util.*; import lombok.AccessLevel; import lombok.Data; +import lombok.EqualsAndHashCode; import lombok.Setter; /** @@ -24,6 +25,7 @@ public class MethodMetrics { private String signature; @Setter(AccessLevel.NONE) + @EqualsAndHashCode.Exclude private boolean finalized; @Setter(AccessLevel.NONE) @@ -37,8 +39,13 @@ public class MethodMetrics { @Setter(AccessLevel.NONE) private Set accessedVariables = new HashSet<>(); + @Setter(AccessLevel.NONE) private Set accessedForeignClasses = new HashSet<>(); + + @Setter(AccessLevel.NONE) private Set accessedForeignAttributes = new HashSet<>(); + + @Setter(AccessLevel.NONE) private Set accessedOwnAttributes = new HashSet<>(); @Setter(AccessLevel.NONE) @@ -204,6 +211,7 @@ public void addTypeParameterFqn(String fqn) { // --- Collection getters: lazy-cached unmodifiable views ---------------- @Setter(AccessLevel.NONE) + @EqualsAndHashCode.Exclude private Set accessedVariablesView; public Set getAccessedVariables() { @@ -216,6 +224,7 @@ public Set getAccessedVariables() { } @Setter(AccessLevel.NONE) + @EqualsAndHashCode.Exclude private Set accessedForeignClassesView; public Set getAccessedForeignClasses() { @@ -228,6 +237,7 @@ public Set getAccessedForeignClasses() { } @Setter(AccessLevel.NONE) + @EqualsAndHashCode.Exclude private Set accessedForeignAttributesView; public Set getAccessedForeignAttributes() { @@ -240,6 +250,7 @@ public Set getAccessedForeignAttributes() { } @Setter(AccessLevel.NONE) + @EqualsAndHashCode.Exclude private Set accessedOwnAttributesView; public Set getAccessedOwnAttributes() { @@ -252,6 +263,7 @@ public Set getAccessedOwnAttributes() { } @Setter(AccessLevel.NONE) + @EqualsAndHashCode.Exclude private Set calledForeignMethodsView; public Set getCalledForeignMethods() { @@ -264,6 +276,7 @@ public Set getCalledForeignMethods() { } @Setter(AccessLevel.NONE) + @EqualsAndHashCode.Exclude private Set calledForeignMethodClassesView; public Set getCalledForeignMethodClasses() { @@ -276,6 +289,7 @@ public Set getCalledForeignMethodClasses() { } @Setter(AccessLevel.NONE) + @EqualsAndHashCode.Exclude private Set changingMethodsView; public Set getChangingMethods() { @@ -288,6 +302,7 @@ public Set getChangingMethods() { } @Setter(AccessLevel.NONE) + @EqualsAndHashCode.Exclude private Set changingClassesView; public Set getChangingClasses() { @@ -300,6 +315,7 @@ public Set getChangingClasses() { } @Setter(AccessLevel.NONE) + @EqualsAndHashCode.Exclude private Set typeParameterFqnsView; public Set getTypeParameterFqns() { @@ -312,6 +328,7 @@ public Set getTypeParameterFqns() { } @Setter(AccessLevel.NONE) + @EqualsAndHashCode.Exclude private List normalizedBodyLinesView; public List getNormalizedBodyLines() { diff --git a/codebase-graph-builder/src/test/java/org/hjug/graphbuilder/graphbuilder/SourceFileGraphBuilderTestPathExclusionTest.java b/codebase-graph-builder/src/test/java/org/hjug/graphbuilder/graphbuilder/SourceFileGraphBuilderTestPathExclusionTest.java index a7a1f316..66e61832 100644 --- a/codebase-graph-builder/src/test/java/org/hjug/graphbuilder/graphbuilder/SourceFileGraphBuilderTestPathExclusionTest.java +++ b/codebase-graph-builder/src/test/java/org/hjug/graphbuilder/graphbuilder/SourceFileGraphBuilderTestPathExclusionTest.java @@ -23,6 +23,14 @@ */ class SourceFileGraphBuilderTestPathExclusionTest { + @Test + void directoryMatchingDoesNotExcludeLongerDirectoryNames() { + assertTrue(SourceFileGraphBuilder.isInConfiguredDirectory( + Path.of("project/src/test/java/Example.java"), "src/test")); + assertFalse(SourceFileGraphBuilder.isInConfiguredDirectory( + Path.of("project/src/testFixtures/java/Example.java"), "src/test")); + } + @DisplayName("Java builder excludes test files with forward-slash testSourceDirectory on Windows-style paths") @Test void javaBuilder_excludesTestsWithForwardSlashPatternOnWindowsPaths(@TempDir Path tempDir) throws IOException { diff --git a/codebase-graph-builder/src/test/java/org/hjug/graphbuilder/metrics/ClassMetricsFinalizationImmutabilityTest.java b/codebase-graph-builder/src/test/java/org/hjug/graphbuilder/metrics/ClassMetricsFinalizationImmutabilityTest.java index 1c6fe1d9..8783d68c 100644 --- a/codebase-graph-builder/src/test/java/org/hjug/graphbuilder/metrics/ClassMetricsFinalizationImmutabilityTest.java +++ b/codebase-graph-builder/src/test/java/org/hjug/graphbuilder/metrics/ClassMetricsFinalizationImmutabilityTest.java @@ -6,6 +6,7 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.Stream; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.function.Executable; @@ -25,6 +26,31 @@ */ class ClassMetricsFinalizationImmutabilityTest { + @Test + void methodMetricsDoesNotExposeBackingCollectionSetters() { + Set forbiddenSetters = Set.of( + "setAccessedForeignClasses", "setAccessedForeignAttributes", "setAccessedOwnAttributes"); + + assertTrue(Stream.of(MethodMetrics.class.getMethods()) + .map(java.lang.reflect.Method::getName) + .noneMatch(forbiddenSetters::contains)); + } + + @Test + void methodMetricsEqualityDoesNotChangeWhenViewsAreCachedOrMetricsAreFrozen() { + MethodMetrics first = new MethodMetrics("method", "method()V"); + MethodMetrics second = new MethodMetrics("method", "method()V"); + first.addAccessedForeignClass("com.example.Foreign"); + second.addAccessedForeignClass("com.example.Foreign"); + + int initialHashCode = first.hashCode(); + first.getAccessedForeignClasses(); + first.freeze(); + + assertEquals(second, first); + assertEquals(initialHashCode, first.hashCode()); + } + private static ClassMetrics newPopulated(String fqn) { ClassMetrics m = new ClassMetrics(fqn); m.setClassName(fqn.substring(fqn.lastIndexOf('.') + 1)); diff --git a/codebase-graph-builder/src/test/java/org/hjug/graphbuilder/visitor/DependencyVisitorLogicJavaKotlinParityTest.java b/codebase-graph-builder/src/test/java/org/hjug/graphbuilder/visitor/DependencyVisitorLogicJavaKotlinParityTest.java index e493a9a6..87b21742 100644 --- a/codebase-graph-builder/src/test/java/org/hjug/graphbuilder/visitor/DependencyVisitorLogicJavaKotlinParityTest.java +++ b/codebase-graph-builder/src/test/java/org/hjug/graphbuilder/visitor/DependencyVisitorLogicJavaKotlinParityTest.java @@ -6,6 +6,8 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.Collections; +import java.util.HashMap; +import java.util.Map; import org.hjug.graphbuilder.GraphDependencyCollector; import org.jgrapht.Graph; import org.jgrapht.graph.DefaultDirectedWeightedGraph; @@ -214,35 +216,24 @@ private void compareProjectEdges( .collect(java.util.stream.Collectors.toMap( v -> v.substring(kotlinPkgPrefix.length()), v -> v, (a, b) -> a)); - // Compare edge pairs and weights - for (var entry : javaNormalized.entrySet()) { - String normV = entry.getKey(); - String javaV = entry.getValue(); - - for (String normT : javaNormalized.keySet()) { - String javaT = javaNormalized.get(normT); - DefaultWeightedEdge javaEdge = javaGraph.getEdge(javaV, javaT); - if (javaEdge != null) { - double javaWeight = javaGraph.getEdgeWeight(javaEdge); - - // Find corresponding Kotlin vertices - String kotlinV = kotlinNormalized.get(normV); - String kotlinT = kotlinNormalized.get(normT); - - assertNotNull(kotlinV, "Kotlin vertex missing for normalized: " + normV); - assertNotNull(kotlinT, "Kotlin vertex missing for normalized: " + normT); - - DefaultWeightedEdge kotlinEdge = kotlinGraph.getEdge(kotlinV, kotlinT); - assertNotNull(kotlinEdge, "Kotlin edge missing for: " + normV + " -> " + normT); - - double kotlinWeight = kotlinGraph.getEdgeWeight(kotlinEdge); - assertEquals( - javaWeight, - kotlinWeight, - "Edge weight mismatch for " + normV + " -> " + normT + ": Java=" + javaWeight + " Kotlin=" - + kotlinWeight); - } + assertEquals( + normalizedEdges(javaGraph, javaNormalized), + normalizedEdges(kotlinGraph, kotlinNormalized), + "Java and Kotlin project edges and weights should match exactly"); + } + + private Map normalizedEdges( + Graph graph, Map normalizedVertices) { + Map edges = new HashMap<>(); + Map vertexNames = new HashMap<>(); + normalizedVertices.forEach((normalized, vertex) -> vertexNames.put(vertex, normalized)); + for (DefaultWeightedEdge edge : graph.edgeSet()) { + String source = vertexNames.get(graph.getEdgeSource(edge)); + String target = vertexNames.get(graph.getEdgeTarget(edge)); + if (source != null && target != null) { + edges.put(source + " -> " + target, graph.getEdgeWeight(edge)); } } + return edges; } } diff --git a/cost-benefit-calculator/src/main/java/org/hjug/cbc/CycleRanker.java b/cost-benefit-calculator/src/main/java/org/hjug/cbc/CycleRanker.java index 3ff110b8..d23cd56d 100644 --- a/cost-benefit-calculator/src/main/java/org/hjug/cbc/CycleRanker.java +++ b/cost-benefit-calculator/src/main/java/org/hjug/cbc/CycleRanker.java @@ -27,6 +27,10 @@ public class CycleRanker { @Getter private CodebaseGraphDTO codebaseGraphDTO; + public CycleRanker(String repositoryPath) { + this(repositoryPath, repositoryPath); + } + /** * Build a unified {@link CodebaseGraphDTO} from a directory that may contain * both Java and Kotlin source files. diff --git a/cost-benefit-calculator/src/test/java/org/hjug/cbc/CycleRankerKotlinTest.java b/cost-benefit-calculator/src/test/java/org/hjug/cbc/CycleRankerKotlinTest.java index 658e3dc0..3ca60929 100644 --- a/cost-benefit-calculator/src/test/java/org/hjug/cbc/CycleRankerKotlinTest.java +++ b/cost-benefit-calculator/src/test/java/org/hjug/cbc/CycleRankerKotlinTest.java @@ -57,6 +57,11 @@ */ class CycleRankerKotlinTest { + @Test + void singlePathConstructorRemainsAvailable() { + assertNotNull(new CycleRanker("repository")); + } + @TempDir public File tempFolder;