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

Rework non_local_definitions lint to only use a syntactic heuristic #127117

Merged
merged 5 commits into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 1 addition & 10 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -592,10 +592,7 @@ lint_non_local_definitions_cargo_update = the {$macro_kind} `{$macro_name}` may
lint_non_local_definitions_deprecation = this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>

lint_non_local_definitions_impl = non-local `impl` definition, `impl` blocks should be written at the same level as their item
.remove_help = remove `{$may_remove_part}` to make the `impl` local
.without_trait = methods and associated constants are still usable outside the current expression, only `impl Local` and `impl dyn Local` can ever be private, and only if the type is nested in the same item as the `impl`
.with_trait = an `impl` is never scoped, even when it is nested inside an item, as it may impact type checking outside of that item, which can be the case if neither the trait or the self type are at the same nesting level as the `impl`
.bounds = `impl` may be usable in bounds, etc. from outside the expression, which might e.g. make something constructible that previously wasn't, because it's still on a publicly-visible type
.non_local = an `impl` is never scoped, even when it is nested inside an item, as it may impact type checking outside of that item, which can be the case if neither the trait or the self type are at the same nesting level as the `impl`
.doctest = make this doc-test a standalone test with its own `fn main() {"{"} ... {"}"}`
.exception = items in an anonymous const item (`const _: () = {"{"} ... {"}"}`) are treated as in the same scope as the anonymous const's declaration for the purpose of this lint
.const_anon = use a const-anon item to suppress this lint
Expand All @@ -617,12 +614,6 @@ lint_non_local_definitions_macro_rules = non-local `macro_rules!` definition, `#
remove the `#[macro_export]` or make this doc-test a standalone test with its own `fn main() {"{"} ... {"}"}`
.non_local = a `macro_rules!` definition is non-local if it is nested inside an item and has a `#[macro_export]` attribute

lint_non_local_definitions_may_move = may need to be moved as well

lint_non_local_definitions_of_trait_not_local = `{$of_trait_str}` is not local

lint_non_local_definitions_self_ty_not_local = `{$self_ty_str}` is not local

lint_non_snake_case = {$sort} `{$name}` should have a snake case name
.rename_or_convert_suggestion = rename the identifier or convert it to a snake case raw identifier
.cannot_convert_note = `{$sc}` cannot be used as a raw identifier
Expand Down
38 changes: 1 addition & 37 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1375,12 +1375,7 @@ pub(crate) enum NonLocalDefinitionsDiag {
body_name: String,
cargo_update: Option<NonLocalDefinitionsCargoUpdateNote>,
const_anon: Option<Option<Span>>,
move_to: Option<(Span, Vec<Span>)>,
doctest: bool,
may_remove: Option<(Span, String)>,
has_trait: bool,
self_ty_str: String,
of_trait_str: Option<String>,
macro_to_change: Option<(String, &'static str)>,
},
MacroRules {
Expand All @@ -1401,22 +1396,13 @@ impl<'a> LintDiagnostic<'a, ()> for NonLocalDefinitionsDiag {
body_name,
cargo_update,
const_anon,
move_to,
doctest,
may_remove,
has_trait,
self_ty_str,
of_trait_str,
macro_to_change,
} => {
diag.primary_message(fluent::lint_non_local_definitions_impl);
diag.arg("depth", depth);
diag.arg("body_kind_descr", body_kind_descr);
diag.arg("body_name", body_name);
diag.arg("self_ty_str", self_ty_str);
if let Some(of_trait_str) = of_trait_str {
diag.arg("of_trait_str", of_trait_str);
}

if let Some((macro_to_change, macro_kind)) = macro_to_change {
diag.arg("macro_to_change", macro_to_change);
Expand All @@ -1427,34 +1413,12 @@ impl<'a> LintDiagnostic<'a, ()> for NonLocalDefinitionsDiag {
diag.subdiagnostic(cargo_update);
}

if has_trait {
diag.note(fluent::lint_bounds);
diag.note(fluent::lint_with_trait);
} else {
diag.note(fluent::lint_without_trait);
}
diag.note(fluent::lint_non_local);

if let Some((move_help, may_move)) = move_to {
let mut ms = MultiSpan::from_span(move_help);
for sp in may_move {
ms.push_span_label(sp, fluent::lint_non_local_definitions_may_move);
}
diag.span_help(ms, fluent::lint_non_local_definitions_impl_move_help);
}
if doctest {
diag.help(fluent::lint_doctest);
}

if let Some((span, part)) = may_remove {
diag.arg("may_remove_part", part);
diag.span_suggestion(
span,
fluent::lint_remove_help,
"",
Applicability::MaybeIncorrect,
);
}

if let Some(const_anon) = const_anon {
diag.note(fluent::lint_exception);
if let Some(const_anon) = const_anon {
Expand Down
Loading
Loading