Skip to content

Commit

Permalink
Refactor config deserialization
Browse files Browse the repository at this point in the history
  • Loading branch information
@camsteffen
camsteffen committed May 1, 2021
1 parent b9c8e68 commit 1e22e56
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 132 deletions.
4 changes: 2 additions & 2 deletions clippy_lints/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,7 @@ mod zero_sized_map_values;
// end lints modules, do not remove this comment, it’s used in `update_lints`

pub use crate::utils::conf::Conf;
use crate::utils::conf::TryConf;

/// Register all pre expansion lints
///
Expand Down Expand Up @@ -421,8 +422,7 @@ pub fn read_conf(sess: &Session) -> Conf {
file_name
};

let (conf, errors) = utils::conf::read(&file_name);

let TryConf { conf, errors } = 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.struct_err(&format!(
Expand Down
232 changes: 104 additions & 128 deletions clippy_lints/src/utils/conf.rs
View file
Original file line number Diff line number Diff line change
@@ -1,98 +1,111 @@
//! Read configurations files.
#![deny(clippy::missing_docs_in_private_items)]
#![allow(clippy::module_name_repetitions)]

use std::lazy::SyncLazy;
use serde::de::{Deserializer, IgnoredAny, IntoDeserializer, MapAccess, Visitor};
use serde::Deserialize;
use std::error::Error;
use std::path::{Path, PathBuf};
use std::sync::Mutex;
use std::{env, fmt, fs, io};

/// Error from reading a configuration file.
#[derive(Debug)]
pub enum Error {
/// An I/O error.
Io(io::Error),
/// Not valid toml or doesn't fit the expected config format
Toml(String),
/// Conf with parse errors
#[derive(Default)]
pub struct TryConf {
pub conf: Conf,
pub errors: Vec<String>,
}

impl fmt::Display for Error {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::Io(err) => err.fmt(f),
Self::Toml(err) => err.fmt(f),
impl TryConf {
fn from_error(error: impl Error) -> Self {
Self {
conf: Conf::default(),
errors: vec![error.to_string()],
}
}
}

impl From<io::Error> for Error {
fn from(e: io::Error) -> Self {
Self::Io(e)
}
}
macro_rules! define_Conf {
($(
#[$doc:meta]
$(#[conf_deprecated($dep:literal)])?
($name:ident: $ty:ty $(= $default:expr)?),
)*) => {
/// Clippy lint configuration
pub struct Conf {
$(#[$doc] pub $name: $ty,)*
}

/// Vec of errors that might be collected during config toml parsing
static ERRORS: SyncLazy<Mutex<Vec<Error>>> = SyncLazy::new(|| Mutex::new(Vec::new()));
mod defaults {
$(pub fn $name() -> $ty { define_Conf!(@default $($default)?) })*
}

macro_rules! define_Conf {
($(#[$doc:meta] ($config:ident, $config_str:literal: $Ty:ty, $default:expr),)+) => {
mod helpers {
use serde::Deserialize;
/// Type used to store lint configuration.
#[derive(Deserialize)]
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
pub struct Conf {
$(
#[$doc]
#[serde(default = $config_str)]
#[serde(with = $config_str)]
pub $config: $Ty,
)+
#[allow(dead_code)]
#[serde(default)]
third_party: Option<::toml::Value>,
impl Default for Conf {
fn default() -> Self {
Self { $($name: defaults::$name(),)* }
}
}

$(
mod $config {
use serde::Deserialize;
pub fn deserialize<'de, D: serde::Deserializer<'de>>(deserializer: D) -> Result<$Ty, D::Error> {
use super::super::{ERRORS, Error};

Ok(
<$Ty>::deserialize(deserializer).unwrap_or_else(|e| {
ERRORS
.lock()
.expect("no threading here")
.push(Error::Toml(e.to_string()));
super::$config()
})
)
}
}
impl<'de> Deserialize<'de> for TryConf {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error> where D: Deserializer<'de> {
deserializer.deserialize_map(ConfVisitor)
}
}

#[derive(Deserialize)]
#[serde(field_identifier, rename_all = "kebab-case")]
#[allow(non_camel_case_types)]
enum Field { $($name,)* third_party, }

struct ConfVisitor;

impl<'de> Visitor<'de> for ConfVisitor {
type Value = TryConf;

fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
formatter.write_str("Conf")
}

#[must_use]
fn $config() -> $Ty {
let x = $default;
x
fn visit_map<V>(self, mut map: V) -> Result<Self::Value, V::Error> where V: MapAccess<'de> {
let mut errors = 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(format!("deprecated field `{}`. {}", name, $dep));)?
match map.next_value() {
Err(e) => errors.push(e.to_string()),
Ok(value) => match $name {
Some(_) => errors.push(format!("duplicate field `{}`", name)),
None => $name = Some(value),
}
}
})*
// white-listed; ignore
Field::third_party => drop(map.next_value::<IgnoredAny>())
}
}
)+
let conf = Conf { $($name: $name.unwrap_or_else(defaults::$name),)* };
Ok(TryConf { conf, errors })
}
}
};
(@default) => (Default::default());
(@default $default:expr) => ($default);
}

pub use self::helpers::Conf;
define_Conf! {
/// Lint: CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR. The minimum rust version that the project supports
(msrv, "msrv": Option<String>, None),
(msrv: Option<String>),
/// Lint: BLACKLISTED_NAME. The list of blacklisted names to lint about. NB: `bar` is not here since it has legitimate uses
(blacklisted_names, "blacklisted_names": Vec<String>, ["foo", "baz", "quux"].iter().map(ToString::to_string).collect()),
(blacklisted_names: Vec<String> = ["foo", "baz", "quux"].iter().map(ToString::to_string).collect()),
/// Lint: COGNITIVE_COMPLEXITY. The maximum cognitive complexity a function can have
(cognitive_complexity_threshold, "cognitive_complexity_threshold": u64, 25),
(cognitive_complexity_threshold: u64 = 25),
/// DEPRECATED LINT: CYCLOMATIC_COMPLEXITY. Use the Cognitive Complexity lint instead.
(cyclomatic_complexity_threshold, "cyclomatic_complexity_threshold": Option<u64>, None),
#[conf_deprecated("Please use `cognitive-complexity-threshold` instead.")]
(cyclomatic_complexity_threshold: Option<u64>),
/// Lint: DOC_MARKDOWN. The list of words this lint should not consider as identifiers needing ticks
(doc_valid_idents, "doc_valid_idents": Vec<String>, [
(doc_valid_idents: Vec<String> = [
"KiB", "MiB", "GiB", "TiB", "PiB", "EiB",
"DirectX",
"ECMAScript",
Expand All @@ -113,54 +126,47 @@ define_Conf! {
"CamelCase",
].iter().map(ToString::to_string).collect()),
/// Lint: TOO_MANY_ARGUMENTS. The maximum number of argument a function or method can have
(too_many_arguments_threshold, "too_many_arguments_threshold": u64, 7),
(too_many_arguments_threshold: u64 = 7),
/// Lint: TYPE_COMPLEXITY. The maximum complexity a type can have
(type_complexity_threshold, "type_complexity_threshold": u64, 250),
(type_complexity_threshold: u64 = 250),
/// Lint: MANY_SINGLE_CHAR_NAMES. The maximum number of single char bindings a scope may have
(single_char_binding_names_threshold, "single_char_binding_names_threshold": u64, 4),
(single_char_binding_names_threshold: u64 = 4),
/// Lint: BOXED_LOCAL, USELESS_VEC. The maximum size of objects (in bytes) that will be linted. Larger objects are ok on the heap
(too_large_for_stack, "too_large_for_stack": u64, 200),
(too_large_for_stack: u64 = 200),
/// Lint: ENUM_VARIANT_NAMES. The minimum number of enum variants for the lints about variant names to trigger
(enum_variant_name_threshold, "enum_variant_name_threshold": u64, 3),
(enum_variant_name_threshold: u64 = 3),
/// Lint: LARGE_ENUM_VARIANT. The maximum size of a enum's variant to avoid box suggestion
(enum_variant_size_threshold, "enum_variant_size_threshold": u64, 200),
(enum_variant_size_threshold: u64 = 200),
/// Lint: VERBOSE_BIT_MASK. The maximum allowed size of a bit mask before suggesting to use 'trailing_zeros'
(verbose_bit_mask_threshold, "verbose_bit_mask_threshold": u64, 1),
(verbose_bit_mask_threshold: u64 = 1),
/// Lint: DECIMAL_LITERAL_REPRESENTATION. The lower bound for linting decimal literals
(literal_representation_threshold, "literal_representation_threshold": u64, 16384),
(literal_representation_threshold: u64 = 16384),
/// Lint: TRIVIALLY_COPY_PASS_BY_REF. The maximum size (in bytes) to consider a `Copy` type for passing by value instead of by reference.
(trivial_copy_size_limit, "trivial_copy_size_limit": Option<u64>, None),
(trivial_copy_size_limit: Option<u64>),
/// Lint: LARGE_TYPE_PASS_BY_MOVE. The minimum size (in bytes) to consider a type for passing by reference instead of by value.
(pass_by_value_size_limit, "pass_by_value_size_limit": u64, 256),
(pass_by_value_size_limit: u64 = 256),
/// Lint: TOO_MANY_LINES. The maximum number of lines a function or method can have
(too_many_lines_threshold, "too_many_lines_threshold": u64, 100),
(too_many_lines_threshold: u64 = 100),
/// Lint: LARGE_STACK_ARRAYS, LARGE_CONST_ARRAYS. The maximum allowed size for arrays on the stack
(array_size_threshold, "array_size_threshold": u64, 512_000),
(array_size_threshold: u64 = 512_000),
/// Lint: VEC_BOX. The size of the boxed type in bytes, where boxing in a `Vec` is allowed
(vec_box_size_threshold, "vec_box_size_threshold": u64, 4096),
(vec_box_size_threshold: u64 = 4096),
/// Lint: TYPE_REPETITION_IN_BOUNDS. The maximum number of bounds a trait can have to be linted
(max_trait_bounds, "max_trait_bounds": u64, 3),
(max_trait_bounds: u64 = 3),
/// Lint: STRUCT_EXCESSIVE_BOOLS. The maximum number of bools a struct can have
(max_struct_bools, "max_struct_bools": u64, 3),
(max_struct_bools: u64 = 3),
/// Lint: FN_PARAMS_EXCESSIVE_BOOLS. The maximum number of bools function parameters can have
(max_fn_params_bools, "max_fn_params_bools": u64, 3),
(max_fn_params_bools: u64 = 3),
/// Lint: WILDCARD_IMPORTS. Whether to allow certain wildcard imports (prelude, super in tests).
(warn_on_all_wildcard_imports, "warn_on_all_wildcard_imports": bool, false),
(warn_on_all_wildcard_imports: bool),
/// Lint: DISALLOWED_METHOD. The list of disallowed methods, written as fully qualified paths.
(disallowed_methods, "disallowed_methods": Vec<String>, Vec::<String>::new()),
(disallowed_methods: Vec<String>),
/// Lint: UNREADABLE_LITERAL. Should the fraction of a decimal be linted to include separators.
(unreadable_literal_lint_fractions, "unreadable_literal_lint_fractions": bool, true),
(unreadable_literal_lint_fractions: bool = true),
/// Lint: UPPER_CASE_ACRONYMS. Enables verbose mode. Triggers if there is more than one uppercase char next to each other
(upper_case_acronyms_aggressive, "upper_case_acronyms_aggressive": bool, false),
(upper_case_acronyms_aggressive: bool),
/// Lint: _CARGO_COMMON_METADATA. For internal testing only, ignores the current `publish` settings in the Cargo manifest.
(cargo_ignore_publish, "cargo_ignore_publish": bool, false),
}

impl Default for Conf {
#[must_use]
fn default() -> Self {
toml::from_str("").expect("we never error on empty config files")
}
(cargo_ignore_publish: bool),
}

/// Search for the configuration file.
Expand Down Expand Up @@ -194,43 +200,13 @@ pub fn lookup_conf_file() -> io::Result<Option<PathBuf>> {
}
}

/// Produces a `Conf` filled with the default values and forwards the errors
///
/// Used internally for convenience
fn default(errors: Vec<Error>) -> (Conf, Vec<Error>) {
(Conf::default(), errors)
}

/// Read the `toml` configuration file.
///
/// In case of error, the function tries to continue as much as possible.
pub fn read(path: &Path) -> (Conf, Vec<Error>) {
pub fn read(path: &Path) -> TryConf {
let content = match fs::read_to_string(path) {
Err(e) => return TryConf::from_error(e),
Ok(content) => content,
Err(err) => return default(vec![err.into()]),
};

assert!(ERRORS.lock().expect("no threading -> mutex always safe").is_empty());
match toml::from_str(&content) {
Ok(toml) => {
let mut errors = ERRORS.lock().expect("no threading -> mutex always safe").split_off(0);

let toml_ref: &Conf = &toml;

let cyc_field: Option<u64> = toml_ref.cyclomatic_complexity_threshold;

if cyc_field.is_some() {
let cyc_err = "found deprecated field `cyclomatic-complexity-threshold`. Please use `cognitive-complexity-threshold` instead.".to_string();
errors.push(Error::Toml(cyc_err));
}

(toml, errors)
},
Err(e) => {
let mut errors = ERRORS.lock().expect("no threading -> mutex always safe").split_off(0);
errors.push(Error::Toml(e.to_string()));

default(errors)
},
}
toml::from_str(&content).unwrap_or_else(TryConf::from_error)
}
2 changes: 1 addition & 1 deletion tests/ui-toml/bad_toml_type/conf_bad_type.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: error reading Clippy's configuration file `$DIR/clippy.toml`: invalid type: integer `42`, expected a sequence
error: error reading Clippy's configuration file `$DIR/clippy.toml`: invalid type: integer `42`, expected a sequence for key `blacklisted-names`

error: aborting due to previous error

2 changes: 1 addition & 1 deletion tests/ui-toml/conf_deprecated_key/conf_deprecated_key.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: error reading Clippy's configuration file `$DIR/clippy.toml`: found deprecated field `cyclomatic-complexity-threshold`. Please use `cognitive-complexity-threshold` instead.
error: 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

0 comments on commit 1e22e56

Please sign in to comment.