Implement feature- and config-based lints

Author: detrumi

src/cargo/core/compiler/mod.rs

@@ -809,9 +809,17 @@ fn build_base_args<'a, 'cfg>(
         cmd.args(args);
     }
 
-    unit.pkg.manifest().lints().set_flags(cmd);
-    if let Some(virtual_lints) = bcx.ws.virtual_lints() {
-        virtual_lints.set_flags(cmd);
+    let unit_cfg = bcx.cfg(unit.kind);
+    let features = bcx.resolve.features(unit.pkg.package_id());
+    if let Some(ref lints) = unit.pkg.manifest().lints() {
+        for ref lint_section in lints.iter() {
+            lint_section.set_lint_flags(unit_cfg, features, cmd);
+        }
+    }
+    if let Some(ref virtual_lints) = bcx.ws.virtual_lints() {
+        for ref lint_section in virtual_lints.iter() {
+            lint_section.set_lint_flags(unit_cfg, features, cmd);
+        }
     }
 
     // -C overflow-checks is implied by the setting of -C debug-assertions,

src/cargo/core/lints.rs

@@ -1,6 +1,9 @@
 use std::collections::BTreeMap;
+use std::collections::HashSet;
+use std::str::FromStr;
 
-use util::{CargoResult, ProcessBuilder};
+use util::{Cfg, CfgExpr, ProcessBuilder};
+use util::errors::CargoResult;
 
 #[derive(Clone, PartialEq, Debug)]
 enum LintKind {
@@ -34,30 +37,50 @@ impl LintKind {
 #[derive(Clone, Debug)]
 pub struct Lints {
     lints: Vec<(String, LintKind)>,
+    cfg: Option<CfgExpr>,
 }
 
 impl Lints {
     pub fn new(
-        manifest_lints: Option<&BTreeMap<String, String>>,
+        cfg: Option<&String>,
+        manifest_lints: &BTreeMap<String, String>,
         warnings: &mut Vec<String>,
     ) -> CargoResult<Lints> {
+        let cfg = if let Some(t) = cfg {
+            if t.starts_with("cfg(") && t.ends_with(')') {
+                Some(CfgExpr::from_str(&t[4..t.len() - 1])?)
+            } else {
+                bail!("expected `cfg(...)`, found {}", t)
+            }
+        } else {
+            None
+        };
+
         let mut lints = vec![];
-        if let Some(lint_section) = manifest_lints {
-            for (lint_name, lint_state) in lint_section.iter() {
-                if let Some(state) = LintKind::try_from_string(lint_state) {
-                    lints.push((lint_name.to_string(), state));
-                } else {
-                    warnings.push(format!(
-                        "invalid lint state for `{}` (expected `warn`, `allow`, `deny` or `forbid`)",
-                        lint_name
-                    ));
-                }
+        for (lint_name, lint_state) in manifest_lints.iter() {
+            if let Some(state) = LintKind::try_from_string(lint_state) {
+                lints.push((lint_name.to_string(), state));
+            } else {
+                warnings.push(format!(
+                    "invalid lint state for `{}` (expected `warn`, `allow`, `deny` or `forbid`)",
+                    lint_name
+                ));
             }
         }
-        Ok(Lints { lints })
+        Ok(Lints { lints, cfg })
+    }
+
+    pub fn set_lint_flags(&self, unit_cfg: &[Cfg], features: &HashSet<String>, cmd: &mut ProcessBuilder) {
+        match self.cfg {
+            None => self.set_flags(cmd),
+            Some(CfgExpr::Value(Cfg::KeyPair(ref key, ref value)))
+                if key == "feature" && features.contains(value) => self.set_flags(cmd),
+            Some(ref cfg) if cfg.matches(unit_cfg) => self.set_flags(cmd),
+            _ => (),
+        }
     }
 
-    pub fn set_flags(&self, cmd: &mut ProcessBuilder) {
+    fn set_flags(&self, cmd: &mut ProcessBuilder) {
         for (lint_name, state) in self.lints.iter() {
             cmd.arg(format!("-{}", state.flag())).arg(lint_name);
         }

src/cargo/core/manifest.rs

@@ -45,7 +45,7 @@ pub struct Manifest {
     original: Rc<TomlManifest>,
     features: Features,
     edition: Edition,
-    lints: Lints,
+    lints: Option<Vec<Lints>>,
     im_a_teapot: Option<bool>,
     default_run: Option<String>,
     metabuild: Option<Vec<String>>,
@@ -70,7 +70,7 @@ pub struct VirtualManifest {
     workspace: WorkspaceConfig,
     profiles: Profiles,
     warnings: Warnings,
-    lints: Lints,
+    lints: Option<Vec<Lints>>,
 }
 
 /// General metadata about a package which is just blindly uploaded to the
@@ -364,7 +364,7 @@ impl Manifest {
         workspace: WorkspaceConfig,
         features: Features,
         edition: Edition,
-        lints: Lints,
+        lints: Option<Vec<Lints>>,
         im_a_teapot: Option<bool>,
         default_run: Option<String>,
         original: Rc<TomlManifest>,
@@ -428,7 +428,7 @@ impl Manifest {
     pub fn warnings(&self) -> &Warnings {
         &self.warnings
     }
-    pub fn lints(&self) -> &Lints {
+    pub fn lints(&self) -> &Option<Vec<Lints>> {
         &self.lints
     }
     pub fn profiles(&self) -> &Profiles {
@@ -539,7 +539,7 @@ impl VirtualManifest {
         patch: HashMap<Url, Vec<Dependency>>,
         workspace: WorkspaceConfig,
         profiles: Profiles,
-        lints: Lints,
+        lints: Option<Vec<Lints>>,
     ) -> VirtualManifest {
         VirtualManifest {
             replace,
@@ -563,7 +563,7 @@ impl VirtualManifest {
         &self.workspace
     }
 
-    pub fn lints(&self) -> &Lints {
+    pub fn lints(&self) -> &Option<Vec<Lints>> {
         &self.lints
     }
 

src/cargo/core/workspace.rs

@@ -246,13 +246,13 @@ impl<'cfg> Workspace<'cfg> {
         }
     }
 
-    pub fn virtual_lints(&self) -> Option<&Lints> {
+    pub fn virtual_lints(&self) -> &Option<Vec<Lints>> {
         let root = self.root_manifest
             .as_ref()
             .unwrap_or(&self.current_manifest);
         match *self.packages.get(root) {
-            MaybePackage::Virtual(ref vm) => Some(vm.lints()),
-            _ => None,
+            MaybePackage::Virtual(ref vm) => vm.lints(),
+            _ => &None,
         }
     }
 

src/cargo/util/toml/mod.rs

@@ -242,6 +242,8 @@ pub struct TomlManifest {
     workspace: Option<TomlWorkspace>,
     badges: Option<BTreeMap<String, BTreeMap<String, String>>>,
     lints: Option<BTreeMap<String, String>>,
+    #[serde(rename = "lints2")]
+    feature_lints: Option<BTreeMap<String, BTreeMap<String, String>>>,
 }
 
 #[derive(Deserialize, Serialize, Clone, Debug, Default)]
@@ -751,6 +753,7 @@ impl TomlManifest {
             badges: self.badges.clone(),
             cargo_features: self.cargo_features.clone(),
             lints: self.lints.clone(),
+            feature_lints: self.feature_lints.clone(),
         });
 
         fn map_deps(
@@ -1000,7 +1003,7 @@ impl TomlManifest {
             ),
         };
         let profiles = Profiles::new(me.profile.as_ref(), config, &features, &mut warnings)?;
-        let lints = Lints::new(me.lints.as_ref(), &mut warnings)?;
+        let lints = lints(me.lints.as_ref(), me.feature_lints.as_ref(), &mut warnings)?;
         let publish = match project.publish {
             Some(VecStringOrBool::VecString(ref vecstring)) => {
                 features
@@ -1114,7 +1117,7 @@ impl TomlManifest {
             (me.replace(&mut cx)?, me.patch(&mut cx)?)
         };
         let profiles = Profiles::new(me.profile.as_ref(), config, &features, &mut warnings)?;
-        let lints = Lints::new(me.lints.as_ref(), &mut warnings)?;
+        let lints = lints(me.lints.as_ref(), me.feature_lints.as_ref(), &mut warnings)?;
         let workspace_config = match me.workspace {
             Some(ref config) => WorkspaceConfig::Root(WorkspaceRootConfig::new(
                 &root,
@@ -1495,3 +1498,24 @@ impl fmt::Debug for PathValue {
         self.0.fmt(f)
     }
 }
+
+fn lints(
+    toml_lints: Option<&BTreeMap<String, String>>,
+    toml_feature_lints: Option<&BTreeMap<String, BTreeMap<String, String>>>,
+    warnings: &mut Vec<String>
+) -> CargoResult<Option<Vec<Lints>>> {
+    let mut lints = vec![];
+    if let Some(toml_lints) = toml_lints {
+        lints.push(Lints::new(None, toml_lints, warnings)?);
+    }
+    if let Some(toml_feature_lints) = toml_feature_lints {
+        for (ref cfg, ref feature_lints) in toml_feature_lints.iter() {
+            lints.push(Lints::new(Some(cfg), feature_lints, warnings)?);
+        }
+    }
+    Ok(if !lints.is_empty() {
+        Some(lints)
+    } else {
+        None
+    })
+}

tests/testsuite/lints.rs

@@ -1,5 +1,4 @@
-use support::{execs, project};
-use support::hamcrest::assert_that;
+use support::project;
 
 #[test]
 fn deny() {
@@ -19,12 +18,10 @@ fn deny() {
         .file("src/lib.rs", "fn foo() {}")
         .build();
 
-    assert_that(
-        p.cargo("build"),
-        execs()
-            .with_status(101)
-            .with_stderr_contains("[..]error: function is never used: `foo`[..]"),
-    );
+    p.cargo("build")
+        .with_status(101)
+        .with_stderr_contains("[..]error: function is never used: `foo`[..]")
+        .run();
 }
 
 #[test]
@@ -44,10 +41,7 @@ fn empty_lints_block() {
         .file("src/lib.rs", "fn foo() {}")
         .build();
 
-    assert_that(
-        p.cargo("build"),
-        execs().with_status(0),
-    );
+    p.cargo("build").with_status(0).run();
 }
 
 #[test]
@@ -68,10 +62,7 @@ fn invalid_state() {
         .file("src/lib.rs", "fn foo() {}")
         .build();
 
-    assert_that(
-        p.cargo("build"),
-        execs().with_status(0),
-    );
+    p.cargo("build").with_status(0).run();
 }
 
 #[test]
@@ -99,12 +90,10 @@ fn virtual_workspace() {
         .file("bar/src/lib.rs", "fn baz() {}")
         .build();
 
-    assert_that(
-        p.cargo("build"),
-        execs()
-            .with_status(101)
-            .with_stderr_contains("[..]error: function is never used: `baz`[..]"),
-    );
+    p.cargo("build")
+        .with_status(101)
+        .with_stderr_contains("[..]error: function is never used: `baz`[..]")
+        .run();
 }
 
 #[test]
@@ -132,12 +121,10 @@ fn member_workspace() {
         .file("bar/src/lib.rs", "fn baz() {}")
         .build();
 
-    assert_that(
-        p.cargo("build"),
-        execs()
-            .with_status(101)
-            .with_stderr_contains("[..]error: function is never used: `baz`[..]"),
-    );
+    p.cargo("build")
+        .with_status(101)
+        .with_stderr_contains("[..]error: function is never used: `baz`[..]")
+        .run();
 }
 
 #[test]
@@ -168,10 +155,36 @@ fn virtual_workspace_overrides() {
         .file("bar/src/lib.rs", "fn baz() {}")
         .build();
 
-    assert_that(
-        p.cargo("build"),
-        execs()
-            .with_status(101)
-            .with_stderr_contains("[..]error: function is never used: `baz`[..]"),
-    );
+    p.cargo("build")
+        .with_status(101)
+        .with_stderr_contains("[..]error: function is never used: `baz`[..]")
+        .run();
+}
+
+#[test]
+fn feature_flag() {
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+            [package]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+
+            [features]
+            bar = []
+
+            [lints2.'cfg(feature = "bar")']
+            dead_code = "deny"
+        "#,
+        )
+        .file("src/lib.rs", "fn foo() {}")
+        .build();
+
+    p.cargo("build").with_status(0).run();
+    p.cargo("build --features bar")
+        .with_status(101)
+        .with_stderr_contains("[..]error: function is never used: `foo`[..]")
+        .run();
 }