Skip to content

Commit

Permalink
Auto merge of #125397 - gurry:125303-wrong-builder-suggestion, r=comp…
Browse files Browse the repository at this point in the history
…iler-errors

Do not suggest unresolvable builder methods

Fixes #125303

The issue was that when a builder method cannot be resolved we are suggesting alternatives that themselves cannot be resolved. This PR adds a check that filters them from the list of suggestions.
  • Loading branch information
bors committed Jun 3, 2024
2 parents 9f2d0b3 + 273a78b commit 865eaf9
Show file tree
Hide file tree
Showing 10 changed files with 141 additions and 26 deletions.
1 change: 1 addition & 0 deletions compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1347,6 +1347,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Some(rcvr),
rcvr_t,
segment.ident,
expr.hir_id,
SelfSource::MethodCall(rcvr),
error,
Some(args),
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -832,6 +832,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
None,
ty.normalized,
item_name,
hir_id,
SelfSource::QPath(qself),
error,
args,
Expand Down
26 changes: 22 additions & 4 deletions compiler/rustc_hir_typeck/src/method/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
rcvr_opt: Option<&'tcx hir::Expr<'tcx>>,
rcvr_ty: Ty<'tcx>,
item_name: Ident,
expr_id: hir::HirId,
source: SelfSource<'tcx>,
error: MethodError<'tcx>,
args: Option<&'tcx [hir::Expr<'tcx>]>,
Expand All @@ -216,6 +217,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
rcvr_opt,
rcvr_ty,
item_name,
expr_id,
source,
args,
sugg_span,
Expand Down Expand Up @@ -549,6 +551,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
rcvr_opt: Option<&'tcx hir::Expr<'tcx>>,
rcvr_ty: Ty<'tcx>,
item_name: Ident,
expr_id: hir::HirId,
source: SelfSource<'tcx>,
args: Option<&'tcx [hir::Expr<'tcx>]>,
sugg_span: Span,
Expand Down Expand Up @@ -681,7 +684,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}

if matches!(source, SelfSource::QPath(_)) && args.is_some() {
self.find_builder_fn(&mut err, rcvr_ty);
self.find_builder_fn(&mut err, rcvr_ty, expr_id);
}

