Split refining_impl_trait lint into _reachable, _internal variants

rust-lang · bors · Mar 17, 2024 · Feb 28, 2024 · Mar 6, 2024 · c121a26ab978681db9133865089a67a3d9723eda
commit c121a26ab978681db9133865089a67a3d9723eda
diff --git a/compiler/rustc_hir_analysis/messages.ftl b/compiler/rustc_hir_analysis/messages.ftl
@@ -347,6 347,7 @@ hir_analysis_rpitit_refined = impl trait in impl method signature does not match
  .label = return type from trait method defined here
  .unmatched_bound_label = this bound is stronger than that defined on the trait
  .note = add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate
  .feedback_note = we are soliciting feedback, see issue #121718 <https://github.com/rust-lang/rust/issues/121718> for more information
 
 hir_analysis_self_in_impl_self =
  `Self` is not valid in the self type of an impl block

diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs
@@ -2,7 2,7 @@ use rustc_data_structures::fx::FxIndexSet;
 use rustc_hir as hir;
 use rustc_hir::def_id::DefId;
 use rustc_infer::infer::{outlives::env::OutlivesEnvironment, TyCtxtInferExt};
-use rustc_lint_defs::builtin::REFINING_IMPL_TRAIT;
 use rustc_lint_defs::builtin::{REFINING_IMPL_TRAIT_INTERNAL, REFINING_IMPL_TRAIT_REACHABLE};
 use rustc_middle::traits::{ObligationCause, Reveal};
 use rustc_middle::ty::{
  self, Ty, TyCtxt, TypeFoldable, TypeFolder, TypeSuperVisitable, TypeVisitable, TypeVisitor,
@@ -24,26 24,23 @@ pub(super) fn check_refining_return_position_impl_trait_in_trait<'tcx>(
  if !tcx.impl_method_has_trait_impl_trait_tys(impl_m.def_id) {
  return;
  }
 
  // unreachable traits don't have any library guarantees, there's no need to do this check.
- if trait_m
  let is_internal = trait_m
  .container_id(tcx)
  .as_local()
  .is_some_and(|trait_def_id| !tcx.effective_visibilities(()).is_reachable(trait_def_id))
- {
- return;
- }
  // If a type in the trait ref is private, then there's also no reason to do this check.
  || impl_trait_ref.args.iter().any(|arg| {
  if let Some(ty) = arg.as_type()
  && let Some(self_visibility) = type_visibility(tcx, ty)
  {
  return !self_visibility.is_public();
  }
  false
  });
 
- // If a type in the trait ref is private, then there's also no reason to do this check.
  let impl_def_id = impl_m.container_id(tcx);
- for arg in impl_trait_ref.args {
- if let Some(ty) = arg.as_type()
- && let Some(self_visibility) = type_visibility(tcx, ty)
- && !self_visibility.is_public()
- {
- return;
- }
- }
-
  let impl_m_args = ty::GenericArgs::identity_for_item(tcx, impl_m.def_id);
  let trait_m_to_impl_m_args = impl_m_args.rebase_onto(tcx, impl_def_id, impl_trait_ref.args);
  let bound_trait_m_sig = tcx.fn_sig(trait_m.def_id).instantiate(tcx, trait_m_to_impl_m_args);
@@ -86,6 83,7 @@ pub(super) fn check_refining_return_position_impl_trait_in_trait<'tcx>(
  trait_m.def_id,
  impl_m.def_id,
  None,
  is_internal,
  );
  return;
  };
@@ -105,6 103,7 @@ pub(super) fn check_refining_return_position_impl_trait_in_trait<'tcx>(
  trait_m.def_id,
  impl_m.def_id,
  None,
  is_internal,
  );
  return;
  }
@@ -199,6 198,7 @@ pub(super) fn check_refining_return_position_impl_trait_in_trait<'tcx>(
  trait_m.def_id,
  impl_m.def_id,
  Some(span),
  is_internal,
  );
  return;
  }
