Properties can now be overridden for the bundled Google/Sun configura…

…tions (#497).

CHANGELOG.md

@@ -1,5 +1,6 @@
 # CheckStyle-IDEA Changelog
 
+* **5.51.0** New: Properties can now be overridden for the bundled Google/Sun configurations (#497).
 * **5.51.0** Fixed: Added a couple of missing DTDs to the resolver.
 * **5.50.0** Fixed: ImportOrder third-party/special imports are ignored if regexes are not defined (#500).
 * **5.50.0** New: Replaced tool window icon with Checkstyle small icon (#519).

README.md

@@ -38,8 +38,7 @@ scanning.
 
 The main configuration option is that of the CheckStyle file. Multiple CheckStyle file may be added, and swapped between
 by using the checkbox. Files may be added using the 'Add' button, or you can use the versions of the standard Sun and 
-Google configuration that are bundled with the selected version of Checkstyle. Note that you cannot enter properties for
-these bundled rules at present.
+Google configuration that are bundled with the selected version of Checkstyle.
 
 If you need to pass authentication information for rules file accessed via HTTP then you can use the `https://user:pass@host/` form to do so.
 

src/main/java/org/infernus/idea/checkstyle/checker/CheckerFactory.java

@@ -138,9 +138,9 @@ private CachedChecker createChecker(@NotNull final ConfigurationLocation locatio
                                         @Nullable final Module module) {
         final ListPropertyResolver propertyResolver;
         try {
+            location.ensurePropertiesAreUpToDate(checkstyleProjectService.underlyingClassLoader());
             final Map<String, String> properties = removeEmptyProperties(location.getProperties());
-            propertyResolver = new ListPropertyResolver(
-                    addEclipseCsProperties(location, module, properties));
+            propertyResolver = new ListPropertyResolver(addEclipseCsProperties(location, module, properties));
         } catch (IOException e) {
             LOG.info("CheckStyle properties could not be loaded: " + location.getLocation(), e);
             return blockAndShowMessage(location, module, e, "checkstyle.file-io-failed", location.getLocation());

src/main/java/org/infernus/idea/checkstyle/config/ProjectConfigurationState.java

@@ -7,21 +7,18 @@
 import com.intellij.openapi.diagnostic.Logger;
 import com.intellij.openapi.project.Project;
 import com.intellij.util.xmlb.annotations.MapAnnotation;
-import org.infernus.idea.checkstyle.CheckStyleBundle;
 import org.infernus.idea.checkstyle.CheckStylePlugin;
 import org.infernus.idea.checkstyle.VersionListReader;
 import org.infernus.idea.checkstyle.csapi.BundledConfig;
 import org.infernus.idea.checkstyle.model.ConfigurationLocation;
 import org.infernus.idea.checkstyle.model.ConfigurationLocationFactory;
 import org.infernus.idea.checkstyle.model.ScanScope;
-import org.infernus.idea.checkstyle.util.Notifications;
 import org.infernus.idea.checkstyle.util.OS;
 import org.infernus.idea.checkstyle.util.ProjectFilePaths;
 import org.infernus.idea.checkstyle.util.Strings;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
-import java.io.IOException;
 import java.util.*;
 import java.util.stream.Collectors;
 
@@ -63,7 +60,7 @@ private ProjectFilePaths projectFilePaths() {
     }
 
     private ProjectSettings defaultProjectSettings() {
-        return ProjectSettings.create(project, projectFilePaths(), defaultConfiguration(project).build());
+        return ProjectSettings.create(projectFilePaths(), defaultConfiguration(project).build());
     }
 
     public ProjectSettings getState() {
@@ -90,7 +87,7 @@ PluginConfigurationBuilder populate(@NotNull final PluginConfigurationBuilder bu
     }
 
     void setCurrentConfig(@NotNull final PluginConfiguration currentPluginConfig) {
-        projectSettings = ProjectSettings.create(project, projectFilePaths(), currentPluginConfig);
+        projectSettings = ProjectSettings.create(projectFilePaths(), currentPluginConfig);
     }
 
     /**
@@ -240,16 +237,15 @@ static class ProjectSettings {
         @MapAnnotation
         private Map<String, String> configuration;
 
-        static ProjectSettings create(@NotNull final Project project,
-                                      @NotNull final ProjectFilePaths projectFilePaths,
+        static ProjectSettings create(@NotNull final ProjectFilePaths projectFilePaths,
                                       @NotNull final PluginConfiguration currentPluginConfig) {
             final Map<String, String> mapForSerialization = new TreeMap<>();
             mapForSerialization.put(CHECKSTYLE_VERSION_SETTING, currentPluginConfig.getCheckstyleVersion());
             mapForSerialization.put(SCANSCOPE_SETTING, currentPluginConfig.getScanScope().name());
             mapForSerialization.put(SUPPRESS_ERRORS, String.valueOf(currentPluginConfig.isSuppressErrors()));
             mapForSerialization.put(COPY_LIBS, String.valueOf(currentPluginConfig.isCopyLibs()));
 
-            serializeLocations(project, mapForSerialization, new ArrayList<>(currentPluginConfig.getLocations()));
+            serializeLocations(mapForSerialization, new ArrayList<>(currentPluginConfig.getLocations()));
 
             String s = serializeThirdPartyClasspath(projectFilePaths, currentPluginConfig.getThirdPartyClasspath());
             if (!Strings.isBlank(s)) {
@@ -274,27 +270,20 @@ public Map<String, String> configuration() {
             return configuration;
         }
 
-        private static void serializeLocations(@NotNull final Project project,
-                                               @NotNull final Map<String, String> storage,
+        private static void serializeLocations(@NotNull final Map<String, String> storage,
                                                @NotNull final List<ConfigurationLocation> configurationLocations) {
             int index = 0;
             for (final ConfigurationLocation configurationLocation : configurationLocations) {
                 storage.put(LOCATION_PREFIX + index, configurationLocation.getDescriptor());
-                try {
-                    final Map<String, String> properties = configurationLocation.getProperties();
-                    if (properties != null) {
-                        for (final Map.Entry<String, String> prop : properties.entrySet()) {
-                            String value = prop.getValue();
-                            if (value == null) {
-                                value = "";
-                            }
-                            storage.put(PROPERTIES_PREFIX + index + "." + prop.getKey(), value);
+                final Map<String, String> properties = configurationLocation.getProperties();
+                if (properties != null) {
+                    for (final Map.Entry<String, String> prop : properties.entrySet()) {
+                        String value = prop.getValue();
+                        if (value == null) {
+                            value = "";
                         }
+                        storage.put(PROPERTIES_PREFIX + index + "." + prop.getKey(), value);
                     }
-                } catch (IOException e) {
-                    LOG.warn("Failed to read properties from " + configurationLocation, e);
-                    Notifications.showError(project, CheckStyleBundle.message("checkstyle" + ""
-                            + ".could-not-read-properties", configurationLocation.getLocation()));
                 }
 
                 ++index;

src/main/java/org/infernus/idea/checkstyle/model/BundledConfigurationLocation.java

@@ -2,8 +2,6 @@
 
 import java.io.IOException;
 import java.io.InputStream;
-import java.util.Collections;
-import java.util.Map;
 
 import com.intellij.openapi.project.Project;
 import org.infernus.idea.checkstyle.csapi.BundledConfig;
@@ -32,12 +30,6 @@ public BundledConfig getBundledConfig() {
         return bundledConfig;
     }
 
-    @Override
-    public Map<String, String> getProperties() {
-        // given the need to instantiate the CS classpath to read these files, the default impl of this currently causes a loop
-        return Collections.emptyMap();
-    }
-
     @Override
     public void setLocation(final String location) {
         // do nothing, we always use the hard-coded location
@@ -62,7 +54,7 @@ protected InputStream resolveFile(@NotNull final ClassLoader checkstyleClassLoad
         }
     }
 
-    public boolean isEditableInConfigDialog() {
+    public boolean isRemovable() {
         return false;
     }
 

src/main/java/org/infernus/idea/checkstyle/model/ConfigurationLocation.java

@@ -101,7 +101,7 @@ public void setDescription(@Nullable final String description) {
         }
     }
 
-    public Map<String, String> getProperties() throws IOException {
+    public Map<String, String> getProperties() {
         return new HashMap<>(properties);
     }
 
@@ -117,7 +117,7 @@ public void setProperties(final Map<String, String> newProperties) {
         this.propertiesCheckedThisSession = false;
     }
 
-    public boolean isEditableInConfigDialog() {
+    public boolean isRemovable() {
         return true;
     }
 
@@ -182,6 +182,15 @@ private void extractPropertyNames(final Element element, final List<String> prop
         }
     }
 
+    @SuppressWarnings("EmptyTryBlock")
+    public void ensurePropertiesAreUpToDate(@NotNull final ClassLoader checkstyleClassLoader) throws IOException {
+        if (!propertiesCheckedThisSession) {
+            try (InputStream ignored = resolve(checkstyleClassLoader)) {
+                // ignored
+            }
+        }
+    }
+
     public InputStream resolve(@NotNull final ClassLoader checkstyleClassLoader) throws IOException {
         InputStream is = resolveFile(checkstyleClassLoader);
 
@@ -317,21 +326,15 @@ private File checkModuleContentRoots(final Module module, final String fileName)
     }
 
     public final boolean hasChangedFrom(final ConfigurationLocation configurationLocation) {
-        return configurationLocation == null
-                || !equals(configurationLocation)
+        return !equals(configurationLocation)
                 || propertiesHaveChanged(configurationLocation);
-
     }
 
     private boolean propertiesHaveChanged(final ConfigurationLocation configurationLocation) {
         if (project.isDefault() && !configurationLocation.canBeResolvedInDefaultProject()) {
             return false;
         }
-        try {
-            return !getProperties().equals(configurationLocation.getProperties());
-        } catch (IOException e) {
-            return true;
-        }
+        return !getProperties().equals(configurationLocation.getProperties());
     }
 
     public String getDescriptor() {
@@ -357,11 +360,7 @@ public String getDescriptor() {
     ConfigurationLocation cloneCommonPropertiesTo(final ConfigurationLocation cloned) {
         cloned.setDescription(getDescription());
         cloned.setLocation(getLocation());
-        try {
-            cloned.setProperties(new HashMap<>(getProperties()));
-        } catch (IOException e) {
-            throw new RuntimeException("Failed to resolve properties for " + this);
-        }
+        cloned.setProperties(new HashMap<>(getProperties()));
         return cloned;
     }
 

src/main/java/org/infernus/idea/checkstyle/ui/CheckStyleConfigPanel.java

@@ -144,8 +144,8 @@ private JPanel buildRuleFilePanel() {
         tableDecorator.setAddAction(new AddLocationAction());
         tableDecorator.setEditAction(new EditPropertiesAction());
         tableDecorator.setRemoveAction(new RemoveLocationAction());
-        tableDecorator.setEditActionUpdater(new EnableWhenSelectedAndEditable());
-        tableDecorator.setRemoveActionUpdater(new EnableWhenSelectedAndEditable());
+        tableDecorator.setEditActionUpdater(new EnableWhenSelected());
+        tableDecorator.setRemoveActionUpdater(new EnableWhenSelectedAndRemovable());
         tableDecorator.setPreferredSize(DECORATOR_DIMENSIONS);
 
         final JPanel container = new JPanel(new BorderLayout());
@@ -521,11 +521,19 @@ public void actionPerformed(final ActionEvent e) {
         }
     }
 
-    private class EnableWhenSelectedAndEditable implements AnActionButtonUpdater {
+    private class EnableWhenSelectedAndRemovable implements AnActionButtonUpdater {
         @Override
         public boolean isEnabled(@NotNull final AnActionEvent e) {
             final int selectedItem = locationTable.getSelectedRow();
-            return selectedItem >= 0 && locationModel.getLocationAt(selectedItem).isEditableInConfigDialog();
+            return selectedItem >= 0 && locationModel.getLocationAt(selectedItem).isRemovable();
+        }
+    }
+
+    private class EnableWhenSelected implements AnActionButtonUpdater {
+        @Override
+        public boolean isEnabled(@NotNull final AnActionEvent e) {
+            final int selectedItem = locationTable.getSelectedRow();
+            return selectedItem >= 0;
         }
     }
 }

src/main/java/org/infernus/idea/checkstyle/ui/LocationDialogue.java

@@ -317,15 +317,11 @@ public void actionPerformed(final ActionEvent e) {
 
                 case COMPLETE:
                 case ERROR:
-                    try {
-                        final Map<String, String> properties = configurationLocation.getProperties();
-                        if (properties == null || properties.isEmpty()) {
-                            moveToStep(Step.SELECT);
-                        } else {
-                            moveToStep(Step.PROPERTIES);
-                        }
-                    } catch (IOException e1) {
+                    final Map<String, String> properties = configurationLocation.getProperties();
+                    if (properties == null || properties.isEmpty()) {
                         moveToStep(Step.SELECT);
+                    } else {
+                        moveToStep(Step.PROPERTIES);
                     }
                     return;
 

src/main/resources/META-INF/plugin.xml

@@ -24,6 +24,7 @@
     <change-notes>
         <![CDATA[
 <ul>
+    <li>5.51.0: New: Properties can now be overridden for the bundled Google/Sun configurations (#497).</li>
     <li>5.51.0: Fixed: Added a couple of missing DTDs to the resolver.</li>
     <li>5.50.0: Fixed: ImportOrder third-party/special imports are ignored if regexes are not defined (#500).</li>
     <li>5.50.0: New: Replaced tool window icon with Checkstyle small icon (#519).</li>
@@ -39,7 +40,6 @@
     <li>5.44.0: New: Added Checkstyle 8.36.2.</li>
     <li>5.43.0: New: Added Checkstyle 8.36.1.</li>
     <li>5.42.0: New: Added Checkstyle 8.36.</li>
-    <li>5.41.0: New: Added Checkstyle 8.35.</li>
     <li><em>For older changes please see the changelog.</em></li>
 </ul>
         ]]>