Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# From https://github.com/gitattributes/gitattributes/blob/master/Java.gitattributes
# Java sources
*.java text diff=java
*.kt text diff=kotlin
*.groovy text diff=java
*.scala text diff=java
*.gradle text diff=java
*.gradle.kts text diff=kotlin

# These files are text and should be normalized (Convert crlf => lf)
*.css text diff=css
*.scss text diff=css
*.sass text
*.df text
*.htm text diff=html
*.html text diff=html
*.js text
*.mjs text
*.cjs text
*.jsp text
*.jspf text
*.jspx text
*.properties text
*.tld text
*.tag text
*.tagx text
*.xml text
*.md text

# These files are binary and should be left untouched
# (binary is a macro for -text -diff)
*.class binary
*.dll binary
*.ear binary
*.jar binary
*.so binary
*.war binary
*.jks binary

# Common build-tool wrapper scripts ('.cmd' versions are handled by 'Common.gitattributes')
mvnw text eol=lf
gradlew text eol=lf

# These are explicitly windows files and should use crlf
*.bat text eol=crlf
2 changes: 1 addition & 1 deletion .github/workflows/maven-pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:
- name: Set up JDK 11
uses: actions/setup-java@v4
with:
java-version: 11
java-version: 17
distribution: 'zulu'

- name: Build With Maven
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/maven.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:
- name: Set up JDK 11
uses: actions/setup-java@v4
with:
java-version: 11
java-version: 17
distribution: 'zulu'

- name: Build With Maven
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ jobs:
- name: Set up JDK 11
uses: actions/setup-java@v3
with:
java-version: '11'
java-version: '17'
distribution: 'temurin'
cache: maven
- name: Set release version
Expand Down
39 changes: 39 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# RefactorFirst Agent Guide

## Essential Commands
- Full build: `mvn clean install`
- Skip tests: `mvn clean install -DskipTests`
- Run specific module tests: `mvn clean test -pl <module-name>`
- Run single test class: `mvn clean test -pl effort-ranker -Dtest=<TestClassName>`
- Format code: `mvn spotless:apply`
- Check formatting: `mvn spotless:check`
- Build with OWASP dependency check: `mvn clean install -Plocal`

## Key Architecture Points
- 11-module Maven build with data flowing left-to-right through the pipeline
- Central DTO: `CodebaseGraphDTO` (JGraphT graphs + disharmony lists + metrics)
- CLI entry point: `org.hjug.refactorfirst.Main` → `ReportCommand`
- Fat jar location: `cli/target/refactor-first-cli-*.jar`

## Testing Notes
- JUnit 5 with parameterized tests
- Test fixtures in `test-resources/src/test/resources`
- For graph algorithm changes, check `JavaGraphBuilderTest` and `CircularReferenceCheckerTests`
- Mutation testing via PIT available but not in default build

## Java & Toolchain
- Java 17 minimum (OpenRewrite supports 17, 21, 25)
- Lombok `@Data`/`@Builder` used extensively - avoid adding boilerplate it already removes
- SLF4J logging: `log.debug()` for verbose per-class output, `log.info()` sparingly
- Spotless enforces Palantir Java format

## Maven Plugin Usage
Generate reports directly:
`mvn org.hjug.refactorfirst.plugin:refactor-first-maven-plugin:0.8.0:htmlReport`

