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.herb...@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()); + } }