diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 7c10e91a29f..805c38983e2 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -26,7 +26,9 @@ use crate::lints::rules::blanket_hint_mostly_unused; use crate::lints::rules::check_im_a_teapot; use crate::lints::rules::implicit_minimum_version_req; use crate::lints::rules::non_kebab_case_bins; +use crate::lints::rules::non_kebab_case_features; use crate::lints::rules::non_kebab_case_packages; +use crate::lints::rules::non_snake_case_features; use crate::lints::rules::non_snake_case_packages; use crate::lints::rules::redundant_readme; use crate::ops; @@ -1380,6 +1382,20 @@ impl<'gctx> Workspace<'gctx> { &mut run_error_count, self.gctx, )?; + non_kebab_case_features( + pkg.into(), + &path, + &cargo_lints, + &mut run_error_count, + self.gctx, + )?; + non_snake_case_features( + pkg.into(), + &path, + &cargo_lints, + &mut run_error_count, + self.gctx, + )?; redundant_readme( pkg.into(), &path, diff --git a/src/cargo/lints/rules/mod.rs b/src/cargo/lints/rules/mod.rs index 93b41d60690..29527686b3e 100644 --- a/src/cargo/lints/rules/mod.rs +++ b/src/cargo/lints/rules/mod.rs @@ -2,7 +2,9 @@ mod blanket_hint_mostly_unused; mod im_a_teapot; mod implicit_minimum_version_req; mod non_kebab_case_bins; +mod non_kebab_case_features; mod non_kebab_case_packages; +mod non_snake_case_features; mod non_snake_case_packages; mod redundant_readme; mod unknown_lints; @@ -11,7 +13,9 @@ pub use blanket_hint_mostly_unused::blanket_hint_mostly_unused; pub use im_a_teapot::check_im_a_teapot; pub use implicit_minimum_version_req::implicit_minimum_version_req; pub use non_kebab_case_bins::non_kebab_case_bins; +pub use non_kebab_case_features::non_kebab_case_features; pub use non_kebab_case_packages::non_kebab_case_packages; +pub use non_snake_case_features::non_snake_case_features; pub use non_snake_case_packages::non_snake_case_packages; pub use redundant_readme::redundant_readme; pub use unknown_lints::output_unknown_lints; @@ -21,7 +25,9 @@ pub const LINTS: &[crate::lints::Lint] = &[ implicit_minimum_version_req::LINT, im_a_teapot::LINT, non_kebab_case_bins::LINT, + non_kebab_case_features::LINT, non_kebab_case_packages::LINT, + non_snake_case_features::LINT, non_snake_case_packages::LINT, redundant_readme::LINT, unknown_lints::LINT, diff --git a/src/cargo/lints/rules/non_kebab_case_features.rs b/src/cargo/lints/rules/non_kebab_case_features.rs new file mode 100644 index 00000000000..428592d6669 --- /dev/null +++ b/src/cargo/lints/rules/non_kebab_case_features.rs @@ -0,0 +1,154 @@ +use std::path::Path; + +use annotate_snippets::AnnotationKind; +use annotate_snippets::Group; +use annotate_snippets::Level; +use annotate_snippets::Origin; +use annotate_snippets::Patch; +use annotate_snippets::Snippet; +use cargo_util_schemas::manifest::TomlToolLints; + +use crate::CargoResult; +use crate::GlobalContext; +use crate::core::Package; +use crate::lints::Lint; +use crate::lints::LintLevel; +use crate::lints::LintLevelReason; +use crate::lints::RESTRICTION; +use crate::lints::get_key_value_span; +use crate::lints::rel_cwd_manifest_path; + +pub const LINT: Lint = Lint { + name: "non_kebab_case_features", + desc: "features should have a kebab-case name", + primary_group: &RESTRICTION, + edition_lint_opts: None, + feature_gate: None, + docs: Some( + r#" +### What it does + +Detect feature names that are not kebab-case. + +### Why it is bad + +Having multiple naming styles within a workspace can be confusing. + +### Drawbacks + +Users would expect that a feature tightly coupled to a dependency would match the dependency's name. + +### Example + +```toml +[features] +foo_bar = [] +``` + +Should be written as: + +```toml +[features] +foo-bar = [] +``` +"#, + ), +}; + +pub fn non_kebab_case_features( + pkg: &Package, + manifest_path: &Path, + cargo_lints: &TomlToolLints, + error_count: &mut usize, + gctx: &GlobalContext, +) -> CargoResult<()> { + let (lint_level, reason) = LINT.level( + cargo_lints, + pkg.manifest().edition(), + pkg.manifest().unstable_features(), + ); + + if lint_level == LintLevel::Allow { + return Ok(()); + } + + let manifest_path = rel_cwd_manifest_path(manifest_path, gctx); + + lint_package(pkg, &manifest_path, lint_level, reason, error_count, gctx) +} + +pub fn lint_package( + pkg: &Package, + manifest_path: &str, + lint_level: LintLevel, + reason: LintLevelReason, + error_count: &mut usize, + gctx: &GlobalContext, +) -> CargoResult<()> { + for original_name in pkg.summary().features().keys() { + let original_name = &**original_name; + let kebab_case = heck::ToKebabCase::to_kebab_case(original_name); + if kebab_case == original_name { + continue; + } + + let manifest = pkg.manifest(); + let document = manifest.document(); + let contents = manifest.contents(); + let level = lint_level.to_diagnostic_level(); + let emitted_source = LINT.emitted_source(lint_level, reason); + + let mut primary = Group::with_title(level.primary_title(LINT.desc)); + if let Some(document) = document + && let Some(contents) = contents + && let Some(span) = get_key_value_span(document, &["features", original_name]) + { + primary = primary.element( + Snippet::source(contents) + .path(manifest_path) + .annotation(AnnotationKind::Primary.span(span.key)), + ); + } else if let Some(document) = document + && let Some(contents) = contents + && let Some(dep_span) = get_key_value_span(document, &["dependencies", original_name]) + && let Some(optional_span) = + get_key_value_span(document, &["dependencies", original_name, "optional"]) + { + primary = primary.element( + Snippet::source(contents) + .path(manifest_path) + .annotation(AnnotationKind::Primary.span(dep_span.key).label("source of feature name")) + .annotation( + AnnotationKind::Context + .span(optional_span.key.start..optional_span.value.end) + .label("cause of feature"), + ), + ).element(Level::NOTE.message("see also ")); + } else { + primary = primary.element(Origin::path(manifest_path)); + } + primary = primary.element(Level::NOTE.message(emitted_source)); + let mut report = vec![primary]; + if let Some(document) = document + && let Some(contents) = contents + && let Some(span) = get_key_value_span(document, &["features", original_name]) + { + let mut help = Group::with_title(Level::HELP.secondary_title( + "to change the feature name to kebab case, convert the `features` key", + )); + help = help.element( + Snippet::source(contents) + .path(manifest_path) + .patch(Patch::new(span.key, kebab_case.as_str())), + ); + report.push(help); + } + + if lint_level.is_error() { + *error_count += 1; + } + gctx.shell().print_report(&report, lint_level.force())?; + } + + Ok(()) +} diff --git a/src/cargo/lints/rules/non_snake_case_features.rs b/src/cargo/lints/rules/non_snake_case_features.rs new file mode 100644 index 00000000000..983141fd470 --- /dev/null +++ b/src/cargo/lints/rules/non_snake_case_features.rs @@ -0,0 +1,154 @@ +use std::path::Path; + +use annotate_snippets::AnnotationKind; +use annotate_snippets::Group; +use annotate_snippets::Level; +use annotate_snippets::Origin; +use annotate_snippets::Patch; +use annotate_snippets::Snippet; +use cargo_util_schemas::manifest::TomlToolLints; + +use crate::CargoResult; +use crate::GlobalContext; +use crate::core::Package; +use crate::lints::Lint; +use crate::lints::LintLevel; +use crate::lints::LintLevelReason; +use crate::lints::RESTRICTION; +use crate::lints::get_key_value_span; +use crate::lints::rel_cwd_manifest_path; + +pub const LINT: Lint = Lint { + name: "non_snake_case_features", + desc: "features should have a snake-case name", + primary_group: &RESTRICTION, + edition_lint_opts: None, + feature_gate: None, + docs: Some( + r#" +### What it does + +Detect feature names that are not snake-case. + +### Why it is bad + +Having multiple naming styles within a workspace can be confusing. + +### Drawbacks + +Users would expect that a feature tightly coupled to a dependency would match the dependency's name. + +### Example + +```toml +[features] +foo-bar = [] +``` + +Should be written as: + +```toml +[features] +foo_bar = [] +``` +"#, + ), +}; + +pub fn non_snake_case_features( + pkg: &Package, + manifest_path: &Path, + cargo_lints: &TomlToolLints, + error_count: &mut usize, + gctx: &GlobalContext, +) -> CargoResult<()> { + let (lint_level, reason) = LINT.level( + cargo_lints, + pkg.manifest().edition(), + pkg.manifest().unstable_features(), + ); + + if lint_level == LintLevel::Allow { + return Ok(()); + } + + let manifest_path = rel_cwd_manifest_path(manifest_path, gctx); + + lint_package(pkg, &manifest_path, lint_level, reason, error_count, gctx) +} + +pub fn lint_package( + pkg: &Package, + manifest_path: &str, + lint_level: LintLevel, + reason: LintLevelReason, + error_count: &mut usize, + gctx: &GlobalContext, +) -> CargoResult<()> { + for original_name in pkg.summary().features().keys() { + let original_name = &**original_name; + let snake_case = heck::ToSnakeCase::to_snake_case(original_name); + if snake_case == original_name { + continue; + } + + let manifest = pkg.manifest(); + let document = manifest.document(); + let contents = manifest.contents(); + let level = lint_level.to_diagnostic_level(); + let emitted_source = LINT.emitted_source(lint_level, reason); + + let mut primary = Group::with_title(level.primary_title(LINT.desc)); + if let Some(document) = document + && let Some(contents) = contents + && let Some(span) = get_key_value_span(document, &["features", original_name]) + { + primary = primary.element( + Snippet::source(contents) + .path(manifest_path) + .annotation(AnnotationKind::Primary.span(span.key)), + ); + } else if let Some(document) = document + && let Some(contents) = contents + && let Some(dep_span) = get_key_value_span(document, &["dependencies", original_name]) + && let Some(optional_span) = + get_key_value_span(document, &["dependencies", original_name, "optional"]) + { + primary = primary.element( + Snippet::source(contents) + .path(manifest_path) + .annotation(AnnotationKind::Primary.span(dep_span.key).label("source of feature name")) + .annotation( + AnnotationKind::Context + .span(optional_span.key.start..optional_span.value.end) + .label("cause of feature"), + ), + ).element(Level::NOTE.message("see also ")); + } else { + primary = primary.element(Origin::path(manifest_path)); + } + primary = primary.element(Level::NOTE.message(emitted_source)); + let mut report = vec![primary]; + if let Some(document) = document + && let Some(contents) = contents + && let Some(span) = get_key_value_span(document, &["features", original_name]) + { + let mut help = Group::with_title(Level::HELP.secondary_title( + "to change the feature name to snake case, convert the `features` key", + )); + help = help.element( + Snippet::source(contents) + .path(manifest_path) + .patch(Patch::new(span.key, snake_case.as_str())), + ); + report.push(help); + } + + if lint_level.is_error() { + *error_count += 1; + } + gctx.shell().print_report(&report, lint_level.force())?; + } + + Ok(()) +} diff --git a/src/doc/src/reference/lints.md b/src/doc/src/reference/lints.md index 0698d4a6156..95668f05f44 100644 --- a/src/doc/src/reference/lints.md +++ b/src/doc/src/reference/lints.md @@ -20,7 +20,9 @@ Note: [Cargo's linting system is unstable](unstable.md#lintscargo) and can only These lints are all set to the 'allow' level by default. - [`implicit_minimum_version_req`](#implicit_minimum_version_req) +- [`non_kebab_case_features`](#non_kebab_case_features) - [`non_kebab_case_packages`](#non_kebab_case_packages) +- [`non_snake_case_features`](#non_snake_case_features) - [`non_snake_case_packages`](#non_snake_case_packages) ## Warn-by-default @@ -148,6 +150,38 @@ name = "foo-bar" ``` +## `non_kebab_case_features` +Group: `restriction` + +Level: `allow` + +### What it does + +Detect feature names that are not kebab-case. + +### Why it is bad + +Having multiple naming styles within a workspace can be confusing. + +### Drawbacks + +Users would expect that a feature tightly coupled to a dependency would match the dependency's name. + +### Example + +```toml +[features] +foo_bar = [] +``` + +Should be written as: + +```toml +[features] +foo-bar = [] +``` + + ## `non_kebab_case_packages` Group: `restriction` @@ -180,6 +214,38 @@ name = "foo-bar" ``` +## `non_snake_case_features` +Group: `restriction` + +Level: `allow` + +### What it does + +Detect feature names that are not snake-case. + +### Why it is bad + +Having multiple naming styles within a workspace can be confusing. + +### Drawbacks + +Users would expect that a feature tightly coupled to a dependency would match the dependency's name. + +### Example + +```toml +[features] +foo-bar = [] +``` + +Should be written as: + +```toml +[features] +foo_bar = [] +``` + + ## `non_snake_case_packages` Group: `restriction` diff --git a/tests/testsuite/lints/mod.rs b/tests/testsuite/lints/mod.rs index 15123cc5ec6..b4474998b9c 100644 --- a/tests/testsuite/lints/mod.rs +++ b/tests/testsuite/lints/mod.rs @@ -8,7 +8,9 @@ mod error; mod implicit_minimum_version_req; mod inherited; mod non_kebab_case_bins; +mod non_kebab_case_features; mod non_kebab_case_packages; +mod non_snake_case_features; mod non_snake_case_packages; mod redundant_readme; mod unknown_lints; diff --git a/tests/testsuite/lints/non_kebab_case_features.rs b/tests/testsuite/lints/non_kebab_case_features.rs new file mode 100644 index 00000000000..12a01f8f0de --- /dev/null +++ b/tests/testsuite/lints/non_kebab_case_features.rs @@ -0,0 +1,92 @@ +use crate::prelude::*; +use cargo_test_support::project; +use cargo_test_support::registry::Package; +use cargo_test_support::str; + +#[cargo_test] +fn feature_name_explicit() { + let foo = project() + .file( + "Cargo.toml", + r#" +[package] +name = "foo" +version = "0.0.1" +edition = "2015" +authors = [] + +[features] +foo_bar = [] + +[lints.cargo] +non_kebab_case_features = "warn" + "#, + ) + .file("src/lib.rs", "") + .build(); + + foo.cargo("check -Zcargo-lints") + .masquerade_as_nightly_cargo(&["cargo-lints", "test-dummy-unstable"]) + .with_stderr_data(str![[r#" +[WARNING] features should have a kebab-case name + --> Cargo.toml:9:1 + | +9 | foo_bar = [] + | ^^^^^^^ + | + = [NOTE] `cargo::non_kebab_case_features` is set to `warn` in `[lints]` +[HELP] to change the feature name to kebab case, convert the `features` key + | +9 - foo_bar = [] +9 + foo-bar = [] + | +[CHECKING] foo v0.0.1 ([ROOT]/foo) +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + +"#]]) + .run(); +} + +#[cargo_test] +fn feature_name_implicit() { + Package::new("foo_bar", "0.0.1").publish(); + + let foo = project() + .file( + "Cargo.toml", + r#" +[package] +name = "foo" +version = "0.0.1" +edition = "2015" +authors = [] + +[dependencies] +foo_bar = { version = "0.0.1", optional = true } + +[lints.cargo] +non_kebab_case_features = "warn" + "#, + ) + .file("src/lib.rs", "") + .build(); + + foo.cargo("check -Zcargo-lints") + .masquerade_as_nightly_cargo(&["cargo-lints", "test-dummy-unstable"]) + .with_stderr_data(str![[r#" +[WARNING] features should have a kebab-case name + --> Cargo.toml:9:1 + | +9 | foo_bar = { version = "0.0.1", optional = true } + | ^^^^^^^ source of feature name --------------- cause of feature + | + = [NOTE] see also + = [NOTE] `cargo::non_kebab_case_features` is set to `warn` in `[lints]` +[UPDATING] `dummy-registry` index +[LOCKING] 1 package to latest compatible version +[CHECKING] foo v0.0.1 ([ROOT]/foo) +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + +"#]]) + .run(); +} diff --git a/tests/testsuite/lints/non_snake_case_features.rs b/tests/testsuite/lints/non_snake_case_features.rs new file mode 100644 index 00000000000..b230af1de73 --- /dev/null +++ b/tests/testsuite/lints/non_snake_case_features.rs @@ -0,0 +1,92 @@ +use crate::prelude::*; +use cargo_test_support::project; +use cargo_test_support::registry::Package; +use cargo_test_support::str; + +#[cargo_test] +fn feature_name_explicit() { + let foo = project() + .file( + "Cargo.toml", + r#" +[package] +name = "foo" +version = "0.0.1" +edition = "2015" +authors = [] + +[features] +foo-bar = [] + +[lints.cargo] +non_snake_case_features = "warn" + "#, + ) + .file("src/lib.rs", "") + .build(); + + foo.cargo("check -Zcargo-lints") + .masquerade_as_nightly_cargo(&["cargo-lints", "test-dummy-unstable"]) + .with_stderr_data(str![[r#" +[WARNING] features should have a snake-case name + --> Cargo.toml:9:1 + | +9 | foo-bar = [] + | ^^^^^^^ + | + = [NOTE] `cargo::non_snake_case_features` is set to `warn` in `[lints]` +[HELP] to change the feature name to snake case, convert the `features` key + | +9 - foo-bar = [] +9 + foo_bar = [] + | +[CHECKING] foo v0.0.1 ([ROOT]/foo) +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + +"#]]) + .run(); +} + +#[cargo_test] +fn feature_name_implicit() { + Package::new("foo-bar", "0.0.1").publish(); + + let foo = project() + .file( + "Cargo.toml", + r#" +[package] +name = "foo" +version = "0.0.1" +edition = "2015" +authors = [] + +[dependencies] +foo-bar = { version = "0.0.1", optional = true } + +[lints.cargo] +non_snake_case_features = "warn" + "#, + ) + .file("src/lib.rs", "") + .build(); + + foo.cargo("check -Zcargo-lints") + .masquerade_as_nightly_cargo(&["cargo-lints", "test-dummy-unstable"]) + .with_stderr_data(str![[r#" +[WARNING] features should have a snake-case name + --> Cargo.toml:9:1 + | +9 | foo-bar = { version = "0.0.1", optional = true } + | ^^^^^^^ source of feature name --------------- cause of feature + | + = [NOTE] see also + = [NOTE] `cargo::non_snake_case_features` is set to `warn` in `[lints]` +[UPDATING] `dummy-registry` index +[LOCKING] 1 package to latest compatible version +[CHECKING] foo v0.0.1 ([ROOT]/foo) +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + +"#]]) + .run(); +}