Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deserialize Msrv directly in Conf #11683

Merged
merged 1 commit into from
Oct 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion book/src/lint_configuration.md
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`)
Copy link
Member

@flip1995 flip1995 Oct 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, why is the default value of MSRV an empty stack? 🤔

EDIT: Ah I see why now. But this might be confusing to the user. I'm not sure what to do about this though, so I don't want to block this PR on this. I really like the code improvements and the pointing to the value.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah damn I didn't think about that, I think we need to change that in general since other config values also show internal type names which isn't helpful for public documentation

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I don't think this has to be done in this PR though. So feel free to r=me here now. But if you feel like doing it in this PR, I won't stop you 🙃


---
**Affected lints:**
Expand Down
65 changes: 4 additions & 61 deletions clippy_lints/src/lib.rs
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
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
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
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 @@ -641,15 +615,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 @@ -662,7 +629,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 @@ -672,6 +639,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
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