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

Add unknown-lints lint #13639

Closed
wants to merge 3 commits into from
Closed
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
67 changes: 57 additions & 10 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ use crate::sources::{PathSource, CRATES_IO_INDEX, CRATES_IO_REGISTRY};
use crate::util::edit_distance;
use crate::util::errors::{CargoResult, ManifestError};
use crate::util::interning::InternedString;
use crate::util::lints::check_implicit_features;
use crate::util::lints::{check_implicit_features, unknown_lints};
use crate::util::toml::{read_manifest, InheritableFields};
use crate::util::{context::ConfigRelativePath, Filesystem, GlobalContext, IntoUrl};
use cargo_util::paths;
use cargo_util::paths::normalize_path;
use cargo_util_schemas::manifest;
use cargo_util_schemas::manifest::RustVersion;
use cargo_util_schemas::manifest::{RustVersion, TomlManifest};
use cargo_util_schemas::manifest::{TomlDependency, TomlProfiles};
use pathdiff::diff_paths;

Expand Down Expand Up @@ -119,6 +119,43 @@ pub enum MaybePackage {
Virtual(VirtualManifest),
}

impl MaybePackage {
pub fn contents(&self) -> &str {
match self {
MaybePackage::Package(pkg) => pkg.manifest().contents(),
MaybePackage::Virtual(vm) => vm.contents(),
}
}

pub fn document(&self) -> &toml_edit::ImDocument<String> {
match self {
MaybePackage::Package(pkg) => pkg.manifest().document(),
MaybePackage::Virtual(vm) => vm.document(),
}
}

pub fn edition(&self) -> Edition {
match self {
MaybePackage::Package(p) => p.manifest().edition(),
MaybePackage::Virtual(_) => Edition::default(),
}
}

pub fn original_toml(&self) -> &TomlManifest {
match self {
MaybePackage::Package(pkg) => pkg.manifest().original_toml(),
MaybePackage::Virtual(vm) => vm.original_toml(),
}
}

pub fn resolved_toml(&self) -> &TomlManifest {
match self {
MaybePackage::Package(pkg) => pkg.manifest().resolved_toml(),
MaybePackage::Virtual(vm) => vm.resolved_toml(),
}
}
}

