[x] add a couple of guppy linters

Introduce the idea of "package linters" -- linters that run once for every
crate in the workspace. Implement a couple of linters:
* a project linter which ensures that there are no banned direct dependencies
* a package linter which ensures that license and authors are set correctly for every crate.

Also implement the logic for running project and package linters.

Closes: diem#3364
Approved by: bmwill

devtools/x-lint/Cargo.toml

@@ -10,4 +10,6 @@ license = "Apache-2.0"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
+guppy = "0.1.8"
 once_cell = "1.3.1"
+x-core = { version = "0.1.0", path = "../x-core" }

devtools/x-lint/src/lib.rs

@@ -8,6 +8,7 @@
 
 pub mod content;
 pub mod file;
+pub mod package;
 pub mod project;
 mod runner;
 
@@ -140,6 +141,10 @@ impl<'l> LintSource<'l> {
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
 pub enum LintKind<'l> {
     Project,
+    Package {
+        name: &'l str,
+        workspace_path: &'l Path,
+    },
     File(&'l Path),
     Content(&'l Path),
 }
@@ -148,6 +153,10 @@ impl<'l> fmt::Display for LintKind<'l> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
             LintKind::Project => write!(f, "project"),
+            LintKind::Package {
+                name,
+                workspace_path,
+            } => write!(f, "package '{}' (at {})", name, workspace_path.display()),
             LintKind::File(path) => write!(f, "file {}", path.display()),
             LintKind::Content(path) => write!(f, "content {}", path.display()),
         }
@@ -165,6 +174,7 @@ pub enum SystemError {
         cmd: &'static str,
         status: ExitStatus,
     },
+    Guppy(guppy::Error),
     Io(io::Error),
 }
 
@@ -176,6 +186,7 @@ impl fmt::Display for SystemError {
                 None => write!(f, "'{}' terminated by signal", cmd),
             },
             SystemError::Io(err) => write!(f, "IO error: {}", err),
+            SystemError::Guppy(err) => write!(f, "guppy error: {}", err),
         }
     }
 }
@@ -185,10 +196,17 @@ impl error::Error for SystemError {
         match self {
             SystemError::Exec { .. } => None,
             SystemError::Io(err) => Some(err),
+            SystemError::Guppy(err) => Some(err),
         }
     }
 }
 
+impl From<guppy::Error> for SystemError {
+    fn from(err: guppy::Error) -> Self {
+        SystemError::Guppy(err)
+    }
+}
+
 impl From<io::Error> for SystemError {
     fn from(err: io::Error) -> Self {
         SystemError::Io(err)
@@ -199,6 +217,7 @@ pub mod prelude {
     pub use super::{
         content::{ContentContext, ContentLinter},
         file::FileContext,
+        package::{PackageContext, PackageLinter},
         project::{ProjectContext, ProjectLinter},
         LintFormatter, LintKind, LintLevel, LintMessage, LintSource, Linter, Result, RunStatus,
         SkipReason, SystemError,

devtools/x-lint/src/package.rs

@@ -0,0 +1,60 @@
+// Copyright (c) The Libra Core Contributors
+// SPDX-License-Identifier: Apache-2.0
+
+use crate::{prelude::*, LintContext};
+use guppy::{
+    graph::{PackageGraph, PackageMetadata},
+    PackageId,
+};
+use std::path::Path;
+
+/// Represents a linter that runs once per package.
+pub trait PackageLinter: Linter {
+    fn run<'l>(
+        &self,
+        ctx: &PackageContext<'l>,
+        out: &mut LintFormatter<'l, '_>,
+    ) -> Result<RunStatus<'l>>;
+}
+
+/// Lint context for an individual package.
+#[derive(Copy, Clone, Debug)]
+pub struct PackageContext<'l> {
+    project_ctx: ProjectContext<'l>,
+    workspace_path: &'l Path,
+    metadata: &'l PackageMetadata,
+}
+
+impl<'l> PackageContext<'l> {
+    pub fn new(
+        project_ctx: ProjectContext<'l>,
+        package_graph: &'l PackageGraph,
+        workspace_path: &'l Path,
+        id: &PackageId,
+    ) -> Self {
+        Self {
+            project_ctx,
+            workspace_path,
+            metadata: package_graph.metadata(id).expect("package id is valid"),
+        }
+    }
+
+    /// Returns the relative path for this package in the workspace.
+    pub fn workspace_path(&self) -> &'l Path {
+        self.workspace_path
+    }
+
+    /// Returns the metadata for this package.
+    pub fn metadata(&self) -> &'l PackageMetadata {
+        self.metadata
+    }
+}
+
+impl<'l> LintContext<'l> for PackageContext<'l> {
+    fn kind(&self) -> LintKind<'l> {
+        LintKind::Package {
+            name: self.metadata.name(),
+            workspace_path: self.workspace_path,
+        }
+    }
+}

