Skip to content

Commit

Permalink
Implement feature- and config-based lints
Browse files Browse the repository at this point in the history
  • Loading branch information
detrumi committed Sep 21, 2018
1 parent 54ffb7d commit e6a64ea
Show file tree
Hide file tree
Showing 6 changed files with 130 additions and 62 deletions.
14 changes: 11 additions & 3 deletions src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
51 changes: 37 additions & 14 deletions src/cargo/core/lints.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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);
}
Expand Down
12 changes: 6 additions & 6 deletions src/cargo/core/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>>,
Expand All @@ -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
Expand Down Expand Up @@ -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>,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -539,7 +539,7 @@ impl VirtualManifest {
patch: HashMap<Url, Vec<Dependency>>,
workspace: WorkspaceConfig,
profiles: Profiles,
lints: Lints,
lints: Option<Vec<Lints>>,
) -> VirtualManifest {
VirtualManifest {
replace,
Expand All @@ -563,7 +563,7 @@ impl VirtualManifest {
&self.workspace
}

pub fn lints(&self) -> &Lints {
pub fn lints(&self) -> &Option<Vec<Lints>> {
&self.lints
}

Expand Down
6 changes: 3 additions & 3 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand Down
28 changes: 26 additions & 2 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
})
}
81 changes: 47 additions & 34 deletions tests/testsuite/lints.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use support::{execs, project};
use support::hamcrest::assert_that;
use support::project;

#[test]
fn deny() {
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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();
}

0 comments on commit e6a64ea

Please sign in to comment.