Skip to content

Commit 28d0312

Browse files
kperkper
committed
Implement assertions and fixes to not emit empty spans without suggestions
1 parent 4b8f431 commit 28d0312

File tree

8 files changed

+81
-117
lines changed

8 files changed

+81
-117
lines changed

‎compiler/rustc_errors/src/diagnostic.rs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -567,6 +567,11 @@ impl Diagnostic {
567567
style: SuggestionStyle,
568568
) -> &mut Self {
569569
assert!(!suggestion.is_empty());
570+
debug_assert!(
571+
!(suggestion.iter().any(|(sp, text)| sp.is_empty() && text.is_empty())),
572+
"Span must not be empty and have no suggestion"
573+
);
574+
570575
self.push_suggestion(CodeSuggestion {
571576
substitutions: vec![Substitution {
572577
parts: suggestion
@@ -644,6 +649,10 @@ impl Diagnostic {
644649
applicability: Applicability,
645650
style: SuggestionStyle,
646651
) -> &mut Self {
652+
debug_assert!(
653+
!(sp.is_empty() && suggestion.to_string().is_empty()),
654+
"Span must not be empty and have no suggestion"
655+
);
647656
self.push_suggestion(CodeSuggestion {
648657
substitutions: vec![Substitution {
649658
parts: vec![SubstitutionPart { snippet: suggestion.to_string(), span: sp }],
@@ -684,6 +693,12 @@ impl Diagnostic {
684693
) -> &mut Self {
685694
let mut suggestions: Vec<_> = suggestions.collect();
686695
suggestions.sort();
696+
697+
debug_assert!(
698+
!(sp.is_empty() && suggestions.iter().any(|suggestion| suggestion.is_empty())),
699+
"Span must not be empty and have no suggestion"
700+
);
701+
687702
let substitutions = suggestions
688703
.into_iter()
689704
.map(|snippet| Substitution { parts: vec![SubstitutionPart { snippet, span: sp }] })
@@ -705,8 +720,18 @@ impl Diagnostic {
705720
suggestions: impl Iterator<Item = Vec<(Span, String)>>,
706721
applicability: Applicability,
707722
) -> &mut Self {
723+
let suggestions: Vec<_> = suggestions.collect();
724+
debug_assert!(
725+
!(suggestions
726+
.iter()
727+
.flat_map(|suggs| suggs)
728+
.any(|(sp, suggestion)| sp.is_empty() && suggestion.is_empty())),
729+
"Span must not be empty and have no suggestion"
730+
);
731+
708732
self.push_suggestion(CodeSuggestion {
709733
substitutions: suggestions
734+
.into_iter()
710735
.map(|sugg| Substitution {
711736
parts: sugg
712737
.into_iter()

‎compiler/rustc_expand/src/base.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use rustc_span::edition::Edition;
2222
use rustc_span::hygiene::{AstPass, ExpnData, ExpnKind, LocalExpnId};
2323
use rustc_span::source_map::SourceMap;
2424
use rustc_span::symbol::{kw, sym, Ident, Symbol};
25-
use rustc_span::{FileName, Span, DUMMY_SP};
25+
use rustc_span::{BytePos, FileName, Span, DUMMY_SP};
2626
use smallvec::{smallvec, SmallVec};
2727

2828
use std::default::Default;
@@ -1228,8 +1228,9 @@ pub fn expr_to_spanned_string<'a>(
12281228
ast::LitKind::Str(s, style) => return Ok((s, style, expr.span)),
12291229
ast::LitKind::ByteStr(_) => {
12301230
let mut err = cx.struct_span_err(l.span, err_msg);
1231+
let span = expr.span.shrink_to_lo();
12311232
err.span_suggestion(
1232-
expr.span.shrink_to_lo(),
1233+
span.with_hi(span.lo() + BytePos(1)),
12331234
"consider removing the leading `b`",
12341235
"",
12351236
Applicability::MaybeIncorrect,

‎compiler/rustc_hir_analysis/src/astconv/mod.rs‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3051,24 +3051,27 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
30513051
.map_or(false, |s| s.trim_end().ends_with('<'));
30523052

30533053
let is_global = poly_trait_ref.trait_ref.path.is_global();
3054-
let sugg = Vec::from_iter([
3055-
(
3056-
self_ty.span.shrink_to_lo(),
3057-
format!(
3058-
"{}dyn {}",
3059-
if needs_bracket { "<" } else { "" },
3060-
if is_global { "(" } else { "" },
3061-
),
3054+
3055+
let mut sugg = Vec::from_iter([(
3056+
self_ty.span.shrink_to_lo(),
3057+
format!(
3058+
"{}dyn {}",
3059+
if needs_bracket { "<" } else { "" },
3060+
if is_global { "(" } else { "" },
30623061
),
3063-
(
3062+
)]);
3063+
3064+
if is_global || needs_bracket {
3065+
sugg.push((
30643066
self_ty.span.shrink_to_hi(),
30653067
format!(
30663068
"{}{}",
30673069
if is_global { ")" } else { "" },
30683070
if needs_bracket { ">" } else { "" },
30693071
),
3070-
),
3071-
]);
3072+
));
3073+
}
3074+
30723075
if self_ty.span.edition() >= Edition::Edition2021 {
30733076
let msg = "trait objects must include the `dyn` keyword";
30743077
let label = "add `dyn` keyword before this trait";

‎compiler/rustc_parse/src/parser/diagnostics.rs‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1374,9 +1374,17 @@ impl<'a> Parser<'a> {
13741374
kind: IncDecRecovery,
13751375
(pre_span, post_span): (Span, Span),
13761376
) -> MultiSugg {
1377+
let mut patches = Vec::new();
1378+
1379+
if !pre_span.is_empty() {
1380+
patches.push((pre_span, String::new()));
1381+
}
1382+
1383+
patches.push((post_span, format!(" {}= 1", kind.op.chr())));
1384+
13771385
MultiSugg {
13781386
msg: format!("use `{}= 1` instead", kind.op.chr()),
1379-
patches: vec![(pre_span, String::new()), (post_span, format!(" {}= 1", kind.op.chr()))],
1387+
patches,
13801388
applicability: Applicability::MachineApplicable,
13811389
}
13821390
}

‎src/tools/clippy/clippy_lints/src/manual_assert.rs‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,13 @@ impl<'tcx> LateLintPass<'tcx> for ManualAssert {
6969
"only a `panic!` in `if`-then statement",
7070
|diag| {
7171
// comments can be noisy, do not show them to the user
72-
diag.tool_only_span_suggestion(
73-
expr.span.shrink_to_lo(),
74-
"add comments back",
75-
comments,
76-
applicability);
72+
if !comments.is_empty() {
73+
diag.tool_only_span_suggestion(
74+
expr.span.shrink_to_lo(),
75+
"add comments back",
76+
comments,
77+
applicability);
78+
}
7779
diag.span_suggestion(
7880
expr.span,
7981
"try instead",

‎src/tools/clippy/clippy_lints/src/needless_late_init.rs‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -180,10 +180,13 @@ fn assignment_suggestions<'tcx>(
180180
let suggestions = assignments
181181
.iter()
182182
.flat_map(|assignment| {
183-
[
184-
assignment.span.until(assignment.rhs_span),
185-
assignment.rhs_span.shrink_to_hi().with_hi(assignment.span.hi()),
186-
]
183+
let mut spans = vec![assignment.span.until(assignment.rhs_span)];
184+
185+
if assignment.rhs_span.hi() != assignment.span.hi() {
186+
spans.push(assignment.rhs_span.shrink_to_hi().with_hi(assignment.span.hi()));
187+
}
188+
189+
spans
187190
})
188191
.map(|span| (span, String::new()))
189192
.collect::<Vec<(Span, String)>>();

‎src/tools/clippy/tests/ui/manual_assert.edition2018.stderr‎

Lines changed: 8 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -4,104 +4,65 @@ error: only a `panic!` in `if`-then statement
44
LL | / if !a.is_empty() {
55
LL | | panic!("qaqaq{:?}", a);
66
LL | | }
7-
| |_____^
7+
| |_____^ help: try instead: `assert!(a.is_empty(), "qaqaq{:?}", a);`
88
|
99
= note: `-D clippy::manual-assert` implied by `-D warnings`
10-
help: try instead
11-
|
12-
LL | assert!(a.is_empty(), "qaqaq{:?}", a);
13-
|
1410

1511
error: only a `panic!` in `if`-then statement
1612
--> $DIR/manual_assert.rs:34:5
1713
|
1814
LL | / if !a.is_empty() {
1915
LL | | panic!("qwqwq");
2016
LL | | }
21-
| |_____^
22-
|
23-
help: try instead
24-
|
25-
LL | assert!(a.is_empty(), "qwqwq");
26-
|
17+
| |_____^ help: try instead: `assert!(a.is_empty(), "qwqwq");`
2718

2819
error: only a `panic!` in `if`-then statement
2920
--> $DIR/manual_assert.rs:51:5
3021
|
3122
LL | / if b.is_empty() {
3223
LL | | panic!("panic1");
3324
LL | | }
34-
| |_____^
35-
|
36-
help: try instead
37-
|
38-
LL | assert!(!b.is_empty(), "panic1");
39-
|
25+
| |_____^ help: try instead: `assert!(!b.is_empty(), "panic1");`
4026

4127
error: only a `panic!` in `if`-then statement
4228
--> $DIR/manual_assert.rs:54:5
4329
|
4430
LL | / if b.is_empty() && a.is_empty() {
4531
LL | | panic!("panic2");
4632
LL | | }
47-
| |_____^
48-
|
49-
help: try instead
50-
|
51-
LL | assert!(!(b.is_empty() && a.is_empty()), "panic2");
52-
|
33+
| |_____^ help: try instead: `assert!(!(b.is_empty() && a.is_empty()), "panic2");`
5334

5435
error: only a `panic!` in `if`-then statement
5536
--> $DIR/manual_assert.rs:57:5
5637
|
5738
LL | / if a.is_empty() && !b.is_empty() {
5839
LL | | panic!("panic3");
5940
LL | | }
60-
| |_____^
61-
|
62-
help: try instead
63-
|
64-
LL | assert!(!(a.is_empty() && !b.is_empty()), "panic3");
65-
|
41+
| |_____^ help: try instead: `assert!(!(a.is_empty() && !b.is_empty()), "panic3");`
6642

6743
error: only a `panic!` in `if`-then statement
6844
--> $DIR/manual_assert.rs:60:5
6945
|
7046
LL | / if b.is_empty() || a.is_empty() {
7147
LL | | panic!("panic4");
7248
LL | | }
73-
| |_____^
74-
|
75-
help: try instead
76-
|
77-
LL | assert!(!(b.is_empty() || a.is_empty()), "panic4");
78-
|
49+
| |_____^ help: try instead: `assert!(!(b.is_empty() || a.is_empty()), "panic4");`
7950

8051
error: only a `panic!` in `if`-then statement
8152
--> $DIR/manual_assert.rs:63:5
8253
|
8354
LL | / if a.is_empty() || !b.is_empty() {
8455
LL | | panic!("panic5");
8556
LL | | }
86-
| |_____^
87-
|
88-
help: try instead
89-
|
90-
LL | assert!(!(a.is_empty() || !b.is_empty()), "panic5");
91-
|
57+
| |_____^ help: try instead: `assert!(!(a.is_empty() || !b.is_empty()), "panic5");`
9258

9359
error: only a `panic!` in `if`-then statement
9460
--> $DIR/manual_assert.rs:66:5
9561
|
9662
LL | / if a.is_empty() {
9763
LL | | panic!("with expansion {}", one!())
9864
LL | | }
99-
| |_____^
100-
|
101-
help: try instead
102-
|
103-
LL | assert!(!a.is_empty(), "with expansion {}", one!());
104-
|
65+
| |_____^ help: try instead: `assert!(!a.is_empty(), "with expansion {}", one!());`
10566

10667
error: only a `panic!` in `if`-then statement
10768
--> $DIR/manual_assert.rs:73:5

‎src/tools/clippy/tests/ui/manual_assert.edition2021.stderr‎

Lines changed: 8 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -4,104 +4,65 @@ error: only a `panic!` in `if`-then statement
44
LL | / if !a.is_empty() {
55
LL | | panic!("qaqaq{:?}", a);
66
LL | | }
7-
| |_____^
7+
| |_____^ help: try instead: `assert!(a.is_empty(), "qaqaq{:?}", a);`
88
|
99
= note: `-D clippy::manual-assert` implied by `-D warnings`
10-
help: try instead
11-
|
12-
LL | assert!(a.is_empty(), "qaqaq{:?}", a);
13-
|
1410

1511
error: only a `panic!` in `if`-then statement
1612
--> $DIR/manual_assert.rs:34:5
1713
|
1814
LL | / if !a.is_empty() {
1915
LL | | panic!("qwqwq");
2016
LL | | }
21-
| |_____^
22-
|
23-
help: try instead
24-
|
25-
LL | assert!(a.is_empty(), "qwqwq");
26-
|
17+
| |_____^ help: try instead: `assert!(a.is_empty(), "qwqwq");`
2718

2819
error: only a `panic!` in `if`-then statement
2920
--> $DIR/manual_assert.rs:51:5
3021
|
3122
LL | / if b.is_empty() {
3223
LL | | panic!("panic1");
3324
LL | | }
34-
| |_____^
35-
|
36-
help: try instead
37-
|
38-
LL | assert!(!b.is_empty(), "panic1");
39-
|
25+
| |_____^ help: try instead: `assert!(!b.is_empty(), "panic1");`
4026

4127
error: only a `panic!` in `if`-then statement
4228
--> $DIR/manual_assert.rs:54:5
4329
|
4430
LL | / if b.is_empty() && a.is_empty() {
4531
LL | | panic!("panic2");
4632
LL | | }
47-
| |_____^
48-
|
49-
help: try instead
50-
|
51-
LL | assert!(!(b.is_empty() && a.is_empty()), "panic2");
52-
|
33+
| |_____^ help: try instead: `assert!(!(b.is_empty() && a.is_empty()), "panic2");`
5334

5435
error: only a `panic!` in `if`-then statement
5536
--> $DIR/manual_assert.rs:57:5
5637
|
5738
LL | / if a.is_empty() && !b.is_empty() {
5839
LL | | panic!("panic3");
5940
LL | | }
60-
| |_____^
61-
|
62-
help: try instead
63-
|
64-
LL | assert!(!(a.is_empty() && !b.is_empty()), "panic3");
65-
|
41+
| |_____^ help: try instead: `assert!(!(a.is_empty() && !b.is_empty()), "panic3");`
6642

6743
error: only a `panic!` in `if`-then statement
6844
--> $DIR/manual_assert.rs:60:5
6945
|
7046
LL | / if b.is_empty() || a.is_empty() {
7147
LL | | panic!("panic4");
7248
LL | | }
73-
| |_____^
74-
|
75-
help: try instead
76-
|
77-
LL | assert!(!(b.is_empty() || a.is_empty()), "panic4");
78-
|
49+
| |_____^ help: try instead: `assert!(!(b.is_empty() || a.is_empty()), "panic4");`
7950

8051
error: only a `panic!` in `if`-then statement
8152
--> $DIR/manual_assert.rs:63:5
8253
|
8354
LL | / if a.is_empty() || !b.is_empty() {
8455
LL | | panic!("panic5");
8556
LL | | }
86-
| |_____^
87-
|
88-
help: try instead
89-
|
90-
LL | assert!(!(a.is_empty() || !b.is_empty()), "panic5");
91-
|
57+
| |_____^ help: try instead: `assert!(!(a.is_empty() || !b.is_empty()), "panic5");`
9258

9359
error: only a `panic!` in `if`-then statement
9460
--> $DIR/manual_assert.rs:66:5
9561
|
9662
LL | / if a.is_empty() {
9763
LL | | panic!("with expansion {}", one!())
9864
LL | | }
99-
| |_____^
100-
|
101-
help: try instead
102-
|
103-
LL | assert!(!a.is_empty(), "with expansion {}", one!());
104-
|
65+
| |_____^ help: try instead: `assert!(!a.is_empty(), "with expansion {}", one!());`
10566

10667
error: only a `panic!` in `if`-then statement
10768
--> $DIR/manual_assert.rs:73:5

0 commit comments

Comments
 (0)