Merge branch 'pr-128'

Improve progress report of ReviewCodeCmd #128

Author: adangel

ReleaseNotes.md

@@ -18,6 +18,7 @@ This is a minor release.
 *   [#123](https://github.com/pmd/pmd-eclipse-plugin/pull/123): Support external configuration changes
 *   [#124](https://github.com/pmd/pmd-eclipse-plugin/issues/124): No repository found containing: osgi.bundle,org.slf4j.log4j,1.7.2.v20130115-1340
 *   [#125](https://github.com/pmd/pmd-eclipse-plugin/issues/125): NoSuchMethodError SizeBasedTriggeringPolicy.setMaxFileSize(Ljava/lang/String;)V
+*   [#128](https://github.com/pmd/pmd-eclipse-plugin/pull/128): Improve progress report of ReviewCodeCmd
 
 ### API Changes
 

net.sourceforge.pmd.eclipse.plugin.test/src/main/java/net/sourceforge/pmd/eclipse/runtime/cmd/ReviewCodeCmdNonJavaTest.java

@@ -62,8 +62,9 @@ public void checkCodeForNonJavaProject() throws Exception {
         cmd.performExecute();
         cmd.join();
 
-        // 2 files are there: .project and src/somefile.ext
-        Assert.assertEquals(2, cmd.getStepCount());
+        // 2 files are there: .project and src/somefile.js, but only the javascript file is considered
+        // due to file extensions -> step count = 1
+        Assert.assertEquals(1, cmd.getStepCount());
         // only one file has an extension, that could be mapped to a language, and therefore pmd was executed
         Assert.assertEquals(1, cmd.getFileCount());
 

net.sourceforge.pmd.eclipse.plugin/src/main/java/net/sourceforge/pmd/eclipse/runtime/cmd/BaseVisitor.java

@@ -264,6 +264,9 @@ private boolean isIncluded(IFile file) throws PropertiesException {
      *            the resource to process
      */
     protected final void reviewResource(IResource resource) {
+        if (isCanceled()) {
+            return;
+        }
 
         IFile file = (IFile) resource.getAdapter(IFile.class);
         if (file == null || file.getFileExtension() == null) {
@@ -309,7 +312,7 @@ protected final void reviewResource(IResource resource) {
             final File sourceCodeFile = file.getRawLocation().toFile();
             if (included && getRuleSets().applies(sourceCodeFile) && isFileInWorkingSet(file)
                     && languageVersion != null) {
-                subTask("PMD checking: " + file.getName());
+                subTask("PMD checking: " + file.getProject() + ": " + file.getName());
 
                 long start = System.currentTimeMillis();
 

net.sourceforge.pmd.eclipse.plugin/src/main/java/net/sourceforge/pmd/eclipse/runtime/cmd/ReviewCodeCmd.java

@@ -6,6 +6,7 @@
 
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -176,7 +177,14 @@ public void execute() {
             ruleCount = 0;
             pmdDuration = 0;
 
-            beginTask("PMD checking...", getStepCount());
+            String projectList = determineProjectList();
+            int totalWork = determineTotalWork();
+            LOG.info("Found {} resources in projects {}", totalWork, projectList);
+            setStepCount(totalWork); // mostly for unit tests
+
+            StringBuilder mainTaskName = new StringBuilder(projectList.length() + 20);
+            mainTaskName.append("Executing PMD for ").append(projectList).append(" ...");
+            beginTask(mainTaskName.toString(), totalWork);
 
             // Lancer PMD
             // PMDPlugin fills resources if it's a full build and
@@ -257,6 +265,114 @@ public void run() {
         PMDPlugin.getDefault().changedFiles(markedFiles());
     }
 
+    private int determineTotalWork() {
+        boolean useFileExtensions = PMDPlugin.getDefault().loadPreferences().isDetermineFiletypesAutomatically();
+        Map<IProject, Set<String>> fileExtensionsPerProject = new HashMap<>();
+        CountVisitor2 visitor = new CountVisitor2(useFileExtensions, fileExtensionsPerProject);
+
+        for (IResource resource : resources) {
+            determineFileExtensions(fileExtensionsPerProject, resource);
+
+            try {
+                if (resource instanceof IProject && ((IProject) resource).hasNature(JavaCore.NATURE_ID)) {
+                    for (IResource sourceFolder : getJavaProjectSourceFolders((IProject) resource)) {
+                        sourceFolder.accept(visitor);
+                    }
+                } else {
+                    resource.accept(visitor);
+                }
+            } catch (CoreException e) {
+                LOG.warn("Error while counting resources for {}", resource, e);
+            }
+        }
+        if (resourceDelta != null) {
+            IResource resource = resourceDelta.getResource();
+            determineFileExtensions(fileExtensionsPerProject, resource);
+            try {
+                resource.accept(visitor);
+            } catch (CoreException e) {
+                LOG.warn("Error while counting resources for {} (delta)", resource, e);
+            }
+        }
+        return visitor.getCount();
+    }
+
+    private void determineFileExtensions(Map<IProject, Set<String>> fileExtensionsPerProject, IResource resource) {
+        IProject project = resource.getProject();
+        if (project != null && !fileExtensionsPerProject.containsKey(project)) {
+            try {
+                RuleSets rulesets = rulesetsFrom(resource);
+                Set<String> fileExtensions = determineFileExtensions(rulesets);
+                fileExtensionsPerProject.put(project, fileExtensions);
+            } catch (PropertiesException e) {
+                LOG.warn("Error while determining file extensions for project {}", project, e);
+                fileExtensionsPerProject.put(project, Collections.<String>emptySet());
+            }
+        }
+    }
+
+    private static class CountVisitor2 implements IResourceVisitor {
+        private final boolean useFileExtensions;
+        private final Map<IProject, Set<String>> fileExtensionsPerProject;
+        private int count;
+
+        CountVisitor2(boolean useFileExtensions, Map<IProject, Set<String>> fileExtensionsPerProject) {
+            this.useFileExtensions = useFileExtensions;
+            this.fileExtensionsPerProject = fileExtensionsPerProject;
+        }
+
+        CountVisitor2(boolean useFileExtensions, IProject project, Set<String> fileExtensions) {
+            this.useFileExtensions = useFileExtensions;
+            this.fileExtensionsPerProject = new HashMap<>();
+            this.fileExtensionsPerProject.put(project, fileExtensions);
+        }
+
+        @Override
+        public boolean visit(IResource resource) throws CoreException {
+            if (resource instanceof IFile) {
+                if (useFileExtensions) {
+                    Set<String> extensions = fileExtensionsPerProject.get(resource.getProject());
+                    if (extensions != null && extensions.contains(resource.getFileExtension().toLowerCase(Locale.ROOT))) {
+                        count++;
+                    }
+                } else {
+                    // count all files
+                    count++;
+                }
+            }
+            return true;
+        }
+
+        public int getCount() {
+            return count;
+        }
+    }
+
+    private String determineProjectList() {
+        Set<IProject> projects = new HashSet<>();
+        for (IResource resource : resources) {
+            IProject project = resource.getProject();
+            if (project != null) {
+                projects.add(project);
+            }
+        }
+        if (resourceDelta != null) {
+            IProject project = resourceDelta.getResource().getProject();
+            if (project != null) {
+                projects.add(project);
+            }
+        }
+        StringBuilder projectList = new StringBuilder(projects.size() * 20);
+        projectList.append('[');
+        for (IProject project : projects) {
+            projectList.append(project.getName());
+            projectList.append(", ");
+        }
+        projectList.delete(projectList.length() - 2, projectList.length());
+        projectList.append(']');
+        return projectList.toString();
+    }
+
     public Map<IFile, Set<MarkerInfo2>> getMarkers() {
         return markersByFile;
     }
@@ -365,18 +481,14 @@ private ISchedulingRule getSchedulingRule() {
      * Process the list of workbench resources
      */
     private void processResources() {
-        Set<String> projects = new HashSet<String>();
         for (IResource resource : resources) {
-            projects.add(resource.getProject().getName());
-        }
-        LOG.info("ReviewCodeCmd started with {} selected resources on project {}", resources.size(), projects);
-
-        for (IResource resource : resources) {
-            // if resource is a project, visit only its source folders
-            if (resource instanceof IProject) {
-                processProject((IProject) resource);
-            } else {
-                processResource(resource);
+            if (!isCanceled()) {
+                // if resource is a project, visit only its source folders
+                if (resource instanceof IProject) {
+                    processProject((IProject) resource);
+                } else {
+                    processResource(resource);
+                }
             }
         }
     }
@@ -396,7 +508,7 @@ private RuleSets rulesetsFrom(IResource resource) throws PropertiesException {
     }
 
     /**
-     * Review a single resource.
+     * Review a single resource. The given resource might be a directory, though.
      */
     private void processResource(IResource resource) {
         try {
@@ -412,7 +524,7 @@ private void processResource(IResource resource) {
             // final PMDEngine pmdEngine = getPmdEngineForProject(project);
             int targetCount = 0;
             if (resource.exists()) {
-                targetCount = countResourceElement(resource);
+                targetCount = countResourceElement(resource, fileExtensions);
             }
             // Could add a property that lets us set the max number to analyze
             if (properties.isFullBuildEnabled() || isUserInitiated() || targetCount <= MAXIMUM_RESOURCE_COUNT) {
@@ -441,7 +553,6 @@ private void processResource(IResource resource) {
                         + "If you want to execute PMD, please check \"Full build enabled\" in the project settings.",
                         resource.getName(), targetCount, MAXIMUM_RESOURCE_COUNT);
             }
-            worked(1); // TODO - temp fix? BR
 
         } catch (PropertiesException e) {
             throw new RuntimeException(e);
@@ -461,7 +572,7 @@ private Set<String> determineFileExtensions(RuleSets ruleSets) {
                 fileExtensions.add(extension.toLowerCase(Locale.ROOT));
             }
         }
-        LOG.info("Determined applicable file extensions: {}", fileExtensions);
+        LOG.debug("Determined applicable file extensions: {}", fileExtensions);
         return fileExtensions;
     }
 
@@ -470,8 +581,7 @@ private Set<String> determineFileExtensions(RuleSets ruleSets) {
      */
     private void processProject(IProject project) {
         try {
-            setStepCount(countResourceElement(project));
-            LOG.info("ReviewCodeCmd: visiting {}: {} resources found.", project, getStepCount());
+            subTask("Review " + project);
 
             if (project.hasNature(JavaCore.NATURE_ID)) {
                 processJavaProject(project);
@@ -485,6 +595,13 @@ private void processProject(IProject project) {
     }
 
     private void processJavaProject(IProject project) throws CoreException {
+        for (IResource sourceFolder : getJavaProjectSourceFolders(project)) {
+            processResource(sourceFolder);
+        }
+    }
+
+    private List<IResource> getJavaProjectSourceFolders(IProject project) throws CoreException {
+        List<IResource> sourceFolders = new ArrayList<>();
         final IJavaProject javaProject = JavaCore.create(project);
         final IClasspathEntry[] entries = javaProject.getRawClasspath();
         final IWorkspaceRoot root = ResourcesPlugin.getWorkspace().getRoot();
@@ -507,10 +624,11 @@ private void processJavaProject(IProject project) throws CoreException {
                 if (sourceContainer == null) {
                     LOG.warn("Source container {} for project {} is not valid", entrie.getPath(), project);
                 } else {
-                    processResource(sourceContainer);
+                    sourceFolders.add(sourceContainer);
                 }
             }
         }
+        return sourceFolders;
     }
 
     private void taskScope(int activeRuleCount, int totalRuleCount) {
@@ -650,25 +768,33 @@ private void applyMarkers() {
     /**
      * Count the number of sub-resources of a resource.
      *
-     * @param resource
-     *            a project
+     * @param resource a project
+     * @param fileExtensions the file extensions that should match
      * @return the element count
      */
-    private int countResourceElement(IResource resource) {
+    private int countResourceElement(IResource resource, Set<String> fileExtensions) {
+        boolean checkFileExtensions = PMDPlugin.getDefault().loadPreferences().isDetermineFiletypesAutomatically();
 
         if (resource instanceof IFile) {
+            if (checkFileExtensions && fileExtensions != null) {
+                if (fileExtensions.contains(resource.getFileExtension().toLowerCase(Locale.ROOT))) {
+                    return 1;
+                } else {
+                    return 0;
+                }
+            }
             return 1;
         }
 
-        final CountVisitor visitor = new CountVisitor();
+        final CountVisitor2 visitor = new CountVisitor2(checkFileExtensions, resource.getProject(), fileExtensions);
 
         try {
             resource.accept(visitor);
         } catch (CoreException e) {
             LOG.error("Exception when counting elements of a project: {}", e.toString(), e);
         }
 
-        return visitor.count;
+        return visitor.getCount();
     }
 
     /**