From 08e7ec40478203a7c1e0959461515ecdaf5754a7 Mon Sep 17 00:00:00 2001 From: Philipp Krones Date: Wed, 27 Jul 2022 17:18:20 +0100 Subject: [PATCH 1/2] Read and use deprecated configuration (as well as emitting a warning) Co-authored-by: Andy Caldwell --- clippy_lints/src/lib.rs | 11 +++++++++- clippy_lints/src/utils/conf.rs | 20 +++++++++++++------ tests/ui-toml/conf_deprecated_key/clippy.toml | 2 +- .../conf_deprecated_key.rs | 10 ++++++++++ .../conf_deprecated_key.stderr | 13 ++++++++++-- 5 files changed, 46 insertions(+), 10 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 5a311163239b..5bec503402c3 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -487,7 +487,7 @@ pub fn read_conf(sess: &Session) -> Conf { }, }; - let TryConf { conf, errors } = utils::conf::read(&file_name); + let TryConf { conf, errors, warnings } = utils::conf::read(&file_name); // all conf errors are non-fatal, we just use the default conf in case of error for error in errors { sess.err(&format!( @@ -497,6 +497,15 @@ pub fn read_conf(sess: &Session) -> Conf { )); } + for warning in warnings { + sess.struct_warn(&format!( + "error reading Clippy's configuration file `{}`: {}", + file_name.display(), + format_error(warning) + )) + .emit(); + } + conf } diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 6e033b3be2d8..60f4b388761d 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -68,6 +68,7 @@ pub enum DisallowedType { pub struct TryConf { pub conf: Conf, pub errors: Vec>, + pub warnings: Vec>, } impl TryConf { @@ -75,6 +76,7 @@ impl TryConf { Self { conf: Conf::default(), errors: vec![Box::new(error)], + warnings: vec![], } } } @@ -97,7 +99,7 @@ fn conf_error(s: String) -> Box { macro_rules! define_Conf { ($( $(#[doc = $doc:literal])+ - $(#[conf_deprecated($dep:literal)])? + $(#[conf_deprecated($dep:literal, $new_conf:ident)])? ($name:ident: $ty:ty = $default:expr), )*) => { /// Clippy lint configuration @@ -137,17 +139,23 @@ macro_rules! define_Conf { fn visit_map(self, mut map: V) -> Result where V: MapAccess<'de> { let mut errors = Vec::new(); + let mut warnings = Vec::new(); $(let mut $name = None;)* // could get `Field` here directly, but get `str` first for diagnostics while let Some(name) = map.next_key::<&str>()? { match Field::deserialize(name.into_deserializer())? { $(Field::$name => { - $(errors.push(conf_error(format!("deprecated field `{}`. {}", name, $dep)));)? + $(warnings.push(conf_error(format!("deprecated field `{}`. {}", name, $dep)));)? match map.next_value() { Err(e) => errors.push(conf_error(e.to_string())), Ok(value) => match $name { Some(_) => errors.push(conf_error(format!("duplicate field `{}`", name))), - None => $name = Some(value), + None => { + $name = Some(value); + // $new_conf is the same as one of the defined `$name`s, so + // this variable is defined in line 2 of this function. + $($new_conf = Some(value);)? + }, } } })* @@ -156,7 +164,7 @@ macro_rules! define_Conf { } } let conf = Conf { $($name: $name.unwrap_or_else(defaults::$name),)* }; - Ok(TryConf { conf, errors }) + Ok(TryConf { conf, errors, warnings }) } } @@ -216,8 +224,8 @@ define_Conf! { /// DEPRECATED LINT: CYCLOMATIC_COMPLEXITY. /// /// Use the Cognitive Complexity lint instead. - #[conf_deprecated("Please use `cognitive-complexity-threshold` instead")] - (cyclomatic_complexity_threshold: Option = None), + #[conf_deprecated("Please use `cognitive-complexity-threshold` instead", cognitive_complexity_threshold)] + (cyclomatic_complexity_threshold: u64 = 25), /// Lint: DOC_MARKDOWN. /// /// The list of words this lint should not consider as identifiers needing ticks. The value diff --git a/tests/ui-toml/conf_deprecated_key/clippy.toml b/tests/ui-toml/conf_deprecated_key/clippy.toml index ac47b195042e..138160d7ac8b 100644 --- a/tests/ui-toml/conf_deprecated_key/clippy.toml +++ b/tests/ui-toml/conf_deprecated_key/clippy.toml @@ -1,5 +1,5 @@ # that one is an error -cyclomatic-complexity-threshold = 42 +cyclomatic-complexity-threshold = 2 # that one is white-listed [third-party] diff --git a/tests/ui-toml/conf_deprecated_key/conf_deprecated_key.rs b/tests/ui-toml/conf_deprecated_key/conf_deprecated_key.rs index f328e4d9d04c..b4e677ea124b 100644 --- a/tests/ui-toml/conf_deprecated_key/conf_deprecated_key.rs +++ b/tests/ui-toml/conf_deprecated_key/conf_deprecated_key.rs @@ -1 +1,11 @@ fn main() {} + +#[warn(clippy::cognitive_complexity)] +fn cognitive_complexity() { + let x = vec![1, 2, 3]; + for i in x { + if i == 1 { + println!("{}", i); + } + } +} diff --git a/tests/ui-toml/conf_deprecated_key/conf_deprecated_key.stderr b/tests/ui-toml/conf_deprecated_key/conf_deprecated_key.stderr index 90021a034a3d..3b4c72044da8 100644 --- a/tests/ui-toml/conf_deprecated_key/conf_deprecated_key.stderr +++ b/tests/ui-toml/conf_deprecated_key/conf_deprecated_key.stderr @@ -1,4 +1,13 @@ -error: error reading Clippy's configuration file `$DIR/clippy.toml`: deprecated field `cyclomatic-complexity-threshold`. Please use `cognitive-complexity-threshold` instead +warning: error reading Clippy's configuration file `$DIR/clippy.toml`: deprecated field `cyclomatic-complexity-threshold`. Please use `cognitive-complexity-threshold` instead -error: aborting due to previous error +error: the function has a cognitive complexity of (3/2) + --> $DIR/conf_deprecated_key.rs:4:4 + | +LL | fn cognitive_complexity() { + | ^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::cognitive-complexity` implied by `-D warnings` + = help: you could split it up into multiple smaller functions + +error: aborting due to previous error; 1 warning emitted From ea25ef10cf942172e09b463c305b934fabccc8e2 Mon Sep 17 00:00:00 2001 From: Andy Caldwell Date: Thu, 28 Jul 2022 23:05:24 +0100 Subject: [PATCH 2/2] Harden duplicates checking and add tests --- clippy_lints/src/utils/conf.rs | 12 +++++++++--- tests/ui-toml/conf_deprecated_key/clippy.toml | 2 +- tests/ui-toml/duplicated_keys/clippy.toml | 5 +++++ tests/ui-toml/duplicated_keys/duplicated_keys.rs | 1 + tests/ui-toml/duplicated_keys/duplicated_keys.stderr | 8 ++++++++ 5 files changed, 24 insertions(+), 4 deletions(-) create mode 100644 tests/ui-toml/duplicated_keys/clippy.toml create mode 100644 tests/ui-toml/duplicated_keys/duplicated_keys.rs create mode 100644 tests/ui-toml/duplicated_keys/duplicated_keys.stderr diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 60f4b388761d..1dd22cb3185e 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -92,8 +92,8 @@ impl fmt::Display for ConfError { impl Error for ConfError {} -fn conf_error(s: String) -> Box { - Box::new(ConfError(s)) +fn conf_error(s: impl Into) -> Box { + Box::new(ConfError(s.into())) } macro_rules! define_Conf { @@ -154,7 +154,13 @@ macro_rules! define_Conf { $name = Some(value); // $new_conf is the same as one of the defined `$name`s, so // this variable is defined in line 2 of this function. - $($new_conf = Some(value);)? + $(match $new_conf { + Some(_) => errors.push(conf_error(concat!( + "duplicate field `", stringify!($new_conf), + "` (provided as `", stringify!($name), "`)" + ))), + None => $new_conf = Some(value), + })? }, } } diff --git a/tests/ui-toml/conf_deprecated_key/clippy.toml b/tests/ui-toml/conf_deprecated_key/clippy.toml index 138160d7ac8b..30cd9eecd98d 100644 --- a/tests/ui-toml/conf_deprecated_key/clippy.toml +++ b/tests/ui-toml/conf_deprecated_key/clippy.toml @@ -1,4 +1,4 @@ -# that one is an error +# that one is a warning cyclomatic-complexity-threshold = 2 # that one is white-listed diff --git a/tests/ui-toml/duplicated_keys/clippy.toml b/tests/ui-toml/duplicated_keys/clippy.toml new file mode 100644 index 000000000000..63a893cc6c79 --- /dev/null +++ b/tests/ui-toml/duplicated_keys/clippy.toml @@ -0,0 +1,5 @@ +cognitive-complexity-threshold = 2 +# This is the deprecated name for the same key +cyclomatic-complexity-threshold = 3 +# Check we get duplication warning regardless of order +cognitive-complexity-threshold = 4 diff --git a/tests/ui-toml/duplicated_keys/duplicated_keys.rs b/tests/ui-toml/duplicated_keys/duplicated_keys.rs new file mode 100644 index 000000000000..f328e4d9d04c --- /dev/null +++ b/tests/ui-toml/duplicated_keys/duplicated_keys.rs @@ -0,0 +1 @@ +fn main() {} diff --git a/tests/ui-toml/duplicated_keys/duplicated_keys.stderr b/tests/ui-toml/duplicated_keys/duplicated_keys.stderr new file mode 100644 index 000000000000..d99490a242d4 --- /dev/null +++ b/tests/ui-toml/duplicated_keys/duplicated_keys.stderr @@ -0,0 +1,8 @@ +error: error reading Clippy's configuration file `$DIR/clippy.toml`: duplicate field `cognitive_complexity_threshold` (provided as `cyclomatic_complexity_threshold`) + +error: error reading Clippy's configuration file `$DIR/clippy.toml`: duplicate field `cognitive-complexity-threshold` + +warning: error reading Clippy's configuration file `$DIR/clippy.toml`: deprecated field `cyclomatic-complexity-threshold`. Please use `cognitive-complexity-threshold` instead + +error: aborting due to 2 previous errors; 1 warning emitted +