Add new `disallowed_fields` lint #16218

GuillaumeGomez · 2025-12-11T20:10:12Z

This is something that we need for cg_gcc (cc @antoyo). It's almost the same as #6674 (which I used as base).

changelog: Add new disallowed_fields lint

GuillaumeGomez · 2025-12-11T20:12:12Z

clippy_lints/src/disallowed_fields.rs

+                // Very round-about way to get the field `DefId` from the expr: first we get its
+                // parent `Ty`. Then we go through all its fields to find the one with the expected
+                // name and get the `DefId` from it.
+                if let Some(parent_ty) = cx.typeck_results().expr_ty_opt(e)


I'm really not sure this is the best way to get the DefId from Expr::Field but couldn't find another way. I'd love to know if there is one!

This needs to use the adjusted type.

clippy_config/src/conf.rs

GuillaumeGomez · 2025-12-11T20:14:06Z

clippy_utils/src/paths.rs

            .filter_by_name_unhygienic(name)
            .find(|assoc_item| ns.matches(Some(assoc_item.namespace())))
            .map(|assoc_item| assoc_item.def_id),
+        ItemKind::Struct(_, _, rustc_hir::VariantData::Struct { fields, .. }) => fields.iter().find_map(|field| {


Basically added support to look for fields in lookup_path (above in this file). it's used to check if the paths are valid in the config file.

So to link to a field (for example with struct X { y: u8 }: X::y.

This is going to cause a problem. Fields and associated items can have the same name. You can add a Field variant to PathNS and constrain matches to that.

Fair enough, added it as well.

GuillaumeGomez · 2025-12-11T20:33:21Z

Finally updated all things that needed to be updated. :')

Jarcho · 2025-12-11T21:13:55Z

This doesn't prevent reading via destructuring. e.g. let Foo { x } = y

samueltardieu · 2025-12-11T21:56:27Z

r? @Jarcho

GuillaumeGomez · 2025-12-15T15:17:01Z

Thanks for the review @Jarcho! Just a heads-up: gonna try to come back to this PR in the next days.

Jarcho · 2025-12-15T15:14:27Z

clippy_lints/src/disallowed_fields.rs

+use rustc_hir::def_id::DefIdMap;
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty::{Adt, TyCtxt};


nit: We don't normally import TyKind variants and use ty::Adt instead.

Noted, removed the Adt import.

Jarcho · 2025-12-15T15:18:38Z

clippy_lints/src/disallowed_fields.rs

+                // Very round-about way to get the field `DefId` from the expr: first we get its
+                // parent `Ty`. Then we go through all its fields to find the one with the expected
+                // name and get the `DefId` from it.
+                if let Some(parent_ty) = cx.typeck_results().expr_ty_opt(e)
+                    && let Adt(adt_def, ..) = parent_ty.kind()
+                    && let Some(field_def_id) = adt_def.all_fields().find_map(|field| {
+                        if field.name == ident.name {
+                            Some(field.did)
+                        } else {
+                            None
+                        }
+                    })


There's get_field_by_name in clippy_utils for this.

It returns a Ty and I need a DefId so sadly it doesn't work in this case.

Can you extract this into it's own util function then. Something like get_field_def_by_name. It can join the other field resolving functions.

Sure, done!

rustbot · 2025-12-30T15:48:42Z

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.

GuillaumeGomez · 2025-12-30T15:49:30Z

It now checks patterns as well. I added UI tests to ensure it. Ready for a new review round. :)

GuillaumeGomez · 2026-01-01T16:57:20Z

Forgot to check it was Adt(Struct), fixed that.

GuillaumeGomez · 2026-01-07T14:05:36Z

CI passed! \o/

samueltardieu

How are fields in a variant handled, especially homonyms?

View changes since this review

samueltardieu · 2026-01-07T14:51:52Z

clippy_utils/src/ty/mod.rs

+    adt_def
+        .all_fields()
+        .find_map(|field| if field.name == name { Some(field.did) } else { None })


This shares some code with the code in check_pat(), maybe this could be factored further.

Function is 4 lines so not sure if it's really worth it. I can add a fixme comment if someone wants to make this improvement later on though if you want?

GuillaumeGomez · 2026-01-07T16:11:03Z

How are fields in a variant handled, especially homonyms?

Easy: they're not. :)

rustbot assigned samueltardieu Dec 11, 2025

rustbot added needs-fcp S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Dec 11, 2025

GuillaumeGomez force-pushed the disallowed_fields branch from 48e76b6 to 73d1c63 Compare December 11, 2025 20:11

GuillaumeGomez commented Dec 11, 2025

View reviewed changes

antoyo reviewed Dec 11, 2025

View reviewed changes

clippy_config/src/conf.rs Outdated Show resolved Hide resolved

GuillaumeGomez commented Dec 11, 2025

View reviewed changes

GuillaumeGomez force-pushed the disallowed_fields branch 2 times, most recently from eacd758 to d017b2c Compare December 11, 2025 20:22

rustbot assigned Jarcho and unassigned samueltardieu Dec 11, 2025

Jarcho requested changes Dec 15, 2025

View reviewed changes

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 15, 2025

GuillaumeGomez force-pushed the disallowed_fields branch from d017b2c to ad311dc Compare December 30, 2025 15:48

GuillaumeGomez added 2 commits January 1, 2026 17:25

Add new disallowed_fields lint

72ff51a

Add new PathNS::Field variant

50526ba

GuillaumeGomez force-pushed the disallowed_fields branch from ad311dc to 50526ba Compare January 1, 2026 16:25

Create new clippy_utils::ty::get_field_def_id_by_name function

6d980a1

samueltardieu reviewed Jan 7, 2026

View reviewed changes

Add new disallowed_fields lint #16218

Are you sure you want to change the base?

Add new disallowed_fields lint #16218

Conversation

GuillaumeGomez commented Dec 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jarcho Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GuillaumeGomez commented Dec 11, 2025

Uh oh!

Jarcho commented Dec 11, 2025

Uh oh!

samueltardieu commented Dec 11, 2025

Uh oh!

GuillaumeGomez commented Dec 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustbot commented Dec 30, 2025

Uh oh!

GuillaumeGomez commented Dec 30, 2025

Uh oh!

GuillaumeGomez commented Jan 1, 2026

Uh oh!

GuillaumeGomez commented Jan 7, 2026

Uh oh!

samueltardieu left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GuillaumeGomez commented Jan 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Add new `disallowed_fields` lint #16218

Add new `disallowed_fields` lint #16218

Jarcho Dec 15, 2025 •

edited

Loading

samueltardieu left a comment •

edited by rustbot

Loading