Warn about non-printable characters in string literals

[matklad]

Currently a string literal with control characters like \0 or \v is accepted without any warnings. The only exception is \r, which gives a hard error.

It makes more sense to treat all non-printable, non \t, \n ASCII characters as a warning.

Steps to fix:

1. Add NonPrintableAsii to EscapeError
2. Produce this error somewhere around here
3. Add lexer-level tests
4. Handle this "error" in unespcape_error_reporting. Note that, unlike other real errors, this one should be just a warning.
5. Adjust the affected ui tests

I am not sure how to make this warning work with #[allow] lint infrastructure: we definitely can't do this in the lexer.

[matklad]

cc @petrochenkov

@rustbot modify labels: +A-parser, +A-diagnostics, +E-medium

[petrochenkov]

I am not sure how to make this warning work with #[allow] lint infrastructure: we definitely can't do this in the lexer.

Attach the lint to CRATE_NODE_ID, then it'll be able to be enabled/disabled at crate level, but not with higher granularity (some existing early lints behave this way).

[basil-cow]

@rustbot claim

[basil-cow]

I'm not sure how to handle this: callers assume that errors unescape_str produces are hard errors, so if we don't span_err on it, that inconsistency causes an ICE later in type deduction (maybe inconsistent array length?). At least that what I think is happening, not entirely sure. Should I change output type to something like Ok(c) | Warn(c, warn) | Err(err)?

[matklad]

Hm, indeed, unescape_str and friends interface is not fully-prepared for emitting warnings.

One approach here is to extend the interface. I think something like this will be cleanest:

struct EscapeError {
    fallback: char,
    kind: EscapeErrorKind,
}

impl EscapeErrorKind {
    fn is_warning(&self) -> bool;
}

That is, even if we emit error, we still produce a fallback char that should be inserted into the string.

However, this particular warning can be handled with less machinery. At the call-site, we check if char is an unprintable character, and range is of length one. Range length seems seems like a hacky, but correct way to figure out if the char was escaped.

The benefit of the first approach is that it centralizes the knowledge about this warning to the rustc_lexer crate. The drawback is that it makes an already highly awkward interface even more cumbersome.

The benefit of the second approach is simplicity. Moreover, it can be argued that warnings not necessary belong to rustc_lexer, as they don't affect the language definition.

I personally would prefer to start with the second approach: it seems significantly easier to implement, and is very isolated, so changing it to the second one in the future would be easy. OTOH, starting with 2 and simplifying to 1 would be much harder to do.

[Alexendoo]

Triage: Hi, are you still working on this issue @Areredify?

[basil-cow]

@Alexendoo nope, you can grab it, check out my pr earlier for reference if you want

[basil-cow]

oh, sorry, it's a triage, I thought you wanted to grab it 😅

[Alexendoo]

@rustbot release-assignment