Skip to content

Commit

Permalink
Auto merge of #10231 - Alexendoo:regex-spans, r=llogiq
Browse files Browse the repository at this point in the history 
`invalid_regex`: Show full error when string value doesn't match source

changelog: [`invalid_regex`]: Show full error when parsing non-literals or regular strings containing escape sequences

Fixes #4170, the escape sequence there causes the span to be incorrect which will have caused most of the confusion
  • Loading branch information
@bors
bors committed Jan 29, 2023
2 parents 1480cea + 986f40f commit 96c28d1
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 62 deletions.
89 changes: 44 additions & 45 deletions clippy_lints/src/regex.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
use std::fmt::Display;

use clippy_utils::consts::{constant, Constant};
use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
use clippy_utils::source::snippet_opt;
use clippy_utils::{match_def_path, paths};
use if_chain::if_chain;
use rustc_ast::ast::{LitKind, StrStyle};
Expand Down Expand Up @@ -77,13 +80,45 @@ impl<'tcx> LateLintPass<'tcx> for Regex {
}
}

#[must_use]
fn str_span(base: Span, c: regex_syntax::ast::Span, offset: u8) -> Span {
let offset = u32::from(offset);
let end = base.lo() + BytePos(u32::try_from(c.end.offset).expect("offset too large") + offset);
let start = base.lo() + BytePos(u32::try_from(c.start.offset).expect("offset too large") + offset);
assert!(start <= end);
Span::new(start, end, base.ctxt(), base.parent())
fn lint_syntax_error(cx: &LateContext<'_>, error: &regex_syntax::Error, unescaped: &str, base: Span, offset: u8) {
let parts: Option<(_, _, &dyn Display)> = match &error {
regex_syntax::Error::Parse(e) => Some((e.span(), e.auxiliary_span(), e.kind())),
regex_syntax::Error::Translate(e) => Some((e.span(), None, e.kind())),
_ => None,
};

let convert_span = |regex_span: &regex_syntax::ast::Span| {
let offset = u32::from(offset);
let start = base.lo() + BytePos(u32::try_from(regex_span.start.offset).expect("offset too large") + offset);
let end = base.lo() + BytePos(u32::try_from(regex_span.end.offset).expect("offset too large") + offset);

Span::new(start, end, base.ctxt(), base.parent())
};

if let Some((primary, auxiliary, kind)) = parts
&& let Some(literal_snippet) = snippet_opt(cx, base)
&& let Some(inner) = literal_snippet.get(offset as usize..)
// Only convert to native rustc spans if the parsed regex matches the
// source snippet exactly, to ensure the span offsets are correct
&& inner.get(..unescaped.len()) == Some(unescaped)
{
let spans = if let Some(auxiliary) = auxiliary {
vec![convert_span(primary), convert_span(auxiliary)]
} else {
vec![convert_span(primary)]
};

span_lint(cx, INVALID_REGEX, spans, &format!("regex syntax error: {kind}"));
} else {
span_lint_and_help(
cx,
INVALID_REGEX,
base,
&error.to_string(),
None,
"consider using a raw string literal: `r\"..\"`",
);
}
}

fn const_str<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> Option<String> {
Expand Down Expand Up @@ -155,25 +190,7 @@ fn check_regex<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, utf8: bool) {
span_lint_and_help(cx, TRIVIAL_REGEX, expr.span, "trivial regex", None, repl);
}
},
Err(regex_syntax::Error::Parse(e)) => {
span_lint(
cx,
INVALID_REGEX,
str_span(expr.span, *e.span(), offset),
&format!("regex syntax error: {}", e.kind()),
);
},
Err(regex_syntax::Error::Translate(e)) => {
span_lint(
cx,
INVALID_REGEX,
str_span(expr.span, *e.span(), offset),
&format!("regex syntax error: {}", e.kind()),
);
},
Err(e) => {
span_lint(cx, INVALID_REGEX, expr.span, &format!("regex syntax error: {e}"));
},
Err(e) => lint_syntax_error(cx, &e, r, expr.span, offset),
}
}
} else if let Some(r) = const_str(cx, expr) {
Expand All @@ -183,25 +200,7 @@ fn check_regex<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, utf8: bool) {
span_lint_and_help(cx, TRIVIAL_REGEX, expr.span, "trivial regex", None, repl);
}
},
Err(regex_syntax::Error::Parse(e)) => {
span_lint(
cx,
INVALID_REGEX,
expr.span,
&format!("regex syntax error on position {}: {}", e.span().start.offset, e.kind()),
);
},
Err(regex_syntax::Error::Translate(e)) => {
span_lint(
cx,
INVALID_REGEX,
expr.span,
&format!("regex syntax error on position {}: {}", e.span().start.offset, e.kind()),
);
},
Err(e) => {
span_lint(cx, INVALID_REGEX, expr.span, &format!("regex syntax error: {e}"));
},
Err(e) => span_lint(cx, INVALID_REGEX, expr.span, &e.to_string()),
}
}
}
4 changes: 4 additions & 0 deletions tests/ui/regex.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ fn syntax_error() {

let raw_string_error = Regex::new(r"[...\/...]");
let raw_string_error = Regex::new(r#"[...\/...]"#);

let escaped_string_span = Regex::new("\\b\\c");

let aux_span = Regex::new("(?ixi)");
}

fn trivial_regex() {
Expand Down
66 changes: 49 additions & 17 deletions tests/ui/regex.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ error: regex syntax error: invalid character class range, the start must be <= t
LL | let some_unicode = Regex::new("[é-è]");
| ^^^

error: regex syntax error on position 0: unclosed group
error: regex parse error:
(
^
error: unclosed group
--> $DIR/regex.rs:18:33
|
LL | let some_regex = Regex::new(OPENING_PAREN);
Expand All @@ -43,25 +46,37 @@ LL | let binary_pipe_in_wrong_position = BRegex::new("|");
|
= help: the regex is unlikely to be useful as it is

error: regex syntax error on position 0: unclosed group
error: regex parse error:
(
^
error: unclosed group
--> $DIR/regex.rs:21:41
|
LL | let some_binary_regex = BRegex::new(OPENING_PAREN);
| ^^^^^^^^^^^^^

error: regex syntax error on position 0: unclosed group
error: regex parse error:
(
^
error: unclosed group
--> $DIR/regex.rs:22:56
|
LL | let some_binary_regex_builder = BRegexBuilder::new(OPENING_PAREN);
| ^^^^^^^^^^^^^

error: regex syntax error on position 0: unclosed group
error: regex parse error:
(
^
error: unclosed group
--> $DIR/regex.rs:34:37
|
LL | let set_error = RegexSet::new(&[OPENING_PAREN, r"[a-z]+/.(com|org|net)"]);
| ^^^^^^^^^^^^^

error: regex syntax error on position 0: unclosed group
error: regex parse error:
(
^
error: unclosed group
--> $DIR/regex.rs:35:39
|
LL | let bset_error = BRegexSet::new(&[OPENING_PAREN, r"[a-z]+/.(com|org|net)"]);
Expand All @@ -79,93 +94,110 @@ error: regex syntax error: unrecognized escape sequence
LL | let raw_string_error = Regex::new(r#"[...//...]"#);
| ^^

error: regex parse error:
/b/c
^^
error: unrecognized escape sequence
--> $DIR/regex.rs:40:42
|
LL | let escaped_string_span = Regex::new("/b/c");
| ^^^^^^^^
|
= help: consider using a raw string literal: `r".."`

error: regex syntax error: duplicate flag
--> $DIR/regex.rs:42:34
|
LL | let aux_span = Regex::new("(?ixi)");
| ^ ^

error: trivial regex
--> $DIR/regex.rs:42:33
--> $DIR/regex.rs:46:33
|
LL | let trivial_eq = Regex::new("^foobar$");
| ^^^^^^^^^^
|
= help: consider using `==` on `str`s

error: trivial regex
--> $DIR/regex.rs:44:48
--> $DIR/regex.rs:48:48
|
LL | let trivial_eq_builder = RegexBuilder::new("^foobar$");
| ^^^^^^^^^^
|
= help: consider using `==` on `str`s

error: trivial regex
--> $DIR/regex.rs:46:42
--> $DIR/regex.rs:50:42
|
LL | let trivial_starts_with = Regex::new("^foobar");
| ^^^^^^^^^
|
= help: consider using `str::starts_with`

error: trivial regex
--> $DIR/regex.rs:48:40
--> $DIR/regex.rs:52:40
|
LL | let trivial_ends_with = Regex::new("foobar$");
| ^^^^^^^^^
|
= help: consider using `str::ends_with`

error: trivial regex
--> $DIR/regex.rs:50:39
--> $DIR/regex.rs:54:39
|
LL | let trivial_contains = Regex::new("foobar");
| ^^^^^^^^
|
= help: consider using `str::contains`

error: trivial regex
--> $DIR/regex.rs:52:39
--> $DIR/regex.rs:56:39
|
LL | let trivial_contains = Regex::new(NOT_A_REAL_REGEX);
| ^^^^^^^^^^^^^^^^
|
= help: consider using `str::contains`

error: trivial regex
--> $DIR/regex.rs:54:40
--> $DIR/regex.rs:58:40
|
LL | let trivial_backslash = Regex::new("a/.b");
| ^^^^^^^
|
= help: consider using `str::contains`

error: trivial regex
--> $DIR/regex.rs:57:36
--> $DIR/regex.rs:61:36
|
LL | let trivial_empty = Regex::new("");
| ^^
|
= help: the regex is unlikely to be useful as it is

error: trivial regex
--> $DIR/regex.rs:59:36
--> $DIR/regex.rs:63:36
|
LL | let trivial_empty = Regex::new("^");
| ^^^
|
= help: the regex is unlikely to be useful as it is

error: trivial regex
--> $DIR/regex.rs:61:36
--> $DIR/regex.rs:65:36
|
LL | let trivial_empty = Regex::new("^$");
| ^^^^
|
= help: consider using `str::is_empty`

error: trivial regex
--> $DIR/regex.rs:63:44
--> $DIR/regex.rs:67:44
|
LL | let binary_trivial_empty = BRegex::new("^$");
| ^^^^
|
= help: consider using `str::is_empty`

error: aborting due to 23 previous errors
error: aborting due to 25 previous errors

0 comments on commit 96c28d1

Please sign in to comment.