-
-
Notifications
You must be signed in to change notification settings - Fork 14.7k
Unconstrained parameter fix #148788
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
Open
TomtheCoder2
wants to merge
21
commits into
rust-lang:main
Choose a base branch
from
TomtheCoder2:unconstrained-parameter-fix
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Unconstrained parameter fix #148788
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
0ade480
Fix unconstrained parameter issue
TomtheCoder2 a8a7e19
fix tests
TomtheCoder2 12340a8
fix rustdoc ui test
TomtheCoder2 69165e1
fix tests
TomtheCoder2 e7d4c8a
fix tests (added the help messages in the code too)
TomtheCoder2 1065e84
fixed tests (hope the last time)
TomtheCoder2 3d49089
prepared the search for the generics in the impl and struct definitio…
TomtheCoder2 bf798a3
first check if the struct Foo has a missing generic parameter
TomtheCoder2 179940d
adapted test outputs
TomtheCoder2 9d1025b
fixed tests
TomtheCoder2 c735d38
fixed some comments
TomtheCoder2 8200d5c
moved from multi part suggestions, to multiple single suggestions
TomtheCoder2 71ecd9b
fixed the two failing tests
TomtheCoder2 5d77bb2
more readable comments
TomtheCoder2 48c4e3c
fixed typo
TomtheCoder2 78b460f
fmt
TomtheCoder2 d5406c4
moved the suggestion code to the error module
TomtheCoder2 2461e5d
added doc comments for the new functions
TomtheCoder2 1cdc8cb
added new test cases for to exercise the new branches added in the
TomtheCoder2 0b83de8
adapted test outputs to the new behaviour: suggesting adding the
TomtheCoder2 7664723
fixed test
TomtheCoder2 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
217 changes: 217 additions & 0 deletions
217
compiler/rustc_hir_analysis/src/errors/remove_or_use_generic.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,217 @@ | ||
| use std::ops::ControlFlow; | ||
|
|
||
| use rustc_errors::{Applicability, Diag}; | ||
| use rustc_hir::def::DefKind; | ||
| use rustc_hir::def_id::{DefId, LocalDefId}; | ||
| use rustc_hir::intravisit::{self, Visitor, walk_lifetime}; | ||
| use rustc_hir::{GenericArg, HirId, LifetimeKind, Path, QPath, TyKind}; | ||
| use rustc_middle::hir::nested_filter::All; | ||
| use rustc_middle::ty::{GenericParamDef, GenericParamDefKind, TyCtxt}; | ||
|
|
||
| use crate::hir::def::Res; | ||
|
|
||
| /// Use a Visitor to find usages of the type or lifetime parameter | ||
| struct ParamUsageVisitor<'tcx> { | ||
| tcx: TyCtxt<'tcx>, | ||
| /// The `DefId` of the generic parameter we are looking for. | ||
| param_def_id: DefId, | ||
| found: bool, | ||
| } | ||
|
|
||
| impl<'tcx> Visitor<'tcx> for ParamUsageVisitor<'tcx> { | ||
| type NestedFilter = All; | ||
|
|
||
| fn maybe_tcx(&mut self) -> Self::MaybeTyCtxt { | ||
| self.tcx | ||
| } | ||
|
|
||
| type Result = ControlFlow<()>; | ||
|
|
||
| fn visit_path(&mut self, path: &Path<'tcx>, _id: HirId) -> Self::Result { | ||
| if let Some(res_def_id) = path.res.opt_def_id() { | ||
| if res_def_id == self.param_def_id { | ||
| self.found = true; | ||
| return ControlFlow::Break(()); | ||
| } | ||
| } | ||
| intravisit::walk_path(self, path) | ||
| } | ||
|
|
||
| fn visit_lifetime(&mut self, lifetime: &'tcx rustc_hir::Lifetime) -> Self::Result { | ||
| if let LifetimeKind::Param(id) = lifetime.kind { | ||
| if let Some(local_def_id) = self.param_def_id.as_local() { | ||
| if id == local_def_id { | ||
| self.found = true; | ||
| return ControlFlow::Break(()); | ||
| } | ||
| } | ||
| } | ||
| walk_lifetime(self, lifetime) | ||
| } | ||
| } | ||
|
|
||
| /// Adds a suggestion to a diagnostic to either remove an unused generic parameter, or use it. | ||
| /// | ||
| /// # Examples | ||
| /// | ||
| /// - `impl<T> Struct { ... }` where `T` is unused -> suggests removing `T` or using it. | ||
| /// - `impl<'a> Struct { ... }` where `'a` is unused -> suggests removing `'a`. | ||
| /// - `impl<T> Struct { // T used in here }` where `T` is used in the body but not in the self type -> suggests adding `T` to the self type and struct definition. | ||
| /// - `impl<T> Struct { ... }` where the struct has a generic parameter with a default -> suggests adding `T` to the self type. | ||
| pub(crate) fn suggest_to_remove_or_use_generic( | ||
| tcx: TyCtxt<'_>, | ||
| diag: &mut Diag<'_>, | ||
| impl_def_id: LocalDefId, | ||
| param: &GenericParamDef, | ||
| is_lifetime: bool, | ||
| ) { | ||
| let node = tcx.hir_node_by_def_id(impl_def_id); | ||
| let hir_impl = node.expect_item().expect_impl(); | ||
|
|
||
| let Some((index, _)) = hir_impl | ||
| .generics | ||
| .params | ||
| .iter() | ||
| .enumerate() | ||
| .find(|(_, par)| par.def_id.to_def_id() == param.def_id) | ||
| else { | ||
| return; | ||
| }; | ||
|
|
||
| // Get the Struct/ADT definition ID from the self type | ||
| let struct_def_id = if let TyKind::Path(QPath::Resolved(_, path)) = hir_impl.self_ty.kind | ||
| && let Res::Def(DefKind::Struct | DefKind::Enum | DefKind::Union, def_id) = path.res | ||
| { | ||
| def_id | ||
| } else { | ||
| return; | ||
| }; | ||
|
|
||
| // Count how many generic parameters are defined in the struct definition | ||
| let generics = tcx.generics_of(struct_def_id); | ||
| let total_params = generics | ||
| .own_params | ||
| .iter() | ||
| .filter(|p| { | ||
| if is_lifetime { | ||
| matches!(p.kind, GenericParamDefKind::Lifetime) | ||
| } else { | ||
| matches!(p.kind, GenericParamDefKind::Type { .. }) | ||
| } | ||
| }) | ||
| .count(); | ||
|
|
||
| // Count how many arguments are currently provided in the impl | ||
| let mut provided_params = 0; | ||
| let mut last_segment_args = None; | ||
|
|
||
| if let TyKind::Path(QPath::Resolved(_, path)) = hir_impl.self_ty.kind | ||
| && let Some(seg) = path.segments.last() | ||
| && let Some(args) = seg.args | ||
| { | ||
| last_segment_args = Some(args); | ||
| provided_params = args | ||
| .args | ||
| .iter() | ||
| .filter(|arg| match arg { | ||
| GenericArg::Lifetime(_) => is_lifetime, | ||
| GenericArg::Type(_) => !is_lifetime, | ||
| _ => false, | ||
| }) | ||
| .count(); | ||
| } | ||
|
|
||
| let mut visitor = ParamUsageVisitor { tcx, param_def_id: param.def_id, found: false }; | ||
| for item_ref in hir_impl.items { | ||
| let _ = visitor.visit_impl_item_ref(item_ref); | ||
| if visitor.found { | ||
| break; | ||
| } | ||
| } | ||
| let is_param_used = visitor.found; | ||
|
|
||
| let mut suggestions = vec![]; | ||
|
|
||
| // Option A: Remove (Only if not used in body) | ||
| if !is_param_used { | ||
| suggestions.push((hir_impl.generics.span_for_param_removal(index), String::new())); | ||
| } | ||
|
|
||
| // Option B: Suggest adding only if there's an available parameter in the struct definition | ||
| // or the parameter is already used somewhere, then we suggest adding to the impl struct and the struct definition | ||
| if provided_params < total_params || is_param_used { | ||
| if let Some(args) = last_segment_args { | ||
| // Struct already has <...>, append to it | ||
| suggestions.push((args.span().unwrap().shrink_to_hi(), format!(", {}", param.name))); | ||
| } else if let TyKind::Path(QPath::Resolved(_, path)) = hir_impl.self_ty.kind { | ||
| // Struct has no <...> yet, add it | ||
| let seg = path.segments.last().unwrap(); | ||
| suggestions.push((seg.ident.span.shrink_to_hi(), format!("<{}>", param.name))); | ||
| } | ||
| if is_param_used { | ||
| // If the parameter is used in the body, we also want to suggest adding it to the struct definition if it's not already there | ||
| let struct_span = tcx.def_span(struct_def_id); | ||
| let struct_generic_params = tcx | ||
| .hir_node_by_def_id(struct_def_id.expect_local()) | ||
| .expect_item() | ||
| .expect_struct() | ||
| .1 | ||
| .params; | ||
| if struct_generic_params.len() > 0 { | ||
| let last_param = struct_generic_params.last().unwrap(); | ||
| suggestions.push((last_param.span.shrink_to_hi(), format!(", {}", param.name))); | ||
| } else { | ||
| suggestions.push((struct_span.shrink_to_hi(), format!("<{}>", param.name))); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if suggestions.is_empty() { | ||
| return; | ||
| } | ||
|
|
||
| let parameter_type = if is_lifetime { "lifetime" } else { "type" }; | ||
| if is_param_used { | ||
| let msg = format!( | ||
| "make use of the {} parameter `{}` in the `self` type", | ||
| parameter_type, param.name | ||
| ); | ||
| diag.span_suggestion( | ||
| suggestions[0].0, | ||
| msg, | ||
| suggestions[0].1.clone(), | ||
| Applicability::MaybeIncorrect, | ||
| ); | ||
| let msg2 = format!( | ||
| "and add it to the struct definition of {} as well since it's used in the body of the impl", | ||
| tcx.def_path_str(struct_def_id) | ||
| ); | ||
| diag.span_suggestion( | ||
| suggestions[1].0, | ||
| msg2, | ||
| suggestions[1].1.clone(), | ||
| Applicability::MaybeIncorrect, | ||
| ); | ||
| } else { | ||
| let msg = if suggestions.len() == 2 { | ||
| format!("either remove the unused {} parameter `{}`", parameter_type, param.name) | ||
| } else { | ||
| format!("remove the unused {} parameter `{}`", parameter_type, param.name) | ||
| }; | ||
| diag.span_suggestion( | ||
| suggestions[0].0, | ||
| msg, | ||
| suggestions[0].1.clone(), | ||
| Applicability::MaybeIncorrect, | ||
| ); | ||
| if suggestions.len() == 2 { | ||
| let msg = format!("or make use of it"); | ||
| diag.span_suggestion( | ||
| suggestions[1].0, | ||
| msg, | ||
| suggestions[1].1.clone(), | ||
| Applicability::MaybeIncorrect, | ||
| ); | ||
| } | ||
| }; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.