Configuration options (most important):
- `showDetails`: Shows God Class metrics in table (default: false)
- `backEdgeAnalysisCount`: 0 = analyze all back edges (default: 50)
- `analyzeCycles`: Whether to analyze cycles (default: true)
- `excludeTests`: Exclude test classes (default: true)
- `minifyHtml`: Minify HTML report (default: false)
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.*;
import java.util.stream.Collectors;
import java.util.stream.Stream;
Expand Down Expand Up @@ -80,7 +79,7 @@ private CodebaseGraphDTO processWithOpenRewrite(String repositoryPath, GraphBuil
new GraphMetricsCollector(classReferencesGraph, packageReferencesGraph);
MetricsCollectingVisitor metricsVisitor = new MetricsCollectingVisitor(metricsCollector);

try (Stream<Path> pathStream = Files.walk(Paths.get(srcDirectory.getAbsolutePath()))) {
try (Stream<Path> pathStream = Files.walk(Path.of(srcDirectory.getAbsolutePath()))) {
List<Path> list;
if (config.isExcludeTests()) {
list = pathStream
Expand All @@ -90,12 +89,10 @@ private CodebaseGraphDTO processWithOpenRewrite(String repositoryPath, GraphBuil
list = pathStream.collect(Collectors.toList());
}

javaParser
.parse(list, Paths.get(srcDirectory.getAbsolutePath()), ctx)
.forEach(cu -> {
javaVisitor.visit(cu, ctx);
metricsVisitor.visit(cu, ctx);
});
javaParser.parse(list, Path.of(srcDirectory.getAbsolutePath()), ctx).forEach(cu -> {
javaVisitor.visit(cu, ctx);
metricsVisitor.visit(cu, ctx);
});
}

removeClassesNotInCodebase(dependencyCollector.getPackagesInCodebase(), classReferencesGraph);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,7 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, Ex
currentPackageName = type.getPackageName();

// Get or create metrics - this ensures it's stored in the collector
if (metricsCollector instanceof GraphMetricsCollector) {
GraphMetricsCollector gmc = (GraphMetricsCollector) metricsCollector;
if (metricsCollector instanceof GraphMetricsCollector gmc) {
currentClassMetrics = gmc.getAllClassMetrics().computeIfAbsent(currentClassName, ClassMetrics::new);
} else {
currentClassMetrics = metricsCollector.getClassMetrics(currentClassName);
Expand All @@ -85,16 +84,32 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, Ex
currentClassMetrics.setParentClass(parentType.getFullyQualifiedName());
}

// Handle record components
boolean isRecord = classDecl.getKind() == J.ClassDeclaration.Kind.Type.Record;
if (isRecord) {
// Record components are in the primary constructor
List<Statement> primaryConstructor = classDecl.getPrimaryConstructor();
if (primaryConstructor != null) {
for (Statement stmt : primaryConstructor) {
if (stmt instanceof J.VariableDeclarations varDecl) {
for (J.VariableDeclarations.NamedVariable var : varDecl.getVariables()) {
// Record components are implicitly public final fields with accessor methods
String varName = var.getSimpleName();
currentClassMetrics.addAttribute(varName, true); // public
}
}
}
}
}

// Count protected members
int protectedMembers = 0;
for (Statement statement : classDecl.getBody().getStatements()) {
if (statement instanceof J.VariableDeclarations) {
J.VariableDeclarations varDecl = (J.VariableDeclarations) statement;
if (statement instanceof J.VariableDeclarations varDecl) {
if (varDecl.getModifiers().stream().anyMatch(mod -> mod.getType() == J.Modifier.Type.Protected)) {
protectedMembers++;
}
} else if (statement instanceof J.MethodDeclaration) {
J.MethodDeclaration methodDecl = (J.MethodDeclaration) statement;
} else if (statement instanceof J.MethodDeclaration methodDecl) {
if (methodDecl.getModifiers().stream().anyMatch(mod -> mod.getType() == J.Modifier.Type.Protected)) {
protectedMembers++;
}
Expand Down Expand Up @@ -191,6 +206,23 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, Ex
public J.VariableDeclarations visitVariableDeclarations(
J.VariableDeclarations multiVariable, ExecutionContext ctx) {
if (currentClassName != null && currentMethodSignature == null) {
// Skip record components in primary constructor - they're already counted in visitClassDeclaration
J.ClassDeclaration enclosingClass = getCursor().firstEnclosing(J.ClassDeclaration.class);
if (enclosingClass != null && enclosingClass.getKind() == J.ClassDeclaration.Kind.Type.Record) {
// Check if this VariableDeclarations is in the primary constructor
// The parent is JRightPadded, grandparent is the JContainer/List
Object grandParent = getCursor().getParent().getParent().getValue();
List<Statement> primaryConstructor = enclosingClass.getPrimaryConstructor();
if (primaryConstructor != null) {
if (grandParent == primaryConstructor
|| (grandParent instanceof JContainer<?> container
&& primaryConstructor.equals(container.getElements()))) {
// This is a record component in the primary constructor, skip it
return super.visitVariableDeclarations(multiVariable, ctx);
}
}
}

for (J.VariableDeclarations.NamedVariable var : multiVariable.getVariables()) {
String varName = var.getSimpleName();
boolean isPublic = multiVariable.hasModifier(J.Modifier.Type.Public);
Expand Down Expand Up @@ -238,8 +270,8 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
JavaType.Method methodType = method.getMethodType();
if (methodType != null && !methodType.isConstructor()) {
JavaType declaringType = methodType.getDeclaringType();
if (declaringType instanceof JavaType.FullyQualified) {
String declaringFqn = ((JavaType.FullyQualified) declaringType).getFullyQualifiedName();
if (declaringType instanceof JavaType.FullyQualified qualified) {
String declaringFqn = qualified.getFullyQualifiedName();
if (!declaringFqn.equals(currentClassName)) {
StringBuilder sig = new StringBuilder();
sig.append(declaringFqn)
Expand Down Expand Up @@ -271,8 +303,7 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
public J.FieldAccess visitFieldAccess(J.FieldAccess fieldAccess, ExecutionContext ctx) {
if (currentMethodMetrics != null && fieldAccess.getType() != null) {
JavaType type = fieldAccess.getType();
if (type instanceof JavaType.Variable) {
JavaType.Variable varType = (JavaType.Variable) type;
if (type instanceof JavaType.Variable varType) {
if (varType.getOwner() instanceof JavaType.FullyQualified) {
JavaType.FullyQualified owner = (JavaType.FullyQualified) varType.getOwner();
String ownerFqn = owner.getFullyQualifiedName();
Expand Down Expand Up @@ -311,8 +342,7 @@ private String buildMethodSignature(J.MethodDeclaration method) {
sig.append(method.getSimpleName()).append("(");
boolean first = true;
for (org.openrewrite.java.tree.Statement param : method.getParameters()) {
if (param instanceof J.VariableDeclarations) {
J.VariableDeclarations varDecl = (J.VariableDeclarations) param;
if (param instanceof J.VariableDeclarations varDecl) {
if (!first) {
sig.append(",");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@
import org.openrewrite.java.JavaIsoVisitor;
import org.openrewrite.java.JavadocVisitor;
import org.openrewrite.java.tree.*;
import org.openrewrite.java.tree.J.VariableDeclarations;
import org.openrewrite.java.tree.Javadoc;
import org.openrewrite.java.tree.Statement;
import org.openrewrite.java.tree.TypeTree;

/**
* BUG: Static method calls and definitions are not being captured, but were previously being captured.
Expand Down Expand Up @@ -110,6 +113,25 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, P
}
}

// Handle record components (record header parameters)
if (classDecl.getKind() == J.ClassDeclaration.Kind.Type.Record) {
List<Statement> primaryConstructor = classDecl.getPrimaryConstructor();
if (primaryConstructor != null) {
for (Statement stmt : primaryConstructor) {
if (stmt instanceof VariableDeclarations varDecl) {
TypeTree typeExpression = varDecl.getTypeExpression();
if (typeExpression != null) {
typeProcessor.processType(owningFqn, typeExpression.getType());
}
// Also process annotations on record components
for (J.Annotation annotation : varDecl.getLeadingAnnotations()) {
typeProcessor.processAnnotation(owningFqn, annotation, getCursor());
}
}
}
}
}

return super.visitClassDeclaration(classDecl, p);
} finally {
currentOwnerFqn = previousOwner;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@ public Set<String> extractDependencies(JavaType javaType) {
}

private void extractDependenciesRecursive(JavaType javaType, Set<String> dependencies) {
if (javaType instanceof JavaType.Class) {
extractFromClass((JavaType.Class) javaType, dependencies);
} else if (javaType instanceof JavaType.Parameterized) {
extractFromParameterized((JavaType.Parameterized) javaType, dependencies);
} else if (javaType instanceof JavaType.GenericTypeVariable) {
extractFromGenericTypeVariable((JavaType.GenericTypeVariable) javaType, dependencies);
} else if (javaType instanceof JavaType.Array) {
extractFromArray((JavaType.Array) javaType, dependencies);
if (javaType instanceof JavaType.Class clazz) {
extractFromClass(clazz, dependencies);
} else if (javaType instanceof JavaType.Parameterized parameterized) {
extractFromParameterized(parameterized, dependencies);
} else if (javaType instanceof JavaType.GenericTypeVariable variable) {
extractFromGenericTypeVariable(variable, dependencies);
} else if (javaType instanceof JavaType.Array array) {
extractFromArray(array, dependencies);
}
}

Expand Down Expand Up @@ -62,10 +62,10 @@ private void extractFromGenericTypeVariable(JavaType.GenericTypeVariable typeVar
log.debug("Type parameter type name: {}", typeVariable.getName());

for (JavaType bound : typeVariable.getBounds()) {
if (bound instanceof JavaType.Class) {
dependencies.add(((JavaType.Class) bound).getFullyQualifiedName());
} else if (bound instanceof JavaType.Parameterized) {
dependencies.add(((JavaType.Parameterized) bound).getFullyQualifiedName());
if (bound instanceof JavaType.Class clazz) {
dependencies.add(clazz.getFullyQualifiedName());
} else if (bound instanceof JavaType.Parameterized parameterized) {
dependencies.add(parameterized.getFullyQualifiedName());
} else {
log.debug("Unknown type bound: {}", bound);
}
Expand Down
Loading
Loading