refining_impl_trait: Decide whether explicit opt-in is needed

View all comments

What is refining_impl_trait?

The refining_impl_trait lints detect usages of return-position impl traits in trait signatures which are refined by implementations, meaning the implementation adds information about the return type that is not present in the trait.

Example

pub 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();
}

The above code compiles, but produces a warning:

warning: impl trait in impl method signature does not match trait method signature
 --> src/main.rs:8:29
  |
4 |     fn as_display(&self) -> impl Display;
  |                             ------------ return type from trait method defined here
...
8 |     fn as_display(&self) -> Self {
  |                             ^^^^
  |
  = 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
help: replace the return type so that it matches the trait
  |
8 |     fn as_display(&self) -> impl std::fmt::Display {
  |                             ~~~~~~~~~~~~~~~~~~~~~~

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.

Future possibilities

Add an explicit #[refine] attribute. This would make it easier to opt in to refining on a per-method basis and make the refine check feel more like a builtin feature of the language.

Make refining_impl_trait error-by-default in a future edition. This would mean we are committed to explicit refinement for impl Traits. Since this would happen over an edition, we could choose to go even further and make it a hard error so you could not blanket-allow it in your crate.

Keep all refining_impl_trait warn-by-default.

Make refining_impl_trait_internal allow-by-default, but keep refining_impl_trait_reachable warn-by-default. This would mean that we think semver hazards for publicly-reachable traits in a crate are important enough to lint against. But for uses internal to a crate, opting in is unnecessary overhead for the user.

Make all refining_impl_trait allow-by-default. This would mean that we think most uses of refinement for return-position impl Trait are deliberate and unlikely to cause semver hazards or other unexpected behavior.

Discussion

The language team is collecting feedback about this lint. We would like to know

• Whether users expect refinement to require an explicit opt-in.
• Whether users see value in the refinement check to prevent accidental leakage of implementation details.
• Whether users expect refinement to work the same between publicly-reachable impls and internal-only impls.
• Whether users would like to see a #[refine] attribute in the future.

Read the issue's explanation of refining_impl_trait, the example, and the listed future possibilities before reviewing the linked comment discussion. Compare the proposed opt-in, edition, lint-default, and #[refine] options against the stated API and semver concerns. Done means reaching and recording a clear decision about the intended policy.