From 8eb36bd8a783b85f867451b8a0a5abbb79324e88 Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Fri, 24 May 2019 08:21:21 -0400 Subject: [PATCH] - Remove unused import. - Remove trailing white spaces on all lines. - Use final. - Use for-each (in test). - Use try-with-resources (in test). - Document empty blocks. --- .../org/apache/commons/csv/CSVFormat.java | 10 +- .../org/apache/commons/csv/CSVParser.java | 13 ++- .../org/apache/commons/csv/CSVFormatTest.java | 1 + .../org/apache/commons/csv/CSVParserTest.java | 92 ++++++++++--------- .../apache/commons/csv/CSVPrinterTest.java | 3 + .../org/apache/commons/csv/LexerTest.java | 1 + 6 files changed, 64 insertions(+), 56 deletions(-) diff --git a/src/main/java/org/apache/commons/csv/CSVFormat.java b/src/main/java/org/apache/commons/csv/CSVFormat.java index 33130b0b..f5553064 100644 --- a/src/main/java/org/apache/commons/csv/CSVFormat.java +++ b/src/main/java/org/apache/commons/csv/CSVFormat.java @@ -723,7 +723,7 @@ public final class CSVFormat implements Serializable { private final boolean trim; private final boolean autoFlush; - + private final boolean allowDuplicateHeaderNames; /** @@ -1691,7 +1691,7 @@ public final class CSVFormat implements Serializable { public CSVFormat withAllowMissingColumnNames(final boolean allowMissingColumnNames) { return new CSVFormat(delimiter, quoteCharacter, quoteMode, commentMarker, escapeCharacter, ignoreSurroundingSpaces, ignoreEmptyLines, recordSeparator, nullString, headerComments, header, - skipHeaderRecord, allowMissingColumnNames, ignoreHeaderCase, trim, trailingDelimiter, autoFlush, + skipHeaderRecord, allowMissingColumnNames, ignoreHeaderCase, trim, trailingDelimiter, autoFlush, allowDuplicateHeaderNames); } @@ -2257,8 +2257,8 @@ public final class CSVFormat implements Serializable { skipHeaderRecord, allowMissingColumnNames, ignoreHeaderCase, trim, trailingDelimiter, autoFlush, allowDuplicateHeaderNames); } - - public CSVFormat withAllowDuplicateHeaderNames(boolean allowDuplicateHeaderNames) { + + public CSVFormat withAllowDuplicateHeaderNames(final boolean allowDuplicateHeaderNames) { return new CSVFormat(delimiter, quoteCharacter, quoteMode, commentMarker, escapeCharacter, ignoreSurroundingSpaces, ignoreEmptyLines, recordSeparator, nullString, headerComments, header, skipHeaderRecord, allowMissingColumnNames, ignoreHeaderCase, trim, trailingDelimiter, autoFlush, @@ -2268,7 +2268,7 @@ public final class CSVFormat implements Serializable { public CSVFormat withAllowDuplicateHeaderNames() { return withAllowDuplicateHeaderNames(true); } - + public boolean getAllowDuplicateHeaderNames() { return allowDuplicateHeaderNames; } diff --git a/src/main/java/org/apache/commons/csv/CSVParser.java b/src/main/java/org/apache/commons/csv/CSVParser.java index eff74022..c034d700 100644 --- a/src/main/java/org/apache/commons/csv/CSVParser.java +++ b/src/main/java/org/apache/commons/csv/CSVParser.java @@ -40,7 +40,6 @@ import java.util.List; import java.util.Map; import java.util.NoSuchElementException; import java.util.TreeMap; -import java.util.stream.Collectors; /** * Parses CSV files according to the specified format. @@ -410,7 +409,7 @@ public final class CSVParser implements Iterable, Closeable { this.format = format; this.lexer = new Lexer(format, new ExtendedBufferedReader(reader)); this.csvRecordIterator = new CSVRecordIterator(); - Headers headers = createHeaders(); + final Headers headers = createHeaders(); this.headerMap = headers.headerMap; this.headerNames = headers.headerNames; this.characterOffset = characterOffset; @@ -454,18 +453,18 @@ public final class CSVParser implements Iterable, Closeable { * Header column positions (0-based) */ final Map headerMap; - + /** * Header names in column order */ final List headerNames; - Headers(Map headerMap, List headerNames) { + Headers(final Map headerMap, final List headerNames) { this.headerMap = headerMap; this.headerNames = headerNames; } } - + /** * Creates the name to index mapping if the format defines a header. * @@ -502,7 +501,7 @@ public final class CSVParser implements Iterable, Closeable { if (!emptyHeader && !this.format.getAllowDuplicateHeaderNames()) { throw new IllegalArgumentException( String.format("The header contains a duplicate name: \"%s\" in %s." - + " If this is valid then use CSVFormat.withAllowDuplicateHeaderNames().", + + " If this is valid then use CSVFormat.withAllowDuplicateHeaderNames().", header, Arrays.toString(headerRecord))); } if (emptyHeader && !this.format.getAllowMissingColumnNames()) { @@ -519,7 +518,7 @@ public final class CSVParser implements Iterable, Closeable { } } } - } + } if (headerNames == null) { headerNames = Collections.emptyList(); //immutable } else { diff --git a/src/test/java/org/apache/commons/csv/CSVFormatTest.java b/src/test/java/org/apache/commons/csv/CSVFormatTest.java index a85c4411..6d30e371 100644 --- a/src/test/java/org/apache/commons/csv/CSVFormatTest.java +++ b/src/test/java/org/apache/commons/csv/CSVFormatTest.java @@ -45,6 +45,7 @@ import org.junit.Test; public class CSVFormatTest { public enum EmptyEnum { + // empty enum. } public enum Header { diff --git a/src/test/java/org/apache/commons/csv/CSVParserTest.java b/src/test/java/org/apache/commons/csv/CSVParserTest.java index f71e479d..10bb3c54 100644 --- a/src/test/java/org/apache/commons/csv/CSVParserTest.java +++ b/src/test/java/org/apache/commons/csv/CSVParserTest.java @@ -84,8 +84,8 @@ public class CSVParserTest { } private void parseFully(final CSVParser parser) { - for (final Iterator records = parser.iterator(); records.hasNext(); ) { - records.next(); + for (final CSVRecord csvRecord : parser) { + Assert.assertNotNull(csvRecord); } } @@ -310,7 +310,7 @@ public class CSVParserTest { assertTrue(parser.getHeaderNames().isEmpty()); } } - + @Test public void testEmptyFile() throws Exception { try (final CSVParser parser = CSVParser.parse("", CSVFormat.DEFAULT)) { @@ -423,6 +423,7 @@ public class CSVParserTest { /** * Tests an exported Excel worksheet with a header row and rows that have more columns than the headers + * @throws Exception */ @Test public void testExcelHeaderCountLessThanData() throws Exception { @@ -529,7 +530,7 @@ public class CSVParserTest { try { headerNames.add("This is a read-only list."); fail(); - } catch (UnsupportedOperationException e) { + } catch (final UnsupportedOperationException e) { // Yes. } } @@ -785,54 +786,57 @@ public class CSVParserTest { final String fiveRows = "1\n2\n3\n4\n5\n"; // Iterator hasNext() shouldn't break sequence - CSVParser parser = CSVFormat.DEFAULT.parse(new StringReader(fiveRows)); - int recordNumber = 0; - final Iterator iter = parser.iterator(); - recordNumber = 0; - while (iter.hasNext()) { - final CSVRecord record = iter.next(); - recordNumber++; - assertEquals(String.valueOf(recordNumber), record.get(0)); - if (recordNumber >= 2) { - break; + try (CSVParser parser = CSVFormat.DEFAULT.parse(new StringReader(fiveRows))) { + int recordNumber = 0; + final Iterator iter = parser.iterator(); + recordNumber = 0; + while (iter.hasNext()) { + final CSVRecord record = iter.next(); + recordNumber++; + assertEquals(String.valueOf(recordNumber), record.get(0)); + if (recordNumber >= 2) { + break; + } + } + iter.hasNext(); + while (iter.hasNext()) { + final CSVRecord record = iter.next(); + recordNumber++; + assertEquals(String.valueOf(recordNumber), record.get(0)); } - } - iter.hasNext(); - while (iter.hasNext()) { - final CSVRecord record = iter.next(); - recordNumber++; - assertEquals(String.valueOf(recordNumber), record.get(0)); } // Consecutive enhanced for loops shouldn't break sequence - parser = CSVFormat.DEFAULT.parse(new StringReader(fiveRows)); - recordNumber = 0; - for (final CSVRecord record : parser) { - recordNumber++; - assertEquals(String.valueOf(recordNumber), record.get(0)); - if (recordNumber >= 2) { - break; + try (CSVParser parser = CSVFormat.DEFAULT.parse(new StringReader(fiveRows))) { + int recordNumber = 0; + for (final CSVRecord record : parser) { + recordNumber++; + assertEquals(String.valueOf(recordNumber), record.get(0)); + if (recordNumber >= 2) { + break; + } + } + for (final CSVRecord record : parser) { + recordNumber++; + assertEquals(String.valueOf(recordNumber), record.get(0)); } - } - for (final CSVRecord record : parser) { - recordNumber++; - assertEquals(String.valueOf(recordNumber), record.get(0)); } // Consecutive enhanced for loops with hasNext() peeking shouldn't break sequence - parser = CSVFormat.DEFAULT.parse(new StringReader(fiveRows)); - recordNumber = 0; - for (final CSVRecord record : parser) { - recordNumber++; - assertEquals(String.valueOf(recordNumber), record.get(0)); - if (recordNumber >= 2) { - break; + try (CSVParser parser = CSVFormat.DEFAULT.parse(new StringReader(fiveRows))) { + int recordNumber = 0; + for (final CSVRecord record : parser) { + recordNumber++; + assertEquals(String.valueOf(recordNumber), record.get(0)); + if (recordNumber >= 2) { + break; + } + } + parser.iterator().hasNext(); + for (final CSVRecord record : parser) { + recordNumber++; + assertEquals(String.valueOf(recordNumber), record.get(0)); } - } - parser.iterator().hasNext(); - for (final CSVRecord record : parser) { - recordNumber++; - assertEquals(String.valueOf(recordNumber), record.get(0)); } } @@ -1166,7 +1170,7 @@ public class CSVParserTest { assertEquals("3", record.get("Z")); Assert.assertEquals(3, record.size()); } - + @Test public void testRepeatedHeadersAreReturnedInCSVRecordHeaderNames() throws IOException { final Reader in = new StringReader("header1,header2,header1\n1,2,3\n4,5,6"); diff --git a/src/test/java/org/apache/commons/csv/CSVPrinterTest.java b/src/test/java/org/apache/commons/csv/CSVPrinterTest.java index 7fe6c5cf..8966a4d7 100644 --- a/src/test/java/org/apache/commons/csv/CSVPrinterTest.java +++ b/src/test/java/org/apache/commons/csv/CSVPrinterTest.java @@ -232,6 +232,7 @@ public class CSVPrinterTest { try (final Writer writer = mock(Writer.class)) { final CSVFormat csvFormat = CSVFormat.DEFAULT; try (CSVPrinter csvPrinter = new CSVPrinter(writer, csvFormat)) { + // empty } verify(writer, never()).flush(); verify(writer, times(1)).close(); @@ -242,6 +243,7 @@ public class CSVPrinterTest { try (final Writer writer = mock(Writer.class)) { final CSVFormat csvFormat = CSVFormat.DEFAULT.withAutoFlush(false); try (CSVPrinter csvPrinter = new CSVPrinter(writer, csvFormat)) { + // empty } verify(writer, never()).flush(); verify(writer, times(1)).close(); @@ -254,6 +256,7 @@ public class CSVPrinterTest { try (final Writer writer = mock(Writer.class)) { final CSVFormat csvFormat = CSVFormat.DEFAULT.withAutoFlush(true); try (CSVPrinter csvPrinter = new CSVPrinter(writer, csvFormat)) { + // empty } verify(writer, times(1)).flush(); verify(writer, times(1)).close(); diff --git a/src/test/java/org/apache/commons/csv/LexerTest.java b/src/test/java/org/apache/commons/csv/LexerTest.java index 6e05fb90..d6023d62 100644 --- a/src/test/java/org/apache/commons/csv/LexerTest.java +++ b/src/test/java/org/apache/commons/csv/LexerTest.java @@ -50,6 +50,7 @@ public class LexerTest { formatWithEscaping = CSVFormat.DEFAULT.withEscape('\\'); } + @SuppressWarnings("resource") private Lexer createLexer(final String input, final CSVFormat format) { return new Lexer(format, new ExtendedBufferedReader(new StringReader(input))); }