Auto merge of #9252 - Metaswitch:use-deprecated-config, r=Jarcho

Read and use deprecated configuration (as well as emitting a warning) Original change written by `@flip1995` I've simply rebased to master and fixed up the formatting/tests. This change teaches the configuration parser which config key replaced a deprecated key and attempts to populate the latter from the former. If both keys are provided this fails with a duplicate key error (rather than attempting to guess which the user intended). Currently this on affects `cyclomatic-complexity-threshold` -> `cognitive-complexity-threshold` but will also be used in #8974 to handle `blacklisted-names` -> `disallowed-names`. ``` changelog: deprecated configuration keys are still applied as if they were provided as their non-deprecated name. ``` - [x] `cargo test` passes locally - [x] Run `cargo dev fmt`
rust-lang · Jul 29, 2022 · 53a09d4 · 53a09d4
2 parents 3c7e7db + ea25ef1
commit 53a09d4
Show file tree

Hide file tree

Showing 8 changed files with 69 additions and 13 deletions.
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -486,7 +486,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!(
@@ -496,6 +496,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
@@ -68,13 +68,15 @@ pub enum DisallowedType {
 pub struct TryConf {
     pub conf: Conf,
     pub errors: Vec<Box<dyn Error>>,
+    pub warnings: Vec<Box<dyn Error>>,
 }
 
 impl TryConf {
     fn from_error(error: impl Error + 'static) -> Self {
         Self {
             conf: Conf::default(),
             errors: vec![Box::new(error)],
+            warnings: vec![],
         }
     }
 }
@@ -90,14 +92,14 @@ impl fmt::Display for ConfError {
 
 impl Error for ConfError {}
 
-fn conf_error(s: String) -> Box<dyn Error> {
-    Box::new(ConfError(s))
+fn conf_error(s: impl Into<String>) -> Box<dyn Error> {
+    Box::new(ConfError(s.into()))
 }
 
 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,29 @@ macro_rules! define_Conf {
 
             fn visit_map<V>(self, mut map: V) -> Result<Self::Value, V::Error> 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.
+                                        $(match $new_conf {
+                                            Some(_) => errors.push(conf_error(concat!(
+                                                "duplicate field `", stringify!($new_conf),
+                                                "` (provided as `", stringify!($name), "`)"
+                                            ))),
+                                            None => $new_conf = Some(value),
+                                        })?
+                                    },
                                 }
                             }
                         })*
@@ -156,7 +170,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 +230,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<u64> = 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
@@ -1,5 +1,5 @@
-# that one is an error
-cyclomatic-complexity-threshold = 42
+# that one is a warning
+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
@@ -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
@@ -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
 
diff --git a/tests/ui-toml/duplicated_keys/clippy.toml 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
@@ -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
@@ -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
+