Skip to content

feat: Add profile-specific configuration for disallowed methods and types#15779

Merged
blyxyas merged 5 commits into
rust-lang:masterfrom
Lallapallooza:disallowed-profiles
Jun 9, 2026
Merged

feat: Add profile-specific configuration for disallowed methods and types#15779
blyxyas merged 5 commits into
rust-lang:masterfrom
Lallapallooza:disallowed-profiles

Conversation

@Lallapallooza

@Lallapallooza Lallapallooza commented Sep 29, 2025

Copy link
Copy Markdown
Contributor

View all comments

Add profile-scoped disallow lists for methods and types, wiring the new configuration tables through a shared resolver that can be toggled with #[clippy::disallowed_profile] attributes.

  • extend clippy_config::Conf to parse [disallowed-methods-profiles.] and [disallowed-types-profiles.] tables through reusable helpers
  • introduce clippy_utils::disallowed_profiles to resolve #[clippy::disallowed_profile(s)] attributes with caching, diagnostics for bad inputs, and shared symbol registrations
  • teach DisallowedMethods and DisallowedTypes to honor active profiles, reuse the combined profile set, and annotate diagnostics with the triggering profile name
  • add UI TOML coverage for per-profile behavior and refresh the unknown-key fixture for the new configuration entries
  • register the new attributes and symbols so tool attributes remain recognized and pedantic lints stay clean

changelog: [disallowed_methods]: allow selecting per-scope disallow lists via disallowed-methods-profiles and the clippy::disallowed_profile attribute
changelog: [disallowed_types]: allow selecting per-scope disallow lists via disallowed-types-profiles and the clippy::disallowed_profile attribute

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Sep 29, 2025
@rustbot

rustbot commented Sep 29, 2025

Copy link
Copy Markdown
Collaborator

r? @blyxyas

rustbot has assigned @blyxyas.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@Lallapallooza

Copy link
Copy Markdown
Contributor Author

@blyxyas ping

@blyxyas

blyxyas commented Oct 9, 2025

Copy link
Copy Markdown
Member

Hi @Lallapallooza,
Sorry for the delayed feedback, I'm currently a bit busy but I'll work on sending a review for this pull request soon.
Thanks for your contribution!

@blyxyas blyxyas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll recount what I understand here, so a user uses a special configuration for settings profiles in their clippy.toml so any function can opt-in to those profiles, enabling warnings to those designated disallows in that function.

For example, one might want to disallow functions that may execute arbitrary code from a string input, into user-input-handling functions, and this is a way to achieve via Clippy warning of those uses.

Am I correct, have I forgotten/misunderstood something? If that's the case, we might be interested in generalizing this a bit so that both (1.) we can use profiles for other lints without the need of adding a special attribute for each lint, and (2.) people can use the general idea of profiles in Clippy to segment their source code for several lints at the same time.

Also, @ojeda could this be interesting for the Rust4Linux project? Should I give it priority?

View changes since this review

@Lallapallooza

Copy link
Copy Markdown
Contributor Author

You read it correctly. This PR introduces profile-scoped disallow lists in clippy.toml and a per-item opt-in attribute that activates one or more named profiles for the annotated item (fn/mod). Currently, only disallowed_methods and disallowed_types consult these profiles.

I agree that generalizing this is a good idea. I can follow up with a patch if that would be useful. Do you have any preferences for the design or the user interface (i.e., how Clippy users would enable and use profiles)?

@ojeda

ojeda commented Oct 11, 2025

Copy link
Copy Markdown
Contributor

Thanks @blyxyas for the ping! It is an interesting idea. IIUC, currently this can only be done at a crate granularity, right? So it is essentially a way to make Clippy more granular -- I agree that generalizing it makes sense. I will try to think if we could use it in Rust for Linux (@Lallapallooza What are the use cases that motivated this? It may help to give some in the docs to inspire others -- thanks!).

@Lallapallooza

Copy link
Copy Markdown
Contributor Author

Hi @ojeda. My initial contribution was motivated by a need in my project to disallow host-side operations (e.g., any conversion from a device tensor to a host tensor) in specific context (e.g. dir crates/lib/cuda/) so that such code fails to compile.

@blyxyas

blyxyas commented Oct 13, 2025

Copy link
Copy Markdown
Member

Do you have any preferences for the design or the user interface

I'll ask both the entire team, and the style team to see if they have anything to say. In Clippy we don't really have an in-house attribute style guide, so maybe they have some feedback about how this could be implemented.

@ojeda

ojeda commented Oct 13, 2025

Copy link
Copy Markdown
Contributor

Thanks @Lallapallooza, that is good to know (if you have more cases you can think of, then it would be great to hear about them, of course).

