dnrops
/
rust
mirror of https://github.com/rust-lang/rust.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Add spans to `clippy.toml` error messages

This commit is contained in:
beetrees 2023-04-08 14:53:50 +01:00
parent c4909746ad
commit 6f13a37499
No known key found for this signature in database
GPG Key ID: 8791BD754191EBD6
20 changed files with 277 additions and 140 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
Cargo.toml
View File

@ -30,7 +30,7 @@ termize = "0.1"
compiletest_rs = { version = "0.10", features = ["tmp"] }
tester = "0.9"
regex = "1.5"
toml = "0.5"
toml = "0.7.3"
walkdir = "2.3"
# This is used by the `collect-metadata` alias.
filetime = "0.2"

2
clippy_lints/Cargo.toml
View File

@ -21,7 +21,7 @@ regex-syntax = "0.6"
serde = { version = "1.0", features = ["derive"] }
serde_json = { version = "1.0", optional = true }
tempfile = { version = "3.3.0", optional = true }
toml = "0.5"
toml = "0.7.3"
unicode-normalization = "0.1"
unicode-script = { version = "0.5", default-features = false }
semver = "1.0"

39
clippy_lints/src/lib.rs
View File

@ -334,7 +334,7 @@ mod zero_sized_map_values;
pub use crate::utils::conf::{lookup_conf_file, Conf};
use crate::utils::{
conf::{format_error, metadata::get_configuration_metadata, TryConf},
conf::{metadata::get_configuration_metadata, TryConf},
FindAll,
};
@ -370,23 +370,36 @@ pub fn read_conf(sess: &Session, path: &io::Result<(Option<PathBuf>, Vec<String>
},
};
let TryConf { conf, errors, warnings } = utils::conf::read(file_name);
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 {
sess.err(format!(
"error reading Clippy's configuration file `{}`: {}",
file_name.display(),
format_error(error)
));
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 {
sess.struct_warn(format!(
"error reading Clippy's configuration file `{}`: {}",
file_name.display(),
format_error(warning)
))
.emit();
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

2
clippy_lints/src/nonstandard_macro_braces.rs
View File

@ -241,7 +241,7 @@ impl<'de> Deserialize<'de> for MacroMatcher {
V: de::MapAccess<'de>,
{
let mut name = None;
let mut brace: Option<&str> = None;
let mut brace: Option<String> = None;
while let Some(key) = map.next_key()? {
match key {
Field::Name => {

205
clippy_lints/src/utils/conf.rs
View File

@ -2,12 +2,15 @@
#![allow(clippy::module_name_repetitions)]
use rustc_session::Session;
use rustc_span::{BytePos, Pos, SourceFile, Span, SyntaxContext};
use serde::de::{Deserializer, IgnoredAny, IntoDeserializer, MapAccess, Visitor};
use serde::Deserialize;
use std::error::Error;
use std::fmt::{Debug, Display, Formatter};
use std::ops::Range;
use std::path::{Path, PathBuf};
use std::str::FromStr;
use std::{cmp, env, fmt, fs, io, iter};
use std::{cmp, env, fmt, fs, io};
#[rustfmt::skip]
const DEFAULT_DOC_VALID_IDENTS: &[&str] = &[
@ -67,33 +70,70 @@ impl DisallowedPath {
#[derive(Default)]
pub struct TryConf {
pub conf: Conf,
pub errors: Vec<Box<dyn Error>>,
pub warnings: Vec<Box<dyn Error>>,
pub errors: Vec<ConfError>,
pub warnings: Vec<ConfError>,
}
impl TryConf {
fn from_error(error: impl Error + 'static) -> Self {
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![Box::new(error)],
errors: vec![value],
warnings: vec![],
}
}
}
#[derive(Debug)]
struct ConfError(String);
impl fmt::Display for ConfError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
<String as fmt::Display>::fmt(&self.0, f)
impl From<io::Error> for TryConf {
fn from(value: io::Error) -> Self {
ConfError::from(value).into()
}
}
impl Error for ConfError {}
#[derive(Debug)]
pub struct ConfError {
pub message: String,
pub span: Option<Span>,
}
fn conf_error(s: impl Into<String>) -> Box<dyn Error> {
Box::new(ConfError(s.into()))
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,
}
}
}
fn spanned(file: &SourceFile, message: impl Into<String>, span: Range<usize>) -> Self {
Self {
message: message.into(),
span: Some(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,
}
}
}
macro_rules! define_Conf {
@ -117,20 +157,14 @@ macro_rules! define_Conf {
}
}
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;
struct ConfVisitor<'a>(&'a SourceFile);
impl<'de> Visitor<'de> for ConfVisitor {
impl<'de> Visitor<'de> for ConfVisitor<'_> {
type Value = TryConf;
fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
@ -141,32 +175,38 @@ macro_rules! define_Conf {
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 => {
$(warnings.push(conf_error(format!("deprecated field `{}`. {}", name, $dep)));)?
match map.next_value() {
Err(e) => errors.push(conf_error(e.to_string())),
// could get `Field` here directly, but get `String` first for diagnostics
while let Some(name) = map.next_key::<toml::Spanned<String>>()? {
match Field::deserialize(name.get_ref().as_str().into_deserializer()) {
Err(e) => {
let e: FieldError = e;
errors.push(ConfError::spanned(self.0, e.0, name.span()));
}
$(Ok(Field::$name) => {
$(warnings.push(ConfError::spanned(self.0, format!("deprecated field `{}`. {}", name.get_ref(), $dep), name.span()));)?
let raw_value = map.next_value::<toml::Spanned<toml::Value>>()?;
let value_span = raw_value.span();
match <$ty>::deserialize(raw_value.into_inner()) {
Err(e) => errors.push(ConfError::spanned(self.0, e.to_string().replace('\n', " ").trim(), value_span)),
Ok(value) => match $name {
Some(_) => errors.push(conf_error(format!("duplicate field `{}`", name))),
Some(_) => errors.push(ConfError::spanned(self.0, format!("duplicate field `{}`", name.get_ref()), name.span())),
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!(
Some(_) => errors.push(ConfError::spanned(self.0, concat!(
"duplicate field `", stringify!($new_conf),
"` (provided as `", stringify!($name), "`)"
))),
), name.span())),
None => $new_conf = $name.clone(),
})?
},
}
}
})*
// white-listed; ignore
Field::third_party => drop(map.next_value::<IgnoredAny>())
// ignore contents of the third_party key
Ok(Field::third_party) => drop(map.next_value::<IgnoredAny>())
}
}
let conf = Conf { $($name: $name.unwrap_or_else(defaults::$name),)* };
@ -532,19 +572,19 @@ 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(path: &Path) -> TryConf {
let content = match fs::read_to_string(path) {
Err(e) => return TryConf::from_error(e),
Ok(content) => content,
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::from_str::<TryConf>(&content) {
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);
conf
},
Err(e) => TryConf::from_error(e),
Err(e) => TryConf::from_toml_error(&file, &e),
}
}
@ -556,65 +596,42 @@ fn extend_vec_if_indicator_present(vec: &mut Vec<String>, default: &[&str]) {
const SEPARATOR_WIDTH: usize = 4;
// Check whether the error is "unknown field" and, if so, list the available fields sorted and at
// least one per line, more if `CLIPPY_TERMINAL_WIDTH` is set and allows it.
pub fn format_error(error: Box<dyn Error>) -> String {
let s = error.to_string();
#[derive(Debug)]
struct FieldError(String);
if_chain! {
if error.downcast::<toml::de::Error>().is_ok();
if let Some((prefix, mut fields, suffix)) = parse_unknown_field_message(&s);
then {
use fmt::Write;
impl std::error::Error for FieldError {}
fields.sort_unstable();
let (rows, column_widths) = calculate_dimensions(&fields);
let mut msg = String::from(prefix);
for row in 0..rows {
writeln!(msg).unwrap();
for (column, column_width) in column_widths.iter().copied().enumerate() {
let index = column * rows + row;
let field = fields.get(index).copied().unwrap_or_default();
write!(
msg,
"{:SEPARATOR_WIDTH$}{field:column_width$}",
" "
)
.unwrap();
}
}
write!(msg, "\n{suffix}").unwrap();
msg
} else {
s
}
impl Display for FieldError {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
f.pad(&self.0)
}
}
// `parse_unknown_field_message` will become unnecessary if
// https://github.com/alexcrichton/toml-rs/pull/364 is merged.
fn parse_unknown_field_message(s: &str) -> Option<(&str, Vec<&str>, &str)> {
// An "unknown field" message has the following form:
// unknown field `UNKNOWN`, expected one of `FIELD0`, `FIELD1`, ..., `FIELDN` at line X column Y
// ^^ ^^^^ ^^
if_chain! {
if s.starts_with("unknown field");
let slices = s.split("`, `").collect::<Vec<_>>();
let n = slices.len();
if n >= 2;
if let Some((prefix, first_field)) = slices[0].rsplit_once(" `");
if let Some((last_field, suffix)) = slices[n - 1].split_once("` ");
then {
let fields = iter::once(first_field)
.chain(slices[1..n - 1].iter().copied())
.chain(iter::once(last_field))
.collect::<Vec<_>>();
Some((prefix, fields, suffix))
} else {
None
impl serde::de::Error for FieldError {
fn custom<T: Display>(msg: T) -> Self {
Self(msg.to_string())
}
fn unknown_field(field: &str, expected: &'static [&'static str]) -> Self {
// List the available fields sorted and at least one per line, more if `CLIPPY_TERMINAL_WIDTH` is
// set and allows it.
use fmt::Write;
let mut expected = expected.to_vec();
expected.sort_unstable();
let (rows, column_widths) = calculate_dimensions(&expected);
let mut msg = format!("unknown field `{field}`, expected one of");
for row in 0..rows {
writeln!(msg).unwrap();
for (column, column_width) in column_widths.iter().copied().enumerate() {
let index = column * rows + row;
let field = expected.get(index).copied().unwrap_or_default();
write!(msg, "{:SEPARATOR_WIDTH$}{field:column_width$}", " ").unwrap();
}
}
Self(msg)
}
}

2
lintcheck/Cargo.toml
View File

@ -22,7 +22,7 @@ rayon = "1.5.1"
serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0.85"
tar = "0.4"
toml = "0.5"
toml = "0.7.3"
ureq = "2.2"
walkdir = "2.3"

15
src/driver.rs
View File

@ -74,7 +74,7 @@ fn track_clippy_args(parse_sess: &mut ParseSess, args_env_var: &Option<String>)
/// Track files that may be accessed at runtime in `file_depinfo` so that cargo will re-run clippy
/// when any of them are modified
fn track_files(parse_sess: &mut ParseSess, conf_path_string: Option<String>) {
fn track_files(parse_sess: &mut ParseSess) {
let file_depinfo = parse_sess.file_depinfo.get_mut();
// Used by `clippy::cargo` lints and to determine the MSRV. `cargo clippy` executes `clippy-driver`
@ -83,10 +83,7 @@ fn track_files(parse_sess: &mut ParseSess, conf_path_string: Option<String>) {
file_depinfo.insert(Symbol::intern("Cargo.toml"));
}
// `clippy.toml`
if let Some(path) = conf_path_string {
file_depinfo.insert(Symbol::intern(&path));
}
// `clippy.toml` will be automatically tracked as it's loaded with `sess.source_map().load_file()`
// During development track the `clippy-driver` executable so that cargo will re-run clippy whenever
// it is rebuilt
@ -130,17 +127,11 @@ impl rustc_driver::Callbacks for ClippyCallbacks {
#[allow(rustc::bad_opt_access)]
fn config(&mut self, config: &mut interface::Config) {
let conf_path = clippy_lints::lookup_conf_file();
let conf_path_string = if let Ok((Some(path), _)) = &conf_path {
path.to_str().map(String::from)
} else {
None
};
let previous = config.register_lints.take();
let clippy_args_var = self.clippy_args_var.take();
config.parse_sess_created = Some(Box::new(move |parse_sess| {
track_clippy_args(parse_sess, &clippy_args_var);
track_files(parse_sess, conf_path_string);
track_files(parse_sess);
}));
config.register_lints = Some(Box::new(move |sess, lint_store| {
// technically we're ~guaranteed that this is none but might as well call anything that

6
tests/ui-toml/bad_toml/conf_bad_toml.stderr
View File

@ -1,4 +1,8 @@
error: error reading Clippy's configuration file `$DIR/clippy.toml`: expected an equals, found an identifier at line 1 column 4
error: error reading Clippy's configuration file: expected `.`, `=`
--> $DIR/clippy.toml:1:4
|
LL | fn this_is_obviously(not: a, toml: file) {
| ^
error: aborting due to previous error

6
tests/ui-toml/bad_toml_type/conf_bad_type.stderr
View File

@ -1,4 +1,8 @@
error: error reading Clippy's configuration file `$DIR/clippy.toml`: invalid type: integer `42`, expected a sequence for key `disallowed-names`
error: error reading Clippy's configuration file: invalid type: integer `42`, expected a sequence
--> $DIR/clippy.toml:1:20
|
LL | disallowed-names = 42
| ^^
error: aborting due to previous error

12
tests/ui-toml/conf_deprecated_key/conf_deprecated_key.stderr
View File

@ -1,6 +1,14 @@
warning: 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: deprecated field `cyclomatic-complexity-threshold`. Please use `cognitive-complexity-threshold` instead
--> $DIR/clippy.toml:2:1
|
LL | cyclomatic-complexity-threshold = 2
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
warning: error reading Clippy's configuration file `$DIR/clippy.toml`: deprecated field `blacklisted-names`. Please use `disallowed-names` instead
warning: error reading Clippy's configuration file: deprecated field `blacklisted-names`. Please use `disallowed-names` instead
--> $DIR/clippy.toml:3:1
|
LL | blacklisted-names = [ "..", "wibble" ]
| ^^^^^^^^^^^^^^^^^
error: the function has a cognitive complexity of (3/2)
--> $DIR/conf_deprecated_key.rs:6:4

3
tests/ui-toml/duplicated_keys/clippy.toml
View File

@ -1,5 +1,2 @@
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

12
tests/ui-toml/duplicated_keys/duplicated_keys.stderr
View File

@ -1,8 +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: duplicate key `cognitive-complexity-threshold` in document root
--> $DIR/clippy.toml:2:1
|
LL | cognitive-complexity-threshold = 4
| ^
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
error: aborting due to previous error

3
tests/ui-toml/duplicated_keys_deprecated/clippy.toml Normal file
View File

@ -0,0 +1,3 @@
cognitive-complexity-threshold = 2
# This is the deprecated name for the same key
cyclomatic-complexity-threshold = 3

1
tests/ui-toml/duplicated_keys_deprecated/duplicated_keys.rs Normal file
View File

@ -0,0 +1 @@
fn main() {}

14
tests/ui-toml/duplicated_keys_deprecated/duplicated_keys.stderr Normal file
View File

@ -0,0 +1,14 @@
error: error reading Clippy's configuration file: duplicate field `cognitive_complexity_threshold` (provided as `cyclomatic_complexity_threshold`)
--> $DIR/clippy.toml:3:1
|
LL | cyclomatic-complexity-threshold = 3
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
warning: error reading Clippy's configuration file: deprecated field `cyclomatic-complexity-threshold`. Please use `cognitive-complexity-threshold` instead
--> $DIR/clippy.toml:3:1
|
LL | cyclomatic-complexity-threshold = 3
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to previous error; 1 warning emitted

4
tests/ui-toml/duplicated_keys_deprecated_2/clippy.toml Normal file
View File

@ -0,0 +1,4 @@
# This is the deprecated name for cognitive-complexity-threshold
cyclomatic-complexity-threshold = 3
# Check we get duplication warning regardless of order
cognitive-complexity-threshold = 4

1
tests/ui-toml/duplicated_keys_deprecated_2/duplicated_keys.rs Normal file
View File

@ -0,0 +1 @@
fn main() {}

14
tests/ui-toml/duplicated_keys_deprecated_2/duplicated_keys.stderr Normal file
View File

@ -0,0 +1,14 @@
error: error reading Clippy's configuration file: duplicate field `cognitive-complexity-threshold`
--> $DIR/clippy.toml:4:1
|
LL | cognitive-complexity-threshold = 4
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
warning: error reading Clippy's configuration file: deprecated field `cyclomatic-complexity-threshold`. Please use `cognitive-complexity-threshold` instead
--> $DIR/clippy.toml:2:1
|
LL | cyclomatic-complexity-threshold = 3
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to previous error; 1 warning emitted

4
tests/ui-toml/toml_unknown_key/clippy.toml
View File

@ -1,6 +1,8 @@
# that one is an error
foobar = 42
# so is this one
barfoo = 53
# that one is white-listed
# that one is ignored
[third-party]
clippy-feature = "nightly"

70
tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr
View File

@ -1,4 +1,4 @@
error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of
error: error reading Clippy's configuration file: unknown field `foobar`, expected one of
allow-dbg-in-tests
allow-expect-in-tests
allow-mixed-uninlined-format-args
@ -54,7 +54,71 @@ error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown fie
vec-box-size-threshold
verbose-bit-mask-threshold
warn-on-all-wildcard-imports
at line 5 column 1
--> $DIR/clippy.toml:2:1
|
LL | foobar = 42
| ^^^^^^
error: aborting due to previous error
error: error reading Clippy's configuration file: unknown field `barfoo`, expected one of
allow-dbg-in-tests
allow-expect-in-tests
allow-mixed-uninlined-format-args
allow-print-in-tests
allow-unwrap-in-tests
allowed-scripts
arithmetic-side-effects-allowed
arithmetic-side-effects-allowed-binary
arithmetic-side-effects-allowed-unary
array-size-threshold
avoid-breaking-exported-api
await-holding-invalid-types
blacklisted-names
cargo-ignore-publish
cognitive-complexity-threshold
cyclomatic-complexity-threshold
disallowed-macros
disallowed-methods
disallowed-names
disallowed-types
doc-valid-idents
enable-raw-pointer-heuristic-for-send
enforced-import-renames
enum-variant-name-threshold
enum-variant-size-threshold
future-size-threshold
ignore-interior-mutability
large-error-threshold
literal-representation-threshold
matches-for-let-else
max-fn-params-bools
max-include-file-size
max-struct-bools
max-suggested-slice-pattern-length
max-trait-bounds
missing-docs-in-crate-items
msrv
pass-by-value-size-limit
semicolon-inside-block-ignore-singleline
semicolon-outside-block-ignore-multiline
single-char-binding-names-threshold
standard-macro-braces
suppress-restriction-lint-in-const
third-party
too-large-for-stack
too-many-arguments-threshold
too-many-lines-threshold
trivial-copy-size-limit
type-complexity-threshold
unnecessary-box-size
unreadable-literal-lint-fractions
upper-case-acronyms-aggressive
vec-box-size-threshold
verbose-bit-mask-threshold
warn-on-all-wildcard-imports
--> $DIR/clippy.toml:4:1
|
LL | barfoo = 53
| ^^^^^^
error: aborting due to 2 previous errors