From d82c2039d1f78f93eacfce2e07b210a62f1b32bc Mon Sep 17 00:00:00 2001 From: Wilco Kusee Date: Thu, 12 Jul 2018 18:15:30 +0200 Subject: [PATCH 1/7] Add lints section to manifest --- src/cargo/core/compiler/compilation.rs | 11 ++++++++++ src/cargo/core/manifest.rs | 14 +++++++++++++ src/cargo/util/toml/mod.rs | 18 ++++++++++++++++- tests/testsuite/lints.rs | 28 ++++++++++++++++++++++++++ tests/testsuite/main.rs | 1 + 5 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 tests/testsuite/lints.rs diff --git a/src/cargo/core/compiler/compilation.rs b/src/cargo/core/compiler/compilation.rs index 2427941b4d1..14774a17f29 100644 --- a/src/cargo/core/compiler/compilation.rs +++ b/src/cargo/core/compiler/compilation.rs @@ -133,6 +133,17 @@ impl<'cfg> Compilation<'cfg> { if target.edition() != Edition::Edition2015 { p.arg(format!("--edition={}", target.edition())); } + if let Some(lints) = pkg.manifest().lints() { + if !lints.warn.is_empty() { + p.arg("-W").arg(lints.warn.join(",")); + } + if !lints.allow.is_empty() { + p.arg("-A").arg(lints.allow.join(",")); + } + if !lints.deny.is_empty() { + p.arg("-D").arg(lints.deny.join(",")); + } + } Ok(p) } diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs index 88ede658c3e..e4d83e977da 100644 --- a/src/cargo/core/manifest.rs +++ b/src/cargo/core/manifest.rs @@ -44,11 +44,19 @@ pub struct Manifest { original: Rc, features: Features, edition: Edition, + lints: Option, im_a_teapot: Option, default_run: Option, metabuild: Option>, } +#[derive(Clone, Debug)] +pub struct Lints { + pub warn: Vec, + pub allow: Vec, + pub deny: Vec, +} + /// When parsing `Cargo.toml`, some warnings should silenced /// if the manifest comes from a dependency. `ManifestWarning` /// allows this delayed emission of warnings. @@ -361,6 +369,7 @@ impl Manifest { workspace: WorkspaceConfig, features: Features, edition: Edition, + lints: Option, im_a_teapot: Option, default_run: Option, original: Rc, @@ -383,6 +392,7 @@ impl Manifest { features, edition, original, + lints, im_a_teapot, default_run, publish_lockfile, @@ -453,6 +463,10 @@ impl Manifest { &self.features } + pub fn lints(&self) -> &Option { + &self.lints + } + pub fn set_summary(&mut self, summary: Summary) { self.summary = summary; } diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 79b68189a2d..4609bf9893d 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -13,7 +13,7 @@ use toml; use url::Url; use core::dependency::{Kind, Platform}; -use core::manifest::{LibKind, ManifestMetadata, TargetSourcePath, Warnings}; +use core::manifest::{LibKind, Lints, ManifestMetadata, TargetSourcePath, Warnings}; use core::profiles::Profiles; use core::{Dependency, Manifest, PackageId, Summary, Target}; use core::{Edition, EitherManifest, Feature, Features, VirtualManifest}; @@ -240,6 +240,7 @@ pub struct TomlManifest { patch: Option>>, workspace: Option, badges: Option>>, + lints: Option>, } #[derive(Deserialize, Serialize, Clone, Debug, Default)] @@ -748,6 +749,7 @@ impl TomlManifest { workspace: None, badges: self.badges.clone(), cargo_features: self.cargo_features.clone(), + lints: self.lints.clone(), }); fn map_deps( @@ -1018,6 +1020,19 @@ impl TomlManifest { None => false, }; + let lints = me.lints.as_ref().map(|ref ls| { + let mut lints = Lints { warn: vec![], allow: vec![], deny: vec![] }; + for (lint_name, lint_state) in ls.iter() { + match lint_state.as_ref() { + "warn" => lints.warn.push(lint_name.to_string()), + "allow" => lints.allow.push(lint_name.to_string()), + "deny" => lints.deny.push(lint_name.to_string()), + _ => continue, // TODO error here? + } + } + lints + }); + let custom_metadata = project.metadata.clone(); let mut manifest = Manifest::new( summary, @@ -1035,6 +1050,7 @@ impl TomlManifest { workspace_config, features, edition, + lints, project.im_a_teapot, project.default_run.clone(), Rc::clone(me), diff --git a/tests/testsuite/lints.rs b/tests/testsuite/lints.rs new file mode 100644 index 00000000000..84d01cf1ac5 --- /dev/null +++ b/tests/testsuite/lints.rs @@ -0,0 +1,28 @@ +use cargotest::support::{execs, project}; +use hamcrest::assert_that; + +#[test] +fn deny() { + let p = project("foo") + .file( + "Cargo.toml", + r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [lints] + dead_code = "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`[..]"), + ); +} diff --git a/tests/testsuite/main.rs b/tests/testsuite/main.rs index 825615bfb90..6df01671f52 100644 --- a/tests/testsuite/main.rs +++ b/tests/testsuite/main.rs @@ -59,6 +59,7 @@ mod git; mod init; mod install; mod jobserver; +mod lints; mod local_registry; mod lockfile_compat; mod login; From c53c0cc290e0a90e876e21ba6ce93f582906bf01 Mon Sep 17 00:00:00 2001 From: Wilco Kusee Date: Sun, 15 Jul 2018 09:56:59 +0200 Subject: [PATCH 2/7] Warn for unknown lint states --- src/cargo/util/toml/mod.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 4609bf9893d..7d0f44518fa 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -1021,13 +1021,20 @@ impl TomlManifest { }; let lints = me.lints.as_ref().map(|ref ls| { - let mut lints = Lints { warn: vec![], allow: vec![], deny: vec![] }; + let mut lints = Lints { + warn: vec![], + allow: vec![], + deny: vec![], + }; for (lint_name, lint_state) in ls.iter() { match lint_state.as_ref() { "warn" => lints.warn.push(lint_name.to_string()), "allow" => lints.allow.push(lint_name.to_string()), "deny" => lints.deny.push(lint_name.to_string()), - _ => continue, // TODO error here? + _ => warnings.push(format!( + "invalid lint state for `{}` (expected `warn`, `allow` or `deny`)", + lint_name + )), } } lints From 5ac592fa9e9d932748dd85bee2449f879a238f6a Mon Sep 17 00:00:00 2001 From: Wilco Kusee Date: Sun, 15 Jul 2018 10:17:19 +0200 Subject: [PATCH 3/7] Add more unit tests --- tests/testsuite/lints.rs | 47 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tests/testsuite/lints.rs b/tests/testsuite/lints.rs index 84d01cf1ac5..0fbc9cf8351 100644 --- a/tests/testsuite/lints.rs +++ b/tests/testsuite/lints.rs @@ -26,3 +26,50 @@ fn deny() { .with_stderr_contains("[..]error: function is never used: `foo`[..]"), ); } + +#[test] +fn empty_lints_block() { + let p = project("foo") + .file( + "Cargo.toml", + r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [lints] + "#, + ) + .file("src/lib.rs", "fn foo() {}") + .build(); + + assert_that( + p.cargo("build"), + execs().with_status(0), + ); +} + +#[test] +fn invalid_state() { + let p = project("foo") + .file( + "Cargo.toml", + r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [lints] + non_snake_case = "something something" + "#, + ) + .file("src/lib.rs", "fn foo() {}") + .build(); + + assert_that( + p.cargo("build"), + execs().with_status(0), + ); +} From 286ec71098f34f723db7909eee4a21393793dd1c Mon Sep 17 00:00:00 2001 From: Wilco Kusee Date: Tue, 31 Jul 2018 20:10:20 +0200 Subject: [PATCH 4/7] Allow lints in virtual workspace --- src/cargo/core/compiler/compilation.rs | 11 --- src/cargo/core/compiler/mod.rs | 2 + src/cargo/core/lints.rs | 64 ++++++++++++++ src/cargo/core/manifest.rs | 26 +++--- src/cargo/core/mod.rs | 1 + src/cargo/core/workspace.rs | 11 +++ src/cargo/util/toml/mod.rs | 27 ++---- tests/testsuite/lints.rs | 118 +++++++++++++++++++++++-- 8 files changed, 206 insertions(+), 54 deletions(-) create mode 100644 src/cargo/core/lints.rs diff --git a/src/cargo/core/compiler/compilation.rs b/src/cargo/core/compiler/compilation.rs index 14774a17f29..2427941b4d1 100644 --- a/src/cargo/core/compiler/compilation.rs +++ b/src/cargo/core/compiler/compilation.rs @@ -133,17 +133,6 @@ impl<'cfg> Compilation<'cfg> { if target.edition() != Edition::Edition2015 { p.arg(format!("--edition={}", target.edition())); } - if let Some(lints) = pkg.manifest().lints() { - if !lints.warn.is_empty() { - p.arg("-W").arg(lints.warn.join(",")); - } - if !lints.allow.is_empty() { - p.arg("-A").arg(lints.allow.join(",")); - } - if !lints.deny.is_empty() { - p.arg("-D").arg(lints.deny.join(",")); - } - } Ok(p) } diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 10c362e2a16..40a5ae3abf3 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -809,6 +809,8 @@ fn build_base_args<'a, 'cfg>( cmd.args(args); } + bcx.ws.lints().set_flags(cmd, unit.pkg.manifest().lints()); + // -C overflow-checks is implied by the setting of -C debug-assertions, // so we only need to provide -C overflow-checks if it differs from // the value of -C debug-assertions we would provide. diff --git a/src/cargo/core/lints.rs b/src/cargo/core/lints.rs new file mode 100644 index 00000000000..1a8cb9b984c --- /dev/null +++ b/src/cargo/core/lints.rs @@ -0,0 +1,64 @@ +use std::collections::BTreeMap; +use std::collections::HashMap; + +use util::{CargoResult, ProcessBuilder}; + +#[derive(Clone, PartialEq, Debug)] +enum LintKind { + Allow, + Warn, + Deny, +} + +#[derive(Clone, Debug)] +pub struct Lints { + lints: HashMap, +} + +impl Lints { + pub fn new( + manifest_lints: Option<&BTreeMap>, + warnings: &mut Vec, + ) -> CargoResult { + let mut lints = HashMap::new(); + if let Some(lint_section) = manifest_lints { + for (lint_name, lint_state) in lint_section.iter() { + match lint_state.as_ref() { + "allow" => { lints.insert(lint_name.to_string(), LintKind::Allow); }, + "warn" => { lints.insert(lint_name.to_string(), LintKind::Warn); }, + "deny" => { lints.insert(lint_name.to_string(), LintKind::Deny); }, + _ => warnings.push(format!( + "invalid lint state for `{}` (expected `warn`, `allow` or `deny`)", + lint_name + )), + } + } + } + Ok(Lints { lints }) + } + + pub fn set_flags(&self, cmd: &mut ProcessBuilder, package_lints: &Lints) { + let get_kind = |kind: LintKind| { + self.lints.iter() + .filter(|l| *l.1 == kind) + .chain(package_lints.lints.iter() + .filter(|l| *l.1 == kind && !self.lints.contains_key(l.0))) + .map(|l| l.0.to_string()) + .collect::>() + .join(",") + }; + + let allow = get_kind(LintKind::Allow); + if !allow.is_empty() { + cmd.arg("-A").arg(allow); + } + let warn = get_kind(LintKind::Warn); + if !warn.is_empty() { + cmd.arg("-W").arg(warn); + } + let deny = get_kind(LintKind::Deny); + if !deny.is_empty() { + cmd.arg("-D").arg(deny); + } + } +} diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs index e4d83e977da..a5ce2b2639d 100644 --- a/src/cargo/core/manifest.rs +++ b/src/cargo/core/manifest.rs @@ -12,6 +12,7 @@ use toml; use url::Url; use core::interning::InternedString; +use core::lints::Lints; use core::profiles::Profiles; use core::{Dependency, PackageId, PackageIdSpec, SourceId, Summary}; use core::{Edition, Feature, Features, WorkspaceConfig}; @@ -44,19 +45,12 @@ pub struct Manifest { original: Rc, features: Features, edition: Edition, - lints: Option, + lints: Lints, im_a_teapot: Option, default_run: Option, metabuild: Option>, } -#[derive(Clone, Debug)] -pub struct Lints { - pub warn: Vec, - pub allow: Vec, - pub deny: Vec, -} - /// When parsing `Cargo.toml`, some warnings should silenced /// if the manifest comes from a dependency. `ManifestWarning` /// allows this delayed emission of warnings. @@ -76,6 +70,7 @@ pub struct VirtualManifest { workspace: WorkspaceConfig, profiles: Profiles, warnings: Warnings, + lints: Lints, } /// General metadata about a package which is just blindly uploaded to the @@ -369,7 +364,7 @@ impl Manifest { workspace: WorkspaceConfig, features: Features, edition: Edition, - lints: Option, + lints: Lints, im_a_teapot: Option, default_run: Option, original: Rc, @@ -433,6 +428,9 @@ impl Manifest { pub fn warnings(&self) -> &Warnings { &self.warnings } + pub fn lints(&self) -> &Lints { + &self.lints + } pub fn profiles(&self) -> &Profiles { &self.profiles } @@ -463,10 +461,6 @@ impl Manifest { &self.features } - pub fn lints(&self) -> &Option { - &self.lints - } - pub fn set_summary(&mut self, summary: Summary) { self.summary = summary; } @@ -545,6 +539,7 @@ impl VirtualManifest { patch: HashMap>, workspace: WorkspaceConfig, profiles: Profiles, + lints: Lints, ) -> VirtualManifest { VirtualManifest { replace, @@ -552,6 +547,7 @@ impl VirtualManifest { workspace, profiles, warnings: Warnings::new(), + lints, } } @@ -567,6 +563,10 @@ impl VirtualManifest { &self.workspace } + pub fn lints(&self) -> &Lints { + &self.lints + } + pub fn profiles(&self) -> &Profiles { &self.profiles } diff --git a/src/cargo/core/mod.rs b/src/cargo/core/mod.rs index 3312ba3458c..3017abcdb35 100644 --- a/src/cargo/core/mod.rs +++ b/src/cargo/core/mod.rs @@ -21,6 +21,7 @@ pub mod compiler; pub mod dependency; mod features; mod interning; +pub mod lints; pub mod manifest; pub mod package; pub mod package_id; diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index d603516e00a..c2af5ca5c59 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -7,6 +7,7 @@ use std::slice; use glob::glob; use url::Url; +use core::lints::Lints; use core::profiles::Profiles; use core::registry::PackageRegistry; use core::{Dependency, PackageIdSpec}; @@ -245,6 +246,16 @@ impl<'cfg> Workspace<'cfg> { } } + pub fn lints(&self) -> &Lints { + let root = self.root_manifest + .as_ref() + .unwrap_or(&self.current_manifest); + match *self.packages.get(root) { + MaybePackage::Package(ref p) => p.manifest().lints(), + MaybePackage::Virtual(ref vm) => vm.lints(), + } + } + /// Returns the root path of this workspace. /// /// That is, this returns the path of the directory containing the diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 7d0f44518fa..63a820b472b 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -13,7 +13,8 @@ use toml; use url::Url; use core::dependency::{Kind, Platform}; -use core::manifest::{LibKind, Lints, ManifestMetadata, TargetSourcePath, Warnings}; +use core::manifest::{LibKind, ManifestMetadata, TargetSourcePath, Warnings}; +use core::lints::Lints; use core::profiles::Profiles; use core::{Dependency, Manifest, PackageId, Summary, Target}; use core::{Edition, EitherManifest, Feature, Features, VirtualManifest}; @@ -999,6 +1000,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 publish = match project.publish { Some(VecStringOrBool::VecString(ref vecstring)) => { features @@ -1020,26 +1022,6 @@ impl TomlManifest { None => false, }; - let lints = me.lints.as_ref().map(|ref ls| { - let mut lints = Lints { - warn: vec![], - allow: vec![], - deny: vec![], - }; - for (lint_name, lint_state) in ls.iter() { - match lint_state.as_ref() { - "warn" => lints.warn.push(lint_name.to_string()), - "allow" => lints.allow.push(lint_name.to_string()), - "deny" => lints.deny.push(lint_name.to_string()), - _ => warnings.push(format!( - "invalid lint state for `{}` (expected `warn`, `allow` or `deny`)", - lint_name - )), - } - } - lints - }); - let custom_metadata = project.metadata.clone(); let mut manifest = Manifest::new( summary, @@ -1132,6 +1114,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 workspace_config = match me.workspace { Some(ref config) => WorkspaceConfig::Root(WorkspaceRootConfig::new( &root, @@ -1144,7 +1127,7 @@ impl TomlManifest { } }; Ok(( - VirtualManifest::new(replace, patch, workspace_config, profiles), + VirtualManifest::new(replace, patch, workspace_config, profiles, lints), nested_paths, )) } diff --git a/tests/testsuite/lints.rs b/tests/testsuite/lints.rs index 0fbc9cf8351..ee03cedb60d 100644 --- a/tests/testsuite/lints.rs +++ b/tests/testsuite/lints.rs @@ -1,13 +1,13 @@ -use cargotest::support::{execs, project}; -use hamcrest::assert_that; +use support::{execs, project}; +use support::hamcrest::assert_that; #[test] fn deny() { - let p = project("foo") + let p = project() .file( "Cargo.toml", r#" - [project] + [package] name = "foo" version = "0.0.1" authors = [] @@ -29,11 +29,11 @@ fn deny() { #[test] fn empty_lints_block() { - let p = project("foo") + let p = project() .file( "Cargo.toml", r#" - [project] + [package] name = "foo" version = "0.0.1" authors = [] @@ -52,11 +52,11 @@ fn empty_lints_block() { #[test] fn invalid_state() { - let p = project("foo") + let p = project() .file( "Cargo.toml", r#" - [project] + [package] name = "foo" version = "0.0.1" authors = [] @@ -73,3 +73,105 @@ fn invalid_state() { execs().with_status(0), ); } + +#[test] +fn virtual_workspace() { + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["bar"] + + [lints] + dead_code = "deny" + "#, + ) + .file( + "bar/Cargo.toml", + r#" + [project] + name = "bar" + version = "0.1.0" + authors = [] + "#, + ) + .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`[..]"), + ); +} + +#[test] +fn member_workspace() { + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["bar"] + "#, + ) + .file( + "bar/Cargo.toml", + r#" + [project] + name = "bar" + version = "0.1.0" + authors = [] + + [lints] + dead_code = "deny" + "#, + ) + .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`[..]"), + ); +} + +#[test] +fn virtual_workspace_overrides() { + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["bar"] + + [lints] + dead_code = "deny" + "#, + ) + .file( + "bar/Cargo.toml", + r#" + [project] + name = "bar" + version = "0.1.0" + authors = [] + + [lints] + dead_code = "allow" + "#, + ) + .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`[..]"), + ); +} From f4408a14b5715ea139b58585310759ad2593c2ba Mon Sep 17 00:00:00 2001 From: Wilco Kusee Date: Wed, 8 Aug 2018 20:41:32 +0200 Subject: [PATCH 5/7] Add forbid lint kind --- src/cargo/core/lints.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/cargo/core/lints.rs b/src/cargo/core/lints.rs index 1a8cb9b984c..3887bade710 100644 --- a/src/cargo/core/lints.rs +++ b/src/cargo/core/lints.rs @@ -8,6 +8,7 @@ enum LintKind { Allow, Warn, Deny, + Forbid, } #[derive(Clone, Debug)] @@ -27,8 +28,9 @@ impl Lints { "allow" => { lints.insert(lint_name.to_string(), LintKind::Allow); }, "warn" => { lints.insert(lint_name.to_string(), LintKind::Warn); }, "deny" => { lints.insert(lint_name.to_string(), LintKind::Deny); }, + "forbid" => { lints.insert(lint_name.to_string(), LintKind::Forbid); }, _ => warnings.push(format!( - "invalid lint state for `{}` (expected `warn`, `allow` or `deny`)", + "invalid lint state for `{}` (expected `warn`, `allow`, `deny` or `forbid`)", lint_name )), } @@ -60,5 +62,9 @@ impl Lints { if !deny.is_empty() { cmd.arg("-D").arg(deny); } + let forbid = get_kind(LintKind::Forbid); + if !forbid.is_empty() { + cmd.arg("-F").arg(forbid); + } } } From 54ffb7d91bdb4c30a16f997c5f01ea70a3b58a17 Mon Sep 17 00:00:00 2001 From: Wilco Kusee Date: Wed, 8 Aug 2018 22:06:37 +0200 Subject: [PATCH 6/7] Leave lint priorities to rustc --- src/cargo/core/compiler/mod.rs | 5 ++- src/cargo/core/lints.rs | 67 ++++++++++++++++------------------ src/cargo/core/workspace.rs | 6 +-- 3 files changed, 38 insertions(+), 40 deletions(-) diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 40a5ae3abf3..3c24fb9fd12 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -809,7 +809,10 @@ fn build_base_args<'a, 'cfg>( cmd.args(args); } - bcx.ws.lints().set_flags(cmd, unit.pkg.manifest().lints()); + unit.pkg.manifest().lints().set_flags(cmd); + if let Some(virtual_lints) = bcx.ws.virtual_lints() { + virtual_lints.set_flags(cmd); + } // -C overflow-checks is implied by the setting of -C debug-assertions, // so we only need to provide -C overflow-checks if it differs from diff --git a/src/cargo/core/lints.rs b/src/cargo/core/lints.rs index 3887bade710..d0a13d7b956 100644 --- a/src/cargo/core/lints.rs +++ b/src/cargo/core/lints.rs @@ -1,5 +1,4 @@ use std::collections::BTreeMap; -use std::collections::HashMap; use util::{CargoResult, ProcessBuilder}; @@ -11,9 +10,30 @@ enum LintKind { Forbid, } +impl LintKind { + pub fn try_from_string(lint_state: &str) -> Option { + match lint_state.as_ref() { + "allow" => Some(LintKind::Allow), + "warn" => Some(LintKind::Warn), + "deny" => Some(LintKind::Deny), + "forbid" => Some(LintKind::Forbid), + _ => None, + } + } + + pub fn flag(&self) -> char { + match self { + LintKind::Allow => 'A', + LintKind::Warn => 'W', + LintKind::Deny => 'D', + LintKind::Forbid => 'F', + } + } +} + #[derive(Clone, Debug)] pub struct Lints { - lints: HashMap, + lints: Vec<(String, LintKind)>, } impl Lints { @@ -21,50 +41,25 @@ impl Lints { manifest_lints: Option<&BTreeMap>, warnings: &mut Vec, ) -> CargoResult { - let mut lints = HashMap::new(); + let mut lints = vec![]; if let Some(lint_section) = manifest_lints { for (lint_name, lint_state) in lint_section.iter() { - match lint_state.as_ref() { - "allow" => { lints.insert(lint_name.to_string(), LintKind::Allow); }, - "warn" => { lints.insert(lint_name.to_string(), LintKind::Warn); }, - "deny" => { lints.insert(lint_name.to_string(), LintKind::Deny); }, - "forbid" => { lints.insert(lint_name.to_string(), LintKind::Forbid); }, - _ => warnings.push(format!( + 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 }) } - pub fn set_flags(&self, cmd: &mut ProcessBuilder, package_lints: &Lints) { - let get_kind = |kind: LintKind| { - self.lints.iter() - .filter(|l| *l.1 == kind) - .chain(package_lints.lints.iter() - .filter(|l| *l.1 == kind && !self.lints.contains_key(l.0))) - .map(|l| l.0.to_string()) - .collect::>() - .join(",") - }; - - let allow = get_kind(LintKind::Allow); - if !allow.is_empty() { - cmd.arg("-A").arg(allow); - } - let warn = get_kind(LintKind::Warn); - if !warn.is_empty() { - cmd.arg("-W").arg(warn); - } - let deny = get_kind(LintKind::Deny); - if !deny.is_empty() { - cmd.arg("-D").arg(deny); - } - let forbid = get_kind(LintKind::Forbid); - if !forbid.is_empty() { - cmd.arg("-F").arg(forbid); + pub fn set_flags(&self, cmd: &mut ProcessBuilder) { + for (lint_name, state) in self.lints.iter() { + cmd.arg(format!("-{}", state.flag())).arg(lint_name); } } } diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index c2af5ca5c59..7846f9328e7 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -246,13 +246,13 @@ impl<'cfg> Workspace<'cfg> { } } - pub fn lints(&self) -> &Lints { + pub fn virtual_lints(&self) -> Option<&Lints> { let root = self.root_manifest .as_ref() .unwrap_or(&self.current_manifest); match *self.packages.get(root) { - MaybePackage::Package(ref p) => p.manifest().lints(), - MaybePackage::Virtual(ref vm) => vm.lints(), + MaybePackage::Virtual(ref vm) => Some(vm.lints()), + _ => None, } } From e6a64ea1e4f80fd3995fb052b87285d030047b91 Mon Sep 17 00:00:00 2001 From: Wilco Kusee Date: Wed, 15 Aug 2018 21:14:54 +0200 Subject: [PATCH 7/7] Implement feature- and config-based lints --- src/cargo/core/compiler/mod.rs | 14 ++++-- src/cargo/core/lints.rs | 51 +++++++++++++++------ src/cargo/core/manifest.rs | 12 ++--- src/cargo/core/workspace.rs | 6 +-- src/cargo/util/toml/mod.rs | 28 +++++++++++- tests/testsuite/lints.rs | 81 ++++++++++++++++++++-------------- 6 files changed, 130 insertions(+), 62 deletions(-) diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 3c24fb9fd12..d4ff0de6c12 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/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, diff --git a/src/cargo/core/lints.rs b/src/cargo/core/lints.rs index d0a13d7b956..45649e061a3 100644 --- a/src/cargo/core/lints.rs +++ b/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, } impl Lints { pub fn new( - manifest_lints: Option<&BTreeMap>, + cfg: Option<&String>, + manifest_lints: &BTreeMap, warnings: &mut Vec, ) -> CargoResult { + 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, 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); } diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs index a5ce2b2639d..801c25feeb3 100644 --- a/src/cargo/core/manifest.rs +++ b/src/cargo/core/manifest.rs @@ -45,7 +45,7 @@ pub struct Manifest { original: Rc, features: Features, edition: Edition, - lints: Lints, + lints: Option>, im_a_teapot: Option, default_run: Option, metabuild: Option>, @@ -70,7 +70,7 @@ pub struct VirtualManifest { workspace: WorkspaceConfig, profiles: Profiles, warnings: Warnings, - lints: Lints, + lints: Option>, } /// 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>, im_a_teapot: Option, default_run: Option, original: Rc, @@ -428,7 +428,7 @@ impl Manifest { pub fn warnings(&self) -> &Warnings { &self.warnings } - pub fn lints(&self) -> &Lints { + pub fn lints(&self) -> &Option> { &self.lints } pub fn profiles(&self) -> &Profiles { @@ -539,7 +539,7 @@ impl VirtualManifest { patch: HashMap>, workspace: WorkspaceConfig, profiles: Profiles, - lints: Lints, + lints: Option>, ) -> VirtualManifest { VirtualManifest { replace, @@ -563,7 +563,7 @@ impl VirtualManifest { &self.workspace } - pub fn lints(&self) -> &Lints { + pub fn lints(&self) -> &Option> { &self.lints } diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 7846f9328e7..0cbefcc6cd3 100644 --- a/src/cargo/core/workspace.rs +++ b/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> { 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, } } diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 63a820b472b..899d4bc54f6 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -242,6 +242,8 @@ pub struct TomlManifest { workspace: Option, badges: Option>>, lints: Option>, + #[serde(rename = "lints2")] + feature_lints: Option>>, } #[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>, + toml_feature_lints: Option<&BTreeMap>>, + warnings: &mut Vec +) -> CargoResult>> { + 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 + }) +} diff --git a/tests/testsuite/lints.rs b/tests/testsuite/lints.rs index ee03cedb60d..3da4644551b 100644 --- a/tests/testsuite/lints.rs +++ b/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(); }