Skip to content

Commit

Permalink
Auto merge of #11683 - Alexendoo:msrv-config, r=Manishearth,flip1995
Browse files Browse the repository at this point in the history 
Deserialize `Msrv` directly in `Conf`

Gives the error a span pointing to the invalid config value

Also puts `Conf` itself in the `OnceLock` rather than just the `Msrv` for [the `register_late_mod_pass` work](rust-lang/rust#116731) since it will be used from two different callbacks

changelog: none
  • Loading branch information
@bors
bors committed Oct 19, 2023
2 parents fe21991 + 1528c1d commit 9574d28
Show file tree
Hide file tree
Showing 10 changed files with 119 additions and 170 deletions.
2 changes: 1 addition & 1 deletion book/src/lint_configuration.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ Suppress lints whenever the suggested change would cause breakage for other crat
## `msrv`
The minimum rust version that the project supports

**Default Value:** `None` (`Option<String>`)
**Default Value:** `Msrv { stack: [] }` (`crate::Msrv`)

---
**Affected lints:**
Expand Down
65 changes: 4 additions & 61 deletions clippy_lints/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,6 @@ extern crate clippy_utils;
#[macro_use]
extern crate declare_clippy_lint;

use std::io;
use std::path::PathBuf;

use clippy_utils::msrvs::Msrv;
use rustc_data_structures::fx::FxHashSet;
use rustc_lint::{Lint, LintId};
Expand Down Expand Up @@ -362,7 +359,6 @@ mod zero_sized_map_values;
// end lints modules, do not remove this comment, it’s used in `update_lints`

use crate::utils::conf::metadata::get_configuration_metadata;
use crate::utils::conf::TryConf;
pub use crate::utils::conf::{lookup_conf_file, Conf};
use crate::utils::FindAll;

Expand All @@ -374,65 +370,13 @@ use crate::utils::FindAll;
/// level (i.e `#![cfg_attr(...)]`) will still be expanded even when using a pre-expansion pass.
///
/// Used in `./src/driver.rs`.
pub fn register_pre_expansion_lints(store: &mut rustc_lint::LintStore, sess: &Session, conf: &Conf) {
pub fn register_pre_expansion_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
// NOTE: Do not add any more pre-expansion passes. These should be removed eventually.
let msrv = Msrv::read(&conf.msrv, sess);
let msrv = move || msrv.clone();
let msrv = || conf.msrv.clone();

store.register_pre_expansion_pass(move || Box::new(attrs::EarlyAttributes { msrv: msrv() }));
}

#[doc(hidden)]
pub fn read_conf(sess: &Session, path: &io::Result<(Option<PathBuf>, Vec<String>)>) -> Conf {
if let Ok((_, warnings)) = path {
for warning in warnings {
sess.warn(warning.clone());
}
}
let file_name = match path {
Ok((Some(path), _)) => path,
Ok((None, _)) => return Conf::default(),
Err(error) => {
sess.err(format!("error finding Clippy's configuration file: {error}"));
return Conf::default();
},
};

let TryConf { conf, errors, warnings } = utils::conf::read(sess, file_name);
// all conf errors are non-fatal, we just use the default conf in case of error
for error in errors {
if let Some(span) = error.span {
sess.span_err(
span,
format!("error reading Clippy's configuration file: {}", error.message),
);
} else {
sess.err(format!(
"error reading Clippy's configuration file `{}`: {}",
file_name.display(),
error.message
));
}
}

for warning in warnings {
if let Some(span) = warning.span {
sess.span_warn(
span,
format!("error reading Clippy's configuration file: {}", warning.message),
);
} else {
sess.warn(format!(
"error reading Clippy's configuration file `{}`: {}",
file_name.display(),
warning.message
));
}
}

conf
}

#[derive(Default)]
struct RegistrationGroups {
all: Vec<LintId>,
Expand Down Expand Up @@ -558,7 +502,7 @@ fn register_categories(store: &mut rustc_lint::LintStore) {
///
/// Used in `./src/driver.rs`.
#[expect(clippy::too_many_lines)]
pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &Conf) {
pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &'static Conf) {
register_removed_non_tool_lints(store);
register_categories(store);

Expand Down Expand Up @@ -660,8 +604,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|_| Box::new(non_octal_unix_permissions::NonOctalUnixPermissions));
store.register_early_pass(|| Box::new(unnecessary_self_imports::UnnecessarySelfImports));

let msrv = Msrv::read(&conf.msrv, sess);
let msrv = move || msrv.clone();
let msrv = || conf.msrv.clone();
let avoid_breaking_exported_api = conf.avoid_breaking_exported_api;
let allow_expect_in_tests = conf.allow_expect_in_tests;
let allow_unwrap_in_tests = conf.allow_unwrap_in_tests;
Expand Down
3 changes: 1 addition & 2 deletions clippy_lints/src/matches/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -967,7 +967,6 @@ declare_clippy_lint! {
"checks for unnecessary guards in match expressions"
}

#[derive(Default)]
pub struct Matches {
msrv: Msrv,
infallible_destructuring_match_linted: bool,
Expand All @@ -978,7 +977,7 @@ impl Matches {
pub fn new(msrv: Msrv) -> Self {
Self {
msrv,
..Matches::default()
infallible_destructuring_match_linted: false,
}
}
}
Expand Down
3 changes: 1 addition & 2 deletions clippy_lints/src/use_self.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ declare_clippy_lint! {
"unnecessary structure name repetition whereas `Self` is applicable"
}

#[derive(Default)]
pub struct UseSelf {
msrv: Msrv,
stack: Vec<StackItem>,
Expand All @@ -65,7 +64,7 @@ impl UseSelf {
pub fn new(msrv: Msrv) -> Self {
Self {
msrv,
..Self::default()
stack: Vec::new(),
}
}
}
Expand Down
111 changes: 66 additions & 45 deletions clippy_lints/src/utils/conf.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ use serde::de::{Deserializer, IgnoredAny, IntoDeserializer, MapAccess, Visitor};
use serde::Deserialize;
use std::fmt::{Debug, Display, Formatter};
use std::ops::Range;
use std::path::{Path, PathBuf};
use std::path::PathBuf;
use std::str::FromStr;
use std::sync::OnceLock;
use std::{cmp, env, fmt, fs, io};

#[rustfmt::skip]
Expand Down Expand Up @@ -78,62 +79,35 @@ pub struct TryConf {

impl TryConf {
fn from_toml_error(file: &SourceFile, error: &toml::de::Error) -> Self {
ConfError::from_toml(file, error).into()
}
}

impl From<ConfError> for TryConf {
fn from(value: ConfError) -> Self {
Self {
conf: Conf::default(),
errors: vec![value],
errors: vec![ConfError::from_toml(file, error)],
warnings: vec![],
}
}
}

impl From<io::Error> for TryConf {
fn from(value: io::Error) -> Self {
ConfError::from(value).into()
}
}

#[derive(Debug)]
pub struct ConfError {
pub message: String,
pub span: Option<Span>,
pub span: Span,
}

impl ConfError {
fn from_toml(file: &SourceFile, error: &toml::de::Error) -> Self {
if let Some(span) = error.span() {
Self::spanned(file, error.message(), span)
} else {
Self {
message: error.message().to_string(),
span: None,
}
}
let span = error.span().unwrap_or(0..file.source_len.0 as usize);
Self::spanned(file, error.message(), span)
}

fn spanned(file: &SourceFile, message: impl Into<String>, span: Range<usize>) -> Self {
Self {
message: message.into(),
span: Some(Span::new(
span: Span::new(
file.start_pos + BytePos::from_usize(span.start),
file.start_pos + BytePos::from_usize(span.end),
SyntaxContext::root(),
None,
)),
}
}
}

impl From<io::Error> for ConfError {
fn from(value: io::Error) -> Self {
Self {
message: value.to_string(),
span: None,
),
}
}
}
Expand Down Expand Up @@ -297,7 +271,7 @@ define_Conf! {
/// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, OPTION_MAP_UNWRAP_OR, 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, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, ERR_EXPECT, CAST_ABS_TO_UNSIGNED, UNINLINED_FORMAT_ARGS, MANUAL_CLAMP, MANUAL_LET_ELSE, UNCHECKED_DURATION_SUBTRACTION, COLLAPSIBLE_STR_REPLACE, SEEK_FROM_CURRENT, SEEK_REWIND, UNNECESSARY_LAZY_EVALUATIONS, TRANSMUTE_PTR_TO_REF, ALMOST_COMPLETE_RANGE, NEEDLESS_BORROW, DERIVABLE_IMPLS, MANUAL_IS_ASCII_CHECK, MANUAL_REM_EUCLID, MANUAL_RETAIN, TYPE_REPETITION_IN_BOUNDS, TUPLE_ARRAY_CONVERSIONS, MANUAL_TRY_FOLD, MANUAL_HASH_ONE.
///
/// The minimum rust version that the project supports
(msrv: Option<String> = None),
(msrv: crate::Msrv = crate::Msrv::empty()),
/// DEPRECATED LINT: BLACKLISTED_NAME.
///
/// Use the Disallowed Names lint instead
Expand Down Expand Up @@ -645,15 +619,8 @@ pub fn lookup_conf_file() -> io::Result<(Option<PathBuf>, Vec<String>)> {
}
}

/// Read the `toml` configuration file.
///
/// In case of error, the function tries to continue as much as possible.
pub fn read(sess: &Session, path: &Path) -> TryConf {
let file = match sess.source_map().load_file(path) {
Err(e) => return e.into(),
Ok(file) => file,
};
match toml::de::Deserializer::new(file.src.as_ref().unwrap()).deserialize_map(ConfVisitor(&file)) {
fn deserialize(file: &SourceFile) -> TryConf {
match toml::de::Deserializer::new(file.src.as_ref().unwrap()).deserialize_map(ConfVisitor(file)) {
Ok(mut conf) => {
extend_vec_if_indicator_present(&mut conf.conf.doc_valid_idents, DEFAULT_DOC_VALID_IDENTS);
extend_vec_if_indicator_present(&mut conf.conf.disallowed_names, DEFAULT_DISALLOWED_NAMES);
Expand All @@ -666,7 +633,7 @@ pub fn read(sess: &Session, path: &Path) -> TryConf {

conf
},
Err(e) => TryConf::from_toml_error(&file, &e),
Err(e) => TryConf::from_toml_error(file, &e),
}
}

Expand All @@ -676,6 +643,60 @@ fn extend_vec_if_indicator_present(vec: &mut Vec<String>, default: &[&str]) {
}
}

impl Conf {
pub fn read(sess: &Session, path: &io::Result<(Option<PathBuf>, Vec<String>)>) -> &'static Conf {
static CONF: OnceLock<Conf> = OnceLock::new();
CONF.get_or_init(|| Conf::read_inner(sess, path))
}

fn read_inner(sess: &Session, path: &io::Result<(Option<PathBuf>, Vec<String>)>) -> Conf {
match path {
Ok((_, warnings)) => {
for warning in warnings {
sess.warn(warning.clone());
}
},
Err(error) => {
sess.err(format!("error finding Clippy's configuration file: {error}"));
},
}

let TryConf {
mut conf,
errors,
warnings,
} = match path {
Ok((Some(path), _)) => match sess.source_map().load_file(path) {
Ok(file) => deserialize(&file),
Err(error) => {
sess.err(format!("failed to read `{}`: {error}", path.display()));
TryConf::default()
},
},
_ => TryConf::default(),
};

conf.msrv.read_cargo(sess);

// all conf errors are non-fatal, we just use the default conf in case of error
for error in errors {
sess.span_err(
error.span,
format!("error reading Clippy's configuration file: {}", error.message),
);
}

for warning in warnings {
sess.span_warn(
warning.span,
format!("error reading Clippy's configuration file: {}", warning.message),
);
}

conf
}
}

const SEPARATOR_WIDTH: usize = 4;

#[derive(Debug)]
Expand Down
1 change: 1 addition & 0 deletions clippy_utils/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ arrayvec = { version = "0.7", default-features = false }
if_chain = "1.0"
itertools = "0.10.1"
rustc-semver = "1.1"
serde = { version = "1.0" }

[features]
deny-warnings = []
Expand Down
Loading

0 comments on commit 9574d28

Please sign in to comment.