Auto merge of #106914 - matthiaskrgr:rollup-yh0x4gq, r=matthiaskrgr

bors · bors · commit 9a19e7604436 · 2023-01-15T21:35:50.000Z
Rollup of 5 pull requests Successful merges: - #106888 (Add tidy check to ensure that rustdoc GUI tests start with a small description) - #106896 (suggest `is_empty` for collections when casting to `bool`) - #106900 (Fix regression in `unused_braces` with macros) - #106906 (remove redundant clones) - #106909 (Only suggest adding type param if path being resolved was a type) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
diff --git 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/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
@@ -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 },
                     );
                 }
             }
diff --git 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<const N: usize>; let _: A<{ 2 + 3 }>;`)
-                //
+                //   not created by a macro, i.e. do not lint on:
+                //      ```
+                //      struct A<const N: usize>;
+                //      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/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/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
@@ -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
@@ -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;
+                }
+            }
+        },
+    );
+}
diff --git 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"
diff --git 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
@@ -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
@@ -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
@@ -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`.
diff --git 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
@@ -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
@@ -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
 
diff --git 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<S, F>{
-   |             +++
+   |         ^ 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
@@ -3,11 +3,6 @@ error[E0405]: cannot find trait `Oops` in this scope
    |
 LL |     let _: S<impl Oops> = S;
    |                   ^^^^ not found in this scope
-   |
-help: you might be missing a type parameter
-   |
-LL | fn f<Oops>() {
-   |     ++++++
 
 error[E0562]: `impl Trait` only allowed in function and inherent method return types, not in variable binding
   --> $DIR/issue-104513-ice.rs:3:14

Original file line number	Diff line number	Diff line change
`@@ -187,9 +187,9 @@ impl<'tcx> LateLintPass<'tcx> for TyTyKind {`
`187`	`187`	`},`
`188`	`188`	`None => cx.emit_spanned_lint(USAGE_OF_TY_TYKIND, path.span, TykindDiag),`
`189`	`189`	`}`
`190`		`- } else if !ty.span.from_expansion() && path.segments.len() > 1 && let Some(t) = is_ty_or_ty_ctxt(cx, &path) {`
	`190`	`+ } else if !ty.span.from_expansion() && path.segments.len() > 1 && let Some(ty) = is_ty_or_ty_ctxt(cx, &path) {`
`191`	`191`	`cx.emit_spanned_lint(USAGE_OF_QUALIFIED_TY, path.span, TyQualified {`
`192`		`- ty: t.clone(),`
	`192`	`+ ty,`
`193`	`193`	`suggestion: path.span,`
`194`	`194`	`});`
`195`	`195`	`}`
Original file line number	Diff line number	Diff line change
`@@ -32,7 +32,7 @@ impl<'tcx> LateLintPass<'tcx> for PassByValue {`
`32`	`32`	`cx.emit_spanned_lint(`
`33`	`33`	`PASS_BY_VALUE,`
`34`	`34`	`ty.span,`
`35`		`- PassByValueDiag { ty: t.clone(), suggestion: ty.span },`
	`35`	`+ PassByValueDiag { ty: t, suggestion: ty.span },`
`36`	`36`	`);`
`37`	`37`	`}`
`38`	`38`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,5 @@`
	`1`	`+// Small test to ensure the "src-line-numbers" element is only present once on`
	`2`	`+// the page.`
`1`	`3`	`goto: "file://" + \|DOC_PATH\| + "/test_docs/index.html"`
`2`	`4`	`click: ".srclink"`
`3`	`5`	`wait-for: ".src-line-numbers"`
Original file line number	Diff line number	Diff line change
`@@ -50,4 +50,8 @@ fn main() {`
`50`	`50`	`if { return } {`
`51`	`51`
`52`	`52`	`}`
	`53`	`+`
	`54`	`+ // regression test for https://github.com/rust-lang/rust/issues/106899`
	`55`	`+ return println!("!");`
	`56`	`+ //~^ WARN unnecessary braces`
`53`	`57`	`}`