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

Check rust lints when an unknown lint is detected #119819

Merged
merged 1 commit into from
Jan 12, 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
10 changes: 8 additions & 2 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -532,8 +532,14 @@ lint_unknown_gated_lint =
lint_unknown_lint =
unknown lint: `{$name}`
.suggestion = did you mean
.help = did you mean: `{$replace}`
.suggestion = {$from_rustc ->
[true] a lint with a similar name exists in `rustc` lints
*[false] did you mean
}
.help = {$from_rustc ->
[true] a lint with a similar name exists in `rustc` lints: `{$replace}`
*[false] did you mean: `{$replace}`
}
lint_unknown_tool_in_scoped_lint = unknown tool name `{$tool_name}` found in scoped lint: `{$tool_name}::{$lint_name}`
.help = add `#![register_tool({$tool_name})]` to the crate root
Expand Down
24 changes: 17 additions & 7 deletions compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use rustc_middle::ty::{self, print::Printer, GenericArg, RegisteredTools, Ty, Ty
use rustc_session::lint::{BuiltinLintDiagnostics, LintExpectationId};
use rustc_session::lint::{FutureIncompatibleInfo, Level, Lint, LintBuffer, LintId};
use rustc_session::{LintStoreMarker, Session};
use rustc_span::edit_distance::find_best_match_for_name;
use rustc_span::edit_distance::find_best_match_for_names;
use rustc_span::symbol::{sym, Ident, Symbol};
use rustc_span::Span;
use rustc_target::abi;
Expand Down Expand Up @@ -117,7 +117,7 @@ struct LintGroup {
pub enum CheckLintNameResult<'a> {
Ok(&'a [LintId]),
/// Lint doesn't exist. Potentially contains a suggestion for a correct lint name.
NoLint(Option<Symbol>),
NoLint(Option<(Symbol, bool)>),
/// The lint refers to a tool that has not been registered.
NoTool,
/// The lint has been renamed to a new name.
Expand Down Expand Up @@ -377,7 +377,7 @@ impl LintStore {
debug!("lints={:?}", self.by_name.keys().collect::<Vec<_>>());
let tool_prefix = format!("{tool_name}::");
return if self.by_name.keys().any(|lint| lint.starts_with(&tool_prefix)) {
self.no_lint_suggestion(&complete_name)
self.no_lint_suggestion(&complete_name, tool_name.as_str())
} else {
// 2. The tool isn't currently running, so no lints will be registered.
// To avoid giving a false positive, ignore all unknown lints.
Expand Down Expand Up @@ -419,13 +419,14 @@ impl LintStore {
}
}

fn no_lint_suggestion(&self, lint_name: &str) -> CheckLintNameResult<'_> {
fn no_lint_suggestion(&self, lint_name: &str, tool_name: &str) -> CheckLintNameResult<'_> {
let name_lower = lint_name.to_lowercase();

if lint_name.chars().any(char::is_uppercase) && self.find_lints(&name_lower).is_ok() {
// First check if the lint name is (partly) in upper case instead of lower case...
return CheckLintNameResult::NoLint(Some(Symbol::intern(&name_lower)));
return CheckLintNameResult::NoLint(Some((Symbol::intern(&name_lower), false)));
}

// ...if not, search for lints with a similar name
// Note: find_best_match_for_name depends on the sort order of its input vector.
// To ensure deterministic output, sort elements of the lint_groups hash map.
Expand All @@ -441,7 +442,16 @@ impl LintStore {
let groups = groups.iter().map(|k| Symbol::intern(k));
let lints = self.lints.iter().map(|l| Symbol::intern(&l.name_lower()));
let names: Vec<Symbol> = groups.chain(lints).collect();
let suggestion = find_best_match_for_name(&names, Symbol::intern(&name_lower), None);
let mut lookups = vec![Symbol::intern(&name_lower)];
if let Some(stripped) = name_lower.split("::").last() {
lookups.push(Symbol::intern(stripped));
}
let res = find_best_match_for_names(&names, &lookups, None);
let is_rustc = res.map_or_else(
|| false,
|s| name_lower.contains("::") && !s.as_str().starts_with(tool_name),
);
let suggestion = res.map(|s| (s, is_rustc));
CheckLintNameResult::NoLint(suggestion)
}

Expand All @@ -454,7 +464,7 @@ impl LintStore {
match self.by_name.get(&complete_name) {
None => match self.lint_groups.get(&*complete_name) {
// Now we are sure, that this lint exists nowhere
None => self.no_lint_suggestion(lint_name),
None => self.no_lint_suggestion(lint_name, tool_name),
Some(LintGroup { lint_ids, depr, .. }) => {
// Reaching this would be weird, but let's cover this case anyway
if let Some(LintAlias { name, silent }) = depr {
Expand Down
9 changes: 5 additions & 4 deletions compiler/rustc_lint/src/levels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -582,8 +582,9 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
}
CheckLintNameResult::NoLint(suggestion) => {
let name = lint_name.clone();
let suggestion =
suggestion.map(|replace| UnknownLintSuggestion::WithoutSpan { replace });
let suggestion = suggestion.map(|(replace, from_rustc)| {
UnknownLintSuggestion::WithoutSpan { replace, from_rustc }
});
let requested_level = RequestedLevel { level, lint_name };
let lint = UnknownLintFromCommandLine { name, suggestion, requested_level };
self.emit_lint(UNKNOWN_LINTS, lint);
Expand Down Expand Up @@ -990,8 +991,8 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
} else {
name.to_string()
};
let suggestion = suggestion.map(|replace| {
UnknownLintSuggestion::WithSpan { suggestion: sp, replace }
let suggestion = suggestion.map(|(replace, from_rustc)| {
UnknownLintSuggestion::WithSpan { suggestion: sp, replace, from_rustc }
});
let lint = UnknownLint { name, suggestion };
self.emit_spanned_lint(UNKNOWN_LINTS, sp.into(), lint);
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1050,9 +1050,10 @@ pub enum UnknownLintSuggestion {
#[primary_span]
suggestion: Span,
replace: Symbol,
from_rustc: bool,
},
#[help(lint_help)]
WithoutSpan { replace: Symbol },
WithoutSpan { replace: Symbol, from_rustc: bool },
}

#[derive(LintDiagnostic)]
Expand Down
28 changes: 28 additions & 0 deletions compiler/rustc_span/src/edit_distance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,34 @@ pub fn find_best_match_for_name(
find_best_match_for_name_impl(false, candidates, lookup, dist)
}

/// Find the best match for multiple words
///
/// This function is intended for use when the desired match would never be
/// returned due to a substring in `lookup` which is superfluous.
///
/// For example, when looking for the closest lint name to `clippy:missing_docs`,
/// we would find `clippy::erasing_op`, despite `missing_docs` existing and being a better suggestion.
/// `missing_docs` would have a larger edit distance because it does not contain the `clippy` tool prefix.
/// In order to find `missing_docs`, this function takes multiple lookup strings, computes the best match
/// for each and returns the match which had the lowest edit distance. In our example, `clippy:missing_docs` and
/// `missing_docs` would be `lookups`, enabling `missing_docs` to be the best match, as desired.
pub fn find_best_match_for_names(
candidates: &[Symbol],
lookups: &[Symbol],
dist: Option<usize>,
) -> Option<Symbol> {
lookups
.iter()
.map(|s| (s, find_best_match_for_name_impl(false, candidates, *s, dist)))
.filter_map(|(s, r)| r.map(|r| (s, r)))
.min_by(|(s1, r1), (s2, r2)| {
let d1 = edit_distance(s1.as_str(), r1.as_str(), usize::MAX).unwrap();
let d2 = edit_distance(s2.as_str(), r2.as_str(), usize::MAX).unwrap();
d1.cmp(&d2)
})
.map(|(_, r)| r)
}

#[cold]
fn find_best_match_for_name_impl(
use_substring_score: bool,
Expand Down
6 changes: 4 additions & 2 deletions src/tools/clippy/tests/ui/unknown_clippy_lints.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@
#[warn(clippy::if_not_else)]
#[warn(clippy::unnecessary_cast)]
#[warn(clippy::useless_transmute)]
// Shouldn't suggest rustc lint name(`dead_code`)
#[warn(clippy::eq_op)]
// Should suggest rustc lint name(`dead_code`)
#[warn(dead_code)]
// Shouldn't suggest removed/deprecated clippy lint name(`unused_collect`)
#[warn(clippy::unused_self)]
// Shouldn't suggest renamed clippy lint name(`const_static_lifetime`)
#[warn(clippy::redundant_static_lifetimes)]
// issue #118183, should report `missing_docs` from rustc lint
#[warn(missing_docs)]
fn main() {}
4 changes: 3 additions & 1 deletion src/tools/clippy/tests/ui/unknown_clippy_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@
#[warn(clippy::if_not_els)]
#[warn(clippy::UNNecsaRy_cAst)]
#[warn(clippy::useles_transute)]
// Shouldn't suggest rustc lint name(`dead_code`)
// Should suggest rustc lint name(`dead_code`)
#[warn(clippy::dead_cod)]
// Shouldn't suggest removed/deprecated clippy lint name(`unused_collect`)
#[warn(clippy::unused_colle)]
// Shouldn't suggest renamed clippy lint name(`const_static_lifetime`)
#[warn(clippy::const_static_lifetim)]
// issue #118183, should report `missing_docs` from rustc lint
#[warn(clippy::missing_docs)]
fn main() {}
20 changes: 18 additions & 2 deletions src/tools/clippy/tests/ui/unknown_clippy_lints.stderr
flip1995 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,12 @@ error: unknown lint: `clippy::dead_cod`
--> $DIR/unknown_clippy_lints.rs:11:8
|
LL | #[warn(clippy::dead_cod)]
| ^^^^^^^^^^^^^^^^ help: did you mean: `clippy::eq_op`
| ^^^^^^^^^^^^^^^^
|
help: a lint with a similar name exists in `rustc` lints
|
LL | #[warn(dead_code)]
| ~~~~~~~~~

error: unknown lint: `clippy::unused_colle`
--> $DIR/unknown_clippy_lints.rs:13:8
Expand All @@ -49,5 +54,16 @@ error: unknown lint: `clippy::const_static_lifetim`
LL | #[warn(clippy::const_static_lifetim)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: did you mean: `clippy::redundant_static_lifetimes`

error: aborting due to 8 previous errors
error: unknown lint: `clippy::missing_docs`
--> $DIR/unknown_clippy_lints.rs:17:8
|
LL | #[warn(clippy::missing_docs)]
| ^^^^^^^^^^^^^^^^^^^^
|
help: a lint with a similar name exists in `rustc` lints
|
LL | #[warn(missing_docs)]
| ~~~~~~~~~~~~

error: aborting due to 9 previous errors

Loading