Add new disallowed_fields lint
#16218
Conversation
rustbot
added
needs-fcp
S-waiting-on-review
GuillaumeGomez
force-pushed
the
disallowed_fields
branch
from
48e76b6 to
73d1c63
Compare
| // 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) |
There was a problem hiding this comment.
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!
There was a problem hiding this comment.
This needs to use the adjusted type.
| .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| { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
GuillaumeGomez
force-pushed
the
disallowed_fields
branch
from
73d1c63 to
eacd758
Compare
GuillaumeGomez
force-pushed
the
disallowed_fields
branch
from
eacd758 to
d017b2c
Compare
|
Finally updated all things that needed to be updated. :') |
|
This doesn't prevent reading via destructuring. e.g. |
|
r? @Jarcho |
|
Thanks for the review @Jarcho! Just a heads-up: gonna try to come back to this PR in the next days. |
| use rustc_hir::def_id::DefIdMap; | ||
| use rustc_hir::{Expr, ExprKind}; | ||
| use rustc_lint::{LateContext, LateLintPass}; | ||
| use rustc_middle::ty::{Adt, TyCtxt}; |
There was a problem hiding this comment.
nit: We don't normally import TyKind variants and use ty::Adt instead.
| // 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 was a problem hiding this comment.
There's get_field_by_name in clippy_utils for this.
rustbot
added
S-waiting-on-author
5 participants
Fixes #9278.
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_fieldslintr? @samueltardieu