Merge pull request #84 from refactorfirst/upgrade-to-new-pmd

Author: jimbethancourt

.github/workflows/maven.yml

@@ -13,27 +13,29 @@ jobs:
 
     steps:
       - name: Check out Git repository
-        uses: actions/checkout@v2
+        uses: actions/checkout@v3
 
-      - name: Set up JDK 1.8
-        uses: actions/setup-java@v1
+      - name: Set up JDK 11
+        uses: actions/setup-java@v3
         with:
           java-version: 11
+          distribution: 'zulu'
 
       - name: Build With Maven
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}  # Needed to get PR information, if any
           SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
-        run: mvn -B verify org.sonarsource.scanner.maven:sonar-maven-plugin:sonar
+        run: mvn -B verify #org.sonarsource.scanner.maven:sonar-maven-plugin:sonar
 
 
 #      Comment "Build With Maven" and uncomment the below when you want a snapshot build to be deployed
 #      *********Don't forget to switch to Java 1.8 as well********
-#      - name: Publish Maven snapshot
-#        uses: samuelmeuli/action-maven-publish@v1
-#        with:
-#          gpg_private_key: ${{ secrets.gpg_private_key }}
-#          gpg_passphrase: ${{ secrets.gpg_passphrase }}
-#          nexus_username: ${{ secrets.nexus_username }}
-#          nexus_password: ${{ secrets.nexus_password }}
-#          maven_profiles: snapshot-release
+      - name: Publish Maven snapshot
+        uses: samuelmeuli/action-maven-publish@v1
+        with:
+          gpg_private_key: ${{ secrets.gpg_private_key }}
+          gpg_passphrase: ${{ secrets.gpg_passphrase }}
+          nexus_username: ${{ secrets.nexus_username }}
+          nexus_password: ${{ secrets.nexus_password }}
+          maven_profiles: snapshot-release
+          maven_args: -B

cost-benefit-calculator/pom.xml

@@ -12,6 +12,13 @@
     <artifactId>cost-benefit-calculator</artifactId>
 
     <dependencies>
+        <!-- Needed for PMD -->
+        <dependency>
+            <groupId>org.slf4j</groupId>
+            <artifactId>slf4j-api</artifactId>
+            <version>2.0.7</version>
+        </dependency>
+
         <dependency>
             <groupId>org.hjug.refactorfirst.changepronenessranker</groupId>
             <artifactId>change-proneness-ranker</artifactId>

cost-benefit-calculator/src/main/java/org/hjug/cbc/CostBenefitCalculator.java

@@ -1,24 +1,56 @@
 package org.hjug.cbc;
 
-import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
+import static net.sourceforge.pmd.RuleViolation.CLASS_NAME;
+import static net.sourceforge.pmd.RuleViolation.PACKAGE_NAME;
+
 import java.io.File;
 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;
 import lombok.extern.slf4j.Slf4j;
+import net.sourceforge.pmd.*;
+import net.sourceforge.pmd.lang.LanguageRegistry;
 import org.eclipse.jgit.api.errors.GitAPIException;
 import org.eclipse.jgit.lib.Repository;
 import org.hjug.git.ChangePronenessRanker;
 import org.hjug.git.GitLogReader;
 import org.hjug.git.RepositoryLogReader;
 import org.hjug.git.ScmLogInfo;
 import org.hjug.metrics.*;