@@ -239,6 239,7 @@ fn report_mismatched_rpitit_signature<'tcx>(
  trait_m_def_id: DefId,
  impl_m_def_id: DefId,
  unmatched_bound: Option<Span>,
  is_internal: bool,
 ) {
  let mapping = std::iter::zip(
  tcx.fn_sig(trait_m_def_id).skip_binder().bound_vars(),
@@ -291,7 292,7 @@ fn report_mismatched_rpitit_signature<'tcx>(
 
  let span = unmatched_bound.unwrap_or(span);
  tcx.emit_node_span_lint(
- REFINING_IMPL_TRAIT,
  if is_internal { REFINING_IMPL_TRAIT_INTERNAL } else { REFINING_IMPL_TRAIT_REACHABLE },
  tcx.local_def_id_to_hir_id(impl_m_def_id.expect_local()),
  span,
  crate::errors::ReturnPositionImplTraitInTraitRefined {

diff --git a/compiler/rustc_hir_analysis/src/errors.rs b/compiler/rustc_hir_analysis/src/errors.rs
@@ -1072,6 1072,7 @@ pub struct UnusedAssociatedTypeBounds {
 #[derive(LintDiagnostic)]
 #[diag(hir_analysis_rpitit_refined)]
 #[note]
 #[note(hir_analysis_feedback_note)]
 pub(crate) struct ReturnPositionImplTraitInTraitRefined<'tcx> {
  #[suggestion(applicability = "maybe-incorrect", code = "{pre}{return_ty}{post}")]
  pub impl_return_span: Span,

diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs
@@ -313,6 313,12 @@ fn register_builtins(store: &mut LintStore) {
  // MACRO_USE_EXTERN_CRATE
  );
 
  add_lint_group!(
  "refining_impl_trait",
  REFINING_IMPL_TRAIT_REACHABLE,
  REFINING_IMPL_TRAIT_INTERNAL
  );
 
  // Register renamed and removed lints.
  store.register_renamed("single_use_lifetime", "single_use_lifetimes");
  store.register_renamed("elided_lifetime_in_path", "elided_lifetimes_in_paths");

diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs
@@ -77,7 77,8 @@ declare_lint_pass! {
  PROC_MACRO_BACK_COMPAT,
  PROC_MACRO_DERIVE_RESOLUTION_FALLBACK,
  PUB_USE_OF_PRIVATE_EXTERN_CRATE,
- REFINING_IMPL_TRAIT,
  REFINING_IMPL_TRAIT_INTERNAL,
  REFINING_IMPL_TRAIT_REACHABLE,
  RENAMED_AND_REMOVED_LINTS,
  REPR_TRANSPARENT_EXTERNAL_PRIVATE_FIELDS,
  RUST_2021_INCOMPATIBLE_CLOSURE_CAPTURES,
@@ -4310,8 4311,10 @@ declare_lint! {
 }
 
 declare_lint! {
- /// The `refining_impl_trait` lint detects usages of return-position impl
- /// traits in trait signatures which are refined by implementations.
  /// The `refining_impl_trait_reachable` lint detects `impl Trait` return
  /// types in method signatures that are refined by a publically reachable
  /// trait implementation, meaning the implementation adds information about
  /// the return type that is not present in the trait.
  ///
  /// ### Example
  ///
@@ -4333,21 4336,88 @@ declare_lint! {
  /// fn main() {
  /// // users can observe that the return type of
  /// // `<&str as AsDisplay>::as_display()` is `&str`.
- /// let x: &str = "".as_display();
  /// let _x: &str = "".as_display();
  /// }
  /// ```
  ///
  /// {{produces}}
  ///
  /// ### Explanation
  ///
- /// Return-position impl trait in traits (RPITITs) desugar to associated types,
- /// and callers of methods for types where the implementation is known are
  /// Callers of methods for types where the implementation is known are
  /// able to observe the types written in the impl signature. This may be
- /// intended behavior, but may also pose a semver hazard for authors of libraries
- /// who do not wish to make stronger guarantees about the types than what is
- /// written in the trait signature.
- pub REFINING_IMPL_TRAIT,
  /// intended behavior, but may also lead to implementation details being
  /// revealed unintentionally. In particular, it may pose a semver hazard
  /// for authors of libraries who do not wish to make stronger guarantees
  /// about the types than what is written in the trait signature.
  ///
  /// `refining_impl_trait` is a lint group composed of two lints:
  ///
  /// * `refining_impl_trait_reachable`, for refinements that are publically
  /// reachable outside a crate, and
  /// * `refining_impl_trait_internal`, for refinements that are only visible
  /// within a crate.
  ///
  /// We are seeking feedback on each of these lints; see issue
  /// [#121718](https://github.com/rust-lang/rust/issues/121718) for more
  /// information.
  pub REFINING_IMPL_TRAIT_REACHABLE,
  Warn,
  "impl trait in impl method signature does not match trait method signature",
 }
 
 declare_lint! {
  /// The `refining_impl_trait_internal` lint detects `impl Trait` return
  /// types in method signatures that are refined by a trait implementation,
  /// meaning the implementation adds information about the return type that
  /// is not present in the trait.
  ///
  /// ### Example
  ///
  /// ```rust,compile_fail
  /// #![deny(refining_impl_trait)]
  ///
  /// use std::fmt::Display;
  ///
  /// trait AsDisplay {
  /// fn as_display(&self) -> impl Display;
  /// }
  ///
  /// impl<'s> AsDisplay for &'s str {
  /// fn as_display(&self) -> Self {
  /// *self
  /// }
  /// }
  ///
  /// fn main() {
  /// // users can observe that the return type of
  /// // `<&str as AsDisplay>::as_display()` is `&str`.
  /// let _x: &str = "".as_display();
  /// }
  /// ```
  ///
  /// {{produces}}
  ///
  /// ### Explanation
  ///
  /// Callers of methods for types where the implementation is known are
  /// able to observe the types written in the impl signature. This may be
  /// intended behavior, but may also lead to implementation details being
  /// revealed unintentionally. In particular, it may pose a semver hazard
  /// for authors of libraries who do not wish to make stronger guarantees
  /// about the types than what is written in the trait signature.
  ///
  /// `refining_impl_trait` is a lint group composed of two lints:
  ///
  /// * `refining_impl_trait_reachable`, for refinements that are publically
  /// reachable outside a crate, and
  /// * `refining_impl_trait_internal`, for refinements that are only visible
  /// within a crate.
  ///
  /// We are seeking feedback on each of these lints; see issue
  /// [#121718](https://github.com/rust-lang/rust/issues/121718) for more
  /// information.
  pub REFINING_IMPL_TRAIT_INTERNAL,
  Warn,
  "impl trait in impl method signature does not match trait method signature",
 }

diff --git a/src/tools/lint-docs/src/groups.rs b/src/tools/lint-docs/src/groups.rs
@@ -16,6 16,10 @@ static GROUP_DESCRIPTIONS: &[(&str, &str)] = &[
  ("rust-2018-compatibility", "Lints used to transition code from the 2015 edition to 2018"),
  ("rust-2021-compatibility", "Lints used to transition code from the 2018 edition to 2021"),
  ("rust-2024-compatibility", "Lints used to transition code from the 2021 edition to 2024"),
  (
  "refining-impl-trait",
  "Detects refinement of `impl Trait` return types by trait implementations",
  ),
 ];
 
 type LintGroups = BTreeMap<String, BTreeSet<String>>;

diff --git a/tests/ui/async-await/in-trait/async-example-desugared-boxed.stderr b/tests/ui/async-await/in-trait/async-example-desugared-boxed.stderr
@@ -8,11 8,13 @@ LL | fn foo(&self) -> Pin<Box<dyn Future<Output = i32> '_>> {
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate
  = note: we are soliciting feedback, see issue #121718 <https://github.com/rust-lang/rust/issues/121718> for more information
 note: the lint level is defined here
  --> $DIR/async-example-desugared-boxed.rs:13:12
  |
 LL | #[warn(refining_impl_trait)]
  | ^^^^^^^^^^^^^^^^^^^
  = note: `#[warn(refining_impl_trait_reachable)]` implied by `#[warn(refining_impl_trait)]`
 help: replace the return type so that it matches the trait
  |
 LL | fn foo(&self) -> impl Future<Output = i32> {

diff --git a/tests/ui/async-await/in-trait/async-example-desugared-manual.stderr b/tests/ui/async-await/in-trait/async-example-desugared-manual.stderr
@@ -8,11 8,13 @@ LL | fn foo(&self) -> MyFuture {
  | ^^^^^^^^
  |
  = note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate
  = note: we are soliciting feedback, see issue #121718 <https://github.com/rust-lang/rust/issues/121718> for more information
 note: the lint level is defined here
  --> $DIR/async-example-desugared-manual.rs:21:12
  |
 LL | #[warn(refining_impl_trait)]
  | ^^^^^^^^^^^^^^^^^^^
  = note: `#[warn(refining_impl_trait_reachable)]` implied by `#[warn(refining_impl_trait)]`
 help: replace the return type so that it matches the trait
  |
 LL | fn foo(&self) -> impl Future<Output = i32> {

diff --git a/tests/ui/impl-trait/in-trait/bad-item-bound-within-rpitit.stderr b/tests/ui/impl-trait/in-trait/bad-item-bound-within-rpitit.stderr
@@ -22,7 22,8 @@ LL | fn iter(&self) -> impl 'a Iterator<Item = I::Item<'a>> {
  | ^^ this bound is stronger than that defined on the trait
  |
  = note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate
- = note: `#[warn(refining_impl_trait)]` on by default
  = note: we are soliciting feedback, see issue #121718 <https://github.com/rust-lang/rust/issues/121718> for more information
  = note: `#[warn(refining_impl_trait_reachable)]` on by default
 help: replace the return type so that it matches the trait
  |
 LL | fn iter(&self) -> impl Iterator<Item = <Self as Iterable>::Item<'_>> '_ {

diff --git a/tests/ui/impl-trait/in-trait/foreign.rs b/tests/ui/impl-trait/in-trait/foreign.rs
@@ -17,9 17,29 @@ impl Foo for Local {
  }
 }
 
-struct LocalIgnoreRefining;
-impl Foo for LocalIgnoreRefining {
- #[deny(refining_impl_trait)]
 struct LocalOnlyRefiningA;
 impl Foo for LocalOnlyRefiningA {
  #[warn(refining_impl_trait)]
  fn bar(self) -> Arc<String> {
  //~^ WARN impl method signature does not match trait method signature
  Arc::new(String::new())
  }
 }
 
 struct LocalOnlyRefiningB;
 impl Foo for LocalOnlyRefiningB {
  #[warn(refining_impl_trait)]
  #[allow(refining_impl_trait_reachable)]
  fn bar(self) -> Arc<String> {
  //~^ WARN impl method signature does not match trait method signature
  Arc::new(String::new())
  }
 }
 
 struct LocalOnlyRefiningC;
 impl Foo for LocalOnlyRefiningC {
  #[warn(refining_impl_trait)]
  #[allow(refining_impl_trait_internal)]
  fn bar(self) -> Arc<String> {
  Arc::new(String::new())
  }
@@ -34,5 54,7 @@ fn main() {
  let &() = Foreign.bar();
 
  let x: Arc<String> = Local.bar();
- let x: Arc<String> = LocalIgnoreRefining.bar();
  let x: Arc<String> = LocalOnlyRefiningA.bar();
  let x: Arc<String> = LocalOnlyRefiningB.bar();
  let x: Arc<String> = LocalOnlyRefiningC.bar();
 }
diff --git a/tests/ui/impl-trait/in-trait/foreign.stderr b/tests/ui/impl-trait/in-trait/foreign.stderr
@@ -0,0 1,39 @@
 warning: impl trait in impl method signature does not match trait method signature
  --> $DIR/foreign.rs:23:21
  |
 LL | fn bar(self) -> Arc<String> {
  | ^^^^^^^^^^^
  |
  = note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate
  = note: we are soliciting feedback, see issue #121718 <https://github.com/rust-lang/rust/issues/121718> for more information
 note: the lint level is defined here
  --> $DIR/foreign.rs:22:12
  |
 LL | #[warn(refining_impl_trait)]
  | ^^^^^^^^^^^^^^^^^^^
  = note: `#[warn(refining_impl_trait_internal)]` implied by `#[warn(refining_impl_trait)]`
 help: replace the return type so that it matches the trait
  |
 LL | fn bar(self) -> impl Deref<Target = impl Sized> {
  | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 warning: impl trait in impl method signature does not match trait method signature
  --> $DIR/foreign.rs:33:21
  |
 LL | fn bar(self) -> Arc<String> {
  | ^^^^^^^^^^^
  |
  = note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate
  = note: we are soliciting feedback, see issue #121718 <https://github.com/rust-lang/rust/issues/121718> for more information
 note: the lint level is defined here
  --> $DIR/foreign.rs:31:12
  |
 LL | #[warn(refining_impl_trait)]
  | ^^^^^^^^^^^^^^^^^^^
 help: replace the return type so that it matches the trait
  |
 LL | fn bar(self) -> impl Deref<Target = impl Sized> {
  | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 warning: 2 warnings emitted
 
diff --git a/tests/ui/impl-trait/in-trait/refine.rs b/tests/ui/impl-trait/in-trait/refine.rs
@@ -25,13 25,15 @@ impl Foo for C {
 struct Private;
 impl Foo for Private {
  fn bar() -> () {}
  //~^ ERROR impl method signature does not match trait method signature
 }
 
 pub trait Arg<A> {
  fn bar() -> impl Sized;
 }
 impl Arg<Private> for A {
  fn bar() -> () {}
  //~^ ERROR impl method signature does not match trait method signature
 }
 
 pub trait Late {
@@ -52,6 54,7 @@ mod unreachable {
  struct E;
  impl UnreachablePub for E {
  fn bar() {}
  //~^ ERROR impl method signature does not match trait method signature
  }
 }