java: misleading StringUtils method names

[jkronegg]

🤔 What's the problem you've observed?

The https://github.com/cucumber/gherkin/blob/main/java/src/main/java/io/cucumber/gherkin/StringUtils.java class provides methods to trim text but keeping new lines:

• StringUtils.ltrimKeepNewLines(String)
• StringUtils.rtrimKeepNewLines(String)

The method names suggest that all new lines that on the left (respectively on the right) are kept.

These two methods are working well when there is one new line to keep. However, the following unit tests shows an unexpected behavior when the text contains multiple newlines:

import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;

public class StringUtilTest {
    private static final String WHITESPACE = "\u00A0 \t";
    private static final String CUCUMBER = "🥒";

    @Test
    void testRtrimKeepNewlines() {
        assertEquals(WHITESPACE + CUCUMBER + "\n", StringUtils.rtrimKeepNewLines(WHITESPACE + CUCUMBER + "\n" + WHITESPACE));
    }

    @Test
    void testRtrimKeepNewlines_multiline() {
        assertEquals("\n" + WHITESPACE + "\n" + WHITESPACE + CUCUMBER + "\n\n",
                StringUtils.rtrimKeepNewLines("\n" + WHITESPACE + "\n" + WHITESPACE + CUCUMBER + WHITESPACE + "\n" + WHITESPACE + "\n"));
        // expected: the two WHITESPACE at the and are removed
        // actual: only one WHITESPACE is removed at the end
    }

    @Test
    void testLtrimKeepNewlines() {
        assertEquals("\n" + CUCUMBER + WHITESPACE, StringUtils.ltrimKeepNewLines(WHITESPACE + "\n" + CUCUMBER + WHITESPACE));
    }

    @Test
    void testLtrimKeepNewlines_multiline() {
        assertEquals("\n\n" + CUCUMBER + WHITESPACE + "\n" + WHITESPACE + "\n",
                StringUtils.ltrimKeepNewLines("\n" + WHITESPACE + "\n" + WHITESPACE + CUCUMBER + WHITESPACE + "\n" + WHITESPACE + "\n"));
        // expected: the two WHITESPACE at the beginning are removed
        // actual: only one WHITESPACE is removed at the beginning
    }

}

Thus, either this is:

• a bug in the method implementation (unknown impact) or
• the method name is wrong (it should be ltrimKeepNewLine without plural s, respectively rtrimKeepNewLine). Impact is that the developer may think the method removes all whitespaces, not only when there is one newline.

✨ Do you have a proposal for making it better?

Decide if this is an implementation bug or a misleading method name. Then correct accordingly.

[mpkorstanje]

Seeing that this is used exclusively in tables, I think this should be understood as "trim whitespace up to the first non-whitspace character and exclude new lines from the usual definition of whitespace".

If you want to improve the naming, please be aware of the duplicates in other languages. These would have to change too.