+import org.hjug.metrics.rules.CBORule;
 
 @Slf4j
 public class CostBenefitCalculator {
 
-    Map<String, ByteArrayOutputStream> filesToScan = new HashMap<>();
+    private Report report;
+    private String projBaseDir = null;
+
+    // copied from PMD's PmdTaskImpl.java and modified
+    public void runPmdAnalysis(String projectBaseDir) throws IOException {
+        projBaseDir = projectBaseDir;
+        PMDConfiguration configuration = new PMDConfiguration();
+
+        try (PmdAnalysis pmd = PmdAnalysis.create(configuration)) {
+            RuleSetLoader rulesetLoader = pmd.newRuleSetLoader();
+            pmd.addRuleSets(rulesetLoader.loadRuleSetsWithoutException(List.of("category/java/design.xml")));
+
+            Rule cboClassRule = new CBORule();
+            cboClassRule.setLanguage(LanguageRegistry.PMD.getLanguageByFullName("Java"));
+            pmd.addRuleSet(RuleSet.forSingleRule(cboClassRule));
+
+            log.info("files to be scanned: " + Paths.get(projectBaseDir));
+
+            try (Stream<Path> files = Files.walk(Paths.get(projectBaseDir))) {
+                files.forEach(file -> pmd.files().addFile(file));
+            }
+
+            report = pmd.performAnalysisAndCollectReport();
+        }
+    }
 
     public List<RankedDisharmony> calculateGodClassCostBenefitValues(String repositoryPath) {
 
@@ -27,11 +59,16 @@ public List<RankedDisharmony> calculateGodClassCostBenefitValues(String reposito
         log.info("Initiating Cost Benefit calculation");
         try {
             repository = repositoryLogReader.gitRepository(new File(repositoryPath));
+            for (String file :
+                    repositoryLogReader.listRepositoryContentsAtHEAD(repository).keySet()) {
+                log.info("Files at HEAD: {}", file);
+            }
         } catch (IOException e) {
             log.error("Failure to access Git repository", e);
         }
 
-        List<GodClass> godClasses = getGodClasses(getFilesToScan(repositoryLogReader, repository));
+        // pass repo path here, not ByteArrayOutputStream
+        List<GodClass> godClasses = getGodClasses();
 
         List<ScmLogInfo> scmLogInfos = getRankedChangeProneness(repositoryLogReader, repository, godClasses);
 
@@ -40,52 +77,70 @@ public List<RankedDisharmony> calculateGodClassCostBenefitValues(String reposito
 
         List<RankedDisharmony> rankedDisharmonies = new ArrayList<>();
         for (GodClass godClass : godClasses) {
-            rankedDisharmonies.add(new RankedDisharmony(godClass, rankedLogInfosByPath.get(godClass.getFileName())));
+            if (rankedLogInfosByPath.containsKey(godClass.getFileName())) {
+                rankedDisharmonies.add(
+                        new RankedDisharmony(godClass, rankedLogInfosByPath.get(godClass.getFileName())));
+            }
+        }
+
+        rankedDisharmonies.sort(
+                Comparator.comparing(RankedDisharmony::getRawPriority).reversed());
+
+        int godClassPriority = 1;
+        for (RankedDisharmony rankedGodClassDisharmony : rankedDisharmonies) {
+            rankedGodClassDisharmony.setPriority(godClassPriority++);
         }
 
         return rankedDisharmonies;
     }
 
+    private List<GodClass> getGodClasses() {
+        List<GodClass> godClasses = new ArrayList<>();
+        for (RuleViolation violation : report.getViolations()) {
+            if (violation.getRule().getName().contains("GodClass")) {
+                GodClass godClass = new GodClass(
+                        violation.getAdditionalInfo().get(CLASS_NAME),
+                        getFileName(violation),
+                        violation.getAdditionalInfo().get(PACKAGE_NAME),
+                        violation.getDescription());
+                log.info("God Class identified: {}", godClass.getFileName());
+                godClasses.add(godClass);
+            }
+        }
+
+        GodClassRanker godClassRanker = new GodClassRanker();
+        godClassRanker.rankGodClasses(godClasses);
+
+        return godClasses;
+    }
+
     <T extends Disharmony> List<ScmLogInfo> getRankedChangeProneness(
             RepositoryLogReader repositoryLogReader, Repository repository, List<T> disharmonies) {
         List<ScmLogInfo> scmLogInfos = new ArrayList<>();
-        log.info("Calculating Change Proneness for each God Class");
+        log.info("Calculating Change Proneness");
         for (Disharmony disharmony : disharmonies) {
             String path = disharmony.getFileName();
             ScmLogInfo scmLogInfo = null;
             try {
                 scmLogInfo = repositoryLogReader.fileLog(repository, path);
+                log.info("Successfully fetched scmLogInfo for {}", scmLogInfo.getPath());
             } catch (GitAPIException | IOException e) {
-                log.error("Error reading Git repository contents", e);
+                log.error("Error reading Git repository contents.", e);
+            } catch (NullPointerException e) {
+                log.error("Encountered nested class in a class containing a violation.  Class: {}", path);
             }
 
-            scmLogInfos.add(scmLogInfo);
+            if (null != scmLogInfo) {
+                log.info("adding {}", scmLogInfo.getPath());
+                scmLogInfos.add(scmLogInfo);
+            }
         }
 
         ChangePronenessRanker changePronenessRanker = new ChangePronenessRanker(repository, repositoryLogReader);
         changePronenessRanker.rankChangeProneness(scmLogInfos);
         return scmLogInfos;
     }
 
-    private List<GodClass> getGodClasses(Map<String, ByteArrayOutputStream> filesToScan) {
-        PMDGodClassRuleRunner ruleRunner = new PMDGodClassRuleRunner();
-
-        log.info("Identifying God Classes from files in repository");
-        List<GodClass> godClasses = new ArrayList<>();
-        for (Map.Entry<String, ByteArrayOutputStream> entry : filesToScan.entrySet()) {
-            String filePath = entry.getKey();
-            ByteArrayOutputStream value = entry.getValue();
-
-            ByteArrayInputStream inputStream = new ByteArrayInputStream(value.toByteArray());
-            Optional<GodClass> godClassOptional = ruleRunner.runGodClassRule(filePath, inputStream);
-            godClassOptional.ifPresent(godClasses::add);
-        }
-
-        GodClassRanker godClassRanker = new GodClassRanker();
-        godClassRanker.rankGodClasses(godClasses);
-        return godClasses;
-    }
-
     public List<RankedDisharmony> calculateCBOCostBenefitValues(String repositoryPath) {
 
         RepositoryLogReader repositoryLogReader = new GitLogReader();
@@ -97,7 +152,7 @@ public List<RankedDisharmony> calculateCBOCostBenefitValues(String repositoryPat
             log.error("Failure to access Git repository", e);
         }
 
-        List<CBOClass> cboClasses = getCBOClasses(getFilesToScan(repositoryLogReader, repository));
+        List<CBOClass> cboClasses = getCBOClasses();
 
         List<ScmLogInfo> scmLogInfos = getRankedChangeProneness(repositoryLogReader, repository, cboClasses);
 
@@ -109,37 +164,35 @@ public List<RankedDisharmony> calculateCBOCostBenefitValues(String repositoryPat
             rankedDisharmonies.add(new RankedDisharmony(cboClass, rankedLogInfosByPath.get(cboClass.getFileName())));
         }
 
-        return rankedDisharmonies;
-    }
+        rankedDisharmonies.sort(
+                Comparator.comparing(RankedDisharmony::getRawPriority).reversed());
 
-    private List<CBOClass> getCBOClasses(Map<String, ByteArrayOutputStream> filesToScan) {
+        int cboPriority = 1;
+        for (RankedDisharmony rankedCBODisharmony : rankedDisharmonies) {
+            rankedCBODisharmony.setPriority(cboPriority++);
+        }
 
-        CBORuleRunner ruleRunner = new CBORuleRunner();
+        return rankedDisharmonies;
+    }
 
-        log.info("Identifying highly coupled classes from files in repository");
+    private List<CBOClass> getCBOClasses() {
         List<CBOClass> cboClasses = new ArrayList<>();
-        for (Map.Entry<String, ByteArrayOutputStream> entry : filesToScan.entrySet()) {
-            String filePath = entry.getKey();
-            ByteArrayOutputStream value = entry.getValue();
-
-            ByteArrayInputStream inputStream = new ByteArrayInputStream(value.toByteArray());
-            Optional<CBOClass> godClassOptional = ruleRunner.runCBOClassRule(filePath, inputStream);
-            godClassOptional.ifPresent(cboClasses::add);
+        for (RuleViolation violation : report.getViolations()) {
+            if (violation.getRule().getName().contains("CBORule")) {
+                log.info(violation.getDescription());
+                CBOClass godClass = new CBOClass(
+                        violation.getAdditionalInfo().get(CLASS_NAME),
+                        getFileName(violation),
+                        violation.getAdditionalInfo().get(PACKAGE_NAME),
+                        violation.getDescription());
+                log.info("Highly Coupled class identified: {}", godClass.getFileName());
+                cboClasses.add(godClass);
+            }
         }
-
         return cboClasses;
     }
 
-    private Map<String, ByteArrayOutputStream> getFilesToScan(
-            RepositoryLogReader repositoryLogReader, Repository repository) {
-
-        try {
-            if (filesToScan.isEmpty()) {
-                filesToScan = repositoryLogReader.listRepositoryContentsAtHEAD(repository);
-            }
-        } catch (IOException e) {
-            log.error("Error reading Git repository contents", e);
-        }
-        return filesToScan;
+    private String getFileName(RuleViolation violation) {
+        return violation.getFileId().getUriString().replace("file:///" + projBaseDir.replace("\\", "/") + "/", "");
     }
 }

cost-benefit-calculator/src/main/java/org/hjug/cbc/RankedDisharmony.java

@@ -16,7 +16,7 @@ public class RankedDisharmony {
     private final Integer effortRank;
     private final Integer changePronenessRank;
     private final Integer rawPriority;
-    private Integer priority;
+    private Integer priority = 0;
 
     private Integer wmc;
     private Integer wmcRank;