[custom] Fix for feature check to ignore bindings with no feature file at all (#405)

- Change configuration to ignore specific bindings instead of features.
This because now the modbus binding has no feature at all

Signed-off-by: Hilbrand Bouwkamp <hilbrand@h72.nl>

Author: Hilbrand

custom-checks/checkstyle/src/main/java/org/openhab/tools/analysis/checkstyle/KarafAddonFeatureCheck.java

@@ -24,6 +24,7 @@
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.function.Function;
+import java.util.regex.Pattern;
 
 import javax.xml.xpath.XPathConstants;
 import javax.xml.xpath.XPathExpression;
@@ -71,52 +72,51 @@ public class KarafAddonFeatureCheck extends AbstractStaticCheck {
     private final Logger logger = LoggerFactory.getLogger(KarafAddonFeatureCheck.class);
 
     private final Map<String, String> featureNamePatternsMap = new LinkedHashMap<>();
-    private final List<String> excludeFeatureNamesList = new ArrayList<>();
+    private final List<Pattern> excludeAddonsList = new ArrayList<>();
 
     public KarafAddonFeatureCheck() {
         setFileExtensions(XML_EXTENSION);
     }
 
-    /**
-     * Comma separated list of binding names (name after pattern applied) of feature names that will be ignored for
-     * checking.
-     *
-     * @param excludeFeatureNames command separated list
-     */
-    public void setExcludeFeatureNames(String excludeFeatureNames) {
-        if (excludeFeatureNames.trim().isBlank()) {
-            return;
-        }
-        for (String exclude : excludeFeatureNames.split(",")) {
-            excludeFeatureNamesList.add(exclude);
-        }
-    }
-
     /**
      * Comma separated list of key value pairs (separated by colon) transform the calculated expected feature name in
      * the actual feature name.
      * For example openhab-transform-map:openhab-transformation-map.
      * the openhab-transform-map is as expected as derived from the artifactId, but all bundles use transformation.
-     * So with the pattern expected feature name can be constructed.
+     * So with the map expected feature name can be constructed.
      *
-     * @param featureNamePatterns list of key/value pairs of patterns
+     * @param featureNameMappings list of key/value pairs of patterns
      */
-    public void setFeatureNamePatterns(String featureNamePatterns) {
-        if (featureNamePatterns.trim().isBlank()) {
+    public void setFeatureNameMappings(String featureNameMappings) {
+        if (featureNameMappings.trim().isBlank()) {
             return;
         }
-        for (String pattern : featureNamePatterns.split(",")) {
+        for (String pattern : featureNameMappings.split(",")) {
             final String[] keypair = pattern.split(":");
 
             if (keypair.length == 2) {
                 featureNamePatternsMap.put(keypair[0], keypair[1]);
             } else {
                 logger.warn("{} check pattern for option featureNamePatterns is invalid. Value set: {}",
-                        getClass().getName(), featureNamePatterns);
+                        getClass().getName(), featureNameMappings);
             }
         }
     }
 
+    /**
+     * Comma separated list of add-on name patterns that will be excluded from checking.
+     *
+     * @param excludeAddonPatterns command separated list
+     */
+    public void setExcludeAddonPatterns(String excludeAddonPatterns) {
+        if (excludeAddonPatterns.trim().isBlank()) {
+            return;
+        }
+        for (String pattern : excludeAddonPatterns.split(",")) {
+            excludeAddonsList.add(Pattern.compile(pattern));
+        }
+    }
+
     @Override
     protected void processFiltered(@Nullable File file, @Nullable FileText fileText) throws CheckstyleException {
         if (file == null || fileText == null) {
@@ -140,19 +140,29 @@ private void checkMissingFeatureFile(File file, FileText fileText) throws Checks
                     getClass().getSimpleName(), file.getAbsolutePath());
             return;
         }
-        final File featureFile = new File(file.getParent(), FEATURE_XML_PATH);
+        String parent = file.getParent();
+        final File featureFile = new File(parent, FEATURE_XML_PATH);
 
         if (!featureFile.exists()) {
-            logMessage(featureFile.toString(), 0, FEATURE_XML,
-                    MessageFormat.format(MSG_MISSING_FEATURE_XML, featureFile));
+            if (isExcludedAddon(parent)) {
+                logger.debug("Ignore check on none exisiting feature name {}", featureFile);
+            } else {
+                logMessage(featureFile.toString(), 0, FEATURE_XML,
+                        MessageFormat.format(MSG_MISSING_FEATURE_XML, featureFile));
+            }
         }
     }
 
     private void checkFeatureFile(File featureFile, FileText fileText) throws CheckstyleException {
         try {
             final String featureFileString = featureFile.getAbsoluteFile().toString();
-            final FileText pomFile = new FileText(
-                    new File(featureFileString.replace(FEATURE_XML_PATH, ""), POM_XML_FILE_NAME),
+            String addonPath = featureFileString.replace(FEATURE_XML_PATH, "");
+
+            if (isExcludedAddon(new File(addonPath).getName())) {
+                logger.debug("Ignore check on excluded addon with feature name {}", featureFile);
+                return;
+            }
+            final FileText pomFile = new FileText(new File(addonPath, POM_XML_FILE_NAME),
                     StandardCharsets.UTF_8.name());
             final String artifactId = getArtifactId(pomFile);
 
@@ -170,6 +180,10 @@ private void checkFeatureFile(File featureFile, FileText fileText) throws Checks
         }
     }
 
+    private boolean isExcludedAddon(String parent) {
+        return excludeAddonsList.stream().anyMatch(p -> p.matcher(parent).find());
+    }
+
     private void checkFeatures(File featureFile, String artifactId, FileText fileText, Document featureXML) {
         checkName(featureFile, artifactId, fileText, featureXML, FEATURES_NAME_EXPRESSION,
                 name -> !artifactId.equals(name.substring(0, name.indexOf('-'))), MSG_FEATURES_NAME_INVALID,
@@ -179,17 +193,12 @@ private void checkFeatures(File featureFile, String artifactId, FileText fileTex
     private void checkFeature(File featureFile, String artifactId, FileText fileText, Document featureXML) {
         final String featureName = adaptedFeatureName(artifactId);
 
-        for (String exclude : excludeFeatureNamesList) {
-            if (exclude.equals(featureName)) {
-                logger.debug("Ignore check on feature name {}", featureName);
-                return;
-            }
-        }
         checkName(featureFile, featureName, fileText, featureXML, FEATURE_NAME_EXPRESSION,
                 name -> !featureName.equals(name), MSG_FEATURE_NAME_INVALID, FEATURE_SEARCH);
     }
 
     private String adaptedFeatureName(String artifactId) {
+        // skip 'org.' part of artifactId by taking substring(4)
         String featureName = artifactId.substring(4).replaceAll("\\.", "-");
 
         for (Entry<String, String> entry : featureNamePatternsMap.entrySet()) {

custom-checks/checkstyle/src/test/java/org/openhab/tools/analysis/checkstyle/test/KarafAddonFeatureCheckTest.java

@@ -45,6 +45,15 @@ public void testValid() throws Exception {
         verify("valid", ArrayUtils.EMPTY_STRING_ARRAY);
     }
 
+    @Test
+    public void testExcludeAddonPatterns() throws Exception {
+        DefaultConfiguration config = createModuleConfig(KarafAddonFeatureCheck.class);
+        config.addAttribute("excludeAddonPatterns", "excludeAddon.*");
+
+        verify(config, getPath("excludeAddonPatterns" + File.separator + POM_XML_FILE_NAME),
+                ArrayUtils.EMPTY_STRING_ARRAY);
+    }
+
     @Test
     public void testMissingFeatureFile() throws Exception {
         final File featureFile = new File(getPath("missingFeature"), FEATURE_XML_PATH);
@@ -75,16 +84,7 @@ public void testInvalidFeatureName() throws Exception {
     @Test
     public void testPatternFeatureName() throws Exception {
         DefaultConfiguration config = createModuleConfig(KarafAddonFeatureCheck.class);
-        config.addAttribute("featureNamePatterns", "openhab-binding-example:org.openhab.binding.someother");
-        verify(config, getPath("invalidFeatureName" + File.separator + FEATURE_XML_PATH),
-                ArrayUtils.EMPTY_STRING_ARRAY);
-    }
-
-    @Test
-    public void testExcludedFeatureName() throws Exception {
-        DefaultConfiguration config = createModuleConfig(KarafAddonFeatureCheck.class);
-        config.addAttribute("featureNamePatterns", "openhab-binding-example:org.openhab.binding.someother");
-        config.addAttribute("excludeFeatureNames", "org.openhab.binding.someother");
+        config.addAttribute("featureNameMappings", "openhab-binding-example:org.openhab.binding.someother");
         verify(config, getPath("invalidFeatureName" + File.separator + FEATURE_XML_PATH),
                 ArrayUtils.EMPTY_STRING_ARRAY);
     }

custom-checks/checkstyle/src/test/resources/checkstyle/karafAddonFeatureCheck/excludeAddonPatterns/pom.xml

@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+  xmlns="http://maven.apache.org/POM/4.0.0"
+  xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+
+  <parent>
+    <groupId>org.openhab.addons.bundles</groupId>
+    <artifactId>org.openhab.addons.reactor.bundles</artifactId>
+    <version>3.0.0-SNAPSHOT</version>
+  </parent>
+
+  <artifactId>org.openhab.binding.example</artifactId>
+
+  <name>openHAB Add-ons :: Bundles :: Example Binding</name>
+
+</project>

sat-plugin/src/main/resources/rulesets/checkstyle/rules.xml

@@ -161,9 +161,9 @@
 
 	<module name="org.openhab.tools.analysis.checkstyle.KarafAddonFeatureCheck">
 		<property name="severity" value="${checkstyle.karafAddonFeatureCheck.severity}" default="error"/>
-		<property name="featureNamePatterns" value="${checkstyle.karafAddonFeatureCheck.featureNamePatterns}"
+		<property name="featureNameMappings" value="${checkstyle.karafAddonFeatureCheck.featureNameMappings}"
 			default=""/>
-		<property name="excludeFeatureNames" value="${checkstyle.karafAddonFeatureCheck.excludeFeatureNames}"
+		<property name="excludeAddonPatterns" value="${checkstyle.karafAddonFeatureCheck.excludeAddonPatterns}"
 			default=""/>
 	</module>