Skip to content
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
48 changes: 46 additions & 2 deletions crates/xtask-lint-docs/src/main.rs
Copy link
Member

Choose a reason for hiding this comment

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

Note: Dynamic groups (i.e., all or warnings) aren't supported at this time

I am a bit unsure whether we want to have cargo::all. clippy::all is not a real "all" (it includes only all lints that are on by default). So sometimes the name is a bit surprising. Anyhow, this is not a blocker for this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that we should not mirror clippy::all

Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ fn main() -> anyhow::Result<()> {
let mut lint_docs = String::new();
for lint in cargo::lints::LINTS.iter().sorted_by_key(|lint| lint.name) {
if lint.docs.is_some() {
let sectipn = match lint.default_level {
let sectipn = match lint.primary_group.default_level {
LintLevel::Allow => &mut allow,
LintLevel::Warn => &mut warn,
LintLevel::Deny => &mut deny,
Expand All @@ -41,6 +41,8 @@ fn main() -> anyhow::Result<()> {
)?;
writeln!(buf)?;

lint_groups(&mut buf)?;

if !allow.is_empty() {
add_level_section(LintLevel::Allow, &allow, &mut buf)?;
}
Expand Down Expand Up @@ -69,9 +71,51 @@ fn main() -> anyhow::Result<()> {
Ok(())
}

fn lint_groups(buf: &mut String) -> anyhow::Result<()> {
let (max_name_len, max_desc_len) = cargo::lints::LINT_GROUPS.iter().filter(|g| !g.hidden).fold(
(0, 0),
|(max_name_len, max_desc_len), group| {
// We add 9 to account for the "cargo::" prefix and backticks
let name_len = group.name.chars().count() + 9;
let desc_len = group.desc.chars().count();
(max_name_len.max(name_len), max_desc_len.max(desc_len))
},
);
let default_level_len = "Default level".chars().count();
writeln!(buf, "\n")?;
writeln!(
buf,
"| {:<max_name_len$} | {:<max_desc_len$} | Default level |",
"Group", "Description",
)?;
writeln!(
buf,
"|-{}-|-{}-|-{}-|",
"-".repeat(max_name_len),
"-".repeat(max_desc_len),
"-".repeat(default_level_len)
)?;
for group in cargo::lints::LINT_GROUPS.iter() {
if group.hidden {
continue;
}
let group_name = format!("`cargo::{}`", group.name);
writeln!(
buf,
"| {:<max_name_len$} | {:<max_desc_len$} | {:<default_level_len$} |",
group_name,
group.desc,
group.default_level.to_string(),
)?;
}
writeln!(buf, "\n")?;
Ok(())
}

fn add_lint(lint: &Lint, buf: &mut String) -> std::fmt::Result {
writeln!(buf, "## `{}`", lint.name)?;
writeln!(buf, "Set to `{}` by default", lint.default_level)?;
writeln!(buf, "Group: `{}`\n", lint.primary_group.name)?;
writeln!(buf, "Level: `{}`", lint.primary_group.default_level)?;
writeln!(buf, "{}\n", lint.docs.as_ref().unwrap())
}

Expand Down
142 changes: 103 additions & 39 deletions src/cargo/lints/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,25 @@ use cargo_util_schemas::manifest::TomlToolLints;
use pathdiff::diff_paths;

use std::borrow::Cow;
use std::cmp::{Reverse, max_by_key};
use std::fmt::Display;
use std::ops::Range;
use std::path::Path;

pub mod rules;
pub use rules::LINTS;

const LINT_GROUPS: &[LintGroup] = &[TEST_DUMMY_UNSTABLE];
pub const LINT_GROUPS: &[LintGroup] = &[
COMPLEXITY,
CORRECTNESS,
NURSERY,
PEDANTIC,
PERF,
RESTRICTION,
STYLE,
SUSPICIOUS,
TEST_DUMMY_UNSTABLE,
];

/// Scope at which a lint runs: package-level or workspace-level.
pub enum ManifestFor<'a> {
Expand Down Expand Up @@ -140,17 +151,12 @@ fn find_lint_or_group<'a>(
if let Some(lint) = LINTS.iter().find(|l| l.name == name) {
Some((
lint.name,
&lint.default_level,
&lint.primary_group.default_level,
&lint.edition_lint_opts,
&lint.feature_gate,
))
} else if let Some(group) = LINT_GROUPS.iter().find(|g| g.name == name) {
Some((
group.name,
&group.default_level,
&group.edition_lint_opts,
&group.feature_gate,
))
Some((group.name, &group.default_level, &None, &group.feature_gate))
} else {
None
}
Expand Down Expand Up @@ -262,25 +268,88 @@ pub struct LintGroup {
pub name: &'static str,
pub default_level: LintLevel,
pub desc: &'static str,
pub edition_lint_opts: Option<(Edition, LintLevel)>,
pub feature_gate: Option<&'static Feature>,
pub hidden: bool,
}

const COMPLEXITY: LintGroup = LintGroup {
name: "complexity",
desc: "code that does something simple but in a complex way",
default_level: LintLevel::Warn,
feature_gate: None,
hidden: false,
};

const CORRECTNESS: LintGroup = LintGroup {
name: "correctness",
desc: "code that is outright wrong or useless",
default_level: LintLevel::Deny,
feature_gate: None,
hidden: false,
};

const NURSERY: LintGroup = LintGroup {
name: "nursery",
desc: "new lints that are still under development",
default_level: LintLevel::Allow,
feature_gate: None,
hidden: false,
};

const PEDANTIC: LintGroup = LintGroup {
name: "pedantic",
desc: "lints which are rather strict or have occasional false positives",
default_level: LintLevel::Allow,
feature_gate: None,
hidden: false,
};

const PERF: LintGroup = LintGroup {
name: "perf",
desc: "code that can be written to run faster",
default_level: LintLevel::Warn,
feature_gate: None,
hidden: false,
};

const RESTRICTION: LintGroup = LintGroup {
name: "restriction",
desc: "lints which prevent the use of Cargo features",
default_level: LintLevel::Allow,
feature_gate: None,
hidden: false,
};

const STYLE: LintGroup = LintGroup {
name: "style",
desc: "code that should be written in a more idiomatic way",
default_level: LintLevel::Warn,
feature_gate: None,
hidden: false,
};

const SUSPICIOUS: LintGroup = LintGroup {
name: "suspicious",
desc: "code that is most likely wrong or useless",
default_level: LintLevel::Warn,
feature_gate: None,
hidden: false,
};

/// This lint group is only to be used for testing purposes
const TEST_DUMMY_UNSTABLE: LintGroup = LintGroup {
name: "test_dummy_unstable",
desc: "test_dummy_unstable is meant to only be used in tests",
default_level: LintLevel::Allow,
edition_lint_opts: None,
feature_gate: Some(Feature::test_dummy_unstable()),
hidden: true,
};

#[derive(Copy, Clone, Debug)]
pub struct Lint {
pub name: &'static str,
pub desc: &'static str,
pub groups: &'static [LintGroup],
pub default_level: LintLevel,
pub primary_group: &'static LintGroup,
pub edition_lint_opts: Option<(Edition, LintLevel)>,
pub feature_gate: Option<&'static Feature>,
/// This is a markdown formatted string that will be used when generating
Expand All @@ -305,33 +374,28 @@ impl Lint {
return (LintLevel::Allow, LintLevelReason::Default);
}

self.groups
.iter()
.map(|g| {
(
g.name,
level_priority(
g.name,
g.default_level,
g.edition_lint_opts,
pkg_lints,
edition,
),
)
})
.chain(std::iter::once((
self.name,
level_priority(
self.name,
self.default_level,
self.edition_lint_opts,
pkg_lints,
edition,
),
)))
.max_by_key(|(n, (l, _, p))| (l == &LintLevel::Forbid, *p, std::cmp::Reverse(*n)))
.map(|(_, (l, r, _))| (l, r))
.unwrap()
let lint_level_priority = level_priority(
self.name,
self.primary_group.default_level,
self.edition_lint_opts,
pkg_lints,
edition,
);

let group_level_priority = level_priority(
self.primary_group.name,
self.primary_group.default_level,
None,
pkg_lints,
edition,
);

let (_, (l, r, _)) = max_by_key(
(self.name, lint_level_priority),
(self.primary_group.name, group_level_priority),
|(n, (l, _, p))| (l == &LintLevel::Forbid, *p, Reverse(*n)),
);
(l, r)
}

fn emitted_source(&self, lint_level: LintLevel, reason: LintLevelReason) -> String {
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/lints/rules/blanket_hint_mostly_unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@ use crate::GlobalContext;
use crate::core::MaybePackage;
use crate::lints::Lint;
use crate::lints::LintLevel;
use crate::lints::SUSPICIOUS;
use crate::lints::get_key_value_span;
use crate::lints::rel_cwd_manifest_path;

pub const LINT: Lint = Lint {
name: "blanket_hint_mostly_unused",
desc: "blanket_hint_mostly_unused lint",
groups: &[],
default_level: LintLevel::Warn,
primary_group: &SUSPICIOUS,
edition_lint_opts: None,
feature_gate: None,
docs: Some(
Expand Down
3 changes: 1 addition & 2 deletions src/cargo/lints/rules/im_a_teapot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ use crate::lints::rel_cwd_manifest_path;
pub const LINT: Lint = Lint {
name: "im_a_teapot",
desc: "`im_a_teapot` is specified",
groups: &[TEST_DUMMY_UNSTABLE],
default_level: LintLevel::Allow,
primary_group: &TEST_DUMMY_UNSTABLE,
edition_lint_opts: None,
feature_gate: Some(Feature::test_dummy_unstable()),
docs: None,
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/lints/rules/implicit_minimum_version_req.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@ use crate::lints::Lint;
use crate::lints::LintLevel;
use crate::lints::LintLevelReason;
use crate::lints::ManifestFor;
use crate::lints::PEDANTIC;
use crate::lints::get_key_value;
use crate::lints::rel_cwd_manifest_path;
use crate::util::OptVersionReq;

pub const LINT: Lint = Lint {
name: "implicit_minimum_version_req",
desc: "dependency version requirement without an explicit minimum version",
groups: &[],
default_level: LintLevel::Allow,
primary_group: &PEDANTIC,
edition_lint_opts: None,
feature_gate: None,
docs: Some(
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/lints/rules/unknown_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ use crate::lints::LINTS;
use crate::lints::Lint;
use crate::lints::LintLevel;
use crate::lints::ManifestFor;
use crate::lints::SUSPICIOUS;
use crate::lints::get_key_value_span;

pub const LINT: Lint = Lint {
name: "unknown_lints",
desc: "unknown lint",
groups: &[],
default_level: LintLevel::Warn,
primary_group: &SUSPICIOUS,
edition_lint_opts: None,
feature_gate: None,
docs: Some(
Expand Down
26 changes: 23 additions & 3 deletions src/doc/src/reference/lints.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,20 @@

Note: [Cargo's linting system is unstable](unstable.md#lintscargo) and can only be used on nightly toolchains



| Group | Description | Default level |
|----------------------|------------------------------------------------------------------|---------------|
| `cargo::complexity` | code that does something simple but in a complex way | warn |
| `cargo::correctness` | code that is outright wrong or useless | deny |
| `cargo::nursery` | new lints that are still under development | allow |
| `cargo::pedantic` | lints which are rather strict or have occasional false positives | allow |
| `cargo::perf` | code that can be written to run faster | warn |
| `cargo::restriction` | lints which prevent the use of Cargo features | allow |
| `cargo::style` | code that should be written in a more idiomatic way | warn |
| `cargo::suspicious` | code that is most likely wrong or useless | warn |


## Allowed-by-default

These lints are all set to the 'allow' level by default.
Expand All @@ -14,7 +28,9 @@ These lints are all set to the 'warn' level by default.
- [`unknown_lints`](#unknown_lints)

## `blanket_hint_mostly_unused`
Set to `warn` by default
Group: `suspicious`

Level: `warn`

### What it does
Checks if `hint-mostly-unused` being applied to all dependencies.
Expand Down Expand Up @@ -42,7 +58,9 @@ hint-mostly-unused = true


## `implicit_minimum_version_req`
Set to `allow` by default
Group: `pedantic`

Level: `allow`

### What it does

Expand Down Expand Up @@ -91,7 +109,9 @@ serde = "1.0.219"


## `unknown_lints`
Set to `warn` by default
Group: `suspicious`

Level: `warn`

### What it does
Checks for unknown lints in the `[lints.cargo]` table
Expand Down