#175 Add object oriented disharmony detectors

.gitignore

@@ -214,4 +214,4 @@ buildNumber.properties
 
 # End of https://www.toptal.com/developers/gitignore/api/java,maven,intellij,eclipse
 
-./jreleaser-cli.jar
+/jreleaser-cli.jar

CITATIONS.md

@@ -26,4 +26,11 @@ https://doi.org/10.1007/978-3-031-22203-0_14
 **Authors:** Nico Zazworka, Carolyn Seaman, and Forrest Shull  
 **Year:** 2011  
 **Publication:** MTD '11: Proceedings of the 2nd Workshop on Managing Technical Debt  
-**Links:** https://dl.acm.org/doi/10.1145/1985362.1985372
+**Links:** https://dl.acm.org/doi/10.1145/1985362.1985372
+
+
+## Object Oriented Disharmonies
+**Title:** Object Oriented Metrics in Practice   
+**Authors:** Michele Lanza, Radu Marinescu  
+**Year:** 2006
+**Links:** https://doi.org/10.1007/3-540-39538-5

CLAUDE.md

@@ -0,0 +1,97 @@
+# CLAUDE.md
+
+This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.
+
+## Project Overview
+
+RefactorFirst is a Java static analysis tool that scans Git repositories, detects code disharmonies (God Classes, Brain Classes, highly coupled classes, circular dependencies), and produces prioritized refactoring recommendations in HTML/CSV/JSON reports. The core insight is combining code-quality metrics with Git change history so the most painful classes surface first.
+
+## Build Commands
+
+```bash
+# Full build (compile + test + package)
+mvn clean install
+
+# Skip tests for faster iteration
+mvn clean install -DskipTests
+
+# Run all tests
+mvn clean test
+
+# Run tests in a single module
+mvn clean test -pl effort-ranker
+
+# Run a single test class
+mvn clean test -pl effort-ranker -Dtest=BrainClassTest
+
+# Format code (Palantir Java format via Spotless)
+mvn spotless:apply
+
+# Check formatting without modifying
+mvn spotless:check
+
+# Build with OWASP dependency check (slow)
+mvn clean install -Plocal
+```
+
+The CLI fat jar is produced at `cli/target/refactor-first-cli-*.jar` via the Maven Shade plugin.
+
+## Module Architecture
+
+11-module Maven build. Data flows left-to-right:
+
+```
+codebase-graph-builder  ──→  cost-benefit-calculator  ──→  graph-data-generator
+graph-algorithms         ──→        │                  ──→  report
+change-proneness-ranker  ──→        │                  ──→  cli
+effort-ranker            ──→  ──────┘                  ──→  refactor-first-maven-plugin
+test-resources (shared fixtures)
+coverage (JaCoCo aggregation)
+```
+
+**codebase-graph-builder** — The most complex module. Uses OpenRewrite to parse Java source across versions (11/17/21), builds class and package dependency graphs with JGraphT, detects cycle-breaking candidates using two graph algorithms, and returns everything in `CodebaseGraphDTO`. Entry point: `JavaGraphBuilder.getCodebaseGraphDTO()`.
+
+**graph-algorithms** — Two cycle-decomposition algorithms used by `JavaGraphBuilder`:
+- *Directed Feedback Vertex Set* (`org.hjug.feedback.vertex.kernelized`) — kernelized algorithm; identifies the minimum vertex set to remove to break all cycles.
+- *Feedback Arc Set with PageRank* (`org.hjug.feedback.arc.pageRank`) — identifies edges to remove; uses PageRank on the line digraph. Based on Geladaris et al.
+- See `DIAGRAM.md` files in each algorithm package for pseudocode diagrams.
+
+**effort-ranker** — Runs PMD rules (`category/java/design.xml` + custom `CBORule`) to detect God Classes (ATFD, WMC, TCC) and highly coupled classes (CBO). Also detects Brain Classes, Data Classes, Feature Envy, and several other disharmonies in `DisharmonyDetector`.
+
+**change-proneness-ranker** — Uses JGit to read Git commit history and assign change-frequency scores to classes.
+
+**cost-benefit-calculator** — Orchestrates the full pipeline: runs effort-ranker and change-proneness-ranker, combines scores into `RankedDisharmony` objects. Main orchestration class: `CostBenefitCalculator`.
+
+**report** — Generates HTML (with embedded bubble charts), CSV, and JSON output. Reports with >4000 classes switch to a simplified 3D viewer.
+
+**cli** — PicoCLI-based executable. Entry: `org.hjug.refactorfirst.Main` → `ReportCommand`.
+
+**refactor-first-maven-plugin** — Maven plugin wrapper; key config options: `showDetails`, `backEdgeAnalysisCount` (default 50; set 0 for all), `analyzeCycles`, `excludeTests`.
+
+## Key Data Model
+
+`CodebaseGraphDTO` is the central transfer object passed between modules. It holds:
+- JGraphT directed graphs for class and package dependencies
+- Detected disharmony lists (God Classes, Brain Classes, etc.)
+- Metrics per class (ATFD, WMC, TCC, CBO)
+
+`RankedDisharmony` carries the final prioritized output consumed by report generators.
+
+## Architectural Patterns
+
+- **Visitor** — `JavaVisitor` (OpenRewrite) walks the AST to collect class dependencies.
+- **Pipeline** — `CostBenefitCalculator` chains multiple independent rankers then merges results.
+- **DTO** — `CodebaseGraphDTO` decouples parsing from ranking and reporting.
+- Lombok `@Data`/`@Builder` is used extensively; avoid adding boilerplate that Lombok already removes.
+
+## Testing
+
+JUnit 5 (Jupiter) with parameterized tests. Test fixtures live in `test-resources/src/test/resources`. When adding or modifying graph-algorithm behavior, check `JavaGraphBuilderTest` and `CircularReferenceCheckerTests` for integration-level coverage.
+
+Mutation testing via PIT (`pitest-maven`) is configured but not part of the default build; run explicitly if needed.
+
+## Java & Toolchain
+
+- Source/target: Java 11 minimum; OpenRewrite parser supports 11, 17, 21.
+- Logging: SLF4J; use `log.debug()` for verbose per-class output, `log.info()` sparingly.
+- Spotless enforces Palantir Java format — run `mvn spotless:apply` before committing.

change-proneness-ranker/src/main/java/org/hjug/git/ChangePronenessRanker.java

@@ -9,24 +9,33 @@
 public class ChangePronenessRanker {
 
     private final TreeMap<Integer, Integer> changeCountsByTimeStamps = new TreeMap<>();
+    private final TreeMap<Integer, Integer> suffixSums = new TreeMap<>();
     private final Map<String, ScmLogInfo> cachedScmLogInfos = new HashMap<>();
 
     public ChangePronenessRanker(GitLogReader repositoryLogReader) {
         try {
             log.info("Capturing change count based on commit timestamps");
             changeCountsByTimeStamps.putAll(repositoryLogReader.captureChangeCountByCommitTimestamp());
+            computeSuffixSums();
         } catch (IOException | GitAPIException e) {
             log.error("Error reading from repository: {}", e.getMessage());
         }
     }
 
+    private void computeSuffixSums() {
+        int runningSum = 0;
+        for (Map.Entry<Integer, Integer> entry :
+                changeCountsByTimeStamps.descendingMap().entrySet()) {
+            runningSum += entry.getValue();
+            suffixSums.put(entry.getKey(), runningSum);
+        }
+    }
+
     public void rankChangeProneness(List<ScmLogInfo> scmLogInfos) {
         for (ScmLogInfo scmLogInfo : scmLogInfos) {
             if (!cachedScmLogInfos.containsKey(scmLogInfo.getPath())) {
-                int commitsInRepositorySinceCreation =
-                        changeCountsByTimeStamps.tailMap(scmLogInfo.getEarliestCommit()).values().stream()
-                                .mapToInt(i -> i)
-                                .sum();
+                Map.Entry<Integer, Integer> entry = suffixSums.ceilingEntry(scmLogInfo.getEarliestCommit());
+                int commitsInRepositorySinceCreation = (entry != null) ? entry.getValue() : 0;
 
                 scmLogInfo.setChangeProneness((float) scmLogInfo.getCommitCount() / commitsInRepositorySinceCreation);
                 cachedScmLogInfos.put(scmLogInfo.getPath(), scmLogInfo);

codebase-graph-builder/src/main/java/org/hjug/graphbuilder/CodebaseGraphDTO.java

@@ -1,15 +1,66 @@
 package org.hjug.graphbuilder;
 
+import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
-import lombok.Data;
+import java.util.stream.Collectors;
+import lombok.EqualsAndHashCode;
+import lombok.Getter;
+import lombok.ToString;
+import org.hjug.graphbuilder.metrics.DisharmonyDetector.ClassDisharmony;
+import org.hjug.graphbuilder.metrics.DisharmonyDetector.MethodDisharmony;
 import org.jgrapht.Graph;
 import org.jgrapht.graph.DefaultWeightedEdge;
 
-@Data
+@Getter
+@EqualsAndHashCode
+@ToString
 public class CodebaseGraphDTO {
 
     private final Graph<String, DefaultWeightedEdge> classReferencesGraph;
     private final Graph<String, DefaultWeightedEdge> packageReferencesGraph;
     // used for looking up files where classes reside
     private final Map<String, String> classToSourceFilePathMapping;
+
+    private final List<ClassDisharmony> classDisharmonies;
+    private final List<MethodDisharmony> methodDisharmonies;
+    private final Map<String, Long> disharmonyCountByClass;
+
+    public CodebaseGraphDTO(
+            Graph<String, DefaultWeightedEdge> classReferencesGraph,
+            Graph<String, DefaultWeightedEdge> packageReferencesGraph,
+            Map<String, String> classToSourceFilePathMapping,
+            List<ClassDisharmony> classDisharmonies,
+            List<MethodDisharmony> methodDisharmonies) {
+        this.classReferencesGraph = classReferencesGraph;
+        this.packageReferencesGraph = packageReferencesGraph;
+        this.classToSourceFilePathMapping = classToSourceFilePathMapping;
+        this.classDisharmonies = classDisharmonies;
+        this.methodDisharmonies = methodDisharmonies;
+        this.disharmonyCountByClass = buildDisharmonyIndex(classDisharmonies, methodDisharmonies);
+    }
+
+    private static Map<String, Long> buildDisharmonyIndex(
+            List<ClassDisharmony> classDisharmonies, List<MethodDisharmony> methodDisharmonies) {
+        Map<String, Long> counts = new HashMap<>();
+        classDisharmonies.forEach(d -> counts.merge(d.getMetrics().getClassName(), 1L, Long::sum));
+        methodDisharmonies.forEach(m -> counts.merge(m.getClassName(), 1L, Long::sum));
+        return counts;
+    }
+
+    public List<ClassDisharmony> getClassDisharmoniesOfType(String disharmonyType) {
+        return classDisharmonies.stream()
+                .filter(d -> disharmonyType.equals(d.getDisharmonyType()))
+                .collect(Collectors.toList());
+    }
+
+    public List<MethodDisharmony> getMethodDisharmoniesOfType(String disharmonyType) {
+        return methodDisharmonies.stream()
+                .filter(d -> disharmonyType.equals(d.getDisharmonyType()))
+                .collect(Collectors.toList());
+    }
+
+    public long getClassDisharmonyCountForClass(String classFqn) {
+        return disharmonyCountByClass.getOrDefault(classFqn, 0L);
+    }
 }

codebase-graph-builder/src/main/java/org/hjug/graphbuilder/DependencyCollector.java

@@ -1,5 +1,6 @@
 package org.hjug.graphbuilder;
 
+// TODO: Revisit - I don't think this is really needed
 public interface DependencyCollector {
 
     /**

codebase-graph-builder/src/main/java/org/hjug/graphbuilder/JavaGraphBuilder.java

@@ -9,8 +9,12 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import lombok.extern.slf4j.Slf4j;
-import org.hjug.graphbuilder.visitor.JavaMethodDeclarationVisitor;
-import org.hjug.graphbuilder.visitor.JavaVariableTypeVisitor;
+import org.hjug.graphbuilder.metrics.ClassMetrics;
+import org.hjug.graphbuilder.metrics.DisharmonyDetector;
+import org.hjug.graphbuilder.metrics.DisharmonyDetector.ClassDisharmony;
+import org.hjug.graphbuilder.metrics.DisharmonyDetector.MethodDisharmony;
+import org.hjug.graphbuilder.metrics.GraphMetricsCollector;
+import org.hjug.graphbuilder.metrics.MetricsCollectingVisitor;
 import org.hjug.graphbuilder.visitor.JavaVisitor;
 import org.jgrapht.Graph;
 import org.jgrapht.graph.DefaultDirectedWeightedGraph;
@@ -25,38 +29,39 @@ public class JavaGraphBuilder {
     /**
      * Given a java source directory, return a CodebaseGraphDTO using default configuration
      *
-     * @param srcDirectory The source directory to analyze
+     * @param repositoryPath The source directory to analyze
      * @param excludeTests Whether to exclude test files
      * @param testSourceDirectory The test source directory pattern to exclude
      * @return CodebaseGraphDTO
      * @throws IOException
      */
-    public CodebaseGraphDTO getCodebaseGraphDTO(String srcDirectory, boolean excludeTests, String testSourceDirectory)
+    public CodebaseGraphDTO getCodebaseGraphDTO(String repositoryPath, boolean excludeTests, String testSourceDirectory)
             throws IOException {
         GraphBuilderConfig config = GraphBuilderConfig.builder()
                 .excludeTests(excludeTests)
                 .testSourceDirectory(testSourceDirectory)
                 .build();
-        return getCodebaseGraphDTO(srcDirectory, config);
+        return getCodebaseGraphDTO(repositoryPath, config);
     }
 
     /**
      * Given a java source directory and configuration, return a CodebaseGraphDTO
      *
-     * @param srcDirectory The source directory to analyze
+     * @param repositoryPath The source directory to analyze
      * @param config The configuration for the graph builder
      * @return CodebaseGraphDTO
      * @throws IOException
      */
-    public CodebaseGraphDTO getCodebaseGraphDTO(String srcDirectory, GraphBuilderConfig config) throws IOException {
-        if (srcDirectory == null || srcDirectory.isEmpty()) {
+    private CodebaseGraphDTO getCodebaseGraphDTO(String repositoryPath, GraphBuilderConfig config) throws IOException {
+        if (repositoryPath == null || repositoryPath.isEmpty()) {
             throw new IllegalArgumentException("Source directory cannot be null or empty");
         }
-        return processWithOpenRewrite(srcDirectory, config);
+        return processWithOpenRewrite(repositoryPath, config);
     }
 
-    private CodebaseGraphDTO processWithOpenRewrite(String srcDir, GraphBuilderConfig config) throws IOException {
-        File srcDirectory = new File(srcDir);
+    private CodebaseGraphDTO processWithOpenRewrite(String repositoryPath, GraphBuilderConfig config)
+            throws IOException {
+        File srcDirectory = new File(repositoryPath);
 
         JavaParser javaParser = JavaParser.fromJavaVersion().build();
         ExecutionContext ctx = new InMemoryExecutionContext(Throwable::printStackTrace);
@@ -69,11 +74,11 @@ private CodebaseGraphDTO processWithOpenRewrite(String srcDir, GraphBuilderConfi
         final GraphDependencyCollector dependencyCollector =
                 new GraphDependencyCollector(classReferencesGraph, packageReferencesGraph);
 
-        final JavaVisitor<ExecutionContext> javaVisitor = new JavaVisitor<>(dependencyCollector);
-        final JavaVariableTypeVisitor<ExecutionContext> javaVariableTypeVisitor =
-                new JavaVariableTypeVisitor<>(dependencyCollector);
-        final JavaMethodDeclarationVisitor<ExecutionContext> javaMethodDeclarationVisitor =
-                new JavaMethodDeclarationVisitor<>(dependencyCollector);
+        final JavaVisitor<ExecutionContext> javaVisitor = new JavaVisitor<>(repositoryPath, dependencyCollector);
+
+        GraphMetricsCollector metricsCollector =
+                new GraphMetricsCollector(classReferencesGraph, packageReferencesGraph);
+        MetricsCollectingVisitor metricsVisitor = new MetricsCollectingVisitor(metricsCollector);
 
         try (Stream<Path> pathStream = Files.walk(Paths.get(srcDirectory.getAbsolutePath()))) {
             List<Path> list;
@@ -89,15 +94,46 @@ private CodebaseGraphDTO processWithOpenRewrite(String srcDir, GraphBuilderConfi
                     .parse(list, Paths.get(srcDirectory.getAbsolutePath()), ctx)
                     .forEach(cu -> {
                         javaVisitor.visit(cu, ctx);
-                        javaVariableTypeVisitor.visit(cu, ctx);
-                        javaMethodDeclarationVisitor.visit(cu, ctx);
+                        metricsVisitor.visit(cu, ctx);
                     });
         }
 
         removeClassesNotInCodebase(dependencyCollector.getPackagesInCodebase(), classReferencesGraph);
 
+        metricsCollector.finalizeMetrics();
+        DisharmonyDetector detector = new DisharmonyDetector();
+        Collection<ClassMetrics> metrics = metricsCollector.getAllClassMetrics().values();
+
         return new CodebaseGraphDTO(
-                classReferencesGraph, packageReferencesGraph, javaVisitor.getClassToSourceFilePathMapping());
+                classReferencesGraph,
+                packageReferencesGraph,
+                javaVisitor.getClassToSourceFilePathMapping(), // hudson.model.FilePath ->
+                // file:///C:/Code/RefactorFirst/cost-benefit-calculator/hudson/model/FilePath.java
+                getClassDisharmonies(detector, metrics),
+                getMethodDisharmonies(detector, metrics));
+    }
+
+    private static List<MethodDisharmony> getMethodDisharmonies(
+            DisharmonyDetector detector, Collection<ClassMetrics> metrics) {
+        List<MethodDisharmony> methodDisharmonies = new ArrayList<>();
+        methodDisharmonies.addAll(detector.detectBrainMethods(List.copyOf(metrics)));
+        methodDisharmonies.addAll(detector.detectFeatureEnvy(List.copyOf(metrics)));
+        methodDisharmonies.addAll(detector.detectIntensiveCoupling(List.copyOf(metrics)));
+        methodDisharmonies.addAll(detector.detectDispersedCoupling(List.copyOf(metrics)));
+        methodDisharmonies.addAll(detector.detectShotgunSurgery(List.copyOf(metrics)));
+        return methodDisharmonies;
+    }
+
+    private static List<ClassDisharmony> getClassDisharmonies(
+            DisharmonyDetector detector, Collection<ClassMetrics> metrics) {
+        List<ClassDisharmony> classDisharmonies = new ArrayList<>();
+        classDisharmonies.addAll(detector.detectGodClasses(List.copyOf(metrics)));
+        classDisharmonies.addAll(detector.detectDataClasses(List.copyOf(metrics)));
+        classDisharmonies.addAll(detector.detectBrainClasses(List.copyOf(metrics)));
+        classDisharmonies.addAll(detector.detectRefusedParentBequest(List.copyOf(metrics)));
+        classDisharmonies.addAll(detector.detectTraditionBreaker(List.copyOf(metrics)));
+        classDisharmonies.addAll(detector.detectSignificantDuplication(List.copyOf(metrics)));
+        return classDisharmonies;
     }
 
     // remove node if package not in codebase