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

Fix cargo_common_metadata warning on publish = false. #6650

Merged
merged 11 commits into from
Feb 11, 2021
75 changes: 46 additions & 29 deletions clippy_lints/src/cargo_common_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::path::PathBuf;
use crate::utils::{run_lints, span_lint};
use rustc_hir::{hir_id::CRATE_HIR_ID, Crate};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::source_map::DUMMY_SP;

declare_clippy_lint! {
Expand Down Expand Up @@ -51,6 +51,21 @@ declare_clippy_lint! {
"common metadata is defined in `Cargo.toml`"
}

#[derive(Copy, Clone, Debug)]
pub struct CargoCommonMetadata {
ignore_publish: bool,
}

impl CargoCommonMetadata {
pub fn new(ignore_publish: bool) -> Self {
Self { ignore_publish }
}
}

impl_lint_pass!(CargoCommonMetadata => [
CARGO_COMMON_METADATA
]);

fn missing_warning(cx: &LateContext<'_>, package: &cargo_metadata::Package, field: &str) {
let message = format!("package `{}` is missing `{}` metadata", package.name, field);
span_lint(cx, CARGO_COMMON_METADATA, DUMMY_SP, &message);
Expand All @@ -69,8 +84,6 @@ fn is_empty_vec(value: &[String]) -> bool {
value.iter().all(String::is_empty)
}

declare_lint_pass!(CargoCommonMetadata => [CARGO_COMMON_METADATA]);

impl LateLintPass<'_> for CargoCommonMetadata {
fn check_crate(&mut self, cx: &LateContext<'_>, _: &Crate<'_>) {
if !run_lints(cx, &[CARGO_COMMON_METADATA], CRATE_HIR_ID) {
Expand All @@ -80,32 +93,36 @@ impl LateLintPass<'_> for CargoCommonMetadata {
let metadata = unwrap_cargo_metadata!(cx, CARGO_COMMON_METADATA, false);

for package in metadata.packages {
if is_empty_vec(&package.authors) {
missing_warning(cx, &package, "package.authors");
}

if is_empty_str(&package.description) {
missing_warning(cx, &package, "package.description");
}

if is_empty_str(&package.license) && is_empty_path(&package.license_file) {
missing_warning(cx, &package, "either package.license or package.license_file");
}

if is_empty_str(&package.repository) {
missing_warning(cx, &package, "package.repository");
}

if is_empty_path(&package.readme) {
missing_warning(cx, &package, "package.readme");
}

if is_empty_vec(&package.keywords) {
missing_warning(cx, &package, "package.keywords");
}

if is_empty_vec(&package.categories) {
missing_warning(cx, &package, "package.categories");
// only run the lint if publish is `None` (`publish = true` or skipped entirely)
// or if the vector isn't empty (`publish = ["something"]`)
if package.publish.as_ref().filter(|publish| publish.is_empty()).is_none() || self.ignore_publish {
flip1995 marked this conversation as resolved.
Show resolved Hide resolved
if is_empty_vec(&package.authors) {
missing_warning(cx, &package, "package.authors");
}

if is_empty_str(&package.description) {
missing_warning(cx, &package, "package.description");
}

if is_empty_str(&package.license) && is_empty_path(&package.license_file) {
missing_warning(cx, &package, "either package.license or package.license_file");
}

if is_empty_str(&package.repository) {
missing_warning(cx, &package, "package.repository");
}

if is_empty_path(&package.readme) {
missing_warning(cx, &package, "package.readme");
}

if is_empty_vec(&package.keywords) {
missing_warning(cx, &package, "package.keywords");
}

if is_empty_vec(&package.categories) {
missing_warning(cx, &package, "package.categories");
}
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1180,7 +1180,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_early_pass(|| box redundant_else::RedundantElse);
store.register_late_pass(|| box create_dir::CreateDir);
store.register_early_pass(|| box needless_arbitrary_self_type::NeedlessArbitrarySelfType);
store.register_late_pass(|| box cargo_common_metadata::CargoCommonMetadata);
let cargo_ignore_publish = conf.cargo_ignore_publish;
store.register_late_pass(move || box cargo_common_metadata::CargoCommonMetadata::new(cargo_ignore_publish));
store.register_late_pass(|| box multiple_crate_versions::MultipleCrateVersions);
store.register_late_pass(|| box wildcard_dependencies::WildcardDependencies);
let literal_representation_lint_fraction_readability = conf.unreadable_literal_lint_fractions;
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,8 @@ define_Conf! {
(disallowed_methods, "disallowed_methods": Vec<String>, Vec::<String>::new()),
/// Lint: UNREADABLE_LITERAL. Should the fraction of a decimal be linted to include separators.
(unreadable_literal_lint_fractions, "unreadable_literal_lint_fractions": bool, true),
/// Lint: _CARGO_COMMON_METADATA. For internal testing only, ignores the current `publish` settings in the Cargo manifest.
(cargo_ignore_publish, "cargo_ignore_publish": bool, false),
}

impl Default for Conf {
Expand Down
3 changes: 3 additions & 0 deletions tests/compile-test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ fn run_ui_cargo(config: &mut compiletest::Config) {
Some("main.rs") => {},
_ => continue,
}
set_var("CLIPPY_CONF_DIR", case.path());
let paths = compiletest::common::TestPaths {
file: file_path,
base: config.src_base.clone(),
Expand Down Expand Up @@ -241,9 +242,11 @@ fn run_ui_cargo(config: &mut compiletest::Config) {
let tests = compiletest::make_tests(&config);

let current_dir = env::current_dir().unwrap();
let conf_dir = var("CLIPPY_CONF_DIR").unwrap_or_default();
let filter = env::var("TESTNAME").ok();
let res = run_tests(&config, &filter, tests);
env::set_current_dir(current_dir).unwrap();
set_var("CLIPPY_CONF_DIR", conf_dir);

match res {
Ok(true) => {},
Expand Down
1 change: 1 addition & 0 deletions tests/ui-cargo/cargo_common_metadata/fail/clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
cargo-ignore-publish = true
6 changes: 6 additions & 0 deletions tests/ui-cargo/cargo_common_metadata/fail_publish/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "cargo_common_metadata"
version = "0.1.0"
publish = ["some-registry-name"]

[workspace]
4 changes: 4 additions & 0 deletions tests/ui-cargo/cargo_common_metadata/fail_publish/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// compile-flags: --crate-name=cargo_common_metadata
#![warn(clippy::cargo_common_metadata)]

fn main() {}
18 changes: 18 additions & 0 deletions tests/ui-cargo/cargo_common_metadata/fail_publish/src/main.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
error: package `cargo_common_metadata` is missing `package.authors` metadata
|
= note: `-D clippy::cargo-common-metadata` implied by `-D warnings`

error: package `cargo_common_metadata` is missing `package.description` metadata

error: package `cargo_common_metadata` is missing `either package.license or package.license_file` metadata

error: package `cargo_common_metadata` is missing `package.repository` metadata

error: package `cargo_common_metadata` is missing `package.readme` metadata

error: package `cargo_common_metadata` is missing `package.keywords` metadata

error: package `cargo_common_metadata` is missing `package.categories` metadata

error: aborting due to 7 previous errors

Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "cargo_common_metadata"
version = "0.1.0"
publish = true

[workspace]
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// compile-flags: --crate-name=cargo_common_metadata
#![warn(clippy::cargo_common_metadata)]

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
error: package `cargo_common_metadata` is missing `package.authors` metadata
|
= note: `-D clippy::cargo-common-metadata` implied by `-D warnings`

error: package `cargo_common_metadata` is missing `package.description` metadata

error: package `cargo_common_metadata` is missing `either package.license or package.license_file` metadata

error: package `cargo_common_metadata` is missing `package.repository` metadata

error: package `cargo_common_metadata` is missing `package.readme` metadata

error: package `cargo_common_metadata` is missing `package.keywords` metadata

error: package `cargo_common_metadata` is missing `package.categories` metadata

error: aborting due to 7 previous errors

1 change: 1 addition & 0 deletions tests/ui-cargo/cargo_common_metadata/pass/clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
cargo-ignore-publish = true
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "cargo_common_metadata"
version = "0.1.0"
publish = []

[workspace]
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// compile-flags: --crate-name=cargo_common_metadata
#![warn(clippy::cargo_common_metadata)]

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "cargo_common_metadata"
version = "0.1.0"
publish = false

[workspace]
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// compile-flags: --crate-name=cargo_common_metadata
#![warn(clippy::cargo_common_metadata)]

fn main() {}
2 changes: 1 addition & 1 deletion tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `msrv`, `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `pass-by-value-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-trait-bounds`, `max-struct-bools`, `max-fn-params-bools`, `warn-on-all-wildcard-imports`, `disallowed-methods`, `unreadable-literal-lint-fractions`, `third-party` at line 5 column 1
error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `msrv`, `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `pass-by-value-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-trait-bounds`, `max-struct-bools`, `max-fn-params-bools`, `warn-on-all-wildcard-imports`, `disallowed-methods`, `unreadable-literal-lint-fractions`, `cargo-ignore-publish`, `third-party` at line 5 column 1
Copy link
Member

Choose a reason for hiding this comment

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

This is not optimal, because it makes this option visible to the user. Nothing we can do here though, jjust for future reference. (But the error message is also horrible, so it's not that bad)


error: aborting due to previous error