bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Rollup merge of #102922 - kper:bugfix/102902-filtering-json, r=oli-obk

Browse source 
Filtering spans when emitting json

According to the issue #102902, we shouldn't emit spans which have an empty span and no suggested replacement.
This commit is contained in:
Dylan DPC 2022-10-21 17:29:58 +05:30 • committed by GitHub
commit 0a0e9f73af
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 81 additions and 117 deletions
Download patch file Download diff file Expand all files Collapse all files

25
compiler/rustc_errors/src/diagnostic.rs
View file

@ -567,6 +567,11 @@ impl Diagnostic {
style: SuggestionStyle,
) -> &mut Self {
assert!(!suggestion.is_empty());
debug_assert!(
!(suggestion.iter().any(|(sp, text)| sp.is_empty() && text.is_empty())),
"Span must not be empty and have no suggestion"
);
self.push_suggestion(CodeSuggestion {
substitutions: vec![Substitution {
parts: suggestion
@ -644,6 +649,10 @@ impl Diagnostic {
applicability: Applicability,
style: SuggestionStyle,
) -> &mut Self {
debug_assert!(
!(sp.is_empty() && suggestion.to_string().is_empty()),
"Span must not be empty and have no suggestion"
);
self.push_suggestion(CodeSuggestion {
substitutions: vec![Substitution {
parts: vec![SubstitutionPart { snippet: suggestion.to_string(), span: sp }],
@ -684,6 +693,12 @@ impl Diagnostic {
) -> &mut Self {
let mut suggestions: Vec<_> = suggestions.collect();
suggestions.sort();
debug_assert!(
!(sp.is_empty() && suggestions.iter().any(|suggestion| suggestion.is_empty())),
"Span must not be empty and have no suggestion"
);
let substitutions = suggestions
.into_iter()
.map(|snippet| Substitution { parts: vec![SubstitutionPart { snippet, span: sp }] })
@ -705,8 +720,18 @@ impl Diagnostic {
suggestions: impl Iterator<Item = Vec<(Span, String)>>,
applicability: Applicability,
) -> &mut Self {
let suggestions: Vec<_> = suggestions.collect();
debug_assert!(
!(suggestions
.iter()
.flat_map(|suggs| suggs)
.any(|(sp, suggestion)| sp.is_empty() && suggestion.is_empty())),
"Span must not be empty and have no suggestion"
);
self.push_suggestion(CodeSuggestion {
substitutions: suggestions
.into_iter()
.map(|sugg| Substitution {
parts: sugg
.into_iter()

5
compiler/rustc_expand/src/base.rs
View file

@ -22,7 +22,7 @@ use rustc_span::edition::Edition;
use rustc_span::hygiene::{AstPass, ExpnData, ExpnKind, LocalExpnId};
use rustc_span::source_map::SourceMap;
use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_span::{FileName, Span, DUMMY_SP};
use rustc_span::{BytePos, FileName, Span, DUMMY_SP};
use smallvec::{smallvec, SmallVec};
use std::default::Default;
@ -1228,8 +1228,9 @@ pub fn expr_to_spanned_string<'a>(
ast::LitKind::Str(s, style) => return Ok((s, style, expr.span)),
ast::LitKind::ByteStr(_) => {
let mut err = cx.struct_span_err(l.span, err_msg);
let span = expr.span.shrink_to_lo();
err.span_suggestion(
expr.span.shrink_to_lo(),
span.with_hi(span.lo() + BytePos(1)),
"consider removing the leading `b`",
"",
Applicability::MaybeIncorrect,

25
compiler/rustc_hir_analysis/src/astconv/mod.rs
View file

@ -3051,24 +3051,27 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
.map_or(false, |s| s.trim_end().ends_with('<'));
let is_global = poly_trait_ref.trait_ref.path.is_global();
let sugg = Vec::from_iter([
(
self_ty.span.shrink_to_lo(),
format!(
"{}dyn {}",
if needs_bracket { "<" } else { "" },
if is_global { "(" } else { "" },
),
let mut sugg = Vec::from_iter([(
self_ty.span.shrink_to_lo(),
format!(
"{}dyn {}",
if needs_bracket { "<" } else { "" },
if is_global { "(" } else { "" },
),
(
)]);
if is_global || needs_bracket {
sugg.push((
self_ty.span.shrink_to_hi(),
format!(
"{}{}",
if is_global { ")" } else { "" },
if needs_bracket { ">" } else { "" },
),
),
]);
));
}
if self_ty.span.edition() >= Edition::Edition2021 {
let msg = "trait objects must include the `dyn` keyword";
let label = "add `dyn` keyword before this trait";

10
compiler/rustc_parse/src/parser/diagnostics.rs
View file

@ -1374,9 +1374,17 @@ impl<'a> Parser<'a> {
kind: IncDecRecovery,
(pre_span, post_span): (Span, Span),
) -> MultiSugg {
let mut patches = Vec::new();
if !pre_span.is_empty() {
patches.push((pre_span, String::new()));
}
patches.push((post_span, format!(" {}= 1", kind.op.chr())));
MultiSugg {
msg: format!("use `{}= 1` instead", kind.op.chr()),
patches: vec![(pre_span, String::new()), (post_span, format!(" {}= 1", kind.op.chr()))],
patches,
applicability: Applicability::MachineApplicable,
}
}

12
src/tools/clippy/clippy_lints/src/manual_assert.rs
View file

@ -69,11 +69,13 @@ impl<'tcx> LateLintPass<'tcx> for ManualAssert {
"only a `panic!` in `if`-then statement",
|diag| {
// comments can be noisy, do not show them to the user
diag.tool_only_span_suggestion(
expr.span.shrink_to_lo(),
"add comments back",
comments,
applicability);
if !comments.is_empty() {
diag.tool_only_span_suggestion(
expr.span.shrink_to_lo(),
"add comments back",
comments,
applicability);
}
diag.span_suggestion(
expr.span,
"try instead",

11
src/tools/clippy/clippy_lints/src/needless_late_init.rs
View file

@ -180,10 +180,13 @@ fn assignment_suggestions<'tcx>(
let suggestions = assignments
.iter()
.flat_map(|assignment| {
[
assignment.span.until(assignment.rhs_span),
assignment.rhs_span.shrink_to_hi().with_hi(assignment.span.hi()),
]
let mut spans = vec![assignment.span.until(assignment.rhs_span)];
if assignment.rhs_span.hi() != assignment.span.hi() {
spans.push(assignment.rhs_span.shrink_to_hi().with_hi(assignment.span.hi()));
}
spans
})
.map(|span| (span, String::new()))
.collect::<Vec<(Span, String)>>();

55
src/tools/clippy/tests/ui/manual_assert.edition2018.stderr
View file

@ -4,13 +4,9 @@ error: only a `panic!` in `if`-then statement
LL | / if !a.is_empty() {
LL | | panic!("qaqaq{:?}", a);
LL | | }
| |_____^
| |_____^ help: try instead: `assert!(a.is_empty(), "qaqaq{:?}", a);`
|
= note: `-D clippy::manual-assert` implied by `-D warnings`
help: try instead
|
LL | assert!(a.is_empty(), "qaqaq{:?}", a);
|
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:34:5
@ -18,12 +14,7 @@ error: only a `panic!` in `if`-then statement
LL | / if !a.is_empty() {
LL | | panic!("qwqwq");
LL | | }
| |_____^
|
help: try instead
|
LL | assert!(a.is_empty(), "qwqwq");
|
| |_____^ help: try instead: `assert!(a.is_empty(), "qwqwq");`
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:51:5
@ -31,12 +22,7 @@ error: only a `panic!` in `if`-then statement
LL | / if b.is_empty() {
LL | | panic!("panic1");
LL | | }
| |_____^
|
help: try instead
|
LL | assert!(!b.is_empty(), "panic1");
|
| |_____^ help: try instead: `assert!(!b.is_empty(), "panic1");`
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:54:5
@ -44,12 +30,7 @@ error: only a `panic!` in `if`-then statement
LL | / if b.is_empty() && a.is_empty() {
LL | | panic!("panic2");
LL | | }
| |_____^
|
help: try instead
|
LL | assert!(!(b.is_empty() && a.is_empty()), "panic2");
|
| |_____^ help: try instead: `assert!(!(b.is_empty() && a.is_empty()), "panic2");`
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:57:5
@ -57,12 +38,7 @@ error: only a `panic!` in `if`-then statement
LL | / if a.is_empty() && !b.is_empty() {
LL | | panic!("panic3");
LL | | }
| |_____^
|
help: try instead
|
LL | assert!(!(a.is_empty() && !b.is_empty()), "panic3");
|
| |_____^ help: try instead: `assert!(!(a.is_empty() && !b.is_empty()), "panic3");`
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:60:5
@ -70,12 +46,7 @@ error: only a `panic!` in `if`-then statement
LL | / if b.is_empty() || a.is_empty() {
LL | | panic!("panic4");
LL | | }
| |_____^
|
help: try instead
|
LL | assert!(!(b.is_empty() || a.is_empty()), "panic4");
|
| |_____^ help: try instead: `assert!(!(b.is_empty() || a.is_empty()), "panic4");`
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:63:5
@ -83,12 +54,7 @@ error: only a `panic!` in `if`-then statement
LL | / if a.is_empty() || !b.is_empty() {
LL | | panic!("panic5");
LL | | }
| |_____^
|
help: try instead
|
LL | assert!(!(a.is_empty() || !b.is_empty()), "panic5");
|
| |_____^ help: try instead: `assert!(!(a.is_empty() || !b.is_empty()), "panic5");`
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:66:5
@ -96,12 +62,7 @@ error: only a `panic!` in `if`-then statement
LL | / if a.is_empty() {
LL | | panic!("with expansion {}", one!())
LL | | }
| |_____^
|
help: try instead
|
LL | assert!(!a.is_empty(), "with expansion {}", one!());
|
| |_____^ help: try instead: `assert!(!a.is_empty(), "with expansion {}", one!());`
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:73:5

55
src/tools/clippy/tests/ui/manual_assert.edition2021.stderr
View file

@ -4,13 +4,9 @@ error: only a `panic!` in `if`-then statement
LL | / if !a.is_empty() {
LL | | panic!("qaqaq{:?}", a);
LL | | }
| |_____^
| |_____^ help: try instead: `assert!(a.is_empty(), "qaqaq{:?}", a);`
|
= note: `-D clippy::manual-assert` implied by `-D warnings`
help: try instead
|
LL | assert!(a.is_empty(), "qaqaq{:?}", a);
|
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:34:5
@ -18,12 +14,7 @@ error: only a `panic!` in `if`-then statement
LL | / if !a.is_empty() {
LL | | panic!("qwqwq");
LL | | }
| |_____^
|
help: try instead
|
LL | assert!(a.is_empty(), "qwqwq");
|
| |_____^ help: try instead: `assert!(a.is_empty(), "qwqwq");`
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:51:5
@ -31,12 +22,7 @@ error: only a `panic!` in `if`-then statement
LL | / if b.is_empty() {
LL | | panic!("panic1");
LL | | }
| |_____^
|
help: try instead
|
LL | assert!(!b.is_empty(), "panic1");
|
| |_____^ help: try instead: `assert!(!b.is_empty(), "panic1");`
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:54:5
@ -44,12 +30,7 @@ error: only a `panic!` in `if`-then statement
LL | / if b.is_empty() && a.is_empty() {
LL | | panic!("panic2");
LL | | }
| |_____^
|
help: try instead
|
LL | assert!(!(b.is_empty() && a.is_empty()), "panic2");
|
| |_____^ help: try instead: `assert!(!(b.is_empty() && a.is_empty()), "panic2");`
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:57:5
@ -57,12 +38,7 @@ error: only a `panic!` in `if`-then statement
LL | / if a.is_empty() && !b.is_empty() {
LL | | panic!("panic3");
LL | | }
| |_____^
|
help: try instead
|
LL | assert!(!(a.is_empty() && !b.is_empty()), "panic3");
|
| |_____^ help: try instead: `assert!(!(a.is_empty() && !b.is_empty()), "panic3");`
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:60:5
@ -70,12 +46,7 @@ error: only a `panic!` in `if`-then statement
LL | / if b.is_empty() || a.is_empty() {
LL | | panic!("panic4");
LL | | }
| |_____^
|
help: try instead
|
LL | assert!(!(b.is_empty() || a.is_empty()), "panic4");
|
| |_____^ help: try instead: `assert!(!(b.is_empty() || a.is_empty()), "panic4");`
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:63:5
@ -83,12 +54,7 @@ error: only a `panic!` in `if`-then statement
LL | / if a.is_empty() || !b.is_empty() {
LL | | panic!("panic5");
LL | | }
| |_____^
|
help: try instead
|
LL | assert!(!(a.is_empty() || !b.is_empty()), "panic5");
|
| |_____^ help: try instead: `assert!(!(a.is_empty() || !b.is_empty()), "panic5");`
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:66:5
@ -96,12 +62,7 @@ error: only a `panic!` in `if`-then statement
LL | / if a.is_empty() {
LL | | panic!("with expansion {}", one!())
LL | | }
| |_____^
|
help: try instead
|
LL | assert!(!a.is_empty(), "with expansion {}", one!());
|
| |_____^ help: try instead: `assert!(!a.is_empty(), "with expansion {}", one!());`
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:73:5