Skip to content

Commit

Permalink
Rollup merge of #119819 - chenyukang:yukang-fix-118183-lint, r=davidtwco
Browse files Browse the repository at this point in the history
Check rust lints when an unknown lint is detected

Fixes #118183
  • Loading branch information
GuillaumeGomez committed Jan 12, 2024
2 parents 46c3c01 + ca421fe commit 737452a
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 19 deletions.
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
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

0 comments on commit 737452a

Please sign in to comment.