Skip to content

Commit 96c28d1

Browse files
borsbors
committed
Auto merge of #10231 - Alexendoo:regex-spans, r=llogiq
`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
2 parents 1480cea + 986f40f commit 96c28d1

File tree

3 files changed

+97
-62
lines changed

3 files changed

+97
-62
lines changed

‎clippy_lints/src/regex.rs‎

Lines changed: 44 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1+
use std::fmt::Display;
2+
13
use clippy_utils::consts::{constant, Constant};
24
use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
5+
use clippy_utils::source::snippet_opt;
36
use clippy_utils::{match_def_path, paths};
47
use if_chain::if_chain;
58
use rustc_ast::ast::{LitKind, StrStyle};
@@ -77,13 +80,45 @@ impl<'tcx> LateLintPass<'tcx> for Regex {
7780
}
7881
}
7982

80-
#[must_use]
81-
fn str_span(base: Span, c: regex_syntax::ast::Span, offset: u8) -> Span {
82-
let offset = u32::from(offset);
83-
let end = base.lo() + BytePos(u32::try_from(c.end.offset).expect("offset too large") + offset);
84-
let start = base.lo() + BytePos(u32::try_from(c.start.offset).expect("offset too large") + offset);
85-
assert!(start <= end);
86-
Span::new(start, end, base.ctxt(), base.parent())
83+
fn lint_syntax_error(cx: &LateContext<'_>, error: &regex_syntax::Error, unescaped: &str, base: Span, offset: u8) {
84+
let parts: Option<(_, _, &dyn Display)> = match &error {
85+
regex_syntax::Error::Parse(e) => Some((e.span(), e.auxiliary_span(), e.kind())),
86+
regex_syntax::Error::Translate(e) => Some((e.span(), None, e.kind())),
87+
_ => None,
88+
};
89+
90+
let convert_span = |regex_span: &regex_syntax::ast::Span| {
91+
let offset = u32::from(offset);
92+
let start = base.lo() + BytePos(u32::try_from(regex_span.start.offset).expect("offset too large") + offset);
93+
let end = base.lo() + BytePos(u32::try_from(regex_span.end.offset).expect("offset too large") + offset);
94+
95+
Span::new(start, end, base.ctxt(), base.parent())
96+
};
97+
98+
if let Some((primary, auxiliary, kind)) = parts
99+
&& let Some(literal_snippet) = snippet_opt(cx, base)
100+
&& let Some(inner) = literal_snippet.get(offset as usize..)
101+
// Only convert to native rustc spans if the parsed regex matches the
102+
// source snippet exactly, to ensure the span offsets are correct
103+
&& inner.get(..unescaped.len()) == Some(unescaped)
104+
{
105+
let spans = if let Some(auxiliary) = auxiliary {
106+
vec![convert_span(primary), convert_span(auxiliary)]
107+
} else {
108+
vec![convert_span(primary)]
109+
};
110+
111+
span_lint(cx, INVALID_REGEX, spans, &format!("regex syntax error: {kind}"));
112+
} else {
113+
span_lint_and_help(
114+
cx,
115+
INVALID_REGEX,
116+
base,
117+
&error.to_string(),
118+
None,
119+
"consider using a raw string literal: `r\"..\"`",
120+
);
121+
}
87122
}
88123

89124
fn const_str<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> Option<String> {
@@ -155,25 +190,7 @@ fn check_regex<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, utf8: bool) {
155190
span_lint_and_help(cx, TRIVIAL_REGEX, expr.span, "trivial regex", None, repl);
156191
}
157192
},
158-
Err(regex_syntax::Error::Parse(e)) => {
159-
span_lint(
160-
cx,
161-
INVALID_REGEX,
162-
str_span(expr.span, *e.span(), offset),
163-
&format!("regex syntax error: {}", e.kind()),
164-
);
165-
},
166-
Err(regex_syntax::Error::Translate(e)) => {
167-
span_lint(
168-
cx,
169-
INVALID_REGEX,
170-
str_span(expr.span, *e.span(), offset),
171-
&format!("regex syntax error: {}", e.kind()),
172-
);
173-
},
174-
Err(e) => {
175-
span_lint(cx, INVALID_REGEX, expr.span, &format!("regex syntax error: {e}"));
176-
},
193+
Err(e) => lint_syntax_error(cx, &e, r, expr.span, offset),
177194
}
178195
}
179196
} else if let Some(r) = const_str(cx, expr) {
@@ -183,25 +200,7 @@ fn check_regex<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, utf8: bool) {
183200
span_lint_and_help(cx, TRIVIAL_REGEX, expr.span, "trivial regex", None, repl);
184201
}
185202
},
186-
Err(regex_syntax::Error::Parse(e)) => {
187-
span_lint(
188-
cx,
189-
INVALID_REGEX,
190-
expr.span,
191-
&format!("regex syntax error on position {}: {}", e.span().start.offset, e.kind()),
192-
);
193-
},
194-
Err(regex_syntax::Error::Translate(e)) => {
195-
span_lint(
196-
cx,
197-
INVALID_REGEX,
198-
expr.span,
199-
&format!("regex syntax error on position {}: {}", e.span().start.offset, e.kind()),
200-
);
201-
},
202-
Err(e) => {
203-
span_lint(cx, INVALID_REGEX, expr.span, &format!("regex syntax error: {e}"));
204-
},
203+
Err(e) => span_lint(cx, INVALID_REGEX, expr.span, &e.to_string()),
205204
}
206205
}
207206
}

‎tests/ui/regex.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ fn syntax_error() {
3636

3737
let raw_string_error = Regex::new(r"[...\/...]");
3838
let raw_string_error = Regex::new(r#"[...\/...]"#);
39+
40+
let escaped_string_span = Regex::new("\\b\\c");
41+
42+
let aux_span = Regex::new("(?ixi)");
3943
}
4044

4145
fn trivial_regex() {

‎tests/ui/regex.stderr‎

Lines changed: 49 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,10 @@ error: regex syntax error: invalid character class range, the start must be <= t
2929
LL | let some_unicode = Regex::new("[é-è]");
3030
| ^^^
3131

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

46-
error: regex syntax error on position 0: unclosed group
49+
error: regex parse error:
50+
(
51+
^
52+
error: unclosed group
4753
--> $DIR/regex.rs:21:41
4854
|
4955
LL | let some_binary_regex = BRegex::new(OPENING_PAREN);
5056
| ^^^^^^^^^^^^^
5157

52-
error: regex syntax error on position 0: unclosed group
58+
error: regex parse error:
59+
(
60+
^
61+
error: unclosed group
5362
--> $DIR/regex.rs:22:56
5463
|
5564
LL | let some_binary_regex_builder = BRegexBuilder::new(OPENING_PAREN);
5665
| ^^^^^^^^^^^^^
5766

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

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

97+
error: regex parse error:
98+
/b/c
99+
^^
100+
error: unrecognized escape sequence
101+
--> $DIR/regex.rs:40:42
102+
|
103+
LL | let escaped_string_span = Regex::new("/b/c");
104+
| ^^^^^^^^
105+
|
106+
= help: consider using a raw string literal: `r".."`
107+
108+
error: regex syntax error: duplicate flag
109+
--> $DIR/regex.rs:42:34
110+
|
111+
LL | let aux_span = Regex::new("(?ixi)");
112+
| ^ ^
113+
82114
error: trivial regex
83-
--> $DIR/regex.rs:42:33
115+
--> $DIR/regex.rs:46:33
84116
|
85117
LL | let trivial_eq = Regex::new("^foobar$");
86118
| ^^^^^^^^^^
87119
|
88120
= help: consider using `==` on `str`s
89121

90122
error: trivial regex
91-
--> $DIR/regex.rs:44:48
123+
--> $DIR/regex.rs:48:48
92124
|
93125
LL | let trivial_eq_builder = RegexBuilder::new("^foobar$");
94126
| ^^^^^^^^^^
95127
|
96128
= help: consider using `==` on `str`s
97129

98130
error: trivial regex
99-
--> $DIR/regex.rs:46:42
131+
--> $DIR/regex.rs:50:42
100132
|
101133
LL | let trivial_starts_with = Regex::new("^foobar");
102134
| ^^^^^^^^^
103135
|
104136
= help: consider using `str::starts_with`
105137

106138
error: trivial regex
107-
--> $DIR/regex.rs:48:40
139+
--> $DIR/regex.rs:52:40
108140
|
109141
LL | let trivial_ends_with = Regex::new("foobar$");
110142
| ^^^^^^^^^
111143
|
112144
= help: consider using `str::ends_with`
113145

114146
error: trivial regex
115-
--> $DIR/regex.rs:50:39
147+
--> $DIR/regex.rs:54:39
116148
|
117149
LL | let trivial_contains = Regex::new("foobar");
118150
| ^^^^^^^^
119151
|
120152
= help: consider using `str::contains`
121153

122154
error: trivial regex
123-
--> $DIR/regex.rs:52:39
155+
--> $DIR/regex.rs:56:39
124156
|
125157
LL | let trivial_contains = Regex::new(NOT_A_REAL_REGEX);
126158
| ^^^^^^^^^^^^^^^^
127159
|
128160
= help: consider using `str::contains`
129161

130162
error: trivial regex
131-
--> $DIR/regex.rs:54:40
163+
--> $DIR/regex.rs:58:40
132164
|
133165
LL | let trivial_backslash = Regex::new("a/.b");
134166
| ^^^^^^^
135167
|
136168
= help: consider using `str::contains`
137169

138170
error: trivial regex
139-
--> $DIR/regex.rs:57:36
171+
--> $DIR/regex.rs:61:36
140172
|
141173
LL | let trivial_empty = Regex::new("");
142174
| ^^
143175
|
144176
= help: the regex is unlikely to be useful as it is
145177

146178
error: trivial regex
147-
--> $DIR/regex.rs:59:36
179+
--> $DIR/regex.rs:63:36
148180
|
149181
LL | let trivial_empty = Regex::new("^");
150182
| ^^^
151183
|
152184
= help: the regex is unlikely to be useful as it is
153185

154186
error: trivial regex
155-
--> $DIR/regex.rs:61:36
187+
--> $DIR/regex.rs:65:36
156188
|
157189
LL | let trivial_empty = Regex::new("^$");
158190
| ^^^^
159191
|
160192
= help: consider using `str::is_empty`
161193

162194
error: trivial regex
163-
--> $DIR/regex.rs:63:44
195+
--> $DIR/regex.rs:67:44
164196
|
165197
LL | let binary_trivial_empty = BRegex::new("^$");
166198
| ^^^^
167199
|
168200
= help: consider using `str::is_empty`
169201

170-
error: aborting due to 23 previous errors
202+
error: aborting due to 25 previous errors
171203

0 commit comments

Comments
 (0)