Fail gracefully when .eclipse-pmd contains an invalid configuration

This is the first part of resolving issue #20.

ch.acanda.eclipse.pmd.core.tests/src/ch/acanda/eclipse/pmd/builder/LocationResolverTest.java

@@ -12,6 +12,8 @@
 package ch.acanda.eclipse.pmd.builder;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -27,6 +29,8 @@
 import ch.acanda.eclipse.pmd.domain.Location;
 import ch.acanda.eclipse.pmd.domain.LocationContext;
 
+import com.google.common.base.Optional;
+
 /**
  * Unit tests for {@code LocationResolver}.
  * 
@@ -43,11 +47,27 @@ public void resolveFileSystemLocation() {
         final Location location = new Location("/tmp/pmd.xml", LocationContext.FILESYSTEM);
         final IProject project = mock(IProject.class);
 
-        final String result = LocationResolver.resolve(location, project);
+        final Optional<String> result = LocationResolver.resolve(location, project);
 
-        assertEquals("The resolved location in a file system context should be the provided location", "/tmp/pmd.xml", result);
+        assertTrue("A valid file system location should resolve", result.isPresent());
+        assertEquals("The resolved location in a file system context should be the provided location",
+                Paths.get("/tmp", "pmd.xml").toString(), result.get());
     }
 
+    /**
+     * Verifies that {@link LocationResolver#resolve(Location, IProject)} does not throw an exception in a file system
+     * context if the path is invalid.
+     */
+    @Test
+    public void resolveFileSystemLocationWithInvalidPath() {
+        final Location location = new Location("/tmp/?", LocationContext.FILESYSTEM);
+        final IProject project = mock(IProject.class);
+
+        final Optional<String> result = LocationResolver.resolve(location, project);
+
+        assertFalse("The location should not resolve", result.isPresent());
+    }
+
     /**
      * Verifies that {@link LocationResolver#resolve(Location, IProject)} resolves the loacation in a remote context
      * correctly.
@@ -57,11 +77,27 @@ public void resolveRemoteLocation() {
         final Location location = new Location("http://example.org/pmd.xml", LocationContext.REMOTE);
         final IProject project = mock(IProject.class);
 
-        final String result = LocationResolver.resolve(location, project);
+        final Optional<String> result = LocationResolver.resolve(location, project);
 
-        assertEquals("The resolved location in a remote context should be the provided location", "http://example.org/pmd.xml", result);
+        assertTrue("A valid remote location should resolve", result.isPresent());
+        assertEquals("The resolved location in a remote context should be the provided location", "http://example.org/pmd.xml",
+                result.get());
     }
 
+    /**
+     * Verifies that {@link LocationResolver#resolve(Location, IProject)} does not throw an exception in a remote
+     * context if the URI is invalid.
+     */
+    @Test
+    public void resolveRemoteLocationWithInvalidURI() {
+        final Location location = new Location("http:#", LocationContext.REMOTE);
+        final IProject project = mock(IProject.class);
+
+        final Optional<String> result = LocationResolver.resolve(location, project);
+
+        assertFalse("The location should not resolve", result.isPresent());
+    }
+
     /**
      * Verifies that {@link LocationResolver#resolve(Location, IProject)} resolves the location in a project context
      * correctly.
@@ -72,10 +108,26 @@ public void resolveProjectLocation() throws URISyntaxException {
         final IProject project = mock(IProject.class);
         when(project.getLocationURI()).thenReturn(new URI("file:///workspace/project/"));
 
-        final String result = LocationResolver.resolve(location, project);
+        final Optional<String> result = LocationResolver.resolve(location, project);
 
+        assertTrue("A valid project location should resolve", result.isPresent());
         assertEquals("The resolved location in a project context should be the provided location appended to the project location",
-                     Paths.get("/workspace", "project", "pmd.xml").toString(), result);
+                Paths.get("/workspace", "project", "pmd.xml").toString(), result.get());
+    }
+
+    /**
+     * Verifies that {@link LocationResolver#resolve(Location, IProject)} does not throw an exception in a project
+     * context if the path is invalid.
+     */
+    @Test
+    public void resolveProjectLocationWithInvalidPath() throws URISyntaxException {
+        final Location location = new Location("pmd.xml?", LocationContext.PROJECT);
+        final IProject project = mock(IProject.class);
+        when(project.getLocationURI()).thenReturn(new URI("file:///workspace/project/"));
+
+        final Optional<String> result = LocationResolver.resolve(location, project);
+
+        assertFalse("The location should not resolve", result.isPresent());
     }
 
     /**
@@ -93,10 +145,51 @@ public void resolveWorkspaceLocation() throws URISyntaxException {
         when(workspaceRoot.getProject("project")).thenReturn(project);
         when(project.getLocationURI()).thenReturn(new URI("file:///workspace/project/"));
 
-        final String result = LocationResolver.resolve(location, project);
+        final Optional<String> result = LocationResolver.resolve(location, project);
 
+        assertTrue("A valid workspace location should resolve", result.isPresent());
         assertEquals("The resolved location in a workspace context should be the provided location appended to the workspace location",
-                     Paths.get("/workspace", "project", "pmd.xml").toString(), result);
+                Paths.get("/workspace", "project", "pmd.xml").toString(), result.get());
+    }
+
+    /**
+     * Verifies that {@link LocationResolver#resolve(Location, IProject)} does not throw an exception in a workspace
+     * context if the project does not exist.
+     */
+    @Test
+    public void resolveWorkspaceLocationWithMissingProject() {
+        final Location location = new Location("MissingProject/pmd.xml", LocationContext.WORKSPACE);
+        final IProject project = mock(IProject.class);
+        final IWorkspace workspace = mock(IWorkspace.class);
+        final IWorkspaceRoot workspaceRoot = mock(IWorkspaceRoot.class);
+        when(project.getWorkspace()).thenReturn(workspace);
+        when(workspace.getRoot()).thenReturn(workspaceRoot);
+        when(workspaceRoot.getProject("MissingProject")).thenReturn(project);
+        when(project.getLocationURI()).thenReturn(null);
+
+        final Optional<String> result = LocationResolver.resolve(location, project);
+
+        assertFalse("The location should not resolve", result.isPresent());
+    }
+
+    /**
+     * Verifies that {@link LocationResolver#resolve(Location, IProject)} does not throw an exception in a workspace
+     * context if the path contains invalid characters.
+     */
+    @Test
+    public void resolveWorkspaceLocationWithInvalidPath() throws URISyntaxException {
+        final Location location = new Location("project/pmd.xml?", LocationContext.WORKSPACE);
+        final IProject project = mock(IProject.class);
+        final IWorkspace workspace = mock(IWorkspace.class);
+        final IWorkspaceRoot workspaceRoot = mock(IWorkspaceRoot.class);
+        when(project.getWorkspace()).thenReturn(workspace);
+        when(workspace.getRoot()).thenReturn(workspaceRoot);
+        when(workspaceRoot.getProject("project")).thenReturn(project);
+        when(project.getLocationURI()).thenReturn(new URI("file:///workspace/project/"));
+
+        final Optional<String> result = LocationResolver.resolve(location, project);
+
+        assertFalse("The location should not resolve", result.isPresent());
     }
 
 }

ch.acanda.eclipse.pmd.core.tests/src/ch/acanda/eclipse/pmd/wizard/AddRuleSetConfigurationModelTest.java

@@ -42,7 +42,7 @@ public class AddRuleSetConfigurationModelTest {
 
     @BeforeClass
     public static void createRuleSetFile() throws IOException {
-        ruleSetFile = Files.createTempFile("AddRuleSetConfigurationModelTest-", "xml");
+        ruleSetFile = Files.createTempFile("AddRuleSetConfigurationModelTest-", ".xml");
         try (final InputStream in = AddRuleSetConfigurationModelTest.class.getResourceAsStream("AddRuleSetConfigurationModelTest.xml")) {
             Files.copy(in, ruleSetFile, StandardCopyOption.REPLACE_EXISTING);
         }
@@ -78,7 +78,7 @@ public void validateWorkspaceConfigurationWithProjectOutsideWorkspace() throws I
         if (validationResult.hasErrors()) {
             final String msg = "The validation should not result in any errors "
                     + "if the project is located outside the workspace. First error: ";
-            fail(msg + " First error: " + validationResult.getFirstErrorMessage());
+            fail(msg + validationResult.getFirstErrorMessage());
         }
     }
 

ch.acanda.eclipse.pmd.core/src/ch/acanda/eclipse/pmd/builder/LocationResolver.java

@@ -12,13 +12,19 @@
 package ch.acanda.eclipse.pmd.builder;
 
 import java.io.File;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.nio.file.InvalidPathException;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 
 import org.eclipse.core.resources.IProject;
+import org.eclipse.core.resources.IWorkspaceRoot;
 
 import ch.acanda.eclipse.pmd.domain.Location;
 
+import com.google.common.base.Optional;
+
 /**
  * @author Philip Graf
  */
@@ -28,26 +34,23 @@ private LocationResolver() {
         // hide constructor of utility class
     }
 
-    public static String resolve(final Location location, final IProject project) {
-        final String path;
+    public static Optional<String> resolve(final Location location, final IProject project) {
+        final Optional<String> path;
         switch (location.getContext()) {
             case WORKSPACE:
-                // format of the location's path: <project-name>/<project-relative-path>
-                final Path locationPath = Paths.get(toOSPath(location.getPath()));
-                final IProject locationProject = project.getWorkspace().getRoot().getProject(locationPath.getName(0).toString());
-                path = Paths.get(locationProject.getLocationURI())
-                            .resolve(locationPath.subpath(1, locationPath.getNameCount()))
-                            .toString();
-
+                path = resolveWorkspaceLocation(location, project);
                 break;
 
             case PROJECT:
-                path = Paths.get(project.getLocationURI()).resolve(toOSPath(location.getPath())).toString();
+                path = resolveProjectLocation(location, project);
                 break;
 
             case FILESYSTEM:
+                path = resolveFileSystemLocation(location);
+                break;
+
             case REMOTE:
-                path = location.getPath();
+                path = resolveRemoteLocation(location);
                 break;
 
             default:
@@ -56,6 +59,47 @@ public static String resolve(final Location location, final IProject project) {
         return path;
     }
 
+    private static Optional<String> resolveWorkspaceLocation(final Location location, final IProject project) {
+        try {
+            // format of the location's path: <project-name>/<project-relative-path>
+            final Path locationPath = Paths.get(toOSPath(location.getPath()));
+            final String projectName = locationPath.getName(0).toString();
+            final Path projectRelativePath = locationPath.subpath(1, locationPath.getNameCount());
+            final IWorkspaceRoot root = project.getWorkspace().getRoot();
+            final URI locationURI = root.getProject(projectName).getLocationURI();
+            if (locationURI != null) {
+                return Optional.of(Paths.get(locationURI).resolve(projectRelativePath).toString());
+            }
+            return Optional.absent();
+        } catch (final InvalidPathException e) {
+            return Optional.absent();
+        }
+    }
+
+    private static Optional<String> resolveProjectLocation(final Location location, final IProject project) {
+        try {
+            return Optional.of(Paths.get(project.getLocationURI()).resolve(toOSPath(location.getPath())).toString());
+        } catch (final InvalidPathException e) {
+            return Optional.absent();
+        }
+    }
+
+    private static Optional<String> resolveFileSystemLocation(final Location location) {
+        try {
+            return Optional.of(Paths.get(location.getPath()).toString());
+        } catch (final InvalidPathException e) {
+            return Optional.absent();
+        }
+    }
+
+    private static Optional<String> resolveRemoteLocation(final Location location) {
+        try {
+            return Optional.of(new URI(location.getPath()).toString());
+        } catch (final URISyntaxException e) {
+            return Optional.absent();
+        }
+    }
+
     private static String toOSPath(final String path) {
         return path.replace('\\', File.separatorChar).replace('/', File.separatorChar);
     }

ch.acanda.eclipse.pmd.core/src/ch/acanda/eclipse/pmd/cache/RuleSetsCache.java

@@ -80,15 +80,20 @@ private void startWatchingRuleSetFiles(final ProjectModel projectModel) {
         if (fileWatcher.isPresent() && projectModel.isPMDEnabled()) {
             final FileChangedListener listener = new RuleSetFileListener(projectModel);
             final IProject project = ResourcesPlugin.getWorkspace().getRoot().getProject(projectModel.getProjectName());
+
             for (final RuleSetModel ruleSetModel : projectModel.getRuleSets()) {
                 if (ruleSetModel.getLocation().getContext() != LocationContext.REMOTE) {
-                    final Path file = Paths.get(LocationResolver.resolve(ruleSetModel.getLocation(), project));
-                    try {
-                        final Subscription subscription = fileWatcher.get().subscribe(file, listener);
-                        subscriptions.put(projectModel.getProjectName(), subscription);
-                    } catch (final IOException e) {
-                        final String msg = "Cannot watch rule set file %s. Changes to this file will not be picked up for up to an hour.";
-                        PMDPlugin.getDefault().warn(String.format(msg, file.toAbsolutePath()), e);
+                    final Optional<String> resolvedLocation = LocationResolver.resolve(ruleSetModel.getLocation(), project);
+                    if (resolvedLocation.isPresent()) {
+                        final Path file = Paths.get(resolvedLocation.get());
+                        try {
+                            final Subscription subscription = fileWatcher.get().subscribe(file, listener);
+                            subscriptions.put(projectModel.getProjectName(), subscription);
+                        } catch (final IOException e) {
+                            final String msg = "Cannot watch rule set file %s. "
+                                    + "Changes to this file will not be picked up for up to an hour.";
+                            PMDPlugin.getDefault().warn(String.format(msg, file.toAbsolutePath()), e);
+                        }
                     }
                 }
             }

ch.acanda.eclipse.pmd.core/src/ch/acanda/eclipse/pmd/cache/RuleSetsCacheLoader.java

@@ -11,6 +11,7 @@
 
 package ch.acanda.eclipse.pmd.cache;
 
+import static com.google.common.base.Optional.presentInstances;
 import static com.google.common.collect.Iterables.transform;
 import net.sourceforge.pmd.RuleSetFactory;
 import net.sourceforge.pmd.RuleSetNotFoundException;
@@ -27,8 +28,10 @@
 import ch.acanda.eclipse.pmd.repository.ProjectModelRepository;
 
 import com.google.common.base.Function;
+import com.google.common.base.Optional;
 import com.google.common.cache.CacheLoader;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSortedSet;
 
 /**
  * @author Philip Graf
@@ -42,15 +45,16 @@ public RuleSets load(final String projectName) {
         PMDPlugin.getDefault().info("RuleSetsCache: loading rule sets for project " + projectName);
         try {
             final ProjectModel projectModel = repository.load(projectName).or(new ProjectModel(projectName));
-            final Iterable<RuleSetReferenceId> ids = transform(projectModel.getRuleSets(), new ToReferenceId(projectName));
+            final ImmutableSortedSet<RuleSetModel> ruleSetModels = projectModel.getRuleSets();
+            final Iterable<RuleSetReferenceId> ids = presentInstances(transform(ruleSetModels, new ToReferenceId(projectName)));
             return new RuleSetFactory().createRuleSets(ImmutableList.copyOf(ids));
         } catch (final RuleSetNotFoundException e) {
             PMDPlugin.getDefault().error("Cannot load rule sets for project " + projectName, e);
             return new RuleSets();
         }
     }
 
-    private static final class ToReferenceId implements Function<RuleSetModel, RuleSetReferenceId> {
+    private static final class ToReferenceId implements Function<RuleSetModel, Optional<RuleSetReferenceId>> {
 
         private final IProject project;
 
@@ -59,8 +63,14 @@ public ToReferenceId(final String projectName) {
         }
 
         @Override
-        public RuleSetReferenceId apply(final RuleSetModel model) {
-            return new RuleSetReferenceId(LocationResolver.resolve(model.getLocation(), project));
+        public Optional<RuleSetReferenceId> apply(final RuleSetModel model) {
+            final Optional<String> resolvedLocation = LocationResolver.resolve(model.getLocation(), project);
+            return resolvedLocation.transform(new Function<String, RuleSetReferenceId>() {
+                @Override
+                public RuleSetReferenceId apply(final String location) {
+                    return new RuleSetReferenceId(location);
+                }
+            });
         }
 
     }