Multiple active configurations and extended scoping

[JanK411]

Hi @jshiell,

We would like to contribute to your checkstyle-plugin for IntelliJ.
The following new things are possible with our submitted pull request.

• It is now possible to have more than one active configuration
  • A file can now be checked by multiple configurations at the same time.
• It is now possible to use IDE-wide scoping (see "Settings" > "Apperance & Behavior" > "Scopes") to restrict validation of rules based on those scopes.
  • For each checkstlye configuration the scope can be set independently.
  • If you run the "normal" checkstyle scan with the default "" parameter in the "Checkstyle" tab, all active configurations that match the scope of the configuration will be checked

The checkstyle settings, which are available in the checkstyle-idea.xml, are adapted so that the new features are available (one specified scope per configuration as well as several active configurations).
The plugin is designed to be backwards compatible, so that no user has to make manual changes or run into problems. As soon as the configuration is changed in the IDE for the first time, it is saved according to the new requirements.

The new functionality works as well for "check current file" as project- and module scanning. Also the "check before commit" and "realtime scanning" is working fine.

This PR might also resolve Issue #445.

If you have any questions, don't hesitate to ask us.

And thank you for creating and maintaining this wonderful plugin!

[jshiell]

This looks great, thank you! This is definitely of interest to others, so it's a great contribution.

I needed to apply the following changes to get all the tests to pass - could you please verify these are valid?

diff --git a/src/test/java/org/infernus/idea/checkstyle/TestHelper.java b/src/test/java/org/infernus/idea/checkstyle/TestHelper.java
index 68f90a9..774be90 100644
--- a/src/test/java/org/infernus/idea/checkstyle/TestHelper.java
+++ b/src/test/java/org/infernus/idea/checkstyle/TestHelper.java
@@ -7,6 +7,7 @@ import com.intellij.psi.search.scope.packageSet.NamedScope;
 import com.intellij.psi.search.scope.packageSet.NamedScopeManager;
 import org.jetbrains.annotations.NotNull;
 
+import static org.infernus.idea.checkstyle.model.NamedScopeHelper.DEFAULT_SCOPE_ID;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -15,9 +16,12 @@ public class TestHelper {
 
     @NotNull
     public static Project mockProject() {
+        DependencyValidationManager dependencyValidationManager = mock(DependencyValidationManager.class);
+        when(dependencyValidationManager.getScope(DEFAULT_SCOPE_ID)).thenReturn(NAMED_SCOPE);
+
         final Project project = mock(Project.class);
         when(project.getService(NamedScopeManager.class)).thenReturn(new NamedScopeManager(project));
-        when(project.getService(DependencyValidationManager.class)).thenReturn(mock(DependencyValidationManager.class));
+        when(project.getService(DependencyValidationManager.class)).thenReturn(dependencyValidationManager);
         return project;
     }
 }
diff --git a/src/test/java/org/infernus/idea/checkstyle/model/FileConfigurationLocationTest.java b/src/test/java/org/infernus/idea/checkstyle/model/FileConfigurationLocationTest.java
index 75dbbc1..2b88d3d 100644
--- a/src/test/java/org/infernus/idea/checkstyle/model/FileConfigurationLocationTest.java
+++ b/src/test/java/org/infernus/idea/checkstyle/model/FileConfigurationLocationTest.java
@@ -2,8 +2,6 @@ package org.infernus.idea.checkstyle.model;
 
 import com.intellij.openapi.project.Project;
 import com.intellij.openapi.vfs.VirtualFile;
-import com.intellij.psi.search.scope.packageSet.InvalidPackageSet;
-import com.intellij.psi.search.scope.packageSet.NamedScope;
 import org.apache.commons.io.FilenameUtils;
 import org.infernus.idea.checkstyle.TestHelper;
 import org.infernus.idea.checkstyle.util.ProjectFilePaths;
@@ -21,7 +19,6 @@ import java.util.function.Function;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
-import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.when;
 
 @RunWith(MockitoJUnitRunner.class)
@@ -125,7 +122,7 @@ public class FileConfigurationLocationTest {
     private FileConfigurationLocation useWindowsFilePaths() {
         final Project project = TestHelper.mockProject();
 
-        ProjectFilePaths testProjectFilePaths = testProjectFilePaths('\\');
+        ProjectFilePaths testProjectFilePaths = testProjectFilePaths('\\', project);
         when(project.getService(ProjectFilePaths.class)).thenReturn(testProjectFilePaths);
         when(projectPaths.projectPath(project)).thenReturn(projectBase);
         when(projectPaths.projectPath(project)).thenReturn(projectBase);
@@ -137,7 +134,7 @@ public class FileConfigurationLocationTest {
     private FileConfigurationLocation useUnixPaths() {
         final Project project = TestHelper.mockProject();
 
-        ProjectFilePaths testProjectFilePaths = testProjectFilePaths('/');
+        ProjectFilePaths testProjectFilePaths = testProjectFilePaths('/', project);
         when(project.getService(ProjectFilePaths.class)).thenReturn(testProjectFilePaths);
         when(projectPaths.projectPath(project)).thenReturn(projectBase);
         when(projectBase.getPath()).thenReturn(PROJECT_BASE_PATH);
@@ -146,7 +143,7 @@ public class FileConfigurationLocationTest {
     }
 
     @NotNull
-    private ProjectFilePaths testProjectFilePaths(char separatorChar) {
+    private ProjectFilePaths testProjectFilePaths(char separatorChar, Project project) {
         Function<File, String> absolutePathOf = file -> {
             // a nasty hack to pretend we're on a Windows box when required...
             if (file.getPath().startsWith("c:")) {
@@ -156,7 +153,7 @@ public class FileConfigurationLocationTest {
             return FilenameUtils.separatorsToUnix(file.getPath());
         };
 
-        return new ProjectFilePaths(TestHelper.mockProject(), separatorChar, absolutePathOf, projectPaths);
+        return new ProjectFilePaths(project, separatorChar, absolutePathOf, projectPaths);
     }
 
 }

[JanK411]

Nice to have such a quick and positive answer!
I just checked your changes and committed/pushed them. We were also wondering about the three additional failing tests, but this explains it. Thank you.
(There are still six failing JUnit-Tests, but they were already failing before we made our changes:

org.infernus.idea.checkstyle.util.ProjectFilePathsTest#directoryTraversalsInARelativePathShouldNotBeAlteredByTokenisation
org.infernus.idea.checkstyle.util.ProjectFilePathsTest#aPathWithRelativeElementsCanBeMadeProjectRelative
org.infernus.idea.checkstyle.util.ProjectFilePathsTest#aPathWithNoCommonElementsCanBeMadeProjectRelative
org.infernus.idea.checkstyle.util.ProjectFilePathsTest#aUnixLocationContainingTheLegacyProjectPathTokenShouldBeDetokenisedCorrectly
org.infernus.idea.checkstyle.util.ProjectFilePathsTest#aUnixLocationContainingTheProjectPathTokenShouldBeDetokenisedCorrectly
org.infernus.idea.checkstyle.util.ProjectFilePathsTest#anAbsolutePathCanBeMadeProjectRelative

)

[jshiell]

I'll merge this in and aim to get it out next weekend - thanks very much!

One more thing - I notice you say we in the original PR message - would you mind listing your collaborators, so I can properly credit them in the README and CHANGELOG?

[JanK411]

Happy to hear our PR getting into the product so quickly!

@Uschi003 was the one I was working with.

[jshiell]

I've just released it as 5.64.0. Thanks @JanK411 and @Uschi003 for a great contribution!

[Uschi003]

That's great! Thank you very much for accepting this PR!