Merge branch 'fixparallel'

Author: Amarin Phaosawasdi

plugins/checker.framework.change.propagator/src/checker/framework/change/propagator/ComputeQuickFixesJob.java

@@ -1,7 +1,6 @@
 package checker.framework.change.propagator;
 
 import org.checkerframework.eclipse.actions.CheckerWorker;
-import org.checkerframework.eclipse.util.MutexSchedulingRule;
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.Status;
@@ -13,8 +12,8 @@ public class ComputeQuickFixesJob extends Job {
 
     private String checkerID;
 
-    public ComputeQuickFixesJob(String name, ShadowOfShadowProject shadowProject,
-            String checkerID) {
+    public ComputeQuickFixesJob(String name,
+            ShadowOfShadowProject shadowProject, String checkerID) {
         super(name);
         this.shadowProject = shadowProject;
         this.checkerID = checkerID;
@@ -27,9 +26,7 @@ private String[] getShadowSourceFiles() {
     private CheckerWorker getCheckerWorker() {
         CheckerWorker checkerJob = new CheckerWorker(
                 shadowProject.getProject(), getShadowSourceFiles(), checkerID);
-        checkerJob.setUser(true);
         checkerJob.setPriority(Job.BUILD);
-        checkerJob.setRule(new MutexSchedulingRule());
         return checkerJob;
     }
 

plugins/checker.framework.change.propagator/src/checker/framework/change/propagator/ShadowOfShadowProject.java

@@ -22,7 +22,6 @@ public void runChecker(String checkerID) {
         ComputeQuickFixesJob computeQuickFixesJob = new ComputeQuickFixesJob(
                 "ComputeQuickFixes of " + getProject().getElementName(), this,
                 checkerID);
-        computeQuickFixesJob.setUser(true);
         computeQuickFixesJob.setPriority(Job.BUILD);
         computeQuickFixesJob.schedule();
         try {

plugins/checker.framework.errorcentric.view/META-INF/MANIFEST.MF

@@ -17,6 +17,8 @@ Require-Bundle: org.eclipse.ui,
  checker.framework.quickfixes,
  com.google.guava,
  org.eclipse.jdt.ui,
- org.eclipse.ui.workbench.texteditor
+ org.eclipse.ui.workbench.texteditor,
+ org.eclipse.ltk.core.refactoring
 Bundle-ActivationPolicy: lazy
-Import-Package: org.apache.commons.lang3.builder
+Import-Package: org.apache.commons.lang3.builder,
+ org.eclipse.jdt.core.refactoring

plugins/checker.framework.errorcentric.view/src/checker/framework/errorcentric/view/Messages.java

@@ -4,6 +4,7 @@
 
 public class Messages extends NLS {
     private static final String BUNDLE_NAME = "checker.framework.errorcentric.view.messages"; //$NON-NLS-1$
+    public static String ErrorCentricView_compute_change_effect_progress_bar_label;
     public static String ErrorCentricView_refresh_icon;
     public static String ErrorCentricView_refresh_text;
     public static String ErrorCentricView_refresh_tool_tip;

plugins/checker.framework.errorcentric.view/src/checker/framework/errorcentric/view/messages.properties

@@ -1,3 +1,4 @@
+ErrorCentricView_compute_change_effect_progress_bar_label=Computing the effect of: {0}
 ErrorCentricView_refresh_icon=icons/refresh.gif
 ErrorCentricView_refresh_text=Refresh
 ErrorCentricView_refresh_tool_tip=Refresh

plugins/checker.framework.errorcentric.view/src/checker/framework/errorcentric/view/views/ChangeComputationJob.java

@@ -1,11 +1,5 @@
 package checker.framework.errorcentric.view.views;
 
-import static com.google.common.collect.Iterables.filter;
-import static com.google.common.collect.Iterables.transform;
-import static com.google.common.collect.Sets.difference;
-import static com.google.common.collect.Sets.newHashSet;
-import static com.google.common.collect.Sets.union;
-
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -23,44 +17,99 @@
 
 import com.google.common.base.Predicate;
 
+import static com.google.common.collect.Sets.difference;
+import static com.google.common.collect.Sets.filter;
+import static com.google.common.collect.Sets.newHashSet;
+import static com.google.common.collect.Sets.union;
+
+import static com.google.common.collect.Iterables.transform;
+
 public class ChangeComputationJob extends Job {
 
     private MarkerResolutionTreeNode markerResolutionTreeNode;
 
+    // We are using our own synchronization here. See MarkerResolutionTreeNode
+    // for more detaila.
+    private static Object lock = new Object();
+
     public ChangeComputationJob(String name,
             MarkerResolutionTreeNode markerResolutionTreeNode) {
         super(name);
         this.markerResolutionTreeNode = markerResolutionTreeNode;
     }
 
-    // TODO(reprogrammer): I suggest that we break this method into several
-    // smaller ones.
     @Override
     protected IStatus run(IProgressMonitor monitor) {
-        monitor.beginTask(getName(), 25);
-        final List<FixerDescriptor> parentFixerDescriptors = markerResolutionTreeNode
-                .getParentFixerDescriptors();
-        ActionableMarkerResolution resolution = markerResolutionTreeNode
-                .getResolution();
-        resolution.getShadowProject().updateToPrimaryProjectWithChanges(
-                parentFixerDescriptors);
-        monitor.worked(3);
-        resolution.apply();
-        monitor.worked(3);
-        // WorkspaceUtils.saveAllEditors();
-        monitor.worked(3);
-        ShadowOfShadowProject shadowProject = resolution.getShadowProject();
-        shadowProject.runChecker(InferCommandHandler.checkerID);
-        monitor.worked(10);
-        Set<ComparableMarker> allMarkersAfterResolution = shadowProject
-                .getMarkers();
-        Set<ComparableMarker> addedMarkers = difference(
-                allMarkersAfterResolution,
-                resolution.getAllMarkersBeforeResolution());
+        synchronized (lock) {
+            monitor.beginTask(getName(), 23);
+            monitor.subTask("Updating shadow project: " + getThread());
+            final List<FixerDescriptor> parentFixerDescriptors = markerResolutionTreeNode
+                    .getParentFixerDescriptors();
+            ActionableMarkerResolution resolution = markerResolutionTreeNode
+                    .getResolution();
+            ShadowOfShadowProject shadowProject = runChecker(monitor,
+                    parentFixerDescriptors, resolution);
+            monitor.subTask("Getting new markers: " + getThread());
+            Set<ComparableMarker> allMarkersAfterResolution = shadowProject
+                    .getMarkers();
+            Set<ComparableMarker> addedMarkers = difference(
+                    allMarkersAfterResolution,
+                    resolution.getAllMarkersBeforeResolution());
+            monitor.worked(1);
+            HashSet<ActionableMarkerResolution> historicallyNewResolutions = computeResolutions(
+                    monitor, parentFixerDescriptors, resolution, shadowProject,
+                    allMarkersAfterResolution, addedMarkers);
+            addChildrenToTree(monitor, resolution, addedMarkers,
+                    historicallyNewResolutions);
+            monitor.done();
+            return Status.OK_STATUS;
+        }
+    }
+
+    private void addChildrenToTree(IProgressMonitor monitor,
+            ActionableMarkerResolution resolution,
+            Set<ComparableMarker> addedMarkers,
+            HashSet<ActionableMarkerResolution> historicallyNewResolutions) {
+        monitor.subTask("Adding children to tree: " + getThread());
+        Set<ComparableMarker> fixedMarkers = newHashSet();
+        for (ActionableMarkerResolution historicallyNewResolution : historicallyNewResolutions) {
+            fixedMarkers.addAll(historicallyNewResolution
+                    .getMarkersToBeResolvedByFixer());
+        }
+        Set<ComparableMarker> unresolvedMarkers = difference(addedMarkers,
+                fixedMarkers);
+        Set<ErrorTreeNode> errorTreeNodesWithoutResolutions = newHashSet(transform(
+                unresolvedMarkers, marker -> new AddedErrorTreeNode(marker)));
+        TreeUpdater treeUpdater = markerResolutionTreeNode.getTreeUpdater();
+        HashSet<ErrorTreeNode> errorTreeNodesWithResolutions = newHashSet(AddedErrorTreeNode
+                .createTreeNodesFrom(historicallyNewResolutions, treeUpdater));
+        markerResolutionTreeNode
+                .addChildren(union(errorTreeNodesWithResolutions,
+                        errorTreeNodesWithoutResolutions));
+        int errorsFixed = resolution.getMarkersToBeResolvedByFixer().size();
+        markerResolutionTreeNode.setFixedErrorsCount(errorsFixed);
+        markerResolutionTreeNode.addErrorCountToLabel();
         monitor.worked(1);
+        treeUpdater.recomputeDisabledNodes();
+        // Since recomputeDisabledNodes makes a call to viewer.refresh(),
+        // we do not need to call update() any more.
+        // treeUpdater.update(markerResolutionTreeNode);
+        monitor.worked(1);
+    }
+
+    private HashSet<ActionableMarkerResolution> computeResolutions(
+            IProgressMonitor monitor,
+            final List<FixerDescriptor> parentFixerDescriptors,
+            ActionableMarkerResolution resolution,
+            ShadowOfShadowProject shadowProject,
+            Set<ComparableMarker> allMarkersAfterResolution,
+            Set<ComparableMarker> addedMarkers) {
+        monitor.subTask("Getting new resolutions: " + getThread());
         Set<ActionableMarkerResolution> newResolutions = shadowProject
                 .getResolutions(allMarkersAfterResolution, addedMarkers);
+        yieldRule(monitor);
         monitor.worked(3);
+        monitor.subTask("Filtering resolutions: " + getThread());
         HashSet<ActionableMarkerResolution> historicallyNewResolutions = newHashSet(filter(
                 newResolutions, new Predicate<ActionableMarkerResolution>() {
                     @Override
@@ -75,34 +124,23 @@ public boolean apply(
                     }
                 }));
         monitor.worked(1);
-        Set<ComparableMarker> fixedMarkers = newHashSet();
-        for (ActionableMarkerResolution historicallyNewResolution : historicallyNewResolutions) {
-            fixedMarkers.addAll(historicallyNewResolution
-                    .getMarkersToBeResolvedByFixer());
-        }
-        Set<ComparableMarker> unresolvedMarkers = difference(addedMarkers,
-                fixedMarkers);
-        Set<ErrorTreeNode> errorTreeNodesWithoutResolutions = newHashSet(transform(
-                unresolvedMarkers, marker -> new AddedErrorTreeNode(marker)));
-        HashSet<ErrorTreeNode> errorTreeNodesWithResolutions = newHashSet(AddedErrorTreeNode
-                .createTreeNodesFrom(historicallyNewResolutions,
-                        markerResolutionTreeNode.getTreeUpdater()));
-        markerResolutionTreeNode
-                .addChildren(union(errorTreeNodesWithResolutions,
-                        errorTreeNodesWithoutResolutions));
-        int errorsFixed = resolution.getMarkersToBeResolvedByFixer().size();
-        markerResolutionTreeNode.setFixedErrorsCount(errorsFixed);
-        markerResolutionTreeNode.setName(markerResolutionTreeNode.getName()
-                + " (" + errorsFixed + ")");
-        // TODO(reprogrammer): I suggest to redesign the classes such that the
-        // following statement doesn't duplicate the reference to
-        // markerResolutionTreeNode.
-        markerResolutionTreeNode.getTreeUpdater().update(
-                markerResolutionTreeNode);
-        monitor.worked(1);
-        monitor.done();
-        JobManager.done(markerResolutionTreeNode);
-        return Status.OK_STATUS;
+        return historicallyNewResolutions;
+    }
+
+    private ShadowOfShadowProject runChecker(IProgressMonitor monitor,
+            final List<FixerDescriptor> parentFixerDescriptors,
+            ActionableMarkerResolution resolution) {
+        resolution.getShadowProject().updateToPrimaryProjectWithChanges(
+                parentFixerDescriptors);
+        monitor.worked(3);
+        monitor.subTask("Applying resolution: " + getThread());
+        resolution.apply();
+        monitor.worked(3);
+        monitor.subTask("Running checker: " + getThread());
+        ShadowOfShadowProject shadowProject = resolution.getShadowProject();
+        shadowProject.runChecker(InferCommandHandler.checkerID);
+        monitor.worked(10);
+        return shadowProject;
     }
 
 }

plugins/checker.framework.errorcentric.view/src/checker/framework/errorcentric/view/views/ChangeStateViewer.java

@@ -1,31 +1,50 @@
 package checker.framework.errorcentric.view.views;
 
-import static com.google.common.collect.Sets.difference;
-import static com.google.common.collect.Sets.newHashSet;
-
 import java.util.Collection;
 import java.util.HashSet;
+import java.util.Map;
 import java.util.Set;
 
 import org.eclipse.jface.viewers.TreeViewer;
-import org.eclipse.swt.graphics.Color;
-import org.eclipse.swt.widgets.TreeItem;
+import org.eclipse.swt.widgets.Display;
+
+import static com.google.common.collect.Sets.difference;
+import static com.google.common.collect.Sets.newHashSet;
+
+import static com.google.common.collect.Maps.newHashMap;
 
 public class ChangeStateViewer {
     private Set<TreeObject> disabledNodes;
     private TreeViewer viewer;
+    private Map<MarkerResolutionTreeNode, Set<TreeObject>> disabledNodesMap;
 
     public ChangeStateViewer(TreeViewer viewer) {
         this.disabledNodes = newHashSet();
         this.viewer = viewer;
+        this.disabledNodesMap = newHashMap();
     }
 
     public void resetState() {
         disabledNodes.clear();
     }
 
-    public Set<TreeObject> disableChange(
-            MarkerResolutionTreeNode resolutionTreeNode) {
+    public void recomputeDisabledChanges() {
+        Set<TreeObject> treeObjects = newHashSet(disabledNodes);
+        for (TreeObject treeObject : treeObjects) {
+            if (treeObject instanceof MarkerResolutionTreeNode) {
+                disableChange((MarkerResolutionTreeNode) treeObject);
+            }
+        }
+    }
+
+    /**
+     * Disables a change node and (1) all the errors it fixes (2) all the other
+     * "equivalent" change node in the tree, i.e. is the same change and fixes
+     * the same set of errors (3) all the errors that (2) fixes
+     * 
+     * @param resolutionTreeNode
+     */
+    public void disableChange(MarkerResolutionTreeNode resolutionTreeNode) {
         // init
         Set<TreeObject> clonedDisabledNodes = newHashSet(disabledNodes);
         clonedDisabledNodes.add(resolutionTreeNode);
@@ -39,8 +58,16 @@ public Set<TreeObject> disableChange(
                 clonedDisabledNodes, disabledNodes));
         // swap the clone
         disabledNodes = clonedDisabledNodes;
-        viewer.refresh();
-        return newDisabledNodes;
+        Display.getDefault().syncExec(new Runnable() {
+            public void run() {
+                viewer.refresh();
+            }
+        });
+        if (disabledNodesMap.containsKey(resolutionTreeNode)) {
+            disabledNodesMap.get(resolutionTreeNode).addAll(newDisabledNodes);
+        } else {
+            disabledNodesMap.put(resolutionTreeNode, newDisabledNodes);
+        }
     }
 
     private boolean canAddRelatedNodes(Set<TreeObject> nodes) {
@@ -65,16 +92,21 @@ private boolean canAddRelatedNodes(Set<TreeObject> nodes) {
         }
     }
 
-    public void enableChange(Set<TreeObject> nodesToBeEnabled) {
+    private void enableChange(Set<TreeObject> nodesToBeEnabled) {
         disabledNodes.removeAll(nodesToBeEnabled);
         viewer.refresh();
     }
 
+    public void enableChange(MarkerResolutionTreeNode markerResolutionTreeNode) {
+        Set<TreeObject> disabledNodesToBeEnabled = disabledNodesMap
+                .get(markerResolutionTreeNode);
+        enableChange(disabledNodesToBeEnabled);
+    }
+
     private Set<TreeObject> getRelatedNodes(Set<TreeObject> nodes) {
         Set<TreeObject> relatedNodes = new HashSet<>();
-        relatedNodes.addAll(getRelatedNodes(
-                ((MarkerResolutionTreeNode) viewer.getInput()).getChildren(),
-                nodes));
+        relatedNodes.addAll(getRelatedNodes(((MarkerResolutionTreeNode) viewer
+                .getInput()).getExistingChildren(), nodes));
         return relatedNodes;
     }
 
@@ -90,28 +122,11 @@ private Set<TreeObject> getRelatedNodes(TreeObject[] candidates,
                     relatedNodes.addAll(errorsFixed);
                 }
             }
-            relatedNodes.addAll(getRelatedNodes(candidate.getChildren(),
-                    existingNodes));
-        }
-        return relatedNodes;
-    }
 
-    private void setNodeColor(Set<TreeObject> nodes, Color color) {
-        setNodeColor(viewer.getTree().getItems(), nodes, color);
-    }
-
-    private void setNodeColor(TreeItem[] items, Set<TreeObject> nodes,
-            Color color) {
-        for (TreeItem item : items) {
-            if (nodes.contains(item.getData())) {
-                setNodeColor(item, color);
-            }
-            setNodeColor(item.getItems(), nodes, color);
+            relatedNodes.addAll(getRelatedNodes(
+                    candidate.getExistingChildren(), existingNodes));
         }
-    }
-
-    private void setNodeColor(TreeItem item, Color color) {
-        item.setForeground(color);
+        return relatedNodes;
     }
 
     public boolean isDisabled(TreeObject treeObject) {