Skip to content

Commit

Permalink
fix(lints): Remove ability to specify - in lint name
Browse files Browse the repository at this point in the history
  • Loading branch information
Muscraft committed May 1, 2024
1 parent 6fc9e4b commit 49bf098
Show file tree
Hide file tree
Showing 12 changed files with 402 additions and 94 deletions.
13 changes: 4 additions & 9 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1161,7 +1161,6 @@ impl<'gctx> Workspace<'gctx> {
.cloned()
.unwrap_or_default()
.into_iter()
.map(|(k, v)| (k.replace('-', "_"), v))
.collect();

for (path, maybe_pkg) in &self.packages.packages {
Expand Down Expand Up @@ -1214,10 +1213,6 @@ impl<'gctx> Workspace<'gctx> {
.get("cargo")
.cloned()
.unwrap_or(manifest::TomlToolLints::default());
let normalized_lints = cargo_lints
.into_iter()
.map(|(name, lint)| (name.replace('-', "_"), lint))
.collect();

// We should only be using workspace lints if the `[lints]` table is
// present in the manifest, and `workspace` is set to `true`
Expand All @@ -1242,7 +1237,7 @@ impl<'gctx> Workspace<'gctx> {
analyze_cargo_lints_table(
pkg,
&path,
&normalized_lints,
&cargo_lints,
ws_cargo_lints,
ws_contents,
ws_document,
Expand All @@ -1252,23 +1247,23 @@ impl<'gctx> Workspace<'gctx> {
check_im_a_teapot(
pkg,
&path,
&normalized_lints,
&cargo_lints,
ws_cargo_lints,
&mut error_count,
self.gctx,
)?;
check_implicit_features(
pkg,
&path,
&normalized_lints,
&cargo_lints,
ws_cargo_lints,
&mut error_count,
self.gctx,
)?;
unused_dependencies(
pkg,
&path,
&normalized_lints,
&cargo_lints,
ws_cargo_lints,
&mut error_count,
self.gctx,
Expand Down
233 changes: 180 additions & 53 deletions src/cargo/util/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,45 +29,68 @@ pub fn analyze_cargo_lints_table(
let manifest = pkg.manifest();
let manifest_path = rel_cwd_manifest_path(path, gctx);
let ws_path = rel_cwd_manifest_path(ws_path, gctx);

for lint_name in pkg_lints
let mut unknown_lints = Vec::new();
for (lint_name, specified_in) in pkg_lints
.keys()
.chain(ws_lints.map(|l| l.keys()).unwrap_or_default())
.map(|name| (name, SpecifiedIn::Package))
.chain(
ws_lints
.map(|l| l.keys())
.unwrap_or_default()
.map(|name| (name, SpecifiedIn::Workspace)),
)
{
if let Some((name, default_level, edition_lint_opts, feature_gate)) =
let Some((name, default_level, edition_lint_opts, feature_gate)) =
find_lint_or_group(lint_name)
{
let (_, reason, _) = level_priority(
name,
*default_level,
*edition_lint_opts,
pkg_lints,
ws_lints,
manifest.edition(),
);

// Only run analysis on user-specified lints
if !reason.is_user_specified() {
continue;
}
else {
unknown_lints.push((lint_name, specified_in));
continue;
};

// Only run this on lints that are gated by a feature
if let Some(feature_gate) = feature_gate {
verify_feature_enabled(
name,
feature_gate,
reason,
manifest,
&manifest_path,
ws_contents,
ws_document,
&ws_path,
&mut error_count,
gctx,
)?;
}
let (_, reason, _) = level_priority(
name,
*default_level,
*edition_lint_opts,
pkg_lints,
ws_lints,
manifest.edition(),
);

// Only run analysis on user-specified lints
if !reason.is_user_specified() {
continue;
}

// Only run this on lints that are gated by a feature
if let Some(feature_gate) = feature_gate {
verify_feature_enabled(
name,
feature_gate,
reason,
manifest,
&manifest_path,
ws_contents,
ws_document,
&ws_path,
&mut error_count,
gctx,
)?;
}
}

output_unknown_lints(
unknown_lints,
manifest,
&manifest_path,
pkg_lints,
ws_lints,
ws_contents,
ws_document,
&ws_path,
&mut error_count,
gctx,
)?;

if error_count > 0 {
Err(anyhow::anyhow!(
"encountered {error_count} errors(s) while verifying lints",
Expand Down Expand Up @@ -117,31 +140,21 @@ fn verify_feature_enabled(
gctx: &GlobalContext,
) -> CargoResult<()> {
if !manifest.unstable_features().is_enabled(feature_gate) {
let dash_name = lint_name.replace("_", "-");
let dash_feature_name = feature_gate.name().replace("_", "-");
let title = format!("use of unstable lint `{}`", dash_name);
let title = format!("use of unstable lint `{}`", lint_name);
let label = format!(
"this is behind `{}`, which is not enabled",
dash_feature_name
);
let second_title = format!("`cargo::{}` was inherited", dash_name);
let second_title = format!("`cargo::{}` was inherited", lint_name);
let help = format!(
"consider adding `cargo-features = [\"{}\"]` to the top of the manifest",
dash_feature_name
);
let message = match reason {
LintLevelReason::Package => {
let span = get_span(
manifest.document(),
&["lints", "cargo", dash_name.as_str()],
false,
)
.or(get_span(
manifest.document(),
&["lints", "cargo", lint_name],
false,
))
.unwrap();
let span =
get_span(manifest.document(), &["lints", "cargo", lint_name], false).unwrap();

Level::Error
.title(&title)
Expand All @@ -155,15 +168,10 @@ fn verify_feature_enabled(
}
LintLevelReason::Workspace => {
let lint_span = get_span(
ws_document,
&["workspace", "lints", "cargo", dash_name.as_str()],
false,
)
.or(get_span(
ws_document,
&["workspace", "lints", "cargo", lint_name],
false,
))
)
.unwrap();
let inherit_span_key =
get_span(manifest.document(), &["lints", "workspace"], false).unwrap();
Expand Down Expand Up @@ -395,6 +403,11 @@ impl LintLevelReason {
}
}

enum SpecifiedIn {
Package,
Workspace,
}

fn level_priority(
name: &str,
default_level: LintLevel,
Expand Down Expand Up @@ -588,6 +601,120 @@ pub fn check_implicit_features(
Ok(())
}

const UNKNOWN_LINTS: Lint = Lint {
name: "unknown_lints",
desc: "unknown lint",
groups: &[],
default_level: LintLevel::Warn,
edition_lint_opts: None,
feature_gate: None,
};

fn output_unknown_lints(
unknown_lints: Vec<(&String, SpecifiedIn)>,
manifest: &Manifest,
manifest_path: &str,
pkg_lints: &TomlToolLints,
ws_lints: Option<&TomlToolLints>,
ws_contents: &str,
ws_document: &ImDocument<String>,
ws_path: &str,
error_count: &mut usize,
gctx: &GlobalContext,
) -> CargoResult<()> {
let (lint_level, reason) = UNKNOWN_LINTS.level(
pkg_lints,
ws_lints,
manifest.edition(),
manifest.unstable_features(),
);
if lint_level == LintLevel::Allow {
return Ok(());
}

let level = lint_level.to_diagnostic_level();
let mut emitted_source = None;
for (lint_name, specified_in) in unknown_lints {
if lint_level == LintLevel::Forbid || lint_level == LintLevel::Deny {
*error_count += 1;
}
let title = format!("{}: `{lint_name}`", UNKNOWN_LINTS.desc);
let second_title = format!("`cargo::{}` was inherited", lint_name);
let underscore_lint_name = lint_name.replace("-", "_");
let matching = if let Some(lint) = LINTS.iter().find(|l| l.name == underscore_lint_name) {
Some((lint.name, "lint"))
} else if let Some(group) = LINT_GROUPS.iter().find(|g| g.name == underscore_lint_name) {
Some((group.name, "group"))
} else {
None
};
let help =
matching.map(|(name, kind)| format!("there is a {kind} with a similar name: `{name}`"));

let mut message = match specified_in {
SpecifiedIn::Package => {
let span =
get_span(manifest.document(), &["lints", "cargo", lint_name], false).unwrap();

level.title(&title).snippet(
Snippet::source(manifest.contents())
.origin(&manifest_path)
.annotation(Level::Error.span(span))
.fold(true),
)
}
SpecifiedIn::Workspace => {
let lint_span = get_span(
ws_document,
&["workspace", "lints", "cargo", lint_name],
false,
)
.unwrap();
let inherit_span_key =
get_span(manifest.document(), &["lints", "workspace"], false).unwrap();
let inherit_span_value =
get_span(manifest.document(), &["lints", "workspace"], true).unwrap();

level
.title(&title)
.snippet(
Snippet::source(ws_contents)
.origin(&ws_path)
.annotation(Level::Error.span(lint_span))
.fold(true),
)
.footer(
Level::Note.title(&second_title).snippet(
Snippet::source(manifest.contents())
.origin(&manifest_path)
.annotation(
Level::Note
.span(inherit_span_key.start..inherit_span_value.end),
)
.fold(true),
),
)
}
};

if emitted_source.is_none() {
emitted_source = Some(format!(
"`cargo::{}` is set to `{lint_level}` {reason}",
UNKNOWN_LINTS.name
));
message = message.footer(Level::Note.title(emitted_source.as_ref().unwrap()));
}

if let Some(help) = help.as_ref() {
message = message.footer(Level::Help.title(help));
}

gctx.shell().print_message(message)?;
}

Ok(())
}

const UNUSED_OPTIONAL_DEPENDENCY: Lint = Lint {
name: "unused_optional_dependency",
desc: "unused optional dependency",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ baz = { version = "0.1.0", optional = true }
target-dep = { version = "0.1.0", optional = true }
[lints.cargo]
implicit-features = "warn"
implicit_features = "warn"
"#,
)
.file("src/lib.rs", "")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ baz = { version = "0.1.0", optional = true }
baz = ["dep:baz"]
[lints.cargo]
unused-optional-dependency = "allow"
unused_optional_dependency = "allow"
"#,
)
.file("src/lib.rs", "")
Expand Down
1 change: 1 addition & 0 deletions tests/testsuite/lints/mod.rs
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
mod implicit_features;
mod unknown_lints;
mod unused_optional_dependencies;
33 changes: 33 additions & 0 deletions tests/testsuite/lints/unknown_lints/default/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
use cargo_test_support::prelude::*;
use cargo_test_support::str;
use cargo_test_support::{file, project};

#[cargo_test]
fn case() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.0.1"
edition = "2015"
authors = []
[lints.cargo]
this-lint-does-not-exist = "warn"
"#,
)
.file("src/lib.rs", "")
.build();

snapbox::cmd::Command::cargo_ui()
.masquerade_as_nightly_cargo(&["cargo-lints"])
.current_dir(p.root())
.arg("check")
.arg("-Zcargo-lints")
.assert()
.success()
.stdout_matches(str![""])
.stderr_matches(file!["stderr.term.svg"]);
}
Loading

0 comments on commit 49bf098

Please sign in to comment.