Skip to content

Commit

Permalink
Tweak CheckLintNameResult::Tool.
Browse files Browse the repository at this point in the history 
It has a clumsy type, with repeated `&'a [LintId]`, and sometimes
requires an empty string that isn't used in the `Err`+`None` case.

This commit splits it into two variants.
  • Loading branch information
@nnethercote
nnethercote committed Jun 2, 2024
1 parent d070e89 commit 32c8a12
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 53 deletions.
39 changes: 21 additions & 18 deletions compiler/rustc_lint/src/context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,12 +127,16 @@ pub enum CheckLintNameResult<'a> {
Renamed(String),
/// The lint has been removed due to the given reason.
Removed(String),
/// The lint is from a tool. If the Option is None, then either
/// the lint does not exist in the tool or the code was not
/// compiled with the tool and therefore the lint was never
/// added to the `LintStore`. Otherwise the `LintId` will be
/// returned as if it where a rustc lint.
Tool(Result<&'a [LintId], (Option<&'a [LintId]>, String)>),

/// The lint is from a tool. The `LintId` will be returned as if it were a
/// rustc lint. The `Option<String>` indicates if the lint has been
/// renamed.
Tool(&'a [LintId], Option<String>),

/// The lint is from a tool. Either the lint does not exist in the tool or
/// the code was not compiled with the tool and therefore the lint was
/// never added to the `LintStore`.
MissingTool,
}

impl LintStore {
Expand Down Expand Up @@ -385,14 +389,14 @@ impl LintStore {
} else {
// 2. The tool isn't currently running, so no lints will be registered.
// To avoid giving a false positive, ignore all unknown lints.
CheckLintNameResult::Tool(Err((None, String::new())))
CheckLintNameResult::MissingTool
};
}
Some(LintGroup { lint_ids, .. }) => {
return CheckLintNameResult::Tool(Ok(lint_ids));
return CheckLintNameResult::Tool(lint_ids, None);
}
},
Some(Id(id)) => return CheckLintNameResult::Tool(Ok(slice::from_ref(id))),
Some(Id(id)) => return CheckLintNameResult::Tool(slice::from_ref(id), None),
// If the lint was registered as removed or renamed by the lint tool, we don't need
// to treat tool_lints and rustc lints different and can use the code below.
_ => {}
Expand All @@ -412,7 +416,7 @@ impl LintStore {
return if *silent {
CheckLintNameResult::Ok(lint_ids)
} else {
CheckLintNameResult::Tool(Err((Some(lint_ids), (*name).to_string())))
CheckLintNameResult::Tool(lint_ids, Some((*name).to_string()))
};
}
CheckLintNameResult::Ok(lint_ids)
Expand Down Expand Up @@ -473,18 +477,17 @@ impl LintStore {
// Reaching this would be weird, but let's cover this case anyway
if let Some(LintAlias { name, silent }) = depr {
let LintGroup { lint_ids, .. } = self.lint_groups.get(name).unwrap();
return if *silent {
CheckLintNameResult::Tool(Err((Some(lint_ids), complete_name)))
if *silent {
CheckLintNameResult::Tool(lint_ids, Some(complete_name))
} else {
CheckLintNameResult::Tool(Err((Some(lint_ids), (*name).to_string())))
};
CheckLintNameResult::Tool(lint_ids, Some((*name).to_string()))
}
} else {
CheckLintNameResult::Tool(lint_ids, Some(complete_name))
}
CheckLintNameResult::Tool(Err((Some(lint_ids), complete_name)))
}
},
Some(Id(id)) => {
CheckLintNameResult::Tool(Err((Some(slice::from_ref(id)), complete_name)))
}
Some(Id(id)) => CheckLintNameResult::Tool(slice::from_ref(id), Some(complete_name)),
Some(other) => {
debug!("got renamed lint {:?}", other);
CheckLintNameResult::NoLint(None)
Expand Down
60 changes: 25 additions & 35 deletions compiler/rustc_lint/src/levels.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -593,7 +593,7 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
let lint = UnknownLintFromCommandLine { name, suggestion, requested_level };
self.emit_lint(UNKNOWN_LINTS, lint);
}
CheckLintNameResult::Tool(Err((Some(_), ref replace))) => {
CheckLintNameResult::Tool(_, Some(ref replace)) => {
let name = lint_name.clone();
let requested_level = RequestedLevel { level, lint_name };
let lint = DeprecatedLintNameFromCommandLine { name, replace, requested_level };
Expand Down Expand Up @@ -902,62 +902,52 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
}
}

CheckLintNameResult::Tool(result) => {
match *result {
Ok(ids) => {
CheckLintNameResult::Tool(ids, new_lint_name) => {
let src = match new_lint_name {
None => {
let complete_name =
&format!("{}::{}", tool_ident.unwrap().name, name);
let src = LintLevelSource::Node {
LintLevelSource::Node {
name: Symbol::intern(complete_name),
span: sp,
reason,
};
for &id in ids {
if self.check_gated_lint(id, attr.span, false) {
self.insert_spec(id, (level, src));
}
}
if let Level::Expect(expect_id) = level {
self.provider.push_expectation(
expect_id,
LintExpectation::new(reason, sp, false, tool_name),
);
}
}
Err((Some(ids), ref new_lint_name)) => {
let lint = builtin::RENAMED_AND_REMOVED_LINTS;
Some(new_lint_name) => {
self.emit_span_lint(
lint,
builtin::RENAMED_AND_REMOVED_LINTS,
sp.into(),
DeprecatedLintName {
name,
suggestion: sp,
replace: new_lint_name,
},
);

let src = LintLevelSource::Node {
LintLevelSource::Node {
name: Symbol::intern(new_lint_name),
span: sp,
reason,
};
for id in ids {
self.insert_spec(*id, (level, src));
}
if let Level::Expect(expect_id) = level {
self.provider.push_expectation(
expect_id,
LintExpectation::new(reason, sp, false, tool_name),
);
}
}
Err((None, _)) => {
// If Tool(Err(None, _)) is returned, then either the lint does not
// exist in the tool or the code was not compiled with the tool and
// therefore the lint was never added to the `LintStore`. To detect
// this is the responsibility of the lint tool.
};
for &id in *ids {
if self.check_gated_lint(id, attr.span, false) {
self.insert_spec(id, (level, src));
}
}
if let Level::Expect(expect_id) = level {
self.provider.push_expectation(
expect_id,
LintExpectation::new(reason, sp, false, tool_name),
);
}
}

CheckLintNameResult::MissingTool => {
// If `MissingTool` is returned, then either the lint does not
// exist in the tool or the code was not compiled with the tool and
// therefore the lint was never added to the `LintStore`. To detect
// this is the responsibility of the lint tool.
}

&CheckLintNameResult::NoTool => {
Expand Down

0 comments on commit 32c8a12

Please sign in to comment.