Skip to content

Commit

Permalink
Auto merge of rust-lang#10160 - llogiq:default-trim-paths, r=dswij
Browse files Browse the repository at this point in the history
trim paths in `default_trait_access`/`clone_on_copy` suggestions

This should help making the suggestions more palatable. Similar to rust-lang#10153.

---

changelog: trim paths in [`default_trait_access`]/[`clone_on_copy`] suggestions
  • Loading branch information
bors committed Jan 6, 2023
2 parents 3816f9a + 05ba519 commit 3f48ed5
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 27 deletions.
5 changes: 2 additions & 3 deletions clippy_lints/src/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use rustc_hir::def::Res;
use rustc_hir::{Block, Expr, ExprKind, PatKind, QPath, Stmt, StmtKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_middle::ty::print::with_forced_trimmed_paths;
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::symbol::{Ident, Symbol};
use rustc_span::Span;
Expand Down Expand Up @@ -98,9 +99,7 @@ impl<'tcx> LateLintPass<'tcx> for Default {
if let ty::Adt(def, ..) = expr_ty.kind();
if !is_from_proc_macro(cx, expr);
then {
// TODO: Work out a way to put "whatever the imported way of referencing
// this type in this file" rather than a fully-qualified type.
let replacement = format!("{}::default()", cx.tcx.def_path_str(def.did()));
let replacement = with_forced_trimmed_paths!(format!("{}::default()", cx.tcx.def_path_str(def.did())));
span_lint_and_sugg(
cx,
DEFAULT_TRAIT_ACCESS,
Expand Down
14 changes: 8 additions & 6 deletions clippy_lints/src/methods/clone_on_copy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use clippy_utils::ty::is_copy;
use rustc_errors::Applicability;
use rustc_hir::{BindingAnnotation, ByRef, Expr, ExprKind, MatchSource, Node, PatKind, QPath};
use rustc_lint::LateContext;
use rustc_middle::ty::{self, adjustment::Adjust};
use rustc_middle::ty::{self, adjustment::Adjust, print::with_forced_trimmed_paths};
use rustc_span::symbol::{sym, Symbol};

use super::CLONE_DOUBLE_REF;
Expand Down Expand Up @@ -47,10 +47,10 @@ pub(super) fn check(
cx,
CLONE_DOUBLE_REF,
expr.span,
&format!(
&with_forced_trimmed_paths!(format!(
"using `clone` on a double-reference; \
this will copy the reference of type `{ty}` instead of cloning the inner type"
),
)),
|diag| {
if let Some(snip) = sugg::Sugg::hir_opt(cx, arg) {
let mut ty = innermost;
Expand All @@ -61,11 +61,11 @@ pub(super) fn check(
}
let refs = "&".repeat(n + 1);
let derefs = "*".repeat(n);
let explicit = format!("<{refs}{ty}>::clone({snip})");
let explicit = with_forced_trimmed_paths!(format!("<{refs}{ty}>::clone({snip})"));
diag.span_suggestion(
expr.span,
"try dereferencing it",
format!("{refs}({derefs}{}).clone()", snip.deref()),
with_forced_trimmed_paths!(format!("{refs}({derefs}{}).clone()", snip.deref())),
Applicability::MaybeIncorrect,
);
diag.span_suggestion(
Expand Down Expand Up @@ -129,7 +129,9 @@ pub(super) fn check(
cx,
CLONE_ON_COPY,
expr.span,
&format!("using `clone` on type `{ty}` which implements the `Copy` trait"),
&with_forced_trimmed_paths!(format!(
"using `clone` on type `{ty}` which implements the `Copy` trait"
)),
help,
sugg,
app,
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/clone_on_copy.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ error: using `clone` on type `i32` which implements the `Copy` trait
LL | vec.push(42.clone());
| ^^^^^^^^^^ help: try removing the `clone` call: `42`

error: using `clone` on type `std::option::Option<i32>` which implements the `Copy` trait
error: using `clone` on type `Option<i32>` which implements the `Copy` trait
--> $DIR/clone_on_copy.rs:77:17
|
LL | let value = opt.clone()?; // operator precedence needed (*opt)?
Expand Down
8 changes: 4 additions & 4 deletions tests/ui/default_trait_access.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,17 @@ use std::default::Default as D2;
use std::string;

fn main() {
let s1: String = std::string::String::default();
let s1: String = String::default();

let s2 = String::default();

let s3: String = std::string::String::default();
let s3: String = String::default();

let s4: String = std::string::String::default();
let s4: String = String::default();

let s5 = string::String::default();

let s6: String = std::string::String::default();
let s6: String = String::default();

let s7 = std::string::String::default();

Expand Down
16 changes: 8 additions & 8 deletions tests/ui/default_trait_access.stderr
Original file line number Diff line number Diff line change
@@ -1,32 +1,32 @@
error: calling `std::string::String::default()` is more clear than this expression
error: calling `String::default()` is more clear than this expression
--> $DIR/default_trait_access.rs:15:22
|
LL | let s1: String = Default::default();
| ^^^^^^^^^^^^^^^^^^ help: try: `std::string::String::default()`
| ^^^^^^^^^^^^^^^^^^ help: try: `String::default()`
|
note: the lint level is defined here
--> $DIR/default_trait_access.rs:3:9
|
LL | #![deny(clippy::default_trait_access)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: calling `std::string::String::default()` is more clear than this expression
error: calling `String::default()` is more clear than this expression
--> $DIR/default_trait_access.rs:19:22
|
LL | let s3: String = D2::default();
| ^^^^^^^^^^^^^ help: try: `std::string::String::default()`
| ^^^^^^^^^^^^^ help: try: `String::default()`

error: calling `std::string::String::default()` is more clear than this expression
error: calling `String::default()` is more clear than this expression
--> $DIR/default_trait_access.rs:21:22
|
LL | let s4: String = std::default::Default::default();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::string::String::default()`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `String::default()`

error: calling `std::string::String::default()` is more clear than this expression
error: calling `String::default()` is more clear than this expression
--> $DIR/default_trait_access.rs:25:22
|
LL | let s6: String = default::Default::default();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::string::String::default()`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `String::default()`

error: calling `GenericDerivedDefault::default()` is more clear than this expression
--> $DIR/default_trait_access.rs:35:46
Expand Down
10 changes: 5 additions & 5 deletions tests/ui/unnecessary_clone.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,13 @@ LL | t.clone();
|
= note: `-D clippy::clone-on-copy` implied by `-D warnings`

error: using `clone` on type `std::option::Option<T>` which implements the `Copy` trait
error: using `clone` on type `Option<T>` which implements the `Copy` trait
--> $DIR/unnecessary_clone.rs:42:5
|
LL | Some(t).clone();
| ^^^^^^^^^^^^^^^ help: try removing the `clone` call: `Some(t)`

error: using `clone` on a double-reference; this will copy the reference of type `&std::vec::Vec<i32>` instead of cloning the inner type
error: using `clone` on a double-reference; this will copy the reference of type `&Vec<i32>` instead of cloning the inner type
--> $DIR/unnecessary_clone.rs:48:22
|
LL | let z: &Vec<_> = y.clone();
Expand All @@ -57,10 +57,10 @@ LL | let z: &Vec<_> = &(*y).clone();
| ~~~~~~~~~~~~~
help: or try being explicit if you are sure, that you want to clone a reference
|
LL | let z: &Vec<_> = <&std::vec::Vec<i32>>::clone(y);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
LL | let z: &Vec<_> = <&Vec<i32>>::clone(y);
| ~~~~~~~~~~~~~~~~~~~~~

error: using `clone` on type `many_derefs::E` which implements the `Copy` trait
error: using `clone` on type `E` which implements the `Copy` trait
--> $DIR/unnecessary_clone.rs:84:20
|
LL | let _: E = a.clone();
Expand Down

0 comments on commit 3f48ed5

Please sign in to comment.