You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by gg...@apache.org on 2019/06/15 17:52:48 UTC
[commons-csv] branch master updated: Post release fixes (#44)
This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-csv.git
The following commit(s) were added to refs/heads/master by this push:
new 03550ab Post release fixes (#44)
03550ab is described below
commit 03550ab565e55f509def8995c3c852b8686e80b2
Author: Alex Herbert <a....@sussex.ac.uk>
AuthorDate: Sat Jun 15 18:52:44 2019 +0100
Post release fixes (#44)
* Fix checkstyle: remove tabs
* Fix checkstyle: Split long line
* Fix checkstyle: exclude pom.properties
* Update findbugs to allow deliberate fall-through
* Fix pmd: Remove ternary operator returning false
* Fix pmd: Remove implicit final
* Fix pmd: Ignore TooManyStaticImports.
This requires adding the default ruleset and then modifying with
suppressions.
* Add tests to cover use of the IOUtils class.
Requires the CSVFormat to have no quote or escape character, and the
formatted value to be a java.io.Reader.
* Clean-up findbugs exclude filter.
* Removed unused import
* Updated test comments for print tests targeting IOUtils.
* Fix checkstyle: Suppress line length warning in CSVParser.
---
pom.xml | 42 +++++++++-
.../java/org/apache/commons/csv/CSVFormat.java | 6 +-
.../java/org/apache/commons/csv/CSVParser.java | 6 +-
.../checkstyle/checkstyle-suppressions.xml | 23 ++++++
.../resources/findbugs/findbugs-exclude-filter.xml | 35 +++++++++
src/main/resources/pmd/pmd-ruleset.xml | 89 ++++++++++++++++++++++
.../org/apache/commons/csv/CSVPrinterTest.java | 41 ++++++++++
7 files changed, 234 insertions(+), 8 deletions(-)
diff --git a/pom.xml b/pom.xml
index 9b57003..3fe5469 100644
--- a/pom.xml
+++ b/pom.xml
@@ -153,8 +153,9 @@ CSV files of various types.
<checkstyle.version>3.0.0</checkstyle.version>
<checkstyle.header.file>${basedir}/LICENSE-header.txt</checkstyle.header.file>
- <checkstyle.resourceExcludes>LICENSE.txt, NOTICE.txt</checkstyle.resourceExcludes>
+ <checkstyle.resourceExcludes>LICENSE.txt, NOTICE.txt, **/maven-archiver/pom.properties</checkstyle.resourceExcludes>
+ <pmd.version>3.12.0</pmd.version>
<japicmp.skip>false</japicmp.skip>
<commons.release.isDistModule>true</commons.release.isDistModule>
@@ -200,6 +201,32 @@ CSV files of various types.
<configuration>
<configLocation>${basedir}/checkstyle.xml</configLocation>
<enableRulesSummary>false</enableRulesSummary>
+ <suppressionsLocation>${basedir}/src/main/resources/checkstyle/checkstyle-suppressions.xml</suppressionsLocation>
+ </configuration>
+ </plugin>
+ <!-- Allow findbugs to be run interactively; keep in sync with report config below -->
+ <plugin>
+ <groupId>org.codehaus.mojo</groupId>
+ <artifactId>findbugs-maven-plugin</artifactId>
+ <version>${commons.findbugs.version}</version>
+ <configuration>
+ <threshold>Normal</threshold>
+ <effort>Default</effort>
+ <excludeFilterFile>${basedir}/src/main/resources/findbugs/findbugs-exclude-filter.xml</excludeFilterFile>
+ </configuration>
+ </plugin>
+ <!-- Allow pmd to be run interactively; keep in sync with report config below -->
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-pmd-plugin</artifactId>
+ <version>${pmd.version}</version>
+ <configuration>
+ <targetJdk>${maven.compiler.target}</targetJdk>
+ <skipEmptyReport>false</skipEmptyReport>
+ <analysisCache>true</analysisCache>
+ <rulesets>
+ <ruleset>${basedir}/src/main/resources/pmd/pmd-ruleset.xml</ruleset>
+ </rulesets>
</configuration>
</plugin>
@@ -244,6 +271,7 @@ CSV files of various types.
<configuration>
<configLocation>${basedir}/checkstyle.xml</configLocation>
<enableRulesSummary>false</enableRulesSummary>
+ <suppressionsLocation>${basedir}/src/main/resources/checkstyle/checkstyle-suppressions.xml</suppressionsLocation>
</configuration>
<!-- We need to specify reportSets because 2.9.1 creates two reports -->
<reportSets>
@@ -257,15 +285,25 @@ CSV files of various types.
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-pmd-plugin</artifactId>
- <version>3.12.0</version>
+ <version>${pmd.version}</version>
<configuration>
<targetJdk>${maven.compiler.target}</targetJdk>
+ <skipEmptyReport>false</skipEmptyReport>
+ <analysisCache>true</analysisCache>
+ <rulesets>
+ <ruleset>${basedir}/src/main/resources/pmd/pmd-ruleset.xml</ruleset>
+ </rulesets>
</configuration>
</plugin>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>findbugs-maven-plugin</artifactId>
<version>${commons.findbugs.version}</version>
+ <configuration>
+ <threshold>Normal</threshold>
+ <effort>Default</effort>
+ <excludeFilterFile>${basedir}/src/main/resources/findbugs/findbugs-exclude-filter.xml</excludeFilterFile>
+ </configuration>
</plugin>
<plugin>
<groupId>org.codehaus.mojo</groupId>
diff --git a/src/main/java/org/apache/commons/csv/CSVFormat.java b/src/main/java/org/apache/commons/csv/CSVFormat.java
index 1111762..263943f 100644
--- a/src/main/java/org/apache/commons/csv/CSVFormat.java
+++ b/src/main/java/org/apache/commons/csv/CSVFormat.java
@@ -892,7 +892,7 @@ public final class CSVFormat implements Serializable {
*/
public String format(final Object... values) {
final StringWriter out = new StringWriter();
- try (final CSVPrinter csvPrinter = new CSVPrinter(out, this)) {
+ try (CSVPrinter csvPrinter = new CSVPrinter(out, this)) {
csvPrinter.printRecord(values);
return out.toString().trim();
} catch (final IOException e) {
@@ -1713,7 +1713,7 @@ public final class CSVFormat implements Serializable {
* @since 1.7
*/
public CSVFormat withAllowDuplicateHeaderNames() {
- return withAllowDuplicateHeaderNames(true);
+ return withAllowDuplicateHeaderNames(true);
}
/**
@@ -1724,7 +1724,7 @@ public final class CSVFormat implements Serializable {
* @since 1.7
*/
public CSVFormat withAllowDuplicateHeaderNames(final boolean allowDuplicateHeaderNames) {
- return new CSVFormat(delimiter, quoteCharacter, quoteMode, commentMarker, escapeCharacter,
+ return new CSVFormat(delimiter, quoteCharacter, quoteMode, commentMarker, escapeCharacter,
ignoreSurroundingSpaces, ignoreEmptyLines, recordSeparator, nullString, headerComments, header,
skipHeaderRecord, allowMissingColumnNames, ignoreHeaderCase, trim, trailingDelimiter, autoFlush,
allowDuplicateHeaderNames);
diff --git a/src/main/java/org/apache/commons/csv/CSVParser.java b/src/main/java/org/apache/commons/csv/CSVParser.java
index a8b16c2..3b3b3e7 100644
--- a/src/main/java/org/apache/commons/csv/CSVParser.java
+++ b/src/main/java/org/apache/commons/csv/CSVParser.java
@@ -495,7 +495,7 @@ public final class CSVParser implements Iterable<CSVRecord>, Closeable {
if (headerRecord != null) {
for (int i = 0; i < headerRecord.length; i++) {
final String header = headerRecord[i];
- final boolean containsHeader = header == null ? false : hdrMap.containsKey(header);
+ final boolean containsHeader = header != null && hdrMap.containsKey(header);
final boolean emptyHeader = header == null || header.trim().isEmpty();
if (containsHeader) {
if (!emptyHeader && !this.format.getAllowDuplicateHeaderNames()) {
@@ -520,9 +520,9 @@ public final class CSVParser implements Iterable<CSVRecord>, Closeable {
}
}
if (headerNames == null) {
- headerNames = Collections.emptyList(); //immutable
+ headerNames = Collections.emptyList(); //immutable
} else {
- headerNames = Collections.unmodifiableList(headerNames);
+ headerNames = Collections.unmodifiableList(headerNames);
}
return new Headers(hdrMap, headerNames);
}
diff --git a/src/main/resources/checkstyle/checkstyle-suppressions.xml b/src/main/resources/checkstyle/checkstyle-suppressions.xml
new file mode 100644
index 0000000..edc6783
--- /dev/null
+++ b/src/main/resources/checkstyle/checkstyle-suppressions.xml
@@ -0,0 +1,23 @@
+<?xml version="1.0"?>
+<!--
+ 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 suppressions PUBLIC
+ "-//Checkstyle//DTD SuppressionFilter Configuration 1.2//EN"
+ "https://checkstyle.org/dtds/suppressions_1_2.dtd">
+<suppressions>
+ <suppress checks="LineLength" files="[\\/]CSVParser\.java$" lines="504"/>
+</suppressions>
diff --git a/src/main/resources/findbugs/findbugs-exclude-filter.xml b/src/main/resources/findbugs/findbugs-exclude-filter.xml
new file mode 100644
index 0000000..da2896a
--- /dev/null
+++ b/src/main/resources/findbugs/findbugs-exclude-filter.xml
@@ -0,0 +1,35 @@
+<?xml version="1.0"?>
+<!--
+ 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.
+-->
+
+<!--
+ This file contains some false positive bugs detected by spotbugs. Their
+ false positive nature has been analyzed individually and they have been
+ put here to instruct spotbugs it must ignore them.
+-->
+<FindBugsFilter
+ xmlns="https://github.com/spotbugs/filter/3.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="https://github.com/spotbugs/filter/3.0.0 https://raw.githubusercontent.com/spotbugs/spotbugs/3.1.0/spotbugs/etc/findbugsfilter.xsd">
+
+ <Match>
+ <Class name="org.apache.commons.csv.CSVPrinter"/>
+ <BugPattern name="SF_SWITCH_FALLTHROUGH"/>
+ <Method name="printComment"/>
+ </Match>
+
+</FindBugsFilter>
diff --git a/src/main/resources/pmd/pmd-ruleset.xml b/src/main/resources/pmd/pmd-ruleset.xml
new file mode 100644
index 0000000..8d52e45
--- /dev/null
+++ b/src/main/resources/pmd/pmd-ruleset.xml
@@ -0,0 +1,89 @@
+<?xml version="1.0"?>
+<!--
+ 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.
+-->
+<ruleset name="commons-rng-customized"
+ xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 http://pmd.sourceforge.net/ruleset_2_0_0.xsd">
+ <description>
+ This ruleset checks the code for discouraged programming constructs.
+ </description>
+
+ <!-- Default ruleset for maven-pmd-plugin -->
+ <rule ref="category/java/bestpractices.xml/AvoidUsingHardCodedIP"/>
+ <rule ref="category/java/bestpractices.xml/CheckResultSet"/>
+ <rule ref="category/java/bestpractices.xml/UnusedImports"/>
+ <rule ref="category/java/bestpractices.xml/UnusedFormalParameter"/>
+ <rule ref="category/java/bestpractices.xml/UnusedLocalVariable"/>
+ <rule ref="category/java/bestpractices.xml/UnusedPrivateField"/>
+ <rule ref="category/java/bestpractices.xml/UnusedPrivateMethod"/>
+ <rule ref="category/java/codestyle.xml/DontImportJavaLang"/>
+ <rule ref="category/java/codestyle.xml/DuplicateImports"/>
+ <rule ref="category/java/codestyle.xml/ExtendsObject"/>
+ <rule ref="category/java/codestyle.xml/ForLoopShouldBeWhileLoop"/>
+ <rule ref="category/java/codestyle.xml/TooManyStaticImports"/>
+ <rule ref="category/java/codestyle.xml/UnnecessaryFullyQualifiedName"/>
+ <rule ref="category/java/codestyle.xml/UnnecessaryModifier"/>
+ <rule ref="category/java/codestyle.xml/UnnecessaryReturn"/>
+ <rule ref="category/java/codestyle.xml/UselessParentheses"/>
+ <rule ref="category/java/codestyle.xml/UselessQualifiedThis"/>
+ <rule ref="category/java/design.xml/CollapsibleIfStatements"/>
+ <rule ref="category/java/design.xml/SimplifiedTernary"/>
+ <rule ref="category/java/design.xml/UselessOverridingMethod"/>
+ <rule ref="category/java/errorprone.xml/AvoidBranchingStatementAsLastInLoop"/>
+ <rule ref="category/java/errorprone.xml/AvoidDecimalLiteralsInBigDecimalConstructor"/>
+ <rule ref="category/java/errorprone.xml/AvoidMultipleUnaryOperators"/>
+ <rule ref="category/java/errorprone.xml/AvoidUsingOctalValues"/>
+ <rule ref="category/java/errorprone.xml/BrokenNullCheck"/>
+ <rule ref="category/java/errorprone.xml/CheckSkipResult"/>
+ <rule ref="category/java/errorprone.xml/ClassCastExceptionWithToArray"/>
+ <rule ref="category/java/errorprone.xml/DontUseFloatTypeForLoopIndices"/>
+ <rule ref="category/java/errorprone.xml/EmptyCatchBlock"/>
+ <rule ref="category/java/errorprone.xml/EmptyFinallyBlock"/>
+ <rule ref="category/java/errorprone.xml/EmptyIfStmt"/>
+ <rule ref="category/java/errorprone.xml/EmptyInitializer"/>
+ <rule ref="category/java/errorprone.xml/EmptyStatementBlock"/>
+ <rule ref="category/java/errorprone.xml/EmptyStatementNotInLoop"/>
+ <rule ref="category/java/errorprone.xml/EmptySwitchStatements"/>
+ <rule ref="category/java/errorprone.xml/EmptySynchronizedBlock"/>
+ <rule ref="category/java/errorprone.xml/EmptyTryBlock"/>
+ <rule ref="category/java/errorprone.xml/EmptyWhileStmt"/>
+ <rule ref="category/java/errorprone.xml/ImportFromSamePackage"/>
+ <rule ref="category/java/errorprone.xml/JumbledIncrementer"/>
+ <rule ref="category/java/errorprone.xml/MisplacedNullCheck"/>
+ <rule ref="category/java/errorprone.xml/OverrideBothEqualsAndHashcode"/>
+ <rule ref="category/java/errorprone.xml/ReturnFromFinallyBlock"/>
+ <rule ref="category/java/errorprone.xml/UnconditionalIfStatement"/>
+ <rule ref="category/java/errorprone.xml/UnnecessaryConversionTemporary"/>
+ <rule ref="category/java/errorprone.xml/UnusedNullCheckInEquals"/>
+ <rule ref="category/java/errorprone.xml/UselessOperationOnImmutable"/>
+ <rule ref="category/java/multithreading.xml/AvoidThreadGroup"/>
+ <rule ref="category/java/multithreading.xml/DontCallThreadRun"/>
+ <rule ref="category/java/multithreading.xml/DoubleCheckedLocking"/>
+ <rule ref="category/java/performance.xml/BigIntegerInstantiation"/>
+ <rule ref="category/java/performance.xml/BooleanInstantiation"/>
+
+ <!-- Rule customisations. -->
+
+ <rule ref="category/java/codestyle.xml/TooManyStaticImports">
+ <properties>
+ <property name="violationSuppressXPath"
+ value="//ClassOrInterfaceDeclaration[@Image='CSVFormat' or @Image='Lexer']"/>
+ </properties>
+ </rule>
+
+</ruleset>
diff --git a/src/test/java/org/apache/commons/csv/CSVPrinterTest.java b/src/test/java/org/apache/commons/csv/CSVPrinterTest.java
index 913b5ea..75253f2 100644
--- a/src/test/java/org/apache/commons/csv/CSVPrinterTest.java
+++ b/src/test/java/org/apache/commons/csv/CSVPrinterTest.java
@@ -1471,4 +1471,45 @@ public class CSVPrinterTest {
return CSVParser.parse(expected, format).getRecords().get(0).values();
}
+ /**
+ * Test to target the use of {@link IOUtils#copyLarge(java.io.Reader, Writer)} which directly
+ * buffers the value from the Reader to the Writer.
+ *
+ * <p>Requires the format to have no quote or escape character, value to be a
+ * {@link java.io.Reader Reader} and the output <i>MUST</i> be a
+ * {@link java.io.Writer Writer}.</p>
+ *
+ * @throws IOException Not expected to happen
+ */
+ @Test
+ public void testPrintReaderWithoutQuoteToWriter() throws IOException {
+ final StringWriter sw = new StringWriter();
+ final String content = "testValue";
+ try (final CSVPrinter printer = new CSVPrinter(sw, CSVFormat.DEFAULT.withQuote(null))) {
+ final StringReader value = new StringReader(content);
+ printer.print(value);
+ }
+ assertEquals(content, sw.toString());
+ }
+
+ /**
+ * Test to target the use of {@link IOUtils#copy(java.io.Reader, Appendable)} which directly
+ * buffers the value from the Reader to the Appendable.
+ *
+ * <p>Requires the format to have no quote or escape character, value to be a
+ * {@link java.io.Reader Reader} and the output <i>MUST NOT</i> be a
+ * {@link java.io.Writer Writer} but some other Appendable.</p>
+ *
+ * @throws IOException Not expected to happen
+ */
+ @Test
+ public void testPrintReaderWithoutQuoteToAppendable() throws IOException {
+ final StringBuilder sb = new StringBuilder();
+ final String content = "testValue";
+ try (final CSVPrinter printer = new CSVPrinter(sb, CSVFormat.DEFAULT.withQuote(null))) {
+ final StringReader value = new StringReader(content);
+ printer.print(value);
+ }
+ assertEquals(content, sb.toString());
+ }
}