Skip to content

Commit 03550ab

Browse files
aherbertgarydgregory
aherbert
authored and
garydgregory
committed
Post release fixes (apache#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.
1 parent 42b9fdb commit 03550ab

7 files changed

Lines changed: 234 additions & 8 deletions

File tree

‎pom.xml‎

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,8 +153,9 @@ CSV files of various types.
153153

154154
<checkstyle.version>3.0.0</checkstyle.version>
155155
<checkstyle.header.file>${basedir}/LICENSE-header.txt</checkstyle.header.file>
156-
<checkstyle.resourceExcludes>LICENSE.txt, NOTICE.txt</checkstyle.resourceExcludes>
156+
<checkstyle.resourceExcludes>LICENSE.txt, NOTICE.txt, **/maven-archiver/pom.properties</checkstyle.resourceExcludes>
157157

158+
<pmd.version>3.12.0</pmd.version>
158159
<japicmp.skip>false</japicmp.skip>
159160

160161
<commons.release.isDistModule>true</commons.release.isDistModule>
@@ -200,6 +201,32 @@ CSV files of various types.
200201
<configuration>
201202
<configLocation>${basedir}/checkstyle.xml</configLocation>
202203
<enableRulesSummary>false</enableRulesSummary>
204+
<suppressionsLocation>${basedir}/src/main/resources/checkstyle/checkstyle-suppressions.xml</suppressionsLocation>
205+
</configuration>
206+
</plugin>
207+
<!-- Allow findbugs to be run interactively; keep in sync with report config below -->
208+
<plugin>
209+
<groupId>org.codehaus.mojo</groupId>
210+
<artifactId>findbugs-maven-plugin</artifactId>
211+
<version>${commons.findbugs.version}</version>
212+
<configuration>
213+
<threshold>Normal</threshold>
214+
<effort>Default</effort>
215+
<excludeFilterFile>${basedir}/src/main/resources/findbugs/findbugs-exclude-filter.xml</excludeFilterFile>
216+
</configuration>
217+
</plugin>
218+
<!-- Allow pmd to be run interactively; keep in sync with report config below -->
219+
<plugin>
220+
<groupId>org.apache.maven.plugins</groupId>
221+
<artifactId>maven-pmd-plugin</artifactId>
222+
<version>${pmd.version}</version>
223+
<configuration>
224+
<targetJdk>${maven.compiler.target}</targetJdk>
225+
<skipEmptyReport>false</skipEmptyReport>
226+
<analysisCache>true</analysisCache>
227+
<rulesets>
228+
<ruleset>${basedir}/src/main/resources/pmd/pmd-ruleset.xml</ruleset>
229+
</rulesets>
203230
</configuration>
204231
</plugin>
205232

@@ -244,6 +271,7 @@ CSV files of various types.
244271
<configuration>
245272
<configLocation>${basedir}/checkstyle.xml</configLocation>
246273
<enableRulesSummary>false</enableRulesSummary>
274+
<suppressionsLocation>${basedir}/src/main/resources/checkstyle/checkstyle-suppressions.xml</suppressionsLocation>
247275
</configuration>
248276
<!-- We need to specify reportSets because 2.9.1 creates two reports -->
249277
<reportSets>
@@ -257,15 +285,25 @@ CSV files of various types.
257285
<plugin>
258286
<groupId>org.apache.maven.plugins</groupId>
259287
<artifactId>maven-pmd-plugin</artifactId>
260-
<version>3.12.0</version>
288+
<version>${pmd.version}</version>
261289
<configuration>
262290
<targetJdk>${maven.compiler.target}</targetJdk>
291+
<skipEmptyReport>false</skipEmptyReport>
292+
<analysisCache>true</analysisCache>
293+
<rulesets>
294+
<ruleset>${basedir}/src/main/resources/pmd/pmd-ruleset.xml</ruleset>
295+
</rulesets>
263296
</configuration>
264297
</plugin>
265298
<plugin>
266299
<groupId>org.codehaus.mojo</groupId>
267300
<artifactId>findbugs-maven-plugin</artifactId>
268301
<version>${commons.findbugs.version}</version>
302+
<configuration>
303+
<threshold>Normal</threshold>
304+
<effort>Default</effort>
305+
<excludeFilterFile>${basedir}/src/main/resources/findbugs/findbugs-exclude-filter.xml</excludeFilterFile>
306+
</configuration>
269307
</plugin>
270308
<plugin>
271309
<groupId>org.codehaus.mojo</groupId>

‎src/main/java/org/apache/commons/csv/CSVFormat.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -892,7 +892,7 @@ public boolean equals(final Object obj) {
892892
*/
893893
public String format(final Object... values) {
894894
final StringWriter out = new StringWriter();
895-
try (final CSVPrinter csvPrinter = new CSVPrinter(out, this)) {
895+
try (CSVPrinter csvPrinter = new CSVPrinter(out, this)) {
896896
csvPrinter.printRecord(values);
897897
return out.toString().trim();
898898
} catch (final IOException e) {
@@ -1713,7 +1713,7 @@ private void validate() throws IllegalArgumentException {
17131713
* @since 1.7
17141714
*/
17151715
public CSVFormat withAllowDuplicateHeaderNames() {
1716-
return withAllowDuplicateHeaderNames(true);
1716+
return withAllowDuplicateHeaderNames(true);
17171717
}
17181718

17191719
/**
@@ -1724,7 +1724,7 @@ public CSVFormat withAllowDuplicateHeaderNames() {
17241724
* @since 1.7
17251725
*/
17261726
public CSVFormat withAllowDuplicateHeaderNames(final boolean allowDuplicateHeaderNames) {
1727-
return new CSVFormat(delimiter, quoteCharacter, quoteMode, commentMarker, escapeCharacter,
1727+
return new CSVFormat(delimiter, quoteCharacter, quoteMode, commentMarker, escapeCharacter,
17281728
ignoreSurroundingSpaces, ignoreEmptyLines, recordSeparator, nullString, headerComments, header,
17291729
skipHeaderRecord, allowMissingColumnNames, ignoreHeaderCase, trim, trailingDelimiter, autoFlush,
17301730
allowDuplicateHeaderNames);

‎src/main/java/org/apache/commons/csv/CSVParser.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,7 @@ private Headers createHeaders() throws IOException {
495495
if (headerRecord != null) {
496496
for (int i = 0; i < headerRecord.length; i++) {
497497
final String header = headerRecord[i];
498-
final boolean containsHeader = header == null ? false : hdrMap.containsKey(header);
498+
final boolean containsHeader = header != null && hdrMap.containsKey(header);
499499
final boolean emptyHeader = header == null || header.trim().isEmpty();
500500
if (containsHeader) {
501501
if (!emptyHeader && !this.format.getAllowDuplicateHeaderNames()) {
@@ -520,9 +520,9 @@ private Headers createHeaders() throws IOException {
520520
}
521521
}
522522
if (headerNames == null) {
523-
headerNames = Collections.emptyList(); //immutable
523+
headerNames = Collections.emptyList(); //immutable
524524
} else {
525-
headerNames = Collections.unmodifiableList(headerNames);
525+
headerNames = Collections.unmodifiableList(headerNames);
526526
}
527527
return new Headers(hdrMap, headerNames);
528528
}

‎src/main/resources/checkstyle/checkstyle-suppressions.xml‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?xml version="1.0"?>
2+
<!--
3+
Licensed to the Apache Software Foundation (ASF) under one or more
4+
contributor license agreements. See the NOTICE file distributed with
5+
this work for additional information regarding copyright ownership.
6+
The ASF licenses this file to You under the Apache License, Version 2.0
7+
(the "License"); you may not use this file except in compliance with
8+
the License. You may obtain a copy of the License at
9+
10+
http://www.apache.org/licenses/LICENSE-2.0
11+
12+
Unless required by applicable law or agreed to in writing, software
13+
distributed under the License is distributed on an "AS IS" BASIS,
14+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
See the License for the specific language governing permissions and
16+
limitations under the License.
17+
-->
18+
<!DOCTYPE suppressions PUBLIC
19+
"-//Checkstyle//DTD SuppressionFilter Configuration 1.2//EN"
20+
"https://checkstyle.org/dtds/suppressions_1_2.dtd">
21+
<suppressions>
22+
<suppress checks="LineLength" files="[\\/]CSVParser\.java$" lines="504"/>
23+
</suppressions>

‎src/main/resources/findbugs/findbugs-exclude-filter.xml‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<?xml version="1.0"?>
2+
<!--
3+
Licensed to the Apache Software Foundation (ASF) under one or more
4+
contributor license agreements. See the NOTICE file distributed with
5+
this work for additional information regarding copyright ownership.
6+
The ASF licenses this file to You under the Apache License, Version 2.0
7+
(the "License"); you may not use this file except in compliance with
8+
the License. You may obtain a copy of the License at
9+
10+
http://www.apache.org/licenses/LICENSE-2.0
11+
12+
Unless required by applicable law or agreed to in writing, software
13+
distributed under the License is distributed on an "AS IS" BASIS,
14+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
See the License for the specific language governing permissions and
16+
limitations under the License.
17+
-->
18+
19+
<!--
20+
This file contains some false positive bugs detected by spotbugs. Their
21+
false positive nature has been analyzed individually and they have been
22+
put here to instruct spotbugs it must ignore them.
23+
-->
24+
<FindBugsFilter
25+
xmlns="https://github.com/spotbugs/filter/3.0.0"
26+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
27+
xsi:schemaLocation="https://github.com/spotbugs/filter/3.0.0 https://raw.githubusercontent.com/spotbugs/spotbugs/3.1.0/spotbugs/etc/findbugsfilter.xsd">
28+
29+
<Match>
30+
<Class name="org.apache.commons.csv.CSVPrinter"/>
31+
<BugPattern name="SF_SWITCH_FALLTHROUGH"/>
32+
<Method name="printComment"/>
33+
</Match>
34+
35+
</FindBugsFilter>

‎src/main/resources/pmd/pmd-ruleset.xml‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
<?xml version="1.0"?>
2+
<!--
3+
Licensed to the Apache Software Foundation (ASF) under one or more
4+
contributor license agreements. See the NOTICE file distributed with
5+
this work for additional information regarding copyright ownership.
6+
The ASF licenses this file to You under the Apache License, Version 2.0
7+
(the "License"); you may not use this file except in compliance with
8+
the License. You may obtain a copy of the License at
9+
10+
http://www.apache.org/licenses/LICENSE-2.0
11+
12+
Unless required by applicable law or agreed to in writing, software
13+
distributed under the License is distributed on an "AS IS" BASIS,
14+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
See the License for the specific language governing permissions and
16+
limitations under the License.
17+
-->
18+
<ruleset name="commons-rng-customized"
19+
xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
20+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
21+
xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 http://pmd.sourceforge.net/ruleset_2_0_0.xsd">
22+
<description>
23+
This ruleset checks the code for discouraged programming constructs.
24+
</description>
25+
26+
<!-- Default ruleset for maven-pmd-plugin -->
27+
<rule ref="category/java/bestpractices.xml/AvoidUsingHardCodedIP"/>
28+
<rule ref="category/java/bestpractices.xml/CheckResultSet"/>
29+
<rule ref="category/java/bestpractices.xml/UnusedImports"/>
30+
<rule ref="category/java/bestpractices.xml/UnusedFormalParameter"/>
31+
<rule ref="category/java/bestpractices.xml/UnusedLocalVariable"/>
32+
<rule ref="category/java/bestpractices.xml/UnusedPrivateField"/>
33+
<rule ref="category/java/bestpractices.xml/UnusedPrivateMethod"/>
34+
<rule ref="category/java/codestyle.xml/DontImportJavaLang"/>
35+
<rule ref="category/java/codestyle.xml/DuplicateImports"/>
36+
<rule ref="category/java/codestyle.xml/ExtendsObject"/>
37+
<rule ref="category/java/codestyle.xml/ForLoopShouldBeWhileLoop"/>
38+
<rule ref="category/java/codestyle.xml/TooManyStaticImports"/>
39+
<rule ref="category/java/codestyle.xml/UnnecessaryFullyQualifiedName"/>
40+
<rule ref="category/java/codestyle.xml/UnnecessaryModifier"/>
41+
<rule ref="category/java/codestyle.xml/UnnecessaryReturn"/>
42+
<rule ref="category/java/codestyle.xml/UselessParentheses"/>
43+
<rule ref="category/java/codestyle.xml/UselessQualifiedThis"/>
44+
<rule ref="category/java/design.xml/CollapsibleIfStatements"/>
45+
<rule ref="category/java/design.xml/SimplifiedTernary"/>
46+
<rule ref="category/java/design.xml/UselessOverridingMethod"/>
47+
<rule ref="category/java/errorprone.xml/AvoidBranchingStatementAsLastInLoop"/>
48+
<rule ref="category/java/errorprone.xml/AvoidDecimalLiteralsInBigDecimalConstructor"/>
49+
<rule ref="category/java/errorprone.xml/AvoidMultipleUnaryOperators"/>
50+
<rule ref="category/java/errorprone.xml/AvoidUsingOctalValues"/>
51+
<rule ref="category/java/errorprone.xml/BrokenNullCheck"/>
52+
<rule ref="category/java/errorprone.xml/CheckSkipResult"/>
53+
<rule ref="category/java/errorprone.xml/ClassCastExceptionWithToArray"/>
54+
<rule ref="category/java/errorprone.xml/DontUseFloatTypeForLoopIndices"/>
55+
<rule ref="category/java/errorprone.xml/EmptyCatchBlock"/>
56+
<rule ref="category/java/errorprone.xml/EmptyFinallyBlock"/>
57+
<rule ref="category/java/errorprone.xml/EmptyIfStmt"/>
58+
<rule ref="category/java/errorprone.xml/EmptyInitializer"/>
59+
<rule ref="category/java/errorprone.xml/EmptyStatementBlock"/>
60+
<rule ref="category/java/errorprone.xml/EmptyStatementNotInLoop"/>
61+
<rule ref="category/java/errorprone.xml/EmptySwitchStatements"/>
62+
<rule ref="category/java/errorprone.xml/EmptySynchronizedBlock"/>
63+
<rule ref="category/java/errorprone.xml/EmptyTryBlock"/>
64+
<rule ref="category/java/errorprone.xml/EmptyWhileStmt"/>
65+
<rule ref="category/java/errorprone.xml/ImportFromSamePackage"/>
66+
<rule ref="category/java/errorprone.xml/JumbledIncrementer"/>
67+
<rule ref="category/java/errorprone.xml/MisplacedNullCheck"/>
68+
<rule ref="category/java/errorprone.xml/OverrideBothEqualsAndHashcode"/>
69+
<rule ref="category/java/errorprone.xml/ReturnFromFinallyBlock"/>
70+
<rule ref="category/java/errorprone.xml/UnconditionalIfStatement"/>
71+
<rule ref="category/java/errorprone.xml/UnnecessaryConversionTemporary"/>
72+
<rule ref="category/java/errorprone.xml/UnusedNullCheckInEquals"/>
73+
<rule ref="category/java/errorprone.xml/UselessOperationOnImmutable"/>
74+
<rule ref="category/java/multithreading.xml/AvoidThreadGroup"/>
75+
<rule ref="category/java/multithreading.xml/DontCallThreadRun"/>
76+
<rule ref="category/java/multithreading.xml/DoubleCheckedLocking"/>
77+
<rule ref="category/java/performance.xml/BigIntegerInstantiation"/>
78+
<rule ref="category/java/performance.xml/BooleanInstantiation"/>
79+
80+
<!-- Rule customisations. -->
81+
82+
<rule ref="category/java/codestyle.xml/TooManyStaticImports">
83+
<properties>
84+
<property name="violationSuppressXPath"
85+
value="//ClassOrInterfaceDeclaration[@Image='CSVFormat' or @Image='Lexer']"/>
86+
</properties>
87+
</rule>
88+
89+
</ruleset>

‎src/test/java/org/apache/commons/csv/CSVPrinterTest.java‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1471,4 +1471,45 @@ private String[] toFirstRecordValues(final String expected, final CSVFormat form
14711471
return CSVParser.parse(expected, format).getRecords().get(0).values();
14721472
}
14731473

1474+
/**
1475+
* Test to target the use of {@link IOUtils#copyLarge(java.io.Reader, Writer)} which directly
1476+
* buffers the value from the Reader to the Writer.
1477+
*
1478+
* <p>Requires the format to have no quote or escape character, value to be a
1479+
* {@link java.io.Reader Reader} and the output <i>MUST</i> be a
1480+
* {@link java.io.Writer Writer}.</p>
1481+
*
1482+
* @throws IOException Not expected to happen
1483+
*/
1484+
@Test
1485+
public void testPrintReaderWithoutQuoteToWriter() throws IOException {
1486+
final StringWriter sw = new StringWriter();
1487+
final String content = "testValue";
1488+
try (final CSVPrinter printer = new CSVPrinter(sw, CSVFormat.DEFAULT.withQuote(null))) {
1489+
final StringReader value = new StringReader(content);
1490+
printer.print(value);
1491+
}
1492+
assertEquals(content, sw.toString());
1493+
}
1494+
1495+
/**
1496+
* Test to target the use of {@link IOUtils#copy(java.io.Reader, Appendable)} which directly
1497+
* buffers the value from the Reader to the Appendable.
1498+
*
1499+
* <p>Requires the format to have no quote or escape character, value to be a
1500+
* {@link java.io.Reader Reader} and the output <i>MUST NOT</i> be a
1501+
* {@link java.io.Writer Writer} but some other Appendable.</p>
1502+
*
1503+
* @throws IOException Not expected to happen
1504+
*/
1505+
@Test
1506+
public void testPrintReaderWithoutQuoteToAppendable() throws IOException {
1507+
final StringBuilder sb = new StringBuilder();
1508+
final String content = "testValue";
1509+
try (final CSVPrinter printer = new CSVPrinter(sb, CSVFormat.DEFAULT.withQuote(null))) {
1510+
final StringReader value = new StringReader(content);
1511+
printer.print(value);
1512+
}
1513+
assertEquals(content, sb.toString());
1514+
}
14741515
}

0 commit comments

Comments
 (0)