devtools/x-lint/src/project.rs

@@ -1,14 +1,18 @@
 // Copyright (c) The Libra Core Contributors
 // SPDX-License-Identifier: Apache-2.0
 
-use crate::prelude::*;
-use crate::LintContext;
+use crate::{prelude::*, LintContext};
+use guppy::graph::PackageGraph;
 use std::path::{Path, PathBuf};
+use x_core::XCoreContext;
 
 /// Represents a linter that checks some property for the overall project.
 ///
 /// Linters that implement `ProjectLinter` will run once for the whole project.
 pub trait ProjectLinter: Linter {
+    // Since ProjectContext is only 1 word long, clippy complains about passing it by reference. Do
+    // it that way for consistency reasons.
+    #[allow(clippy::trivially_copy_pass_by_ref)]
     /// Executes the lint against the given project context.
     fn run<'l>(
         &self,
@@ -20,23 +24,27 @@ pub trait ProjectLinter: Linter {
 /// Overall linter context for a project.
 #[derive(Copy, Clone, Debug)]
 pub struct ProjectContext<'l> {
-    project_root: &'l Path,
+    core: &'l XCoreContext,
 }
 
-#[allow(dead_code)]
 impl<'l> ProjectContext<'l> {
-    pub fn new(project_root: &'l Path) -> Self {
-        Self { project_root }
+    pub fn new(core: &'l XCoreContext) -> Self {
+        Self { core }
     }
 
     /// Returns the project root.
-    pub fn project_root(&self) -> &'l Path {
-        self.project_root
+    pub fn project_root(self) -> &'l Path {
+        self.core.project_root()
+    }
+
+    /// Returns the package graph, computing it for the first time if necessary.
+    pub fn package_graph(self) -> Result<&'l PackageGraph> {
+        Ok(self.core.package_graph()?)
     }
 
     /// Returns the absolute path from the project root.
-    pub fn full_path(&self, path: impl AsRef<Path>) -> PathBuf {
-        self.project_root.join(path.as_ref())
+    pub fn full_path(self, path: impl AsRef<Path>) -> PathBuf {
+        self.core.project_root().join(path.as_ref())
     }
 }
 

devtools/x-lint/src/runner.rs

@@ -8,24 +8,45 @@ use std::{
     path::Path,
     process::{Command, Stdio},
 };
+use x_core::XCoreContext;
 
 /// Configuration for the lint engine.
 #[derive(Clone, Debug)]
 pub struct LintEngineConfig<'cfg> {
-    project_root: &'cfg Path,
+    core: &'cfg XCoreContext,
+    project_linters: &'cfg [&'cfg dyn ProjectLinter],
+    package_linters: &'cfg [&'cfg dyn PackageLinter],
     content_linters: &'cfg [&'cfg dyn ContentLinter],
     fail_fast: bool,
 }
 
 impl<'cfg> LintEngineConfig<'cfg> {
-    pub fn new(project_root: &'cfg Path) -> Self {
+    pub fn new(core: &'cfg XCoreContext) -> Self {
         Self {
-            project_root,
+            core,
+            project_linters: &[],
+            package_linters: &[],
             content_linters: &[],
             fail_fast: false,
         }
     }
 
+    pub fn with_project_linters(
+        &mut self,
+        project_linters: &'cfg [&'cfg dyn ProjectLinter],
+    ) -> &mut Self {
+        self.project_linters = project_linters;
+        self
+    }
+
+    pub fn with_package_linters(
+        &mut self,
+        package_linters: &'cfg [&'cfg dyn PackageLinter],
+    ) -> &mut Self {
+        self.package_linters = package_linters;
+        self
+    }
+
     pub fn with_content_linters(
         &mut self,
         content_linters: &'cfg [&'cfg dyn ContentLinter],
