You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by gb...@apache.org on 2018/04/07 13:27:57 UTC

[maven-checkstyle-plugin] 01/01: [MCHECKSTYLE-347] StringIndexOutOfBoundsException when checkstyle.violation.ignore set to empty value

This is an automated email from the ASF dual-hosted git repository.

gboue pushed a commit to branch MCHECKSTYLE-347
in repository https://gitbox.apache.org/repos/asf/maven-checkstyle-plugin.git

commit 8fd6c8bbd26ca02cd7c64057676c9934c99c03bc
Author: Guillaume Boué <gb...@apache.org>
AuthorDate: Sat Apr 7 15:23:17 2018 +0200

    [MCHECKSTYLE-347] StringIndexOutOfBoundsException when
    checkstyle.violation.ignore set to empty value
    
    Blank strings should be skipped when parsing ignored violations.
---
 src/it/MCHECKSTYLE-347/checkstyle.xml              | 23 +++++++++++
 src/it/MCHECKSTYLE-347/invoker.properties          | 19 +++++++++
 src/it/MCHECKSTYLE-347/pom.xml                     | 47 ++++++++++++++++++++++
 .../MCHECKSTYLE-347/src/main/java/org/MyClass.java | 26 ++++++++++++
 src/it/MCHECKSTYLE-347/verify.groovy               | 23 +++++++++++
 .../checkstyle/CheckstyleViolationCheckMojo.java   | 17 ++++----
 .../apache/maven/plugins/checkstyle/RuleUtil.java  | 18 ++++++---
 .../maven/plugins/checkstyle/RuleUtilTest.java     | 20 +++++++--
 8 files changed, 174 insertions(+), 19 deletions(-)

diff --git a/src/it/MCHECKSTYLE-347/checkstyle.xml b/src/it/MCHECKSTYLE-347/checkstyle.xml
new file mode 100644
index 0000000..027c8a4
--- /dev/null
+++ b/src/it/MCHECKSTYLE-347/checkstyle.xml
@@ -0,0 +1,23 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  ~ Licensed to the Apache Software Foundation (ASF) under one
+  ~ or more contributor license agreements.  See the NOTICE file
+  ~ distributed with this work for additional information
+  ~ regarding copyright ownership.  The ASF licenses this file
+  ~ to you under the Apache License, Version 2.0 (the
+  ~ "License"); you may not use this file except in compliance
+  ~ with the License.  You may obtain a copy of the License at
+  ~
+  ~   http://www.apache.org/licenses/LICENSE-2.0
+  ~
+  ~ Unless required by applicable law or agreed to in writing,
+  ~ software distributed under the License is distributed on an
+  ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  ~ KIND, either express or implied.  See the License for the
+  ~ specific language governing permissions and limitations
+  ~ under the License.
+  -->
+<!DOCTYPE module PUBLIC "-//Puppy Crawl//DTD Check Configuration 1.2//EN" "http://www.puppycrawl.com/dtds/configuration_1_2.dtd">
+<module name="Checker">
+  <module name="JavadocPackage"/>
+</module>
diff --git a/src/it/MCHECKSTYLE-347/invoker.properties b/src/it/MCHECKSTYLE-347/invoker.properties
new file mode 100644
index 0000000..2b41a40
--- /dev/null
+++ b/src/it/MCHECKSTYLE-347/invoker.properties
@@ -0,0 +1,19 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+invoker.goals=clean checkstyle:check
+invoker.buildResult=failure
diff --git a/src/it/MCHECKSTYLE-347/pom.xml b/src/it/MCHECKSTYLE-347/pom.xml
new file mode 100644
index 0000000..445012a
--- /dev/null
+++ b/src/it/MCHECKSTYLE-347/pom.xml
@@ -0,0 +1,47 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  ~ Licensed to the Apache Software Foundation (ASF) under one
+  ~ or more contributor license agreements.  See the NOTICE file
+  ~ distributed with this work for additional information
+  ~ regarding copyright ownership.  The ASF licenses this file
+  ~ to you under the Apache License, Version 2.0 (the
+  ~ "License"); you may not use this file except in compliance
+  ~ with the License.  You may obtain a copy of the License at
+  ~
+  ~   http://www.apache.org/licenses/LICENSE-2.0
+  ~
+  ~ Unless required by applicable law or agreed to in writing,
+  ~ software distributed under the License is distributed on an
+  ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  ~ KIND, either express or implied.  See the License for the
+  ~ specific language governing permissions and limitations
+  ~ under the License.
+  -->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+  <groupId>org.apache.maven.plugins.checkstyle</groupId>
+  <artifactId>mcheckstytle-347</artifactId>
+  <version>1.0</version>
+  <url>https://issues.apache.org/jira/browse/MCHECKSTYLE-347</url>
+  <description>Tests that blank strings in Checkstyle ignored violations are skipped</description>
+  <properties>
+    <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+    <checkstyle.violation.ignore/>
+  </properties>
+  <build>
+    <plugins>
+      <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-checkstyle-plugin</artifactId>
+        <version>@pom.version@</version>
+        <configuration>
+          <!-- making sure there is one checkstyle error -->
+          <configLocation>checkstyle.xml</configLocation>
+        </configuration>
+      </plugin>
+    </plugins>
+  </build>
+</project>
diff --git a/src/it/MCHECKSTYLE-347/src/main/java/org/MyClass.java b/src/it/MCHECKSTYLE-347/src/main/java/org/MyClass.java
new file mode 100644
index 0000000..2566187
--- /dev/null
+++ b/src/it/MCHECKSTYLE-347/src/main/java/org/MyClass.java
@@ -0,0 +1,26 @@
+package org;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * No Javadoc package.
+ */
+public class MyClass {
+}
diff --git a/src/it/MCHECKSTYLE-347/verify.groovy b/src/it/MCHECKSTYLE-347/verify.groovy
new file mode 100644
index 0000000..2e1e395
--- /dev/null
+++ b/src/it/MCHECKSTYLE-347/verify.groovy
@@ -0,0 +1,23 @@
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+def buildLog = new File( basedir, 'build.log' )
+
+assert buildLog.text.contains( "You have 1 Checkstyle violation" )
diff --git a/src/main/java/org/apache/maven/plugins/checkstyle/CheckstyleViolationCheckMojo.java b/src/main/java/org/apache/maven/plugins/checkstyle/CheckstyleViolationCheckMojo.java
index eeed5c0..88e319b 100644
--- a/src/main/java/org/apache/maven/plugins/checkstyle/CheckstyleViolationCheckMojo.java
+++ b/src/main/java/org/apache/maven/plugins/checkstyle/CheckstyleViolationCheckMojo.java
@@ -28,6 +28,7 @@ import java.io.IOException;
 import java.io.OutputStream;
 import java.io.Reader;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
@@ -602,8 +603,8 @@ public class CheckstyleViolationCheckMojo
     {
         int count = 0;
         int ignoreCount = 0;
-        RuleUtil.Matcher[] ignores =
-            ( violationIgnore == null ) ? null : RuleUtil.parseMatchers( violationIgnore.split( "," ) );
+        List<RuleUtil.Matcher> ignores = violationIgnore == null ? Collections.<RuleUtil.Matcher>emptyList()
+                        : RuleUtil.parseMatchers( violationIgnore.split( "," ) );
 
         String basedir = project.getBasedir().getAbsolutePath();
         String file = "";
@@ -704,19 +705,15 @@ public class CheckstyleViolationCheckMojo
         }
     }
 
-    private boolean ignore( RuleUtil.Matcher[] ignores, String source )
+    private boolean ignore( List<RuleUtil.Matcher> ignores, String source )
     {
-        if ( ignores != null )
+        for ( RuleUtil.Matcher ignore : ignores )
         {
-            for ( RuleUtil.Matcher ignore : ignores )
+            if ( ignore.match( source ) )
             {
-                if ( ignore.match( source ) )
-                {
-                    return true;
-                }
+                return true;
             }
         }
-
         return false;
     }
 
diff --git a/src/main/java/org/apache/maven/plugins/checkstyle/RuleUtil.java b/src/main/java/org/apache/maven/plugins/checkstyle/RuleUtil.java
index ff53a3e..3f136ed 100644
--- a/src/main/java/org/apache/maven/plugins/checkstyle/RuleUtil.java
+++ b/src/main/java/org/apache/maven/plugins/checkstyle/RuleUtil.java
@@ -19,6 +19,11 @@ package org.apache.maven.plugins.checkstyle;
  * under the License.
  */
 
+import java.util.ArrayList;
+import java.util.List;
+
+import org.codehaus.plexus.util.StringUtils;
+
 import com.puppycrawl.tools.checkstyle.api.AuditEvent;
 
 /**
@@ -106,12 +111,15 @@ public final class RuleUtil
         return eventSrcName.substring( eventSrcName.lastIndexOf( '.' ) + 1 );
     }
 
-    public static Matcher[] parseMatchers( String[] specs )
+    public static List<Matcher> parseMatchers( String[] specs )
     {
-        Matcher[] matchers = new Matcher[specs.length];
-        int i = 0;
-        for ( String spec: specs )
+        List<Matcher> matchers = new ArrayList<>();
+        for ( String spec : specs )
         {
+            if ( StringUtils.isBlank( spec ) )
+            {
+                continue;
+            }
             spec = spec.trim();
             Matcher matcher;
             if ( Character.isUpperCase( spec.charAt( 0 ) ) )
@@ -138,7 +146,7 @@ public final class RuleUtil
                 // by default, spec is a package name
                 matcher = new PackageMatcher( spec );
             }
-            matchers[i++] = matcher;
+            matchers.add( matcher );
         }
         return matchers;
     }
diff --git a/src/test/java/org/apache/maven/plugins/checkstyle/RuleUtilTest.java b/src/test/java/org/apache/maven/plugins/checkstyle/RuleUtilTest.java
index 457be55..a7dddd4 100644
--- a/src/test/java/org/apache/maven/plugins/checkstyle/RuleUtilTest.java
+++ b/src/test/java/org/apache/maven/plugins/checkstyle/RuleUtilTest.java
@@ -1,5 +1,7 @@
 package org.apache.maven.plugins.checkstyle;
 
+import java.util.List;
+
 import junit.framework.TestCase;
 
 /*
@@ -50,13 +52,13 @@ public class RuleUtilTest
                 CHECKSTYLE_PACKAGE + ".test.FinalParametersCheck", "test.FinalParametersCheck",
                 CHECKSTYLE_PACKAGE + ".whitespace.HeaderCheck", CHECKSTYLE_PACKAGE + ".test2.FinalParametersCheck" };
 
-        RuleUtil.Matcher[] matchers = RuleUtil.parseMatchers( specs );
+        List<RuleUtil.Matcher> matchers = RuleUtil.parseMatchers( specs );
 
-        for ( int i = 0; i < matchers.length; i++ )
+        for ( int i = 0; i < matchers.size(); i++ )
         {
             String spec = specs[i];
-            RuleUtil.Matcher matcher = matchers[i];
-            for ( int j = 0; j < matchers.length; j++ )
+            RuleUtil.Matcher matcher = matchers.get( i );
+            for ( int j = 0; j < matchers.size(); j++ )
             {
                 String eventSrcName = eventSrcNames[j];
                 assertEquals( spec + " should" + ( ( i == j ) ? " " : " not " ) + "match " + eventSrcName, i == j,
@@ -64,4 +66,14 @@ public class RuleUtilTest
             }
         }
     }
+
+    public void testMatcherWithBlankStrings()
+    {
+        String[] specs = ( "   ,,foo, " ).split( "," );
+
+        List<RuleUtil.Matcher> matchers = RuleUtil.parseMatchers( specs );
+
+        assertEquals( 1, matchers.size() );
+        assertTrue( matchers.get( 0 ).match( CHECKSTYLE_PACKAGE + ".foo.SomeCheck" ) );
+    }
 }

-- 
To stop receiving notification emails like this one, please contact
gboue@apache.org.