Skip to content

Commit b2f88e9

Browse files
fix(eta_reduction): don't lint when method crate has duplicate name\n\nThe redundant_closure_for_method_calls lint generates a textual path\nfor the suggested replacement via get_path_from_caller_to_method_type\n(e.g. ipnetwork::IpNetwork::ip). When a project has a \"diamond\ndependency\" — two direct or transitive dependencies that pull in\ndifferent versions of the same library — that path will resolve to\nwhatever version is in scope at the call site, which may not be the\nversion that the method actually belongs to. Applying the suggestion\nthen breaks the build.\n\nFix: in the MethodCall branch, add a guard using the new helper\nis_method_from_ambiguous_crate that checks whether more than one\nexternal crate is loaded under the same name as the method's defining\ncrate. If so, avoid linting because we cannot generate an unambiguous\npath.\n\nFixes #16788"
1 parent c07baa4 commit b2f88e9

1 file changed

Lines changed: 32 additions & 0 deletions

File tree

clippy_lints/src/eta_reduction.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use clippy_utils::usage::{local_used_after_expr, local_used_in};
66
use clippy_utils::{get_path_from_caller_to_method_type, is_adjusted, is_no_std_crate};
77
use rustc_abi::ExternAbi;
88
use rustc_errors::Applicability;
9+
use rustc_hir::def_id::DefId;
910
use rustc_hir::{BindingMode, Expr, ExprKind, FnRetTy, GenericArgs, Param, PatKind, QPath, Safety, TyKind, find_attr};
1011
use rustc_infer::infer::TyCtxtInferExt;
1112
use rustc_lint::{LateContext, LateLintPass};
@@ -266,6 +267,12 @@ fn check_closure<'tcx>(cx: &LateContext<'tcx>, outer_receiver: Option<&Expr<'tcx
266267
if let Some(method_def_id) = typeck.type_dependent_def_id(body.value.hir_id)
267268
&& !find_attr!(cx.tcx, method_def_id, TrackCaller(..))
268269
&& check_sig(closure_sig, cx.tcx.fn_sig(method_def_id).skip_binder().skip_binder())
270+
// Do not lint when the method's crate has a duplicate name among the loaded
271+
// external crates ("diamond dependency" / multiple crate versions). In that
272+
// situation the path Clippy would generate (e.g. `ipnetwork::IpNetwork::ip`)
273+
// resolves to the *wrong* version at the call site, producing a breaking
274+
// suggestion. See <https://github.com/rust-lang/rust-clippy/issues/16788>.
275+
&& !is_method_from_ambiguous_crate(cx, method_def_id)
269276
{
270277
let mut app = Applicability::MachineApplicable;
271278
let generic_args = match path.args.and_then(GenericArgs::span_ext) {
@@ -385,3 +392,28 @@ fn ty_has_static(ty: Ty<'_>) -> bool {
385392
ty.walk()
386393
.any(|arg| matches!(arg.kind(), GenericArgKind::Lifetime(re) if re.is_static()))
387394
}
395+
396+
/// Returns `true` when more than one external crate is loaded under the same name as
397+
/// `method_def_id`'s defining crate.
398+
///
399+
/// This detects the "diamond dependency" / multiple-crate-version scenario where two
400+
/// packages depend on different versions of the same library (e.g. `ipnetwork 0.20` and
401+
/// `ipnetwork 0.21`). In that case the textual path Clippy would generate for the
402+
/// suggested replacement — such as `ipnetwork::IpNetwork::ip` — has no way to select
403+
/// the correct version: it will resolve to whichever version is *directly* depended on
404+
/// by the user's crate, which may differ from the version the method actually belongs to.
405+
/// Suppressing the lint avoids producing a suggestion that breaks the build.
406+
///
407+
/// For local methods this is never ambiguous, so we return `false` immediately.
408+
fn is_method_from_ambiguous_crate(cx: &LateContext<'_>, method_def_id: DefId) -> bool {
409+
if method_def_id.is_local() {
410+
return false;
411+
}
412+
let method_crate_name = cx.tcx.crate_name(method_def_id.krate);
413+
cx.tcx
414+
.crates(())
415+
.iter()
416+
.filter(|&&krate| cx.tcx.crate_name(krate) == method_crate_name)
417+
.count()
418+
> 1
419+
}

0 commit comments

Comments
 (0)