Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Impl item removals should lint if the fallback item is not public API. #763

Merged
merged 1 commit into from
Apr 18, 2024

Conversation

obi1kenobi
Copy link
Owner

@obi1kenobi obi1kenobi commented Apr 18, 2024

Before this PR, the removal of the marked method in this snippet did not cause an inherent_method_missing lint:

pub struct SecretlyIterator;

impl SecretlyIterator {
    // This method gets removed.
    pub fn next(&mut self) -> Option<<Self as Iterator>::Item> {
        <Self as Iterator>::next()
    }
}

#[doc(hidden)]
impl Iterator for SecretlyIterator {
    type Item = i64;
    
    fn next(&mut self) -> Option<Self::Item> {
        None
    }
}

The reasoning was that the impl Iterator has a matching method and Iterator is in scope by default (assuming the prelude is used), so users of this code wouldn't get a compile error. Instead, they'd automatically fall back to the Iterator::next() method through the magic of Rust method resolution.

With this PR, we improve the analysis for methods/associated functions and associated constants, so that cases like this do cause those lints — it'll now ignore trait implementations marked #[doc(hidden)] when considering if there's a valid fallback for the item that was removed.

@obi1kenobi obi1kenobi enabled auto-merge (squash) April 18, 2024 21:13
@obi1kenobi obi1kenobi added A-lint Area: new or existing lint C-enhancement Category: raise the bar on expectations labels Apr 18, 2024
@obi1kenobi obi1kenobi merged commit 0bfda4f into main Apr 18, 2024
31 of 32 checks passed
@obi1kenobi obi1kenobi deleted the handle_doc_hidden_on_impl branch April 18, 2024 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: new or existing lint C-enhancement Category: raise the bar on expectations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant