#33 "source code style rule definition"

- all static helper classes are now called *Utils
- all other service-style classes have no static methods anymore and use constructor dependency injection

README.md

@@ -202,3 +202,12 @@ Uncovered methods:
 Unresolvable methods (no coverage information available):
    ...
 ```
+
+## Source code style
+
+1. avoid static methods for easier testing
+    1. exception: (package) private helper methods in classes that are static to enforce functional purity
+    1. exception: completely stateless `*Utils` classes with static methods only
+        1. utility classes must be `abstract` and have a private default constructor
+    1. exception: real constants with names in upper case delimited by underscores
+1. `logger` has to be protected final but not static: `protected final Logger logger = LoggerFactory.getLogger(getClass());`

test-gap-analysis-maven-plugin/src/main/java/com/scheible/testgapanalysis/maven/AbstractTestGapMojo.java

@@ -1,6 +1,6 @@
 package com.scheible.testgapanalysis.maven;
 
-import com.scheible.testgapanalysis.jacoco.JaCoCoHelper;
+import com.scheible.testgapanalysis.jacoco.JaCoCoReportParser;
 
 import java.io.File;
 import java.nio.file.Path;
@@ -37,7 +37,7 @@ protected Set<File> findRelevantJaCoCoReportFiles() {
 		final Path outputDirAsPath = outputDir.toPath();
 		final Path testOutputDirAsPath = testOutputDir.toPath();
 
-		JaCoCoHelper.findJaCoCoReportFiles(buildDir).forEach(file -> {
+		JaCoCoReportParser.findJaCoCoReportFiles(buildDir).forEach(file -> {
 			final Path fileAsPath = file.toPath();
 			if (!fileAsPath.startsWith(outputDirAsPath) && !fileAsPath.startsWith(testOutputDirAsPath)) {
 				result.add(file);

test-gap-analysis-maven-plugin/src/main/java/com/scheible/testgapanalysis/maven/DebugCoverageResolutionMojo.java

@@ -2,6 +2,8 @@
 
 import com.scheible.testgapanalysis.DebugCoverageResolution;
 import com.scheible.testgapanalysis.DebugCoverageResolutionReport;
+import com.scheible.testgapanalysis.jacoco.JaCoCoReportParser;
+import com.scheible.testgapanalysis.parser.JavaParser;
 
 import java.io.File;
 
@@ -23,7 +25,9 @@ public class DebugCoverageResolutionMojo extends AbstractTestGapMojo {
 	@Override
 	public void execute() throws MojoExecutionException, MojoFailureException {
 		if (buildDir.exists()) {
-			final DebugCoverageResolutionReport report = DebugCoverageResolution.run(baseDir, sourceDir,
+			final DebugCoverageResolution debugCoverageResolution = new DebugCoverageResolution(new JavaParser(), 
+					new JaCoCoReportParser());
+			final DebugCoverageResolutionReport report = debugCoverageResolution.run(baseDir, sourceDir,
 					findRelevantJaCoCoReportFiles());
 
 			logReport(report);

test-gap-analysis-maven-plugin/src/main/java/com/scheible/testgapanalysis/maven/TestGapAnalysisMojo.java

@@ -4,6 +4,10 @@
 import com.google.gson.GsonBuilder;
 import com.scheible.testgapanalysis.TestGapAnalysis;
 import com.scheible.testgapanalysis.TestGapReport;
+import com.scheible.testgapanalysis.analysis.Analysis;
+import com.scheible.testgapanalysis.git.GitDiffer;
+import com.scheible.testgapanalysis.jacoco.JaCoCoReportParser;
+import com.scheible.testgapanalysis.parser.JavaParser;
 
 import java.io.File;
 import java.io.IOException;
@@ -28,7 +32,9 @@ public class TestGapAnalysisMojo extends AbstractTestGapMojo {
 	@Override
 	public void execute() throws MojoExecutionException, MojoFailureException {
 		if (buildDir.exists()) {
-			final TestGapReport report = TestGapAnalysis.run(baseDir, findRelevantJaCoCoReportFiles(),
+			final TestGapAnalysis testGapAnalysis = new TestGapAnalysis(new Analysis(new JavaParser()),
+					new JaCoCoReportParser(), new GitDiffer());
+			final TestGapReport report = testGapAnalysis.run(baseDir, findRelevantJaCoCoReportFiles(),
 					Optional.ofNullable(referenceCommitHash));
 
 			logReport(report);

test-gap-analysis/spotbugs-exclude.xml

@@ -1,8 +1,8 @@
 <FindBugsFilter>
 	<!-- Path is specified by user, checking therfore impossible. -->
 	<Match>
-		<Class name="com.scheible.testgapanalysis.git.RepositoryStatus" />
-		<Method name="appendChildFile" />
+		<Class name="com.scheible.testgapanalysis.git.GitDiffer" />
+		<Method name="newFile" />
 		<Bug pattern="PATH_TRAVERSAL_IN" />
 	</Match>
 	<Match>

test-gap-analysis/src/main/java/com/scheible/testgapanalysis/DebugCoverageResolution.java

@@ -10,12 +10,12 @@
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.stream.Stream;
 
-import com.scheible.testgapanalysis.common.Files2;
-import com.scheible.testgapanalysis.jacoco.JaCoCoHelper;
+import com.scheible.testgapanalysis.common.FilesUtils;
+import com.scheible.testgapanalysis.jacoco.JaCoCoReportParser;
 import com.scheible.testgapanalysis.jacoco.MethodWithCoverageInfo;
 import com.scheible.testgapanalysis.jacoco.resolver.CoverageResolver;
 import com.scheible.testgapanalysis.jacoco.resolver.CoverageResult;
-import com.scheible.testgapanalysis.parser.JavaParserHelper;
+import com.scheible.testgapanalysis.parser.JavaParser;
 import com.scheible.testgapanalysis.parser.ParsedMethod;
 
 /**
@@ -35,28 +35,36 @@ private ParseResult(final Set<ParsedMethod> methods, final int javaFileCount) {
 		}
 	}
 
-	public static DebugCoverageResolutionReport run(final File workDir, final File sourceDir,
+	private final JavaParser javaParser;
+	private final JaCoCoReportParser jaCoCoReportParser;
+
+	public DebugCoverageResolution(final JavaParser javaParser, final JaCoCoReportParser jaCoCoReportParser) {
+		this.javaParser = javaParser;
+		this.jaCoCoReportParser = jaCoCoReportParser;
+	}
+
+	public DebugCoverageResolutionReport run(final File workDir, final File sourceDir,
 			final Set<File> jaCoCoReportFiles) {
-		final Set<MethodWithCoverageInfo> coverageInfo = JaCoCoHelper.getMethodCoverage(jaCoCoReportFiles);
+		final Set<MethodWithCoverageInfo> coverageInfo = jaCoCoReportParser.getMethodCoverage(jaCoCoReportFiles);
 		final ParseResult parseResult = parseMethods(sourceDir);
 
 		final CoverageResolver resolver = CoverageResolver.with(coverageInfo);
 		final CoverageResult result = resolver.resolve(parseResult.methods);
 
-		return new DebugCoverageResolutionReport(coverageInfo.size(), Files2.toRelative(workDir, jaCoCoReportFiles),
+		return new DebugCoverageResolutionReport(coverageInfo.size(), FilesUtils.toRelative(workDir, jaCoCoReportFiles),
 				parseResult.javaFileCount, result.getResolvedMethods(), result.getEmptyMethods(),
 				result.getUnresolvedMethods(), result.getAmbiguousCoverage());
 	}
 
-	private static ParseResult parseMethods(final File workingDir) throws UncheckedIOException {
+	private ParseResult parseMethods(final File workingDir) throws UncheckedIOException {
 		final AtomicInteger javaFileCount = new AtomicInteger(0);
 		final Set<ParsedMethod> methods = new HashSet<>();
 
 		try (Stream<Path> walkStream = Files.walk(workingDir.toPath())) {
 			walkStream.filter(p -> p.toFile().isFile()).forEach(file -> {
 				if (file.toString().endsWith(".java")) {
 					javaFileCount.incrementAndGet();
-					methods.addAll(JavaParserHelper.getMethods(Files2.readUtf8(file.toFile())));
+					methods.addAll(javaParser.getMethods(FilesUtils.readUtf8(file.toFile())));
 				}
 			});
 		} catch (IOException ex) {

test-gap-analysis/src/main/java/com/scheible/testgapanalysis/TestGapAnalysis.java

@@ -18,9 +18,10 @@
 import com.scheible.testgapanalysis.TestGapReport.TestGapMethod;
 import com.scheible.testgapanalysis.analysis.Analysis;
 import com.scheible.testgapanalysis.analysis.AnalysisResult;
-import com.scheible.testgapanalysis.common.Files2;
+import com.scheible.testgapanalysis.common.FilesUtils;
+import com.scheible.testgapanalysis.git.GitDiffer;
 import com.scheible.testgapanalysis.git.RepositoryStatus;
-import com.scheible.testgapanalysis.jacoco.JaCoCoHelper;
+import com.scheible.testgapanalysis.jacoco.JaCoCoReportParser;
 import com.scheible.testgapanalysis.jacoco.MethodWithCoverageInfo;
 import com.scheible.testgapanalysis.parser.ParsedMethod;
 
@@ -66,26 +67,36 @@ private CoverageResult(final Set<TestGapMethod> coveredMethods, final Set<TestGa
 	public static final Predicate<String> NON_TEST_JAVA_FILE = f -> f.endsWith(".java")
 			&& !f.startsWith("src/test/java/") && !f.contains("/src/test/java/");
 
-	public static TestGapReport run(final File workDir, final Set<File> jaCoCoReportFiles,
+	private final Analysis analysis;
+	private final JaCoCoReportParser jaCoCoReportParser;
+	private final GitDiffer gitDiffer;
+
+	public TestGapAnalysis(final Analysis analysis, final JaCoCoReportParser jaCoCoReportParser,
+			final GitDiffer gitDiffer) {
+		this.analysis = analysis;
+		this.jaCoCoReportParser = jaCoCoReportParser;
+		this.gitDiffer = gitDiffer;
+	}
+
+	public TestGapReport run(final File workDir, final Set<File> jaCoCoReportFiles,
 			final Optional<String> referenceCommitHash) {
-		final Set<MethodWithCoverageInfo> coverageInfo = JaCoCoHelper.getMethodCoverage(jaCoCoReportFiles);
+		final Set<MethodWithCoverageInfo> coverageInfo = jaCoCoReportParser.getMethodCoverage(jaCoCoReportFiles);
 		final RepositoryResult repositoryResult = identifyFileChanges(referenceCommitHash, workDir);
 		final CoverageResult coverageResult = performTestGapAnalysis(repositoryResult.repositoryStatus, coverageInfo);
 
 		return new TestGapReport(workDir.getAbsolutePath(), repositoryResult.repositoryStatus.getOldCommitHash(),
-				repositoryResult.repositoryStatus.getNewCommitHash(), Files2.toRelative(workDir, jaCoCoReportFiles),
+				repositoryResult.repositoryStatus.getNewCommitHash(), FilesUtils.toRelative(workDir, jaCoCoReportFiles),
 				coverageInfo.size(), repositoryResult.newOrChangedFiles, coverageResult.coveredMethods,
 				coverageResult.uncoveredMethods, coverageResult.emptyMethods, coverageResult.unresolvableMethods,
 				coverageResult.ambiguouslyResolvedCoverage);
 	}
 
-	private static RepositoryResult identifyFileChanges(final Optional<String> referenceCommitHash,
-			final File workDir) {
+	private RepositoryResult identifyFileChanges(final Optional<String> referenceCommitHash, final File workDir) {
 		Set<NewOrChangedFile> newOrChangedFiles = emptySet();
 
 		final RepositoryStatus status = referenceCommitHash
-				.map(h -> RepositoryStatus.ofCommitComparedToHead(workDir, h))
-				.orElseGet(() -> RepositoryStatus.ofWorkingCopyChanges(workDir));
+				.map(h -> gitDiffer.ofCommitComparedToHead(workDir, h, NON_TEST_JAVA_FILE))
+				.orElseGet(() -> gitDiffer.ofWorkingCopyChanges(workDir, NON_TEST_JAVA_FILE));
 
 		if (!(status.getAddedFiles().isEmpty() && status.getChangedFiles().isEmpty())) {
 			newOrChangedFiles = Stream.concat(status.getChangedFiles().stream(), status.getAddedFiles().stream())
@@ -99,9 +110,9 @@ private static RepositoryResult identifyFileChanges(final Optional<String> refer
 		return new RepositoryResult(status, newOrChangedFiles);
 	}
 
-	private static CoverageResult performTestGapAnalysis(final RepositoryStatus status,
+	private CoverageResult performTestGapAnalysis(final RepositoryStatus status,
 			final Set<MethodWithCoverageInfo> coverageInfo) {
-		final AnalysisResult result = Analysis.perform(status, NON_TEST_JAVA_FILE, coverageInfo);
+		final AnalysisResult result = analysis.perform(status, coverageInfo);
 
 		return new CoverageResult(toTestGapMethods(result.getCoveredMethods()),
 				toTestGapMethods(result.getUncoveredMethods()), toTestGapMethods(result.getEmptyMethods()),

test-gap-analysis/src/main/java/com/scheible/testgapanalysis/analysis/Analysis.java

@@ -12,7 +12,7 @@
 import com.scheible.testgapanalysis.jacoco.MethodWithCoverageInfo;
 import com.scheible.testgapanalysis.jacoco.resolver.CoverageResolver;
 import com.scheible.testgapanalysis.jacoco.resolver.CoverageResult;
-import com.scheible.testgapanalysis.parser.JavaParserHelper;
+import com.scheible.testgapanalysis.parser.JavaParser;
 import com.scheible.testgapanalysis.parser.ParsedMethod;
 
 /**
@@ -24,18 +24,23 @@ public class Analysis {
 	static final Predicate<MethodCompareWrapper> NON_GETTER_OR_SETTER_METHOD = mcw -> !mcw.getParsedMethod().getName()
 			.startsWith("get") && !mcw.getParsedMethod().getName().startsWith("set");
 
-	public static AnalysisResult perform(final RepositoryStatus status, final Predicate<String> fileFilter,
-			final Set<MethodWithCoverageInfo> coverageInfo) {
-		final Map<String, String> newOrChangedFilesWithContent = status.getNewContents(fileFilter);
+	private final JavaParser javaParser;
+
+	public Analysis(final JavaParser javaParser) {
+		this.javaParser = javaParser;
+	}
+
+	public AnalysisResult perform(final RepositoryStatus status, final Set<MethodWithCoverageInfo> coverageInfo) {
+		final Map<String, String> newOrChangedFilesWithContent = status.getNewContents();
 
 		// all methods of new or changed files in the new state compared to the old state
 		final Set<MethodCompareWrapper> newContentMethods = newOrChangedFilesWithContent.entrySet().stream()
-				.flatMap(e -> JavaParserHelper.getMethods(e.getValue()).stream()).map(MethodCompareWrapper::new)
+				.flatMap(e -> javaParser.getMethods(e.getValue()).stream()).map(MethodCompareWrapper::new)
 				.filter(NON_GETTER_OR_SETTER_METHOD).collect(Collectors.toSet());
 
 		// all methods of changed files in the new state that already existed in the old state
-		final Set<MethodCompareWrapper> oldContentMethods = status.getOldContents(fileFilter).entrySet().stream()
-				.flatMap(e -> JavaParserHelper.getMethods(e.getValue()).stream()).map(MethodCompareWrapper::new)
+		final Set<MethodCompareWrapper> oldContentMethods = status.getOldContents().entrySet().stream()
+				.flatMap(e -> javaParser.getMethods(e.getValue()).stream()).map(MethodCompareWrapper::new)
 				.filter(NON_GETTER_OR_SETTER_METHOD).collect(Collectors.toSet());
 
 		// @formatter:off

test-gap-analysis/src/main/java/com/scheible/testgapanalysis/common/Files2.java → test-gap-analysis/src/main/java/com/scheible/testgapanalysis/common/FilesUtils.java

@@ -17,12 +17,15 @@
  *
  * @author sj
  */
-public class Files2 {
+public abstract class FilesUtils {
 
 	private static final String UTF_8_CHARSET = StandardCharsets.UTF_8.name();
 
 	private static final String BACKSLASH_PATTERN = "\\\\";
 
+	private FilesUtils() {
+	}
+
 	public static String readUtf8(final Class<?> anchor, final String name) {
 		return readUtf8(anchor.getResourceAsStream(name));
 	}

test-gap-analysis/src/main/java/com/scheible/testgapanalysis/common/JavaMethodUtil.java → test-gap-analysis/src/main/java/com/scheible/testgapanalysis/common/JavaMethodUtils.java

@@ -13,7 +13,7 @@
  *
  * @author sj
  */
-public class JavaMethodUtil {
+public class JavaMethodUtils {
 
 	private static final Map<String, String> PRIMITIVE_TYPE_MAPPING = new HashMap<>();
 
@@ -33,7 +33,7 @@ public class JavaMethodUtil {
 	 * ed.)") and returns Java notations. Java notations means for example '.' separator between nested classes.
 	 */
 	public static List<String> parseDescriptorArguments(final String descriptor) {
-		return parseArguments(descriptor).stream().map(JavaMethodUtil::convertType).collect(Collectors.toList());
+		return parseArguments(descriptor).stream().map(JavaMethodUtils::convertType).collect(Collectors.toList());
 	}
 
 	static List<String> parseArguments(final String descriptor) {
@@ -137,7 +137,7 @@ private static String removeArrayBrackets(final String type) {
 	}
 
 	private static List<String> normalizeMethodArguments(final Collection<String> arguments) {
-		return arguments.stream().map(JavaMethodUtil::normalizeArgument).collect(Collectors.toList());
+		return arguments.stream().map(JavaMethodUtils::normalizeArgument).collect(Collectors.toList());
 	}
 
 	private static String normalizeArgument(final String argument) {