@rustbot

This comment has been minimized.

@blyxyas

blyxyas commented Nov 12, 2025

Copy link
Copy Markdown
Member

Hi Lallapallooza, I've talked with the team and I'm going to proceed with the review. Also, sorry for the delayed response, I've just returned from vacation.

I was thinking that this may be useful for this message thread

After learning about disallowed_names ("foo", "bar" and others that are
in fact classical documentation favorites), I also think it would be
very nice to allow those in documentation.

I thought that we already had a configuration option for this, but it seems that we don't have one. So I think that this feature can also support this use case.

@rustbot

This comment has been minimized.

@Lallapallooza

Copy link
Copy Markdown
Contributor Author

@blyxyas Thanks, I have rebased pr.

@blyxyas

blyxyas commented Nov 20, 2025

Copy link
Copy Markdown
Member

I thought that we already had a configuration option for this, but it seems that we don't have one. So I think that this feature can also support this use case.

I'm walking back this statement, we already have another pull request taking care of this. So we won't focus on that front here.

#15600

@blyxyas blyxyas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need some interface changes to be more ergonomic, but apart from that, this looks great!

View changes since this review

Comment thread book/src/lint_configuration.md Outdated
Comment thread clippy_config/src/conf.rs Outdated
Comment thread clippy_config/src/conf.rs Outdated
Comment thread clippy_config/src/conf.rs Outdated
Comment thread clippy_config/src/conf.rs Outdated
Comment thread clippy_config/src/conf.rs Outdated
Comment thread clippy_config/src/conf.rs Outdated
Comment thread clippy_lints/src/disallowed_methods.rs Outdated
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Nov 24, 2025
@rustbot

rustbot commented Nov 24, 2025

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Dec 14, 2025
@Lallapallooza

Copy link
Copy Markdown
Contributor Author

Hi! I have addressed all comments, could you please check again.

