Handle CRLF properly in the lexer #14400
Conversation
|
The alternative to treating a bare CR as an error is to treat a bare CR as if it were an LF. I went with treating it as an error because I thought a bare CR was more likely to be a mistake (since the two common EOL formats are LF and CRLF). |
|
|
|
What the... those files have the same license all the other test files do. And those CR characters are supposed to be ignored. Edit: s/tho/those/. Wat. |
|
Tidy has some nice bugs regarding CRLF handling in a repo without |
|
I hate having to support CRLF. But this looks good to me. |
|
So do I. It's an abomination. |
|
Closing due to inactivity, but it seems like this is something that definitely needs to be fixed! cc #14979 |
|
@alexcrichton How about reviewing it instead of closing it? That's all it's waiting for. |
|
r=me with rebase, thanks a lot for this! Sorry it took so long :( |
Teach StringReader how to emit errors for arbitrary spans, so we don't need to modify peek_span. This allows for emitting errors without having a &mut borrow of the StringReader.
|
Rebased (which largely means rewritten, due to the intervening refactoring). Already passed |
|
Failure: Why is this a failure? It's a fulldeps test. Shouldn't the |
|
I'm guessing this is an issue with cross-compiling. |
|
Fulldeps tests don't work on android since it's cross-compiled. Add in a |
|
@sfackler I could probably also just turn it into a regular |
The lexer already ignores CRLF in between tokens, but it doesn't properly handle carriage returns inside strings and doc comments. Teach it to treat CRLF as LF inside these tokens, and to disallow carriage returns that are not followed by linefeeds. This includes handling an escaped CRLF inside a regular string token the same way it handles an escaped LF. This is technically a breaking change, as bare carriage returns are no longer allowed, and CRLF sequences are now treated as LF inside strings and doc comments, but it's very unlikely to actually affect any real-world code. This change is necessary to have Rust code compile on Windows the same way it does on Unix. The mozilla/rust repository explicitly sets eol=lf for Rust source files, but other Rust repositories don't. Notably, rust-http cannot be compiled on Windows without converting the CRLF line endings back to LF. [breaking-change]
The lexer already ignores CRLF in between tokens, but it doesn't properly handle carriage returns inside strings and doc comments. Teach it to treat CRLF as LF inside these tokens, and to disallow carriage returns that are not followed by linefeeds. This includes handling an escaped CRLF inside a regular string token the same way it handles an escaped LF. This is technically a breaking change, as bare carriage returns are no longer allowed, and CRLF sequences are now treated as LF inside strings and doc comments, but it's very unlikely to actually affect any real-world code. This change is necessary to have Rust code compile on Windows the same way it does on Unix. The mozilla/rust repository explicitly sets eol=lf for Rust source files, but other Rust repositories don't. Notably, rust-http cannot be compiled on Windows without converting the CRLF line endings back to LF. [breaking-change]
The lexer already ignores CRLF in between tokens, but it doesn't
properly handle carriage returns inside strings and doc comments. Teach
it to treat CRLF as LF inside these tokens, and to disallow carriage
returns that are not followed by linefeeds. This includes handling an
escaped CRLF inside a regular string token the same way it handles an
escaped LF.
This is technically a breaking change, as bare carriage returns are no
longer allowed, and CRLF sequences are now treated as LF inside strings
and doc comments, but it's very unlikely to actually affect any
real-world code.
This change is necessary to have Rust code compile on Windows the same
way it does on Unix. The mozilla/rust repository explicitly sets eol=lf
for Rust source files, but other Rust repositories don't. Notably,
rust-http cannot be compiled on Windows without converting the CRLF line
endings back to LF.
[breaking-change]