From fa6ae4c82872d4cd325072400a04e386f4004dc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 24 Nov 2017 14:47:40 -0800 Subject: [PATCH 1/4] Suggest using slice when encountering `let x = ""[..];` --- src/librustc/hir/mod.rs | 17 +++++++ src/librustc/traits/error_reporting.rs | 29 ++++++++++++ .../ui/suggestions/str-array-assignment.rs | 17 +++++++ .../suggestions/str-array-assignment.stderr | 44 +++++++++++++++++++ 4 files changed, 107 insertions(+) create mode 100644 src/test/ui/suggestions/str-array-assignment.rs create mode 100644 src/test/ui/suggestions/str-array-assignment.stderr diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 39ec33eef1fec..986f1b9e4196f 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -244,6 +244,23 @@ impl Path { pub fn is_global(&self) -> bool { !self.segments.is_empty() && self.segments[0].name == keywords::CrateRoot.name() } + + /// Wether this path is any of `::std::ops::{Range, RangeTo, RangeFrom}`. + pub fn is_range(&self) -> bool { + let mut base = ["{{root}}", "std", "ops"].iter().map(|p| p.to_string()).collect::>(); + let range_paths = ["Range", "RangeTo", "RangeFrom"]; + let segments = self.segments.iter() + .map(|segment| format!("{}", segment.name)) + .collect::>(); + for path in &range_paths { + base.push(path.to_string()); + if base == segments { + return true; + } + base.pop(); + } + false + } } impl fmt::Debug for Path { diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index 46ec2be4a1f9b..a8a8b20012b7a 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -581,6 +581,8 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { trait_ref.self_ty())); } + self.suggest_borrow_on_unsized_slice(&obligation.cause.code, &mut err); + // Try to report a help message if !trait_ref.has_infer_types() && self.predicate_can_apply(obligation.param_env, trait_ref) { @@ -821,6 +823,33 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { err.emit(); } + /// When encountering an assignment of an unsized trait, like `let x = ""[..];`, provide a + /// suggestion to borrow the initializer in order to use have a slice instead. + fn suggest_borrow_on_unsized_slice(&self, + code: &ObligationCauseCode<'tcx>, + err: &mut DiagnosticBuilder<'tcx>) { + if let &ObligationCauseCode::VariableType(node_id) = code { + let parent_node = self.tcx.hir.get_parent_node(node_id); + if let Some(hir::map::NodeLocal(ref local)) = self.tcx.hir.find(parent_node) { + if let Some(ref expr) = local.init { + if let hir::ExprIndex(_, ref index) = expr.node { + if let hir::ExprStruct(hir::QPath::Resolved(None, ref path), + ..) = index.node { + if let (Ok(snippet), true) = ( + self.tcx.sess.codemap().span_to_snippet(expr.span), + path.is_range() + ) { + err.span_suggestion(expr.span, + "consider a slice instead", + format!("&{}", snippet)); + } + } + } + } + } + } + } + fn report_arg_count_mismatch( &self, span: Span, diff --git a/src/test/ui/suggestions/str-array-assignment.rs b/src/test/ui/suggestions/str-array-assignment.rs new file mode 100644 index 0000000000000..523e7bea6224b --- /dev/null +++ b/src/test/ui/suggestions/str-array-assignment.rs @@ -0,0 +1,17 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +fn main() { + let s = "abc"; + let t = if true { s[..2] } else { s }; + let u: &str = if true { s[..2] } else { s }; + let v = s[..2]; + let w: &str = s[..2]; +} diff --git a/src/test/ui/suggestions/str-array-assignment.stderr b/src/test/ui/suggestions/str-array-assignment.stderr new file mode 100644 index 0000000000000..225dfbd98fddf --- /dev/null +++ b/src/test/ui/suggestions/str-array-assignment.stderr @@ -0,0 +1,44 @@ +error[E0308]: if and else have incompatible types + --> $DIR/str-array-assignment.rs:13:11 + | +13 | let t = if true { s[..2] } else { s }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected str, found &str + | + = note: expected type `str` + found type `&str` + +error[E0308]: mismatched types + --> $DIR/str-array-assignment.rs:14:27 + | +11 | fn main() { + | - expected `()` because of default return type +... +14 | let u: &str = if true { s[..2] } else { s }; + | ^^^^^^ expected &str, found str + | + = note: expected type `&str` + found type `str` + +error[E0277]: the trait bound `str: std::marker::Sized` is not satisfied + --> $DIR/str-array-assignment.rs:15:7 + | +15 | let v = s[..2]; + | ^ ------ help: consider a slice instead: `&s[..2]` + | | + | `str` does not have a constant size known at compile-time + | + = help: the trait `std::marker::Sized` is not implemented for `str` + = note: all local variables must have a statically known size + +error[E0308]: mismatched types + --> $DIR/str-array-assignment.rs:16:17 + | +16 | let w: &str = s[..2]; + | ^^^^^^ expected &str, found str + | + = note: expected type `&str` + found type `str` + = help: try with `&s[..2]` + +error: aborting due to 4 previous errors + From cde0023b5b71eb4ceafef31dd9036a268ba3b08d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 25 Nov 2017 06:19:39 -0800 Subject: [PATCH 2/4] Add `compile-fail` style comments to tests --- src/test/ui/suggestions/str-array-assignment.rs | 17 ++++++++++++++++- .../ui/suggestions/str-array-assignment.stderr | 14 +++++++------- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/test/ui/suggestions/str-array-assignment.rs b/src/test/ui/suggestions/str-array-assignment.rs index 523e7bea6224b..52b39d390d62b 100644 --- a/src/test/ui/suggestions/str-array-assignment.rs +++ b/src/test/ui/suggestions/str-array-assignment.rs @@ -8,10 +8,25 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -fn main() { +fn main() { //~ NOTE expected `()` because of default return type let s = "abc"; let t = if true { s[..2] } else { s }; + //~^ ERROR if and else have incompatible types + //~| NOTE expected str, found &str + //~| NOTE expected type let u: &str = if true { s[..2] } else { s }; + //~^ ERROR mismatched types + //~| NOTE expected &str, found str + //~| NOTE expected type let v = s[..2]; + //~^ ERROR the trait bound `str: std::marker::Sized` is not satisfied + //~| NOTE consider a slice instead + //~| NOTE `str` does not have a constant size known at compile-time + //~| HELP the trait `std::marker::Sized` is not implemented for `str` + //~| NOTE all local variables must have a statically known size let w: &str = s[..2]; + //~^ ERROR mismatched types + //~| NOTE expected &str, found str + //~| NOTE expected type + //~| HELP try with `&s[..2]` } diff --git a/src/test/ui/suggestions/str-array-assignment.stderr b/src/test/ui/suggestions/str-array-assignment.stderr index 225dfbd98fddf..55006b2760cca 100644 --- a/src/test/ui/suggestions/str-array-assignment.stderr +++ b/src/test/ui/suggestions/str-array-assignment.stderr @@ -8,21 +8,21 @@ error[E0308]: if and else have incompatible types found type `&str` error[E0308]: mismatched types - --> $DIR/str-array-assignment.rs:14:27 + --> $DIR/str-array-assignment.rs:17:27 | -11 | fn main() { +11 | fn main() { //~ NOTE expected `()` because of default return type | - expected `()` because of default return type ... -14 | let u: &str = if true { s[..2] } else { s }; +17 | let u: &str = if true { s[..2] } else { s }; | ^^^^^^ expected &str, found str | = note: expected type `&str` found type `str` error[E0277]: the trait bound `str: std::marker::Sized` is not satisfied - --> $DIR/str-array-assignment.rs:15:7 + --> $DIR/str-array-assignment.rs:21:7 | -15 | let v = s[..2]; +21 | let v = s[..2]; | ^ ------ help: consider a slice instead: `&s[..2]` | | | `str` does not have a constant size known at compile-time @@ -31,9 +31,9 @@ error[E0277]: the trait bound `str: std::marker::Sized` is not satisfied = note: all local variables must have a statically known size error[E0308]: mismatched types - --> $DIR/str-array-assignment.rs:16:17 + --> $DIR/str-array-assignment.rs:27:17 | -16 | let w: &str = s[..2]; +27 | let w: &str = s[..2]; | ^^^^^^ expected &str, found str | = note: expected type `&str` From 97d8d04f3fffd0984a2d2af2f24c104e75be8f01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 25 Nov 2017 06:20:07 -0800 Subject: [PATCH 3/4] Remove index type check (review comment) --- src/librustc/hir/mod.rs | 17 ----------------- src/librustc/traits/error_reporting.rs | 16 +++++----------- src/test/ui/suggestions/str-array-assignment.rs | 2 +- 3 files changed, 6 insertions(+), 29 deletions(-) diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 986f1b9e4196f..39ec33eef1fec 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -244,23 +244,6 @@ impl Path { pub fn is_global(&self) -> bool { !self.segments.is_empty() && self.segments[0].name == keywords::CrateRoot.name() } - - /// Wether this path is any of `::std::ops::{Range, RangeTo, RangeFrom}`. - pub fn is_range(&self) -> bool { - let mut base = ["{{root}}", "std", "ops"].iter().map(|p| p.to_string()).collect::>(); - let range_paths = ["Range", "RangeTo", "RangeFrom"]; - let segments = self.segments.iter() - .map(|segment| format!("{}", segment.name)) - .collect::>(); - for path in &range_paths { - base.push(path.to_string()); - if base == segments { - return true; - } - base.pop(); - } - false - } } impl fmt::Debug for Path { diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index a8a8b20012b7a..46e2c3af5e632 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -832,17 +832,11 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { let parent_node = self.tcx.hir.get_parent_node(node_id); if let Some(hir::map::NodeLocal(ref local)) = self.tcx.hir.find(parent_node) { if let Some(ref expr) = local.init { - if let hir::ExprIndex(_, ref index) = expr.node { - if let hir::ExprStruct(hir::QPath::Resolved(None, ref path), - ..) = index.node { - if let (Ok(snippet), true) = ( - self.tcx.sess.codemap().span_to_snippet(expr.span), - path.is_range() - ) { - err.span_suggestion(expr.span, - "consider a slice instead", - format!("&{}", snippet)); - } + if let hir::ExprIndex(_, _) = expr.node { + if let Ok(snippet) = self.tcx.sess.codemap().span_to_snippet(expr.span) { + err.span_suggestion(expr.span, + "consider a slice instead", + format!("&{}", snippet)); } } } diff --git a/src/test/ui/suggestions/str-array-assignment.rs b/src/test/ui/suggestions/str-array-assignment.rs index 52b39d390d62b..adec495e72a9f 100644 --- a/src/test/ui/suggestions/str-array-assignment.rs +++ b/src/test/ui/suggestions/str-array-assignment.rs @@ -20,7 +20,7 @@ fn main() { //~ NOTE expected `()` because of default return type //~| NOTE expected type let v = s[..2]; //~^ ERROR the trait bound `str: std::marker::Sized` is not satisfied - //~| NOTE consider a slice instead + //~| HELP consider a slice instead //~| NOTE `str` does not have a constant size known at compile-time //~| HELP the trait `std::marker::Sized` is not implemented for `str` //~| NOTE all local variables must have a statically known size From fa44927d2c017359a7d4b14a31e96ee35472b406 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 26 Nov 2017 12:52:00 -0800 Subject: [PATCH 4/4] reword to "consider borrowing here: `{suggestion}`" --- src/librustc/traits/error_reporting.rs | 2 +- src/test/ui/suggestions/str-array-assignment.rs | 2 +- src/test/ui/suggestions/str-array-assignment.stderr | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index 46e2c3af5e632..5703c5c870e88 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -835,7 +835,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { if let hir::ExprIndex(_, _) = expr.node { if let Ok(snippet) = self.tcx.sess.codemap().span_to_snippet(expr.span) { err.span_suggestion(expr.span, - "consider a slice instead", + "consider borrowing here", format!("&{}", snippet)); } } diff --git a/src/test/ui/suggestions/str-array-assignment.rs b/src/test/ui/suggestions/str-array-assignment.rs index adec495e72a9f..6c7f4852a4ae5 100644 --- a/src/test/ui/suggestions/str-array-assignment.rs +++ b/src/test/ui/suggestions/str-array-assignment.rs @@ -20,7 +20,7 @@ fn main() { //~ NOTE expected `()` because of default return type //~| NOTE expected type let v = s[..2]; //~^ ERROR the trait bound `str: std::marker::Sized` is not satisfied - //~| HELP consider a slice instead + //~| HELP consider borrowing here //~| NOTE `str` does not have a constant size known at compile-time //~| HELP the trait `std::marker::Sized` is not implemented for `str` //~| NOTE all local variables must have a statically known size diff --git a/src/test/ui/suggestions/str-array-assignment.stderr b/src/test/ui/suggestions/str-array-assignment.stderr index 55006b2760cca..14b744c2e469c 100644 --- a/src/test/ui/suggestions/str-array-assignment.stderr +++ b/src/test/ui/suggestions/str-array-assignment.stderr @@ -23,7 +23,7 @@ error[E0277]: the trait bound `str: std::marker::Sized` is not satisfied --> $DIR/str-array-assignment.rs:21:7 | 21 | let v = s[..2]; - | ^ ------ help: consider a slice instead: `&s[..2]` + | ^ ------ help: consider borrowing here: `&s[..2]` | | | `str` does not have a constant size known at compile-time |