@blyxyas blyxyas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some more thoughts ฅ(´・ω・`)ฅ

View changes since this review

Comment thread clippy_config/src/conf.rs Outdated
Comment thread clippy_lints/src/disallowed_methods.rs Outdated
Comment thread clippy_lints/src/disallowed_methods.rs Outdated
Comment thread clippy_lints/src/disallowed_methods.rs Outdated
Comment thread clippy_lints/src/disallowed_types.rs Outdated
Comment thread clippy_lints/src/disallowed_methods.rs Outdated
Comment thread tests/ui-toml/disallowed_profiles_methods/main.rs
pub fn active_profiles(&mut self, cx: &LateContext<'_>, hir_id: HirId) -> Option<&ProfileSelection> {
if self.cache.contains_key(&hir_id) {
return self.cache.get(&hir_id).and_then(|selection| selection.as_ref());
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self.cache.contains_key is only self.cache.get().is_some(), so it's performing the same lookup two times. It should be something more akin to:

if let Some(selection) = self.cache.get(&hir_id) {
    return selection;
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added a NOTE why it's needed

Comment thread clippy_lints/src/disallowed_types.rs
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Dec 16, 2025
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Dec 17, 2025
@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) label Dec 17, 2025
std::mem::drop(value); //~ ERROR: use of a disallowed method `std::mem::drop`
}

#[expect(clippy::disallowed_methods)]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need expects for the unknown_profile lint, and trying it before and after the profile attribute.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added tests in both disallowed_profiles_methods and disallowed_profiles_types covering #[expect(...)] placed before and after #[clippy::disallowed_profile("unknown_...")].

Comment on lines +96 to +97
profiles: FxHashMap<Symbol, TypeLookup>,
known_profiles: FxHashSet<Symbol>,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is profiles different from known_profiles? Is there a difference between these two? Seems that known_profiles is just a set from the keys of profiles.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're intentionally different: known_profiles holds every name declared in [profiles.*], while profiles only holds those that contribute entries to this lint.

The distinction is what suppresses an "unknown profile" warning when a profile only defines entries for the other lint.

Added field-level comments to make this explicit. Thanks

Comment thread clippy_lints/src/disallowed_types.rs Outdated
Comment on lines +151 to +165
let mut active_profiles = SmallVec::<[Symbol; 2]>::new();
let mut unknown_profiles = SmallVec::<[ProfileEntry; 2]>::new();
if let Some(selection) = self.profile_cache.active_profiles(cx, hir_id) {
for entry in selection.iter() {
if self.profiles.contains_key(&entry.name) {
active_profiles.push(entry.name);
} else if !self.known_profiles.contains(&entry.name) {
unknown_profiles.push(*entry);
}
}
}

for entry in unknown_profiles {
self.warn_unknown_profile(cx, &entry);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are going to lint them one by one anyways, this self.warn_unknown_profile should be above.

Suggested change
let mut active_profiles = SmallVec::<[Symbol; 2]>::new();
let mut unknown_profiles = SmallVec::<[ProfileEntry; 2]>::new();
if let Some(selection) = self.profile_cache.active_profiles(cx, hir_id) {
for entry in selection.iter() {
if self.profiles.contains_key(&entry.name) {
active_profiles.push(entry.name);
} else if !self.known_profiles.contains(&entry.name) {
unknown_profiles.push(*entry);
}
}
}
for entry in unknown_profiles {
self.warn_unknown_profile(cx, &entry);
}
let mut active_profiles = SmallVec::<[Symbol; 2]>::new();
let mut unknown_profiles = SmallVec::<[ProfileEntry; 2]>::new();
if let Some(selection) = self.profile_cache.active_profiles(cx, hir_id) {
for entry in selection.iter() {
if self.profiles.contains_key(&entry.name) {
active_profiles.push(entry.name);
} else {
self.warn_unknown_profile(cx, &entry);
}
}
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, thanks! Dropped the intermediate unknown_profiles SmallVec and warn inline.

I had to copy entries out of the cache first because warn_unknown_profile takes &mut self and the active_profiles borrow holds &mut self.profile_cache.

Comment thread clippy_lints/src/disallowed_types.rs
use rustc_span::{Span, Symbol};

#[derive(Copy, Clone)]
pub struct ProfileEntry {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need some documentation here, what is the entry for? What does it represent?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a doc comment explaining that each entry represents one profile-name argument from a #[clippy::disallowed_profile(s)] attribute. Thanks!

Comment thread clippy_utils/src/disallowed_profiles.rs
Comment thread clippy_lints/src/disallowed_types.rs Outdated
Comment on lines +153 to +161
if let Some(selection) = self.profile_cache.active_profiles(cx, hir_id) {
for entry in selection.iter() {
if self.profiles.contains_key(&entry.name) {
active_profiles.push(entry.name);
} else if !self.known_profiles.contains(&entry.name) {
unknown_profiles.push(*entry);
}
}
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if let Some(selection) = self.profile_cache.active_profiles(cx, hir_id) {
for entry in selection.iter() {
if self.profiles.contains_key(&entry.name) {
active_profiles.push(entry.name);
} else if !self.known_profiles.contains(&entry.name) {
unknown_profiles.push(*entry);
}
}
}
if let Some(selection) = self.profile_cache.active_profiles(cx, hir_id) {
active_profiles = selection.iter().filter(|profile| {
if !self.profiles.contains_key(&profile.name) { self.warn_unknown_profile(cx, &profile); false}; true
})
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied the inline-warn refactor from the sibling thread. Kept the known_profiles guard so that profiles used for the other lint don't get flagged as unknown here.

@rustbot

This comment has been minimized.

@rustbot

This comment has been minimized.

@Lallapallooza

Copy link
Copy Markdown
Contributor Author

Hi @blyxyas, sorry for leaving this PR sitting for so long.

I've rebased onto current master and addressed all the outstanding review comments (docs on ProfileEntry/ProfileSelection, the profiles vs known_profiles distinction, inlining the unknown-profile warning, and the #[expect] before/after tests).

Replies are in each thread. Whenever you have a moment, could you take another look? Thanks again for the reviews. 🙏

@Lallapallooza Lallapallooza requested a review from blyxyas April 21, 2026 01:35
@Lallapallooza

Copy link
Copy Markdown
Contributor Author

@blyxyas Hi, could you check my patch again please.

@rustbot

This comment has been minimized.

@blyxyas blyxyas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks! ❤️
Thanks for taking this amazing contribution until the end, I think that it will heavily improve workflows, and there are many people interested in this kind of configuration option. I might make a blog post about it.

View changes since this review

- document `ProfileEntry` and `ProfileSelection`
- explain why `profiles` and `known_profiles` are separate
- emit unknown-profile warning inline (drop intermediate SmallVec)
- add tests covering `#[expect]` before/after `#[clippy::disallowed_profile]`
  with an unknown profile name, for both methods and types lints
- sort_by -> sort_by_key to satisfy dogfood
@Lallapallooza Lallapallooza force-pushed the disallowed-profiles branch from 647bc06 to 03128d7 Compare June 9, 2026 11:45
@rustbot

rustbot commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@Lallapallooza

Copy link
Copy Markdown
Contributor Author

@blyxyas Thanks! I rebased MR and now it's mergeable.

@blyxyas blyxyas added this pull request to the merge queue Jun 9, 2026
Merged via the queue into rust-lang:master with commit bf2b6ed Jun 9, 2026
11 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jun 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants