Suggest using string.lines() when string.split("\n"), or string.split("\r\n") is seen

[cowlicks]

What it does

Suggest replacing string.split("\n"), string.split('\n'), and string.split("\r\n") with string.lines().

Note that clippy throws a single_char_split warning for string.split("\n") which should be updated to this.

Lint Name

non_portable_line_iteration

Category

suspicious, style

Advantage

Makes string line iteration portable across Unix and Windows.

Drawbacks

If you need to differentiate between platform specific newlines this will be a false positive.
Also, if the string has a final terminating newline, the result of a .split will differ from .lines.
This could be avoiding by having the lint only trigger on .trim().split(sep).

Example

for line in string.split("\n") { ... }
// or
for line in string.split("\r\n") { ... }
// or
for line in string.split('\n') { ... }

Could be written as:

for line in string.lines() { ... }

[ghost]

It's not equivalent.

fn main() {
    let text = "foo\r\nbar\n\nbaz\n";
    println!("{:?}", text.split("\n").collect::<Vec<_>>());
    println!("{:?}", text.split("\r\n").collect::<Vec<_>>());
    println!("{:?}", text.lines().collect::<Vec<_>>());
}

gives

["foo\r", "bar", "", "baz", ""]
["foo", "bar\n\nbaz\n"]
["foo", "bar", "", "baz"]

Even when the line endings aren't mixed, lines doesn't include the empty element after the final \n.

[fedemartinezdev]

@rustbot claim

[yonip23]

@mikerite so what are you suggesting? Maybe this lint can go into the "pedantic" category, with an appropriate message stating that they're not equivalent, but maybe you can rewrite a bit in order to use the lines api?

[yonip23]

Also, if this lint is still not taken I'd like to claim it 🙂
(Not sure if / what exactly needs to be done yet, but anyway haha)

[ghost]

I think this lint shouldn't exist or it should go into restriction.

The line endings and final blank element won't matter 99% of the time. When it does, the suggestion introduce bugs.

[joshuachp]

@rustbot claim

[cowlicks]

It's not equivalent.

Indeed it is not. Good point about the final "", I'll update the issue. However I think in most cases where someone is doing this, they actually want what .lines() does, but they just don't know it exists. I think most users are not interested in the final "" you get from a case like "foo\nbar\n".split("\n"). Likewise I think in most cases the user is probably actually wants to handle "\r\n" too.

Perhaps a good middle ground would be to only suggest this lint whenever we see foo.trim().split(sep). If you can see from this search that there is a lot of code in the wild that does this.

[torfsen]

@rustbot claim

[torfsen]

Perhaps a good middle ground would be to only suggest this lint whenever we see foo.trim().split(sep). If you can see from this search that there is a lot of code in the wild that does this.

Just for my own records, right now the GitHub search finds

• 1.4k files containing .trim.split('\n')
• 832 files containing .trim.split("\n")
• 9 files containing .trim.split("\r\n")