Skip to content

Commit c121a26

Browse files
tmandrytmandry
committed
Split refining_impl_trait lint into _reachable, _internal variants
1 parent 3c02972 commit c121a26

File tree

13 files changed

+239
-33
lines changed

13 files changed

+239
-33
lines changed

‎compiler/rustc_hir_analysis/messages.ftl‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,7 @@ hir_analysis_rpitit_refined = impl trait in impl method signature does not match
347347
.label = return type from trait method defined here
348348
.unmatched_bound_label = this bound is stronger than that defined on the trait
349349
.note = add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate
350+
.feedback_note = we are soliciting feedback, see issue #121718 <https://github.com/rust-lang/rust/issues/121718> for more information
350351
351352
hir_analysis_self_in_impl_self =
352353
`Self` is not valid in the self type of an impl block

‎compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs‎

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use rustc_data_structures::fx::FxIndexSet;
22
use rustc_hir as hir;
33
use rustc_hir::def_id::DefId;
44
use rustc_infer::infer::{outlives::env::OutlivesEnvironment, TyCtxtInferExt};
5-
use rustc_lint_defs::builtin::REFINING_IMPL_TRAIT;
5+
use rustc_lint_defs::builtin::{REFINING_IMPL_TRAIT_INTERNAL, REFINING_IMPL_TRAIT_REACHABLE};
66
use rustc_middle::traits::{ObligationCause, Reveal};
77
use rustc_middle::ty::{
88
self, Ty, TyCtxt, TypeFoldable, TypeFolder, TypeSuperVisitable, TypeVisitable, TypeVisitor,
@@ -24,26 +24,23 @@ pub(super) fn check_refining_return_position_impl_trait_in_trait<'tcx>(
2424
if !tcx.impl_method_has_trait_impl_trait_tys(impl_m.def_id) {
2525
return;
2626
}
27+
2728
// unreachable traits don't have any library guarantees, there's no need to do this check.
28-
if trait_m
29+
let is_internal = trait_m
2930
.container_id(tcx)
3031
.as_local()
3132
.is_some_and(|trait_def_id| !tcx.effective_visibilities(()).is_reachable(trait_def_id))
32-
{
33-
return;
34-
}
33+
// If a type in the trait ref is private, then there's also no reason to do this check.
34+
|| impl_trait_ref.args.iter().any(|arg| {
35+
if let Some(ty) = arg.as_type()
36+
&& let Some(self_visibility) = type_visibility(tcx, ty)
37+
{
38+
return !self_visibility.is_public();
39+
}
40+
false
41+
});
3542

36-
// If a type in the trait ref is private, then there's also no reason to do this check.
3743
let impl_def_id = impl_m.container_id(tcx);
38-
for arg in impl_trait_ref.args {
39-
if let Some(ty) = arg.as_type()
40-
&& let Some(self_visibility) = type_visibility(tcx, ty)
41-
&& !self_visibility.is_public()
42-
{
43-
return;
44-
}
45-
}
46-
4744
let impl_m_args = ty::GenericArgs::identity_for_item(tcx, impl_m.def_id);
4845
let trait_m_to_impl_m_args = impl_m_args.rebase_onto(tcx, impl_def_id, impl_trait_ref.args);
4946
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>(
8683
trait_m.def_id,
8784
impl_m.def_id,
8885
None,
86+
is_internal,
8987
);
9088
return;
9189
};
@@ -105,6 +103,7 @@ pub(super) fn check_refining_return_position_impl_trait_in_trait<'tcx>(
105103
trait_m.def_id,
106104
impl_m.def_id,
107105
None,
106+
is_internal,
108107
);
109108
return;
110109
}
@@ -199,6 +198,7 @@ pub(super) fn check_refining_return_position_impl_trait_in_trait<'tcx>(
199198
trait_m.def_id,
200199
impl_m.def_id,
201200
Some(span),
201+
is_internal,
202202
);
203203
return;
204204
}
@@ -239,6 +239,7 @@ fn report_mismatched_rpitit_signature<'tcx>(
239239
trait_m_def_id: DefId,
240240
impl_m_def_id: DefId,
241241
unmatched_bound: Option<Span>,
242+
is_internal: bool,
242243
) {
243244
let mapping = std::iter::zip(
244245
tcx.fn_sig(trait_m_def_id).skip_binder().bound_vars(),
@@ -291,7 +292,7 @@ fn report_mismatched_rpitit_signature<'tcx>(
291292

292293
let span = unmatched_bound.unwrap_or(span);
293294
tcx.emit_node_span_lint(
294-
REFINING_IMPL_TRAIT,
295+
if is_internal { REFINING_IMPL_TRAIT_INTERNAL } else { REFINING_IMPL_TRAIT_REACHABLE },
295296
tcx.local_def_id_to_hir_id(impl_m_def_id.expect_local()),
296297
span,
297298
crate::errors::ReturnPositionImplTraitInTraitRefined {

‎compiler/rustc_hir_analysis/src/errors.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1072,6 +1072,7 @@ pub struct UnusedAssociatedTypeBounds {
10721072
#[derive(LintDiagnostic)]
10731073
#[diag(hir_analysis_rpitit_refined)]
10741074
#[note]
1075+
#[note(hir_analysis_feedback_note)]
10751076
pub(crate) struct ReturnPositionImplTraitInTraitRefined<'tcx> {
10761077
#[suggestion(applicability = "maybe-incorrect", code = "{pre}{return_ty}{post}")]
10771078
pub impl_return_span: Span,

‎compiler/rustc_lint/src/lib.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,12 @@ fn register_builtins(store: &mut LintStore) {
313313
// MACRO_USE_EXTERN_CRATE
314314
);
315315

316+
add_lint_group!(
317+
"refining_impl_trait",
318+
REFINING_IMPL_TRAIT_REACHABLE,
319+
REFINING_IMPL_TRAIT_INTERNAL
320+
);
321+
316322
// Register renamed and removed lints.
317323
store.register_renamed("single_use_lifetime", "single_use_lifetimes");
318324
store.register_renamed("elided_lifetime_in_path", "elided_lifetimes_in_paths");

‎compiler/rustc_lint_defs/src/builtin.rs‎

Lines changed: 80 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,8 @@ declare_lint_pass! {
7777
PROC_MACRO_BACK_COMPAT,
7878
PROC_MACRO_DERIVE_RESOLUTION_FALLBACK,
7979
PUB_USE_OF_PRIVATE_EXTERN_CRATE,
80-
REFINING_IMPL_TRAIT,
80+
REFINING_IMPL_TRAIT_INTERNAL,
81+
REFINING_IMPL_TRAIT_REACHABLE,
8182
RENAMED_AND_REMOVED_LINTS,
8283
REPR_TRANSPARENT_EXTERNAL_PRIVATE_FIELDS,
8384
RUST_2021_INCOMPATIBLE_CLOSURE_CAPTURES,
@@ -4310,8 +4311,10 @@ declare_lint! {
43104311
}
43114312

43124313
declare_lint! {
4313-
/// The `refining_impl_trait` lint detects usages of return-position impl
4314-
/// traits in trait signatures which are refined by implementations.
4314+
/// The `refining_impl_trait_reachable` lint detects `impl Trait` return
4315+
/// types in method signatures that are refined by a publically reachable
4316+
/// trait implementation, meaning the implementation adds information about
4317+
/// the return type that is not present in the trait.
43154318
///
43164319
/// ### Example
43174320
///
@@ -4333,21 +4336,88 @@ declare_lint! {
43334336
/// fn main() {
43344337
/// // users can observe that the return type of
43354338
/// // `<&str as AsDisplay>::as_display()` is `&str`.
4336-
/// let x: &str = "".as_display();
4339+
/// let _x: &str = "".as_display();
43374340
/// }
43384341
/// ```
43394342
///
43404343
/// {{produces}}
43414344
///
43424345
/// ### Explanation
43434346
///
4344-
/// Return-position impl trait in traits (RPITITs) desugar to associated types,
4345-
/// and callers of methods for types where the implementation is known are
4347+
/// Callers of methods for types where the implementation is known are
43464348
/// able to observe the types written in the impl signature. This may be
4347-
/// intended behavior, but may also pose a semver hazard for authors of libraries
4348-
/// who do not wish to make stronger guarantees about the types than what is
4349-
/// written in the trait signature.
4350-
pub REFINING_IMPL_TRAIT,
4349+
/// intended behavior, but may also lead to implementation details being
4350+
/// revealed unintentionally. In particular, it may pose a semver hazard
4351+
/// for authors of libraries who do not wish to make stronger guarantees
4352+
/// about the types than what is written in the trait signature.
4353+
///
4354+
/// `refining_impl_trait` is a lint group composed of two lints:
4355+
///
4356+
/// * `refining_impl_trait_reachable`, for refinements that are publically
4357+
/// reachable outside a crate, and
4358+
/// * `refining_impl_trait_internal`, for refinements that are only visible
4359+
/// within a crate.
4360+
///
4361+
/// We are seeking feedback on each of these lints; see issue
4362+
/// [#121718](https://github.com/rust-lang/rust/issues/121718) for more
4363+
/// information.
4364+
pub REFINING_IMPL_TRAIT_REACHABLE,
4365+
Warn,
4366+
"impl trait in impl method signature does not match trait method signature",
4367+
}
4368+
4369+
declare_lint! {
4370+
/// The `refining_impl_trait_internal` lint detects `impl Trait` return
4371+
/// types in method signatures that are refined by a trait implementation,
4372+
/// meaning the implementation adds information about the return type that
4373+
/// is not present in the trait.
4374+
///
4375+
/// ### Example
4376+
///
4377+
/// ```rust,compile_fail
4378+
/// #![deny(refining_impl_trait)]
4379+
///
4380+
/// use std::fmt::Display;
4381+
///
4382+
/// trait AsDisplay {
4383+
/// fn as_display(&self) -> impl Display;
4384+
/// }
4385+
///
4386+
/// impl<'s> AsDisplay for &'s str {
4387+
/// fn as_display(&self) -> Self {
4388+
/// *self
4389+
/// }
4390+
/// }
4391+
///
4392+
/// fn main() {
4393+
/// // users can observe that the return type of
4394+
/// // `<&str as AsDisplay>::as_display()` is `&str`.
4395+
/// let _x: &str = "".as_display();
4396+
/// }
4397+
/// ```
4398+
///
4399+
/// {{produces}}
4400+
///
4401+
/// ### Explanation
4402+
///
4403+
/// Callers of methods for types where the implementation is known are
4404+
/// able to observe the types written in the impl signature. This may be
4405+
/// intended behavior, but may also lead to implementation details being
4406+
/// revealed unintentionally. In particular, it may pose a semver hazard
4407+
/// for authors of libraries who do not wish to make stronger guarantees
4408+
/// about the types than what is written in the trait signature.
4409+
///
4410+
/// `refining_impl_trait` is a lint group composed of two lints:
4411+
///
4412+
/// * `refining_impl_trait_reachable`, for refinements that are publically
4413+
/// reachable outside a crate, and
4414+
/// * `refining_impl_trait_internal`, for refinements that are only visible
4415+
/// within a crate.
4416+
///
4417+
/// We are seeking feedback on each of these lints; see issue
4418+
/// [#121718](https://github.com/rust-lang/rust/issues/121718) for more
4419+
/// information.
4420+
pub REFINING_IMPL_TRAIT_INTERNAL,
43514421
Warn,
43524422
"impl trait in impl method signature does not match trait method signature",
43534423
}

‎src/tools/lint-docs/src/groups.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ static GROUP_DESCRIPTIONS: &[(&str, &str)] = &[
1616
("rust-2018-compatibility", "Lints used to transition code from the 2015 edition to 2018"),
1717
("rust-2021-compatibility", "Lints used to transition code from the 2018 edition to 2021"),
1818
("rust-2024-compatibility", "Lints used to transition code from the 2021 edition to 2024"),
19+
(
20+
"refining-impl-trait",
21+
"Detects refinement of `impl Trait` return types by trait implementations",
22+
),
1923
];
2024

2125
type LintGroups = BTreeMap<String, BTreeSet<String>>;

‎tests/ui/async-await/in-trait/async-example-desugared-boxed.stderr‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,13 @@ LL | fn foo(&self) -> Pin<Box<dyn Future<Output = i32> + '_>> {
88
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
99
|
1010
= note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate
11+
= note: we are soliciting feedback, see issue #121718 <https://github.com/rust-lang/rust/issues/121718> for more information
1112
note: the lint level is defined here
1213
--> $DIR/async-example-desugared-boxed.rs:13:12
1314
|
1415
LL | #[warn(refining_impl_trait)]
1516
| ^^^^^^^^^^^^^^^^^^^
17+
= note: `#[warn(refining_impl_trait_reachable)]` implied by `#[warn(refining_impl_trait)]`
1618
help: replace the return type so that it matches the trait
1719
|
1820
LL | fn foo(&self) -> impl Future<Output = i32> {

‎tests/ui/async-await/in-trait/async-example-desugared-manual.stderr‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,13 @@ LL | fn foo(&self) -> MyFuture {
88
| ^^^^^^^^
99
|
1010
= note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate
11+
= note: we are soliciting feedback, see issue #121718 <https://github.com/rust-lang/rust/issues/121718> for more information
1112
note: the lint level is defined here
1213
--> $DIR/async-example-desugared-manual.rs:21:12
1314
|
1415
LL | #[warn(refining_impl_trait)]
1516
| ^^^^^^^^^^^^^^^^^^^
17+
= note: `#[warn(refining_impl_trait_reachable)]` implied by `#[warn(refining_impl_trait)]`
1618
help: replace the return type so that it matches the trait
1719
|
1820
LL | fn foo(&self) -> impl Future<Output = i32> {

‎tests/ui/impl-trait/in-trait/bad-item-bound-within-rpitit.stderr‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ LL | fn iter(&self) -> impl 'a + Iterator<Item = I::Item<'a>> {
2222
| ^^ this bound is stronger than that defined on the trait
2323
|
2424
= note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate
25-
= note: `#[warn(refining_impl_trait)]` on by default
25+
= note: we are soliciting feedback, see issue #121718 <https://github.com/rust-lang/rust/issues/121718> for more information
26+
= note: `#[warn(refining_impl_trait_reachable)]` on by default
2627
help: replace the return type so that it matches the trait
2728
|
2829
LL | fn iter(&self) -> impl Iterator<Item = <Self as Iterable>::Item<'_>> + '_ {

‎tests/ui/impl-trait/in-trait/foreign.rs‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,29 @@ impl Foo for Local {
1717
}
1818
}
1919

20-
struct LocalIgnoreRefining;
21-
impl Foo for LocalIgnoreRefining {
22-
#[deny(refining_impl_trait)]
20+
struct LocalOnlyRefiningA;
21+
impl Foo for LocalOnlyRefiningA {
22+
#[warn(refining_impl_trait)]
23+
fn bar(self) -> Arc<String> {
24+
//~^ WARN impl method signature does not match trait method signature
25+
Arc::new(String::new())
26+
}
27+
}
28+
29+
struct LocalOnlyRefiningB;
30+
impl Foo for LocalOnlyRefiningB {
31+
#[warn(refining_impl_trait)]
32+
#[allow(refining_impl_trait_reachable)]
33+
fn bar(self) -> Arc<String> {
34+
//~^ WARN impl method signature does not match trait method signature
35+
Arc::new(String::new())
36+
}
37+
}
38+
39+
struct LocalOnlyRefiningC;
40+
impl Foo for LocalOnlyRefiningC {
41+
#[warn(refining_impl_trait)]
42+
#[allow(refining_impl_trait_internal)]
2343
fn bar(self) -> Arc<String> {
2444
Arc::new(String::new())
2545
}
@@ -34,5 +54,7 @@ fn main() {
3454
let &() = Foreign.bar();
3555

3656
let x: Arc<String> = Local.bar();
37-
let x: Arc<String> = LocalIgnoreRefining.bar();
57+
let x: Arc<String> = LocalOnlyRefiningA.bar();
58+
let x: Arc<String> = LocalOnlyRefiningB.bar();
59+
let x: Arc<String> = LocalOnlyRefiningC.bar();
3860
}

0 commit comments

Comments
 (0)