-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Suggest parentheses for possible range method calling #102454
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -2,6 +2,7 @@ | |||||
//! found or is otherwise invalid. | ||||||
|
||||||
use crate::check::FnCtxt; | ||||||
use crate::errors; | ||||||
use rustc_ast::ast::Mutability; | ||||||
use rustc_data_structures::fx::{FxHashMap, FxHashSet}; | ||||||
use rustc_errors::{ | ||||||
|
@@ -271,9 +272,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { | |||||
} | ||||||
}; | ||||||
|
||||||
if self.suggest_constraining_numerical_ty( | ||||||
tcx, actual, source, span, item_kind, item_name, &ty_str, | ||||||
) { | ||||||
if self.suggest_range_for_iter(tcx, actual, source, span, item_name, &ty_str) | ||||||
|| self.suggest_constraining_numerical_ty( | ||||||
tcx, actual, source, span, item_kind, item_name, &ty_str, | ||||||
) | ||||||
{ | ||||||
return None; | ||||||
} | ||||||
|
||||||
|
@@ -1201,6 +1204,81 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { | |||||
false | ||||||
} | ||||||
|
||||||
fn suggest_range_for_iter( | ||||||
chenyukang marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
&self, | ||||||
tcx: TyCtxt<'tcx>, | ||||||
actual: Ty<'tcx>, | ||||||
source: SelfSource<'tcx>, | ||||||
span: Span, | ||||||
item_name: Ident, | ||||||
ty_str: &str, | ||||||
) -> bool { | ||||||
if let SelfSource::MethodCall(expr) = source { | ||||||
for (_, parent) in tcx.hir().parent_iter(expr.hir_id).take(5) { | ||||||
if let Node::Expr(parent_expr) = parent { | ||||||
let lang_item = match parent_expr.kind { | ||||||
chenyukang marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
ExprKind::Struct(ref qpath, _, _) => match **qpath { | ||||||
QPath::LangItem(LangItem::Range, ..) => Some(LangItem::Range), | ||||||
QPath::LangItem(LangItem::RangeTo, ..) => Some(LangItem::RangeTo), | ||||||
QPath::LangItem(LangItem::RangeToInclusive, ..) => { | ||||||
Some(LangItem::RangeToInclusive) | ||||||
} | ||||||
_ => None, | ||||||
}, | ||||||
ExprKind::Call(ref func, _) => match func.kind { | ||||||
// `..=` desugars into `::std::ops::RangeInclusive::new(...)`. | ||||||
ExprKind::Path(QPath::LangItem(LangItem::RangeInclusiveNew, ..)) => { | ||||||
Some(LangItem::RangeInclusiveStruct) | ||||||
} | ||||||
_ => None, | ||||||
}, | ||||||
_ => None, | ||||||
}; | ||||||
|
||||||
if lang_item.is_none() { | ||||||
continue; | ||||||
} | ||||||
|
||||||
let span_included = match parent_expr.kind { | ||||||
hir::ExprKind::Struct(_, eps, _) => { | ||||||
eps.len() > 0 && eps.last().map_or(false, |ep| ep.span.contains(span)) | ||||||
} | ||||||
// `..=` desugars into `::std::ops::RangeInclusive::new(...)`. | ||||||
hir::ExprKind::Call(ref func, ..) => func.span.contains(span), | ||||||
_ => false, | ||||||
}; | ||||||
|
||||||
if !span_included { | ||||||
continue; | ||||||
} | ||||||
|
||||||
debug!("lang_item: {:?}", lang_item); | ||||||
let range_def_id = self.tcx.require_lang_item(lang_item.unwrap(), None); | ||||||
let range_ty = | ||||||
self.tcx.bound_type_of(range_def_id).subst(self.tcx, &[actual.into()]); | ||||||
|
||||||
let pick = | ||||||
self.lookup_probe(span, item_name, range_ty, expr, ProbeScope::AllTraits); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you emit a comment for why we're using also, i think ideally we would use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems
Use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it seems helpful to not emit
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. updated the code to use
othewise try to find methods in all traits. At this function, we are at a point of error handling of range type checking, since error happened so the |
||||||
if pick.is_ok() { | ||||||
let range_span = parent_expr.span.with_hi(expr.span.hi()); | ||||||
tcx.sess.emit_err(errors::MissingParentheseInRange { | ||||||
span, | ||||||
ty_str: ty_str.to_string(), | ||||||
method_name: item_name.as_str().to_string(), | ||||||
add_missing_parentheses: Some(errors::AddMissingParenthesesInRange { | ||||||
func_name: item_name.name.as_str().to_string(), | ||||||
left: range_span.shrink_to_lo(), | ||||||
right: range_span.shrink_to_hi(), | ||||||
}), | ||||||
}); | ||||||
return true; | ||||||
} | ||||||
} | ||||||
} | ||||||
} | ||||||
false | ||||||
} | ||||||
|
||||||
fn suggest_constraining_numerical_ty( | ||||||
&self, | ||||||
tcx: TyCtxt<'tcx>, | ||||||
|
@@ -1263,7 +1341,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { | |||||
// If this is a floating point literal that ends with '.', | ||||||
// get rid of it to stop this from becoming a member access. | ||||||
let snippet = snippet.strip_suffix('.').unwrap_or(&snippet); | ||||||
|
||||||
err.span_suggestion( | ||||||
lit.span, | ||||||
&format!( | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,76 @@ | ||
#![allow(unused)] | ||
fn main() { | ||
chenyukang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
let arr = &[0,1,2,3]; | ||
for _i in 0..arr.len().rev() { //~ERROR not an iterator | ||
// The above error used to say “the method `rev` exists for type `usize`”. | ||
// This regression test ensures it doesn't say that any more. | ||
} | ||
let arr = &[0, 1, 2, 3]; | ||
for _i in 0..arr.len().rev() { | ||
//~^ ERROR can't call method | ||
//~| surround the range in parentheses | ||
// The above error used to say “the method `rev` exists for type `usize`”. | ||
// This regression test ensures it doesn't say that any more. | ||
} | ||
|
||
// Test for #102396 | ||
for i in 1..11.rev() { | ||
//~^ ERROR can't call method | ||
//~| HELP surround the range in parentheses | ||
} | ||
|
||
let end: usize = 10; | ||
for i in 1..end.rev() { | ||
//~^ ERROR can't call method | ||
//~| HELP surround the range in parentheses | ||
} | ||
|
||
for i in 1..(end + 1).rev() { | ||
//~^ ERROR can't call method | ||
//~| HELP surround the range in parentheses | ||
} | ||
|
||
if 1..(end + 1).is_empty() { | ||
//~^ ERROR can't call method | ||
//~| ERROR mismatched types [E0308] | ||
//~| HELP surround the range in parentheses | ||
} | ||
|
||
if 1..(end + 1).is_sorted() { | ||
//~^ ERROR mismatched types [E0308] | ||
//~| ERROR can't call method | ||
//~| HELP surround the range in parentheses | ||
} | ||
|
||
let _res: i32 = 3..6.take(2).sum(); | ||
//~^ ERROR can't call method | ||
//~| ERROR mismatched types [E0308] | ||
//~| HELP surround the range in parentheses | ||
|
||
let _sum: i32 = 3..6.sum(); | ||
//~^ ERROR can't call method | ||
//~| ERROR mismatched types [E0308] | ||
//~| HELP surround the range in parentheses | ||
|
||
let a = 1 as usize; | ||
let b = 10 as usize; | ||
|
||
for _a in a..=b.rev() { | ||
//~^ ERROR can't call method | ||
//~| HELP surround the range in parentheses | ||
} | ||
|
||
let _res = ..10.contains(3); | ||
//~^ ERROR can't call method | ||
//~| HELP surround the range in parentheses | ||
|
||
if 1..end.error_method() { | ||
//~^ ERROR no method named `error_method` | ||
//~| ERROR mismatched types [E0308] | ||
// Won't suggest | ||
} | ||
|
||
let _res = b.take(1)..a; | ||
//~^ ERROR `usize` is not an iterator | ||
|
||
let _res: i32 = ..6.take(2).sum(); | ||
//~^ can't call method `take` on ambiguous numeric type | ||
//~| ERROR mismatched types [E0308] | ||
//~| HELP you must specify a concrete type for this numeric value | ||
// Won't suggest because `RangeTo` dest not implemented `take` | ||
} | ||
chenyukang marked this conversation as resolved.
Show resolved
Hide resolved
|
Uh oh!
There was an error while loading. Please reload this page.