Merge #3419

3419: Linter to check that all common metadata is included r=flip1995 a=ysimonson

Addresses https://github.com/rust-lang-nursery/rust-clippy/issues/1793

This is not ready to be merged yet. First the associated changes to `cargo_metadata` need to be merged: https://github.com/oli-obk/cargo_metadata/pull/55

Co-authored-by: Yusuf Simonson <simonson@gmail.com>

CHANGELOG.md

@@ -628,6 +628,7 @@ All notable changes to this project will be documented in this file.
 [`box_vec`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#box_vec
 [`boxed_local`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#boxed_local
 [`builtin_type_shadow`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#builtin_type_shadow
+[`cargo_common_metadata`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#cargo_common_metadata
 [`cast_lossless`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#cast_lossless
 [`cast_possible_truncation`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#cast_possible_truncation
 [`cast_possible_wrap`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#cast_possible_wrap

Cargo.toml

@@ -47,7 +47,7 @@ rustc_tools_util = { version = "0.1.0", path = "rustc_tools_util"}
 
 [dev-dependencies]
 clippy_dev = { version = "0.0.1", path = "clippy_dev" }
-cargo_metadata = "0.6"
+cargo_metadata = "0.6.2"
 compiletest_rs = "0.3.16"
 lazy_static = "1.0"
 serde_derive = "1.0"

README.md

@@ -9,7 +9,7 @@ We are currently in the process of discussing Clippy 1.0 via the RFC process in
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are 287 lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html)
+[There are 288 lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html)
 
 We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
 

clippy_lints/Cargo.toml

@@ -17,7 +17,7 @@ keywords = ["clippy", "lint", "plugin"]
 edition = "2018"
 
 [dependencies]
-cargo_metadata = "0.6"
+cargo_metadata = "0.6.2"
 itertools = "0.7"
 lazy_static = "1.0.2"
 matches = "0.1.7"

clippy_lints/src/cargo_common_metadata.rs

@@ -0,0 +1,115 @@
+// Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+//! lint on missing cargo common metadata
+
+use crate::rustc::lint::{EarlyContext, EarlyLintPass, LintArray, LintPass};
+use crate::rustc::{declare_tool_lint, lint_array};
+use crate::syntax::{ast::*, source_map::DUMMY_SP};
+use crate::utils::span_lint;
+
+use cargo_metadata;
+
+/// **What it does:** Checks to see if all common metadata is defined in
+/// `Cargo.toml`. See: https://rust-lang-nursery.github.io/api-guidelines/documentation.html#cargotoml-includes-all-common-metadata-c-metadata
+///
+/// **Why is this bad?** It will be more difficult for users to discover the
+/// purpose of the crate, and key information related to it.
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+/// ```toml
+/// # This `Cargo.toml` is missing an authors field:
+/// [package]
+/// name = "clippy"
+/// version = "0.0.212"
+/// description = "A bunch of helpful lints to avoid common pitfalls in Rust"
+/// repository = "https://github.com/rust-lang-nursery/rust-clippy"
+/// readme = "README.md"
+/// license = "MIT/Apache-2.0"
+/// keywords = ["clippy", "lint", "plugin"]
+/// categories = ["development-tools", "development-tools::cargo-plugins"]
+/// ```
+declare_clippy_lint! {
+    pub CARGO_COMMON_METADATA,
+    cargo,
+    "common metadata is defined in `Cargo.toml`"
+}
+
+fn warning(cx: &EarlyContext<'_>, message: &str) {
+    span_lint(cx, CARGO_COMMON_METADATA, DUMMY_SP, message);
+}
+
+fn missing_warning(cx: &EarlyContext<'_>, package: &cargo_metadata::Package, field: &str) {
+    let message = format!("package `{}` is missing `{}` metadata", package.name, field);
+    warning(cx, &message);
+}
+
+fn is_empty_str(value: &Option<String>) -> bool {
+    match value {
+        None => true,
+        Some(value) if value.is_empty() => true,
+        _ => false
+    }
+}
+
+fn is_empty_vec(value: &[String]) -> bool {
+    // This works because empty iterators return true
+    value.iter().all(|v| v.is_empty())
+}
+
+pub struct Pass;
+
+impl LintPass for Pass {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(CARGO_COMMON_METADATA)
+    }
+}
+
+impl EarlyLintPass for Pass {
+    fn check_crate(&mut self, cx: &EarlyContext<'_>, _: &Crate) {
+        let metadata = if let Ok(metadata) = cargo_metadata::metadata_deps(None, true) {
+            metadata
+        } else {
+            warning(cx, "could not read cargo metadata");
+            return;
+        };
+
+        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) {
+                missing_warning(cx, &package, "package.license");
+            }
+
+            if is_empty_str(&package.repository) {
+                missing_warning(cx, &package, "package.repository");
+            }
+
+            if is_empty_str(&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");
+            }
+        }
+    }
+}

clippy_lints/src/lib.rs

@@ -96,6 +96,7 @@ pub mod blacklisted_name;
 pub mod block_in_if_condition;
 pub mod booleans;
 pub mod bytecount;
+pub mod cargo_common_metadata;
 pub mod collapsible_if;
 pub mod const_static_lifetime;
 pub mod copies;
@@ -444,6 +445,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
     reg.register_late_lint_pass(box double_comparison::Pass);
     reg.register_late_lint_pass(box question_mark::Pass);
     reg.register_late_lint_pass(box suspicious_trait_impl::SuspiciousImpl);
+    reg.register_early_lint_pass(box cargo_common_metadata::Pass);
     reg.register_early_lint_pass(box multiple_crate_versions::Pass);
     reg.register_early_lint_pass(box wildcard_dependencies::Pass);
     reg.register_late_lint_pass(box map_unit_fn::Pass);
@@ -985,6 +987,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
     ]);
 
     reg.register_lint_group("clippy::cargo", Some("clippy_cargo"), vec![
+        cargo_common_metadata::CARGO_COMMON_METADATA,
         multiple_crate_versions::MULTIPLE_CRATE_VERSIONS,
         wildcard_dependencies::WILDCARD_DEPENDENCIES,
     ]);