Refactor actions to use correct selection

Previously selectionChanged was ignored and the selected elements
where determined manually by getting the selection from the ui part
the action is used for. However, this makes it impossible to
programmatically call this action and provide a specific selection.

This drastically simplifies the actions, since they mostly care about
the selection and the selection is provided by eclipse automatically.

Also fixes PMD issues and invalid charset handling while removing
review comments.

Author: adangel

net.sourceforge.pmd.eclipse.plugin/src/main/java/net/sourceforge/pmd/eclipse/ui/actions/AbstractUIAction.java

@@ -21,6 +21,7 @@
  */
 public abstract class AbstractUIAction implements IObjectActionDelegate {
 
+    private ISelection selection;
     private IWorkbenchPart targetPart;
 
     protected AbstractUIAction() {
@@ -59,7 +60,12 @@ protected IWorkbenchPartSite targetPartSite() {
     }
 
     protected ISelection targetSelection() {
-        return targetPartSite().getSelectionProvider().getSelection();
+        return selection;
+    }
+
+    @Override
+    public void selectionChanged(IAction action, ISelection selection) {
+        this.selection = selection;
     }
 
     /**

net.sourceforge.pmd.eclipse.plugin/src/main/java/net/sourceforge/pmd/eclipse/ui/actions/CPDCheckProjectAction.java

@@ -47,12 +47,10 @@ public class CPDCheckProjectAction extends AbstractUIAction {
     private static final String SIMPLE_KEY = "Simple Text";
     private static final String CSV_KEY = "CSV";
 
-    /*
-     * @see org.eclipse.ui.IActionDelegate#run(IAction)
-     */
+    @Override
     public void run(final IAction action) { // NOPMD:UnusedFormalParameter
         final IWorkbenchPartSite site = targetPartSite();
-        final ISelection sel = site.getSelectionProvider().getSelection();
+        final ISelection sel = targetSelection();
         final Shell shell = site.getShell();
         final String[] languages = LanguageFactory.supportedLanguages;
 
@@ -64,7 +62,7 @@ public void run(final IAction action) { // NOPMD:UnusedFormalParameter
 
         final CPDCheckDialog dialog = new CPDCheckDialog(shell, languages, formats);
 
-        if (dialog.open() == Dialog.OK && sel instanceof IStructuredSelection) {       
+        if (dialog.open() == Dialog.OK && sel instanceof IStructuredSelection) {
             final StructuredSelection ss = (StructuredSelection) sel;
             final Iterator<?> i = ss.iterator();
             while (i.hasNext()) {
@@ -83,17 +81,10 @@ public void run(final IAction action) { // NOPMD:UnusedFormalParameter
                     LOG.warn("The selected object is not adaptable");
                     LOG.debug("   -> selected object : " + obj);
                 }
-            }           
+            }
         }
     }
 
-
-    /*
-     * @see org.eclipse.ui.IActionDelegate#selectionChanged(IAction, ISelection)
-     */
-    public void selectionChanged(final IAction action, final ISelection selection) { // NOPMD:UnusedFormalParameter
-    }
-
     /**
      * Run the DetectCutAndPaste command against the selected project 
      * and creates optionally the report file.
@@ -173,5 +164,4 @@ private String createFileName(String rendererKey) {
         }
         return fileName;
     }
-    
 }

net.sourceforge.pmd.eclipse.plugin/src/main/java/net/sourceforge/pmd/eclipse/ui/actions/ClearReviewsAction.java

@@ -6,12 +6,13 @@
 
 import java.io.BufferedReader;
 import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.io.PrintWriter;
+import java.io.StringWriter;
 import java.lang.reflect.InvocationTargetException;
 import java.util.Iterator;
+import java.util.Locale;
 
 import org.eclipse.core.resources.IFile;
 import org.eclipse.core.resources.IFolder;
@@ -28,9 +29,6 @@
 import org.eclipse.jface.viewers.ISelection;
 import org.eclipse.jface.viewers.IStructuredSelection;
 import org.eclipse.swt.widgets.Display;
-import org.eclipse.ui.IEditorInput;
-import org.eclipse.ui.IEditorPart;
-import org.eclipse.ui.IFileEditorInput;
 import org.eclipse.ui.IViewActionDelegate;
 import org.eclipse.ui.IViewPart;
 import org.slf4j.Logger;
@@ -39,7 +37,6 @@
 import net.sourceforge.pmd.eclipse.runtime.PMDRuntimeConstants;
 import net.sourceforge.pmd.eclipse.runtime.cmd.AbstractDefaultCommand;
 import net.sourceforge.pmd.eclipse.ui.nls.StringKeys;
-import net.sourceforge.pmd.eclipse.util.IOUtil;
 
 /**
  * Implements the clear reviews action
@@ -52,21 +49,18 @@ public class ClearReviewsAction extends AbstractUIAction implements IResourceVis
     private static final Logger LOG = LoggerFactory.getLogger(ClearReviewsAction.class);
     private IProgressMonitor monitor;
 
-    /**
-     * @see org.eclipse.ui.IViewActionDelegate#init(org.eclipse.ui.IViewPart)
-     */
+    @Override
     public void init(IViewPart view) {
         setActivePart(null, view.getSite().getPage().getActivePart());
     }
 
-    /**
-     * @see org.eclipse.ui.IActionDelegate#run(org.eclipse.jface.action.IAction)
-     */
+    @Override
     public void run(IAction action) {
         LOG.info("Remove violation reviews requested.");
         ProgressMonitorDialog monitorDialog = new ProgressMonitorDialog(Display.getCurrent().getActiveShell());
         try {
             monitorDialog.run(false, false, new IRunnableWithProgress() {
+                @Override
                 public void run(IProgressMonitor monitor) throws InvocationTargetException, InterruptedException {
                     setMonitor(monitor);
                     clearReviews();
@@ -80,13 +74,6 @@ public void run(IProgressMonitor monitor) throws InvocationTargetException, Inte
         }
     }
 
-    /**
-     * @see org.eclipse.ui.IActionDelegate#selectionChanged(org.eclipse.jface.action.IAction,
-     *      org.eclipse.jface.viewers.ISelection)
-     */
-    public void selectionChanged(IAction action, ISelection selection) {
-    }
-
     /**
      * Get the monitor
      *
@@ -131,56 +118,36 @@ protected void monitorSubTask(String message) {
     protected void clearReviews() {
 
         try {
-            // If action is started from a view, the process all selected
-            // resource
-            if (isViewPart()) {
-                ISelection selection = targetSelection();
-
-                if (selection != null && selection instanceof IStructuredSelection) {
-                    IStructuredSelection structuredSelection = (IStructuredSelection) selection;
-                    if (getMonitor() != null) {
-                        getMonitor().beginTask(getString(StringKeys.MONITOR_REMOVE_REVIEWS), IProgressMonitor.UNKNOWN);
-
-                        Iterator<?> i = structuredSelection.iterator();
-                        while (i.hasNext()) {
-                            Object object = i.next();
-                            IResource resource = null;
-
-                            if (object instanceof IMarker) {
-                                resource = ((IMarker) object).getResource();
-                            } else if (object instanceof IAdaptable) {
-                                IAdaptable adaptable = (IAdaptable) object;
-                                resource = (IResource) adaptable.getAdapter(IResource.class);
-                            } else {
-                                LOG.warn("The selected object is not adaptable");
-                                LOG.debug("   -> selected object = " + object);
-                            }
-
-                            if (resource != null) {
-                                resource.accept(this);
-                            } else {
-                                LOG.warn("The selected object cannot adapt to a resource.");
-                                LOG.debug("   -> selected object" + object);
-                            }
+            ISelection selection = targetSelection();
+
+            if (selection instanceof IStructuredSelection) {
+                IStructuredSelection structuredSelection = (IStructuredSelection) selection;
+                if (getMonitor() != null) {
+                    getMonitor().beginTask(getString(StringKeys.MONITOR_REMOVE_REVIEWS), IProgressMonitor.UNKNOWN);
+
+                    Iterator<?> i = structuredSelection.iterator();
+                    while (i.hasNext()) {
+                        Object object = i.next();
+                        IResource resource = null;
+
+                        if (object instanceof IMarker) {
+                            resource = ((IMarker) object).getResource();
+                        } else if (object instanceof IAdaptable) {
+                            IAdaptable adaptable = (IAdaptable) object;
+                            resource = (IResource) adaptable.getAdapter(IResource.class);
+                        } else {
+                            LOG.warn("The selected object is not adaptable");
+                            LOG.debug("   -> selected object = " + object);
                         }
-                    }
-                }
-            }
 
-            // If action is started from an editor, process the file currently
-            // edited
-            if (isEditorPart()) {
-                IEditorInput editorInput = ((IEditorPart) this.targetPart()).getEditorInput();
-                if (editorInput instanceof IFileEditorInput) {
-                    ((IFileEditorInput) editorInput).getFile().accept(this);
-                } else {
-                    LOG.debug("The kind of editor input is not supported. The editor input if of type: "
-                            + editorInput.getClass().getName());
+                        if (resource != null) {
+                            resource.accept(this);
+                        } else {
+                            LOG.warn("The selected object cannot adapt to a resource.");
+                            LOG.debug("   -> selected object" + object);
+                        }
+                    }
                 }
-            } else {
-                // else this is not supported
-                LOG.debug("This action is not supported on this kind of part. This part type is: "
-                        + targetPartClassName());
             }
         } catch (CoreException e) {
             logError("Core Exception when clearing violations reviews", e);
@@ -208,7 +175,7 @@ private static boolean isReviewable(IFile file) {
         if (AbstractDefaultCommand.isJavaFile(file)) {
             return true;
         }
-        return file.getName().toLowerCase().endsWith(".jsp");
+        return file.getName().toLowerCase(Locale.ROOT).endsWith(".jsp");
     }
 
     /**
@@ -223,13 +190,11 @@ private String removeReviews(IFile file) {
             return null;
         }
 
-        ByteArrayOutputStream baos = new ByteArrayOutputStream();
-        PrintWriter out = null;
+        StringWriter modified = new StringWriter();
         boolean noChange = true;
-        try {
+        try (BufferedReader reader = new BufferedReader(new InputStreamReader(file.getContents(), file.getCharset()));
+                PrintWriter out = new PrintWriter(modified)) {
             boolean comment = false;
-            BufferedReader reader = new BufferedReader(new InputStreamReader(file.getContents()));
-            out = new PrintWriter(baos);
 
             while (reader.ready()) {
                 String origLine = reader.readLine();
@@ -265,12 +230,9 @@ private String removeReviews(IFile file) {
             logError(StringKeys.ERROR_CORE_EXCEPTION, e);
         } catch (IOException e) {
             logError(StringKeys.ERROR_IO_EXCEPTION, e);
-        } finally {
-            IOUtil.closeQuietly(baos);
-            IOUtil.closeQuietly(out);
         }
 
-        return noChange ? null : baos.toString();
+        return noChange ? null : modified.toString();
     }
 
     /**
@@ -281,15 +243,15 @@ private String removeReviews(IFile file) {
      */
     private void saveNewContent(IFile file, String newContent) {
         try {
-            file.setContents(new ByteArrayInputStream(newContent.getBytes()), false, true, getMonitor());
+            file.setContents(new ByteArrayInputStream(newContent.getBytes(file.getCharset())), false, true, getMonitor());
         } catch (CoreException e) {
             logError(StringKeys.ERROR_CORE_EXCEPTION, e);
+        } catch (IOException e) {
+            logError(StringKeys.ERROR_IO_EXCEPTION, e);
         }
     }
 
-    /**
-     * @see org.eclipse.core.resources.IResourceVisitor#visit(org.eclipse.core.resources.IResource)
-     */
+    @Override
     public boolean visit(IResource resource) throws CoreException {
         if (resource instanceof IFile) {
             clearReviews((IFile) resource);

net.sourceforge.pmd.eclipse.plugin/src/main/java/net/sourceforge/pmd/eclipse/ui/actions/GenerateReportAction.java

@@ -6,6 +6,7 @@
 
 import java.util.List;
 
+import org.apache.commons.lang3.StringUtils;
 import org.eclipse.core.resources.IProject;
 import org.eclipse.core.resources.IResource;
 import org.eclipse.core.runtime.IAdaptable;
@@ -20,7 +21,6 @@
 import net.sourceforge.pmd.eclipse.ui.nls.StringKeys;
 import net.sourceforge.pmd.eclipse.ui.reports.ReportManager;
 import net.sourceforge.pmd.renderers.Renderer;
-import net.sourceforge.pmd.util.StringUtil;
 
 /**
  * Process GenerateReport action menu. Generate a HTML report on the current
@@ -57,7 +57,7 @@ private boolean checkRenderers() {
 
         for (Renderer renderer : renderers) {
             String issue = renderer.dysfunctionReason();
-            if (StringUtil.isNotEmpty(issue)) {
+            if (StringUtils.isNotBlank(issue)) {
                 errors.append(renderer.getName()).append(": ");
                 errors.append(issue).append("\n");
             }
@@ -71,9 +71,7 @@ private boolean checkRenderers() {
         return false;
     }
 
-    /**
-     * @see org.eclipse.ui.IActionDelegate#run(IAction)
-     */
+    @Override
     public final void run(final IAction action) {
         LOG.info("Generation Report action requested");
         final ISelection sel = targetSelection();
@@ -98,23 +96,15 @@ public final void run(final IAction action) {
         }
     }
 
-    /**
-     * @see org.eclipse.ui.IActionDelegate#selectionChanged(IAction, ISelection)
-     */
-    public final void selectionChanged(IAction action, ISelection selection) {
-        // nothing to do
-    }
-
     /**
      * Get a project from a selection
      * 
      * @param selection
      * @return
      */
     private static IProject getProject(IStructuredSelection selection) {
-
         Object object = selection.getFirstElement();
-        if (object != null && object instanceof IAdaptable) {
+        if (object instanceof IAdaptable) {
             final IResource resource = (IResource) ((IAdaptable) object).getAdapter(IResource.class);
             if (resource != null) {
                 return resource.getProject();