if tcx.ty_is_opaque_future(rcvr_ty) && item_name.name == sym::poll {
Expand Down Expand Up @@ -1942,7 +1945,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

/// Look at all the associated functions without receivers in the type's inherent impls
/// to look for builders that return `Self`, `Option<Self>` or `Result<Self, _>`.
fn find_builder_fn(&self, err: &mut Diag<'_>, rcvr_ty: Ty<'tcx>) {
fn find_builder_fn(&self, err: &mut Diag<'_>, rcvr_ty: Ty<'tcx>, expr_id: hir::HirId) {
let ty::Adt(adt_def, _) = rcvr_ty.kind() else {
return;
};
Expand All @@ -1951,8 +1954,23 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let mut items = impls
.iter()
.flat_map(|i| self.tcx.associated_items(i).in_definition_order())
// Only assoc fn with no receivers.
.filter(|item| matches!(item.kind, ty::AssocKind::Fn) && !item.fn_has_self_parameter)
// Only assoc fn with no receivers and only if
// they are resolvable
.filter(|item| {
matches!(item.kind, ty::AssocKind::Fn)
&& !item.fn_has_self_parameter
&& self
.probe_for_name(
Mode::Path,
item.ident(self.tcx),
None,
IsSuggestion(true),
rcvr_ty,
expr_id,
ProbeScope::TraitsInScope,
)
.is_ok()
})
.filter_map(|item| {
// Only assoc fns that return `Self`, `Option<Self>` or `Result<Self, _>`.
let ret_ty = self
Expand Down
5 changes: 0 additions & 5 deletions tests/ui/resolve/fn-new-doesnt-exist.rs

This file was deleted.

14 changes: 0 additions & 14 deletions tests/ui/resolve/fn-new-doesnt-exist.stderr

This file was deleted.

64 changes: 64 additions & 0 deletions tests/ui/resolve/suggest-builder-fn.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
// Tests that we suggest the right alternatives when
// a builder method cannot be resolved

use std::net::TcpStream;

trait SomeTrait {}

struct Foo<T> {
v : T
}

impl<T: SomeTrait + Default> Foo<T> {
// Should not be suggested if constraint on the impl not met
fn new() -> Self {
Self { v: T::default() }
}
}

struct Bar;

impl Bar {
// Should be suggested
fn build() -> Self {
Self {}
}

// Method with self can't be a builder.
// Should not be suggested
fn build_with_self(&self) -> Self {
Self {}
}
}

mod SomeMod {
use Bar;

impl Bar {
// Public method. Should be suggested
pub fn build_public() -> Self {
Self {}
}

// Private method. Should not be suggested
fn build_private() -> Self {
Self {}
}
}
}

fn main() {
// `new` not found on `TcpStream` and `connect` should be suggested
let _stream = TcpStream::new();
//~^ ERROR no function or associated item named `new` found

// Although `new` is found on `<impl Foo<T>>` it should not be
// suggested because `u8` does not meet the `T: SomeTrait` constraint
let _foo = Foo::<u8>::new();
//~^ ERROR the function or associated item `new` exists for struct `Foo<u8>`, but its trait bounds were not satisfied

// Should suggest only `<impl Bar>::build()` and `SomeMod::<impl Bar>::build_public()`.
// Other methods should not suggested because they are private or are not a builder
let _bar = Bar::new();
//~^ ERROR no function or associated item named `new` found
}
51 changes: 51 additions & 0 deletions tests/ui/resolve/suggest-builder-fn.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
error[E0599]: no function or associated item named `new` found for struct `TcpStream` in the current scope
--> $DIR/suggest-builder-fn.rs:52:29
|
LL | let _stream = TcpStream::new();
| ^^^ function or associated item not found in `TcpStream`
|
note: if you're trying to build a new `TcpStream` consider using one of the following associated functions:
TcpStream::connect
TcpStream::connect_timeout
--> $SRC_DIR/std/src/net/tcp.rs:LL:COL

error[E0599]: the function or associated item `new` exists for struct `Foo<u8>`, but its trait bounds were not satisfied
--> $DIR/suggest-builder-fn.rs:57:27
|
LL | struct Foo<T> {
| ------------- function or associated item `new` not found for this struct
...
LL | let _foo = Foo::<u8>::new();
| ^^^ function or associated item cannot be called on `Foo<u8>` due to unsatisfied trait bounds
|
note: trait bound `u8: SomeTrait` was not satisfied
--> $DIR/suggest-builder-fn.rs:12:9
|
LL | impl<T: SomeTrait + Default> Foo<T> {
| ^^^^^^^^^ ------
| |
| unsatisfied trait bound introduced here

error[E0599]: no function or associated item named `new` found for struct `Bar` in the current scope
--> $DIR/suggest-builder-fn.rs:62:21
|
LL | struct Bar;
| ---------- function or associated item `new` not found for this struct
...
LL | let _bar = Bar::new();
| ^^^ function or associated item not found in `Bar`
|
note: if you're trying to build a new `Bar` consider using one of the following associated functions:
Bar::build
SomeMod::<impl Bar>::build_public
--> $DIR/suggest-builder-fn.rs:23:5
|
LL | fn build() -> Self {
| ^^^^^^^^^^^^^^^^^^
...
LL | pub fn build_public() -> Self {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0599`.
2 changes: 1 addition & 1 deletion tests/ui/suggestions/deref-path-method.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ note: if you're trying to build a new `Vec<_, _>` consider using one of the foll
Vec::<T>::with_capacity
Vec::<T>::try_with_capacity
Vec::<T>::from_raw_parts
and 6 others
and 4 others
--> $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
help: the function `contains` is implemented on `[_]`
|
Expand Down
1 change: 0 additions & 1 deletion tests/ui/suggestions/issue-109291.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ note: if you're trying to build a new `Backtrace` consider using one of the foll
Backtrace::capture
Backtrace::force_capture
Backtrace::disabled
Backtrace::create
--> $SRC_DIR/std/src/backtrace.rs:LL:COL
help: there is an associated function `force_capture` with a similar name
|
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/ufcs/bad-builder.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ note: if you're trying to build a new `Vec<Q>` consider using one of the followi
Vec::<T>::with_capacity
Vec::<T>::try_with_capacity
Vec::<T>::from_raw_parts
and 6 others
and 4 others
--> $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
help: there is an associated function `new` with a similar name
|
Expand Down

0 comments on commit 865eaf9

Please sign in to comment.