Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion src/main/java/org/apache/commons/csv/CSVFormat.java
Original file line number Diff line number Diff line change
Expand Up @@ -2134,6 +2134,15 @@ public boolean isEscapeCharacterSet() {
return escapeCharacter != null;
}

/**
* Tests whether the quote policy encapsulates values only when needed, which is also the policy when none is set.
*
* @return {@code true} if the {@link QuoteMode} is {@link QuoteMode#MINIMAL} or unset.
*/
private boolean isMinimalQuoteMode() {
return quoteMode == null || quoteMode == QuoteMode.MINIMAL;
}

/**
* Tests whether a null string has been defined.
*
Expand Down Expand Up @@ -2272,7 +2281,17 @@ private void print(final Object object, final CharSequence value, final Appendab
out.append(getDelimiterString());
}
if (object == null) {
out.append(value);
if (len == 0 && newRecord && isQuoteCharacterSet() && isMinimalQuoteMode()) {
// Encapsulate like printWithQuotes does for an empty value that starts a record: an
// unquoted one makes the whole line empty, and a parser with ignoreEmptyLines enabled
// then drops the record. ALL, ALL_NON_NULL, and NON_NUMERIC are excluded because they
// encode null as the bare empty field, distinct from a quoted empty string (CSV-203).
final char quoteChar = quoteCharacter.charValue(); // Explicit unboxing is intentional
out.append(quoteChar);
out.append(quoteChar);
} else {
out.append(value);
}
Comment on lines +2284 to +2294

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extending this to ALL breaks JiraCsv203Test.testWithoutNullString (CSV-203), which pins that ALL writes null as the bare empty field so it stays distinct from a quoted "". That's the same encoding rationale as ALL_NON_NULL, so ALL stays excluded. Updated the code comment to say so and added a test assertion pinning it.

} else if (isQuoteCharacterSet()) {
// The original object is needed so can check for Number
printWithQuotes(object, value, out, newRecord);
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/org/apache/commons/csv/CSVFormatTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1557,7 +1557,7 @@ void testWithHeaderComments() {
assertNotEquals(csvFormat, csvFormatTwo); // CSV-244 - should not be equal

assertNotEquals(csvFormatTwo, csvFormat); // CSV-244 - should not be equal
assertEquals(",,,,,,,", string);
assertEquals("\"\",,,,,,,", string);

}

Expand Down
38 changes: 36 additions & 2 deletions src/test/java/org/apache/commons/csv/CSVPrinterTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,14 @@ private Connection getH2Connection() throws SQLException, ClassNotFoundException
return DriverManager.getConnection("jdbc:h2:mem:my_test;", "sa", "");
}

private String printNullRecord(final CSVFormat format) throws IOException {
final StringWriter sw = new StringWriter();
try (CSVPrinter printer = new CSVPrinter(sw, format)) {
printer.printRecord((Object) null);
}
return sw.toString();
}

private CSVPrinter printWithHeaderComments(final StringWriter sw, final Date now, final CSVFormat baseFormat) throws IOException {
// Use withHeaderComments first to test CSV-145
// @formatter:off
Expand Down Expand Up @@ -1721,6 +1729,31 @@ void testPrinter7() throws IOException {
}
}

@Test
void testPrintNullValueStartingRecord() throws IOException {
Comment on lines +1732 to +1733

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added an ALL assertion. It pins the current behavior: null stays the bare empty field per CSV-203, since quoting it would break the null vs "" distinction that JiraCsv203Test checks.

final StringWriter sw = new StringWriter();
try (CSVPrinter printer = new CSVPrinter(sw, CSVFormat.DEFAULT)) {
printer.printRecord("a");
printer.printRecord((Object) null);
printer.printRecord("b");
}
final String csv = sw.toString();
assertEquals("a" + RECORD_SEPARATOR + "\"\"" + RECORD_SEPARATOR + "b" + RECORD_SEPARATOR, csv);
try (CSVParser parser = CSVParser.parse(csv, CSVFormat.DEFAULT)) {
final List<CSVRecord> records = parser.getRecords();
assertEquals(3, records.size());
assertArrayEquals(new String[] { "" }, records.get(1).values());
}
// An explicit MINIMAL quote mode encapsulates it too.
assertEquals("\"\"" + RECORD_SEPARATOR, printNullRecord(CSVFormat.DEFAULT.builder().setQuoteMode(QuoteMode.MINIMAL).get()));
// ALL encodes null as the bare empty field, distinct from a quoted empty string (CSV-203).
assertEquals(RECORD_SEPARATOR, printNullRecord(CSVFormat.DEFAULT.builder().setQuoteMode(QuoteMode.ALL).get()));
// ALL_NON_NULL encodes null as the bare empty field, so it must stay unquoted.
assertEquals(RECORD_SEPARATOR, printNullRecord(CSVFormat.DEFAULT.builder().setQuoteMode(QuoteMode.ALL_NON_NULL).get()));
// Without a quote character there is nothing to encapsulate with.
assertEquals(RECORD_SEPARATOR, printNullRecord(CSVFormat.DEFAULT.builder().setQuote(null).get()));
Comment on lines +1747 to +1754

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Covered by the new ALL assertion above, pinning the bare empty field per CSV-203.

}

@Test
void testPrintNullValues() throws IOException {
final StringWriter sw = new StringWriter();
Expand Down Expand Up @@ -1854,8 +1887,9 @@ void testPrintRecordsWithObjectArray() throws IOException {
printer.printRecords(objectArray);
assertEquals(objectArray.length, printer.getRecordCount());
}
assertEquals(6, charArrayWriter.size());
assertEquals("\n\n\n\n\n\n", charArrayWriter.toString());
final String expected = "\"\"\n\"\"\n\"\"\n\n\"\"\n\"\"\n";
assertEquals(expected.length(), charArrayWriter.size());
assertEquals(expected, charArrayWriter.toString());
}

@Test
Expand Down
Loading