@@ -65,10 +86,59 @@ impl<'cfg> LintEngine<'cfg> {
         let mut skipped = vec![];
         let mut messages = vec![];
 
-        // TODO: add support for project and file lints.
+        // TODO: add support for file linters.
+
+        let project_ctx = ProjectContext::new(self.config.core);
+
+        // Run project linters.
+        if !self.config.project_linters.is_empty() {
+            for linter in self.config.project_linters {
+                let source = project_ctx.source(linter.name());
+                let mut formatter = LintFormatter::new(source, &mut messages);
+                match linter.run(&project_ctx, &mut formatter)? {
+                    RunStatus::Executed => {
+                        // Lint ran successfully.
+                    }
+                    RunStatus::Skipped(reason) => {
+                        skipped.push((source, reason));
+                    }
+                }
+
+                if self.config.fail_fast && !messages.is_empty() {
+                    // At least one issue was found.
+                    return Ok(LintResults { skipped, messages });
+                }
+            }
+        }
+
+        // Run package linters.
+        if !self.config.package_linters.is_empty() {
+            let package_graph = project_ctx.package_graph()?;
 
-        let project_ctx = ProjectContext::new(self.config.project_root);
+            for (workspace_path, id) in package_graph.workspace().members() {
+                let package_ctx =
+                    PackageContext::new(project_ctx, package_graph, workspace_path, id);
+                for linter in self.config.package_linters {
+                    let source = package_ctx.source(linter.name());
+                    let mut formatter = LintFormatter::new(source, &mut messages);
+                    match linter.run(&package_ctx, &mut formatter)? {
+                        RunStatus::Executed => {
+                            // Lint ran successfully.
+                        }
+                        RunStatus::Skipped(reason) => {
+                            skipped.push((source, reason));
+                        }
+                    }
+
+                    if self.config.fail_fast && !messages.is_empty() {
+                        // At least one issue was found.
+                        return Ok(LintResults { skipped, messages });
+                    }
+                }
+            }
+        }
 
+        // Run content linters.
         if !self.config.content_linters.is_empty() {
             let file_list = self.get_file_list()?;
 
@@ -78,7 +148,7 @@ impl<'cfg> LintEngine<'cfg> {
                 .iter()
                 .map(|path| FileContext::new(project_ctx, path));
 
-            'outer: for file_ctx in file_ctxs {
+            for file_ctx in file_ctxs {
                 let linters_to_run = self
                     .config
                     .content_linters
@@ -118,7 +188,7 @@ impl<'cfg> LintEngine<'cfg> {
 
                     if self.config.fail_fast && !messages.is_empty() {
                         // At least one issue was found.
-                        break 'outer;
+                        return Ok(LintResults { skipped, messages });
                     }
                 }
             }
@@ -171,7 +241,7 @@ impl<'cfg> LintEngine<'cfg> {
             .get_or_try_init(|| {
                 // TODO: abstract out SCM and command-running functionality.
                 let output = Command::new("git")
-                    .current_dir(self.config.project_root)
+                    .current_dir(self.config.core.project_root())
                     // The -z causes files to not be quoted, and to be separated by \0.
                     .args(&["ls-files", "-z"])
                     .stderr(Stdio::inherit())

devtools/x/src/config.rs

@@ -14,6 +14,8 @@ use std::{
 pub struct Config {
     /// Package exceptions which need to be run special
     package_exceptions: HashMap<String, Package>,
+    /// Workspace configuration
+    workspace: WorkspaceConfig,
     /// Clippy configureation
     clippy: Clippy,
 }
@@ -33,6 +35,24 @@ fn default_as_true() -> bool {
     true
 }
 
+#[derive(Clone, Serialize, Deserialize, Debug, PartialEq, Eq)]
+#[serde(rename_all = "kebab-case")]
+pub struct WorkspaceConfig {
+    /// Attributes to enforce on workspace crates
+    pub enforced_attributes: EnforcedAttributesConfig,
+    /// Banned direct dependencies
+    pub banned_direct_deps: HashMap<String, String>,
+}
+
+#[derive(Clone, Serialize, Deserialize, Debug, PartialEq, Eq)]
+#[serde(rename_all = "kebab-case")]
+pub struct EnforcedAttributesConfig {
+    /// Ensure the authors of every workspace crate are set to this.
+    pub authors: Option<Vec<String>>,
+    /// Ensure the `license` field of every workspace crate is set to this.
+    pub license: Option<String>,
+}
+
 #[derive(Clone, Serialize, Deserialize, Debug, PartialEq, Eq)]
 #[serde(rename_all = "kebab-case")]
 pub struct Clippy {
@@ -61,6 +81,10 @@ impl Config {
         &self.package_exceptions
     }
 
+    pub fn workspace_config(&self) -> &WorkspaceConfig {
+        &self.workspace
+    }
+
     pub fn allowed_clippy_lints(&self) -> &[String] {
         &self.clippy.allowed
     }

devtools/x/src/context.rs

@@ -1,8 +1,7 @@
 // Copyright (c) The Libra Core Contributors
 // SPDX-License-Identifier: Apache-2.0
 
-use crate::utils::project_root;
-use crate::{config::Config, Result};
+use crate::{config::Config, utils::project_root, Result};
 use x_core::XCoreContext;
 
 /// Global context shared across x commands.