/// Configuration of a workspace in a manifest.
#[derive(Debug, Clone)]
pub enum WorkspaceConfig {
Expand Down Expand Up @@ -1098,9 +1135,7 @@ impl<'gctx> Workspace<'gctx> {
pub fn emit_warnings(&self) -> CargoResult<()> {
for (path, maybe_pkg) in &self.packages.packages {
let path = path.join("Cargo.toml");
if let MaybePackage::Package(pkg) = maybe_pkg {
self.emit_lints(pkg, &path)?
}
self.emit_lints(maybe_pkg, &path)?;
let warnings = match maybe_pkg {
MaybePackage::Package(pkg) => pkg.manifest().warnings().warnings(),
MaybePackage::Virtual(vm) => vm.warnings().warnings(),
Expand All @@ -1126,16 +1161,15 @@ impl<'gctx> Workspace<'gctx> {
Ok(())
}

pub fn emit_lints(&self, pkg: &Package, path: &Path) -> CargoResult<()> {
pub fn emit_lints(&self, maybe_pkg: &MaybePackage, path: &Path) -> CargoResult<()> {
let mut error_count = 0;
let toml_lints = pkg
.manifest()
let resolved_lints = maybe_pkg
.resolved_toml()
.lints
.clone()
.map(|lints| lints.lints)
.unwrap_or(manifest::TomlLints::default());
let cargo_lints = toml_lints
let cargo_lints = resolved_lints
.get("cargo")
.cloned()
.unwrap_or(manifest::TomlToolLints::default());
Expand All @@ -1144,7 +1178,20 @@ impl<'gctx> Workspace<'gctx> {
.map(|(name, lint)| (name.replace('-', "_"), lint))
.collect();

check_implicit_features(pkg, &path, &normalized_lints, &mut error_count, self.gctx)?;
unknown_lints(
maybe_pkg,
&path,
&normalized_lints,
&mut error_count,
self.gctx,
)?;
check_implicit_features(
maybe_pkg,
&path,
&normalized_lints,
&mut error_count,
self.gctx,
)?;
if error_count > 0 {
Err(crate::util::errors::AlreadyPrintedError::new(anyhow!(
"encountered {error_count} errors(s) while running lints"
Expand Down
108 changes: 106 additions & 2 deletions src/cargo/util/lints.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::core::FeatureValue::Dep;
use crate::core::{Edition, FeatureValue, Package};
use crate::core::{Edition, FeatureValue, MaybePackage};
use crate::util::interning::InternedString;
use crate::{CargoResult, GlobalContext};
use annotate_snippets::{Level, Renderer, Snippet};
Expand Down Expand Up @@ -58,6 +58,8 @@ fn rel_cwd_manifest_path(path: &Path, gctx: &GlobalContext) -> String {
.to_string()
}

const LINT_GROUPS: &[LintGroup] = &[RUST_2024_COMPATIBILITY];

#[derive(Copy, Clone, Debug)]
pub struct LintGroup {
pub name: &'static str,
Expand Down Expand Up @@ -136,6 +138,104 @@ impl From<TomlLintLevel> for LintLevel {
}
}

const LINTS: &[Lint] = &[IMPLICIT_FEATURES, UNKNOWN_LINTS];

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

pub fn unknown_lints(
maybe_pkg: &MaybePackage,
path: &Path,
resolved_lints: &TomlToolLints,
error_count: &mut usize,
gctx: &GlobalContext,
) -> CargoResult<()> {
let lint_level = UNKNOWN_LINTS.level(resolved_lints, maybe_pkg.edition());
if lint_level == LintLevel::Allow {
return Ok(());
}

let original_toml = maybe_pkg.original_toml();
let contents = maybe_pkg.contents();
let document = maybe_pkg.document();
let renderer = Renderer::styled().term_width(
gctx.shell()
.err_width()
.diagnostic_terminal_width()
.unwrap_or(annotate_snippets::renderer::DEFAULT_TERM_WIDTH),
);
let level = lint_level.to_diagnostic_level();
let manifest_path = rel_cwd_manifest_path(path, gctx);

let all_names = LINTS
.iter()
.map(|lint| lint.name)
.chain(LINT_GROUPS.iter().map(|group| group.name))
.collect::<HashSet<_>>();

if let Some(lints) = original_toml
.workspace
.as_ref()
.map_or(&None, |ws| &ws.lints)
Comment on lines +181 to +184
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo this should be treated as a separate "workspace" lint that is only subject to workspace.lints so we have consistency between virtual and real workspace packages.

{
if let Some(cargo_lints) = lints.get("cargo") {
for (name, _) in cargo_lints.iter() {
let normalized_name = name.replace("-", "_");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is having me lean more towards normalizing earlier in the process

if all_names.contains(normalized_name.as_str()) {
continue;
}
if lint_level == LintLevel::Forbid || lint_level == LintLevel::Deny {
*error_count += 1;
}
let title = format!("unknown lint: `{}`", name);
let message = level.title(&title).snippet(
Snippet::source(contents)
.origin(&manifest_path)
.annotation(
level.span(
get_span(document, &["workspace", "lints", "cargo", name], false)
.unwrap(),
),
)
.fold(true),
);
writeln!(gctx.shell().err(), "{}", renderer.render(message))?;
}
}
}

if let Some(lints) = original_toml.lints.as_ref().map(|l| &l.lints) {
if let Some(cargo_lints) = lints.get("cargo") {
for (name, _) in cargo_lints.iter() {
let normalized_name = name.replace("-", "_");
if all_names.contains(normalized_name.as_str()) {
continue;
}
if lint_level == LintLevel::Forbid || lint_level == LintLevel::Deny {
*error_count += 1;
}
let title = format!("unknown lint: `{}`", name);
let message =
level.title(&title).snippet(
Snippet::source(contents)
.origin(&manifest_path)
.annotation(level.span(
get_span(document, &["lints", "cargo", name], false).unwrap(),
))
.fold(true),
);
writeln!(gctx.shell().err(), "{}", renderer.render(message))?;
}
}
}
Ok(())
}

/// By default, cargo will treat any optional dependency as a [feature]. As of
/// cargo 1.60, these can be disabled by declaring a feature that activates the
/// optional dependency as `dep:<name>` (see [RFC #3143]).
Expand All @@ -158,12 +258,16 @@ const IMPLICIT_FEATURES: Lint = Lint {
};

pub fn check_implicit_features(
pkg: &Package,
maybe_pkg: &MaybePackage,
path: &Path,
lints: &TomlToolLints,
error_count: &mut usize,
gctx: &GlobalContext,
) -> CargoResult<()> {
let MaybePackage::Package(pkg) = maybe_pkg else {
return Ok(());
};

let lint_level = IMPLICIT_FEATURES.level(lints, pkg.manifest().edition());
if lint_level == LintLevel::Allow {
return Ok(());
Expand Down
6 changes: 2 additions & 4 deletions tests/testsuite/lints/implicit_features/edition_2021/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use cargo_test_support::prelude::*;
use cargo_test_support::project;
use cargo_test_support::registry::Package;
use cargo_test_support::str;
use cargo_test_support::{file, str};

#[cargo_test]
fn case() {
Expand All @@ -23,12 +23,10 @@ bar = { version = "0.1.0", optional = true }
.build();

snapbox::cmd::Command::cargo_ui()
.masquerade_as_nightly_cargo(&["always_nightly"])
.current_dir(p.root())
.arg("check")
.arg("--quiet")
.assert()
.success()
.stdout_matches(str![""])
.stderr_matches(str![""]);
.stderr_matches(file!["stderr.term.svg"]);
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ implicit-features = "warn"
.build();

snapbox::cmd::Command::cargo_ui()
.masquerade_as_nightly_cargo(&["always_nightly"])
.masquerade_as_nightly_cargo(&["cargo-lints"])
.current_dir(p.root())
.arg("check")
.arg("--quiet")
.arg("-Zcargo-lints")
.assert()
.success()
.stdout_matches(str![""])
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ baz = ["dep:baz"]
.build();

snapbox::cmd::Command::cargo_ui()
.masquerade_as_nightly_cargo(&["always_nightly"])
.masquerade_as_nightly_cargo(&["edition2024"])
.current_dir(p.root())
.arg("check")
.assert()
Expand Down
4 changes: 2 additions & 2 deletions tests/testsuite/lints/implicit_features/warn/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ implicit-features = "warn"
.build();

snapbox::cmd::Command::cargo_ui()
.masquerade_as_nightly_cargo(&["always_nightly"])
.masquerade_as_nightly_cargo(&["cargo-lints", "edition2024"])
.current_dir(p.root())
.arg("check")
.arg("--quiet")
.arg("-Zcargo-lints")
.assert()
.success()
.stdout_matches(str![""])
Expand Down
13 changes: 11 additions & 2 deletions tests/testsuite/lints/implicit_features/warn/stderr.term.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading