From 295f5483fee168b385b03db03dfa9986f05ea706 Mon Sep 17 00:00:00 2001 From: clubby789 Date: Sun, 15 Jan 2023 05:06:33 +0000 Subject: [PATCH 1/6] Fix regression in `unused_braces` with macros --- compiler/rustc_lint/src/unused.rs | 12 ++++++++---- tests/ui/lint/unused_braces.fixed | 4 ++++ tests/ui/lint/unused_braces.rs | 4 ++++ tests/ui/lint/unused_braces.stderr | 14 +++++++++++++- 4 files changed, 29 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_lint/src/unused.rs b/compiler/rustc_lint/src/unused.rs index f2ee9ab1a19..36791915964 100644 --- a/compiler/rustc_lint/src/unused.rs +++ b/compiler/rustc_lint/src/unused.rs @@ -1095,17 +1095,21 @@ impl UnusedDelimLint for UnusedBraces { // ``` // - the block has no attribute and was not created inside a macro // - if the block is an `anon_const`, the inner expr must be a literal - // (do not lint `struct A; let _: A<{ 2 + 3 }>;`) - // + // not created by a macro, i.e. do not lint on: + // ``` + // struct A; + // let _: A<{ 2 + 3 }>; + // let _: A<{produces_literal!()}>; + // ``` // FIXME(const_generics): handle paths when #67075 is fixed. if let [stmt] = inner.stmts.as_slice() { if let ast::StmtKind::Expr(ref expr) = stmt.kind { if !Self::is_expr_delims_necessary(expr, followed_by_block, false) && (ctx != UnusedDelimsCtx::AnonConst - || matches!(expr.kind, ast::ExprKind::Lit(_))) + || (matches!(expr.kind, ast::ExprKind::Lit(_)) + && !expr.span.from_expansion())) && !cx.sess().source_map().is_multiline(value.span) && value.attrs.is_empty() - && !expr.span.from_expansion() && !value.span.from_expansion() && !inner.span.from_expansion() { diff --git a/tests/ui/lint/unused_braces.fixed b/tests/ui/lint/unused_braces.fixed index 1a88d985dd8..e691fb37e6c 100644 --- a/tests/ui/lint/unused_braces.fixed +++ b/tests/ui/lint/unused_braces.fixed @@ -50,4 +50,8 @@ fn main() { if { return } { } + + // regression test for https://github.com/rust-lang/rust/issues/106899 + return println!("!"); + //~^ WARN unnecessary braces } diff --git a/tests/ui/lint/unused_braces.rs b/tests/ui/lint/unused_braces.rs index 5ca4811fc32..0d260d2cbc9 100644 --- a/tests/ui/lint/unused_braces.rs +++ b/tests/ui/lint/unused_braces.rs @@ -50,4 +50,8 @@ fn main() { if { return } { } + + // regression test for https://github.com/rust-lang/rust/issues/106899 + return { println!("!") }; + //~^ WARN unnecessary braces } diff --git a/tests/ui/lint/unused_braces.stderr b/tests/ui/lint/unused_braces.stderr index 7773f44ea2d..0b4a1c32180 100644 --- a/tests/ui/lint/unused_braces.stderr +++ b/tests/ui/lint/unused_braces.stderr @@ -68,5 +68,17 @@ LL - consume({ 7 }); LL + consume(7); | -warning: 5 warnings emitted +warning: unnecessary braces around `return` value + --> $DIR/unused_braces.rs:55:12 + | +LL | return { println!("!") }; + | ^^ ^^ + | +help: remove these braces + | +LL - return { println!("!") }; +LL + return println!("!"); + | + +warning: 6 warnings emitted From 92ced4a12e60811b67ed98c79c77c151581b7e07 Mon Sep 17 00:00:00 2001 From: Ezra Shaw Date: Sun, 15 Jan 2023 15:51:52 +1300 Subject: [PATCH 2/6] suggest `is_empty` for collections when casting to `bool` --- compiler/rustc_hir_typeck/src/cast.rs | 46 ++++++++++++++++- tests/ui/cast/cast-as-bool.rs | 6 ++- tests/ui/cast/cast-as-bool.stderr | 9 +++- tests/ui/cast/issue-106883-is-empty.rs | 27 ++++++++++ tests/ui/cast/issue-106883-is-empty.stderr | 58 ++++++++++++++++++++++ 5 files changed, 142 insertions(+), 4 deletions(-) create mode 100644 tests/ui/cast/issue-106883-is-empty.rs create mode 100644 tests/ui/cast/issue-106883-is-empty.stderr diff --git a/compiler/rustc_hir_typeck/src/cast.rs b/compiler/rustc_hir_typeck/src/cast.rs index 042a50f2fd4..0a230fca107 100644 --- a/compiler/rustc_hir_typeck/src/cast.rs +++ b/compiler/rustc_hir_typeck/src/cast.rs @@ -31,7 +31,9 @@ use super::FnCtxt; use crate::type_error_struct; -use rustc_errors::{struct_span_err, Applicability, DelayDm, DiagnosticBuilder, ErrorGuaranteed}; +use rustc_errors::{ + struct_span_err, Applicability, DelayDm, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, +}; use rustc_hir as hir; use rustc_macros::{TypeFoldable, TypeVisitable}; use rustc_middle::mir::Mutability; @@ -270,6 +272,9 @@ impl<'a, 'tcx> CastCheck<'tcx> { } )); } + + self.try_suggest_collection_to_bool(fcx, &mut err); + err.emit(); } CastError::NeedViaInt => { @@ -517,6 +522,9 @@ impl<'a, 'tcx> CastCheck<'tcx> { } else { err.span_label(self.span, "invalid cast"); } + + self.try_suggest_collection_to_bool(fcx, &mut err); + err.emit(); } CastError::SizedUnsizedCast => { @@ -1080,4 +1088,40 @@ impl<'a, 'tcx> CastCheck<'tcx> { }, ); } + + /// Attempt to suggest using `.is_empty` when trying to cast from a + /// collection type to a boolean. + fn try_suggest_collection_to_bool(&self, fcx: &FnCtxt<'a, 'tcx>, err: &mut Diagnostic) { + if self.cast_ty.is_bool() { + let derefed = fcx + .autoderef(self.expr_span, self.expr_ty) + .silence_errors() + .find(|t| matches!(t.0.kind(), ty::Str | ty::Slice(..))); + + if let Some((deref_ty, _)) = derefed { + // Give a note about what the expr derefs to. + if deref_ty != self.expr_ty.peel_refs() { + err.span_note( + self.expr_span, + format!( + "this expression `Deref`s to `{}` which implements `is_empty`", + fcx.ty_to_string(deref_ty) + ), + ); + } + + // Create a multipart suggestion: add `!` and `.is_empty()` in + // place of the cast. + let suggestion = vec![ + (self.expr_span.shrink_to_lo(), "!".to_string()), + (self.span.with_lo(self.expr_span.hi()), ".is_empty()".to_string()), + ]; + + err.multipart_suggestion_verbose(format!( + "consider using the `is_empty` method on `{}` to determine if it contains anything", + fcx.ty_to_string(self.expr_ty), + ), suggestion, Applicability::MaybeIncorrect); + } + } + } } diff --git a/tests/ui/cast/cast-as-bool.rs b/tests/ui/cast/cast-as-bool.rs index 1aed218aeb4..fbebc80d91c 100644 --- a/tests/ui/cast/cast-as-bool.rs +++ b/tests/ui/cast/cast-as-bool.rs @@ -2,8 +2,12 @@ fn main() { let u = 5 as bool; //~ ERROR cannot cast as `bool` //~| HELP compare with zero instead //~| SUGGESTION 5 != 0 + let t = (1 + 2) as bool; //~ ERROR cannot cast as `bool` //~| HELP compare with zero instead //~| SUGGESTION (1 + 2) != 0 - let v = "hello" as bool; //~ ERROR casting `&'static str` as `bool` is invalid + + let v = "hello" as bool; + //~^ ERROR casting `&'static str` as `bool` is invalid + //~| HELP consider using the `is_empty` method on `&'static str` to determine if it contains anything } diff --git a/tests/ui/cast/cast-as-bool.stderr b/tests/ui/cast/cast-as-bool.stderr index 15d94ab69d8..19ac8f10fec 100644 --- a/tests/ui/cast/cast-as-bool.stderr +++ b/tests/ui/cast/cast-as-bool.stderr @@ -5,16 +5,21 @@ LL | let u = 5 as bool; | ^^^^^^^^^ help: compare with zero instead: `5 != 0` error[E0054]: cannot cast as `bool` - --> $DIR/cast-as-bool.rs:5:13 + --> $DIR/cast-as-bool.rs:6:13 | LL | let t = (1 + 2) as bool; | ^^^^^^^^^^^^^^^ help: compare with zero instead: `(1 + 2) != 0` error[E0606]: casting `&'static str` as `bool` is invalid - --> $DIR/cast-as-bool.rs:8:13 + --> $DIR/cast-as-bool.rs:10:13 | LL | let v = "hello" as bool; | ^^^^^^^^^^^^^^^ + | +help: consider using the `is_empty` method on `&'static str` to determine if it contains anything + | +LL | let v = !"hello".is_empty(); + | + ~~~~~~~~~~~ error: aborting due to 3 previous errors diff --git a/tests/ui/cast/issue-106883-is-empty.rs b/tests/ui/cast/issue-106883-is-empty.rs new file mode 100644 index 00000000000..27e0816dd1c --- /dev/null +++ b/tests/ui/cast/issue-106883-is-empty.rs @@ -0,0 +1,27 @@ +use std::ops::Deref; + +struct Foo; + +impl Deref for Foo { + type Target = [u8]; + + fn deref(&self) -> &Self::Target { + &[] + } +} + +fn main() { + let _ = "foo" as bool; + //~^ ERROR casting `&'static str` as `bool` is invalid [E0606] + + let _ = String::from("foo") as bool; + //~^ ERROR non-primitive cast: `String` as `bool` [E0605] + + let _ = Foo as bool; + //~^ ERROR non-primitive cast: `Foo` as `bool` [E0605] +} + +fn _slice(bar: &[i32]) -> bool { + bar as bool + //~^ ERROR casting `&[i32]` as `bool` is invalid [E0606] +} diff --git a/tests/ui/cast/issue-106883-is-empty.stderr b/tests/ui/cast/issue-106883-is-empty.stderr new file mode 100644 index 00000000000..7115c7704ca --- /dev/null +++ b/tests/ui/cast/issue-106883-is-empty.stderr @@ -0,0 +1,58 @@ +error[E0606]: casting `&'static str` as `bool` is invalid + --> $DIR/issue-106883-is-empty.rs:14:13 + | +LL | let _ = "foo" as bool; + | ^^^^^^^^^^^^^ + | +help: consider using the `is_empty` method on `&'static str` to determine if it contains anything + | +LL | let _ = !"foo".is_empty(); + | + ~~~~~~~~~~~ + +error[E0605]: non-primitive cast: `String` as `bool` + --> $DIR/issue-106883-is-empty.rs:17:13 + | +LL | let _ = String::from("foo") as bool; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ an `as` expression can only be used to convert between primitive types or to coerce to a specific trait object + | +note: this expression `Deref`s to `str` which implements `is_empty` + --> $DIR/issue-106883-is-empty.rs:17:13 + | +LL | let _ = String::from("foo") as bool; + | ^^^^^^^^^^^^^^^^^^^ +help: consider using the `is_empty` method on `String` to determine if it contains anything + | +LL | let _ = !String::from("foo").is_empty(); + | + ~~~~~~~~~~~ + +error[E0605]: non-primitive cast: `Foo` as `bool` + --> $DIR/issue-106883-is-empty.rs:20:13 + | +LL | let _ = Foo as bool; + | ^^^^^^^^^^^ an `as` expression can only be used to convert between primitive types or to coerce to a specific trait object + | +note: this expression `Deref`s to `[u8]` which implements `is_empty` + --> $DIR/issue-106883-is-empty.rs:20:13 + | +LL | let _ = Foo as bool; + | ^^^ +help: consider using the `is_empty` method on `Foo` to determine if it contains anything + | +LL | let _ = !Foo.is_empty(); + | + ~~~~~~~~~~~ + +error[E0606]: casting `&[i32]` as `bool` is invalid + --> $DIR/issue-106883-is-empty.rs:25:5 + | +LL | bar as bool + | ^^^^^^^^^^^ + | +help: consider using the `is_empty` method on `&[i32]` to determine if it contains anything + | +LL | !bar.is_empty() + | + ~~~~~~~~~~~ + +error: aborting due to 4 previous errors + +Some errors have detailed explanations: E0605, E0606. +For more information about an error, try `rustc --explain E0605`. From 98cfa68f16cb07a3c4bc18daeb071c53904a487f Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 14 Jan 2023 22:17:06 +0100 Subject: [PATCH 3/6] Add tidy check to ensure that rustdoc GUI tests start with a small description --- src/tools/tidy/src/lib.rs | 1 + src/tools/tidy/src/main.rs | 1 + src/tools/tidy/src/rustdoc_gui_tests.rs | 33 +++++++++++++++++++++++++ 3 files changed, 35 insertions(+) create mode 100644 src/tools/tidy/src/rustdoc_gui_tests.rs diff --git a/src/tools/tidy/src/lib.rs b/src/tools/tidy/src/lib.rs index 40375f1306d..97e56720b98 100644 --- a/src/tools/tidy/src/lib.rs +++ b/src/tools/tidy/src/lib.rs @@ -62,6 +62,7 @@ pub mod features; pub mod mir_opt_tests; pub mod pal; pub mod primitive_docs; +pub mod rustdoc_gui_tests; pub mod style; pub mod target_specific_tests; pub mod tests_placement; diff --git a/src/tools/tidy/src/main.rs b/src/tools/tidy/src/main.rs index ea2886a3c2f..0b9a1b37e94 100644 --- a/src/tools/tidy/src/main.rs +++ b/src/tools/tidy/src/main.rs @@ -80,6 +80,7 @@ fn main() { check!(debug_artifacts, &tests_path); check!(ui_tests, &tests_path); check!(mir_opt_tests, &tests_path, bless); + check!(rustdoc_gui_tests, &tests_path); // Checks that only make sense for the compiler. check!(error_codes, &root_path, &[&compiler_path, &librustdoc_path], verbose); diff --git a/src/tools/tidy/src/rustdoc_gui_tests.rs b/src/tools/tidy/src/rustdoc_gui_tests.rs new file mode 100644 index 00000000000..feb513df34b --- /dev/null +++ b/src/tools/tidy/src/rustdoc_gui_tests.rs @@ -0,0 +1,33 @@ +//! Tidy check to ensure that rustdoc GUI tests start with a small description. + +use std::path::Path; + +pub fn check(path: &Path, bad: &mut bool) { + crate::walk::walk( + &path.join("rustdoc-gui"), + &mut |p| { + // If there is no extension, it's very likely a folder and we want to go into it. + p.extension().map(|e| e != "goml").unwrap_or(false) + }, + &mut |entry, content| { + for line in content.lines() { + if !line.starts_with("// ") { + tidy_error!( + bad, + "{}: rustdoc-gui tests must start with a small description", + entry.path().display(), + ); + return; + } else if line.starts_with("// ") { + let parts = line[2..].trim(); + // We ignore tidy comments. + if parts.starts_with("// tidy-") { + continue; + } + // All good! + return; + } + } + }, + ); +} From 5376670323d14870be7d90b585735354674aa4cc Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 14 Jan 2023 22:18:56 +0100 Subject: [PATCH 4/6] Add small description to GUI test --- tests/rustdoc-gui/basic-code.goml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/rustdoc-gui/basic-code.goml b/tests/rustdoc-gui/basic-code.goml index 108cf8abcb5..971c2f9480e 100644 --- a/tests/rustdoc-gui/basic-code.goml +++ b/tests/rustdoc-gui/basic-code.goml @@ -1,3 +1,5 @@ +// Small test to ensure the "src-line-numbers" element is only present once on +// the page. goto: "file://" + |DOC_PATH| + "/test_docs/index.html" click: ".srclink" wait-for: ".src-line-numbers" From 665d4ea98df9b350cc459816337c9fc839bdf128 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20Kr=C3=BCger?= Date: Sun, 15 Jan 2023 15:02:02 +0100 Subject: [PATCH 5/6] remove redundant clones --- compiler/rustc_lint/src/internal.rs | 4 ++-- compiler/rustc_lint/src/pass_by_value.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_lint/src/internal.rs b/compiler/rustc_lint/src/internal.rs index 5eb54cc0034..6cefaea2bc7 100644 --- a/compiler/rustc_lint/src/internal.rs +++ b/compiler/rustc_lint/src/internal.rs @@ -187,9 +187,9 @@ impl<'tcx> LateLintPass<'tcx> for TyTyKind { }, None => cx.emit_spanned_lint(USAGE_OF_TY_TYKIND, path.span, TykindDiag), } - } else if !ty.span.from_expansion() && path.segments.len() > 1 && let Some(t) = is_ty_or_ty_ctxt(cx, &path) { + } else if !ty.span.from_expansion() && path.segments.len() > 1 && let Some(ty) = is_ty_or_ty_ctxt(cx, &path) { cx.emit_spanned_lint(USAGE_OF_QUALIFIED_TY, path.span, TyQualified { - ty: t.clone(), + ty, suggestion: path.span, }); } diff --git a/compiler/rustc_lint/src/pass_by_value.rs b/compiler/rustc_lint/src/pass_by_value.rs index 57482a9edba..392e13f2fa9 100644 --- a/compiler/rustc_lint/src/pass_by_value.rs +++ b/compiler/rustc_lint/src/pass_by_value.rs @@ -32,7 +32,7 @@ impl<'tcx> LateLintPass<'tcx> for PassByValue { cx.emit_spanned_lint( PASS_BY_VALUE, ty.span, - PassByValueDiag { ty: t.clone(), suggestion: ty.span }, + PassByValueDiag { ty: t, suggestion: ty.span }, ); } } From 566202b97511b27e66312f67faacffb7272d7dbb Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 15 Jan 2023 16:33:08 +0000 Subject: [PATCH 6/6] Only suggest adding type param if path being resolved was a type --- compiler/rustc_resolve/src/late.rs | 2 +- tests/ui/suggestions/constrain-suggest-ice.stderr | 11 +---------- tests/ui/typeck/issue-104513-ice.stderr | 5 ----- 3 files changed, 2 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index ca43762aa21..6ca7cd3e713 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -3373,7 +3373,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { sugg.to_string(), Applicability::MaybeIncorrect, )) - } else if res.is_none() { + } else if res.is_none() && matches!(source, PathSource::Type) { this.report_missing_type_error(path) } else { None diff --git a/tests/ui/suggestions/constrain-suggest-ice.stderr b/tests/ui/suggestions/constrain-suggest-ice.stderr index 477eb278679..2af7c2f6971 100644 --- a/tests/ui/suggestions/constrain-suggest-ice.stderr +++ b/tests/ui/suggestions/constrain-suggest-ice.stderr @@ -24,16 +24,7 @@ error[E0425]: cannot find value `F` in this scope --> $DIR/constrain-suggest-ice.rs:6:9 | LL | F - | ^ - | -help: a local variable with a similar name exists - | -LL | x - | ~ -help: you might be missing a type parameter - | -LL | struct Bug{ - | +++ + | ^ help: a local variable with a similar name exists: `x` error: generic `Self` types are currently not permitted in anonymous constants --> $DIR/constrain-suggest-ice.rs:3:21 diff --git a/tests/ui/typeck/issue-104513-ice.stderr b/tests/ui/typeck/issue-104513-ice.stderr index 5561673f3c6..42cfe38aed8 100644 --- a/tests/ui/typeck/issue-104513-ice.stderr +++ b/tests/ui/typeck/issue-104513-ice.stderr @@ -3,11 +3,6 @@ error[E0405]: cannot find trait `Oops` in this scope | LL | let _: S = S; | ^^^^ not found in this scope - | -help: you might be missing a type parameter - | -LL | fn f() { - | ++++++ error[E0562]: `impl Trait` only allowed in function and inherent method return types, not in variable binding --> $DIR/issue-104513-ice.rs:3:14