Skip to content
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
7 changes: 6 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,18 @@ ui_test = "0.30.2"
regex = "1.5.5"
serde = { version = "1.0.145", features = ["derive"] }
serde_json = "1.0.122"
toml = "0.7.3"
walkdir = "2.3"
filetime = "0.2.9"
itertools = "0.12"
pulldown-cmark = { version = "0.11", default-features = false, features = ["html"] }
askama = { version = "0.14", default-features = false, features = ["alloc", "config", "derive"] }

[dev-dependencies.toml]
version = "0.9.7"
default-features = false
# preserve_order keeps diagnostic output in file order
features = ["parse", "preserve_order"]

[build-dependencies]
rustc_tools_util = { path = "rustc_tools_util", version = "0.4.2" }

Expand Down
7 changes: 6 additions & 1 deletion clippy_lints/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,17 @@ itertools = "0.12"
quine-mc_cluskey = "0.2"
regex-syntax = "0.8"
serde = { version = "1.0", features = ["derive"] }
toml = "0.7.3"
unicode-normalization = "0.1"
unicode-script = { version = "0.5", default-features = false }
semver = "1.0"
url = "2.2"

[dependencies.toml]
version = "0.9.7"
default-features = false
# preserve_order keeps diagnostic output in file order
features = ["parse", "preserve_order"]

[dev-dependencies]
walkdir = "2.3"

Expand Down
186 changes: 90 additions & 96 deletions clippy_lints/src/cargo/lint_groups_priority.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,142 +4,103 @@ use rustc_data_structures::fx::FxHashSet;
use rustc_errors::Applicability;
use rustc_lint::{LateContext, unerased_lint_store};
use rustc_span::{BytePos, Pos, SourceFile, Span, SyntaxContext};
use serde::{Deserialize, Serialize};
use std::collections::BTreeMap;
use std::ops::Range;
use std::path::Path;
use toml::Spanned;
use toml::de::{DeTable, DeValue};

#[derive(Deserialize, Serialize, Debug)]
struct LintConfigTable {
level: String,
priority: Option<i64>,
fn toml_span(range: Range<usize>, file: &SourceFile) -> Span {
Span::new(
file.start_pos + BytePos::from_usize(range.start),
file.start_pos + BytePos::from_usize(range.end),
SyntaxContext::root(),
None,
)
}

#[derive(Deserialize, Debug)]
#[serde(untagged)]
enum LintConfig {
Level(String),
Table(LintConfigTable),
struct LintConfig<'a> {
sp: Range<usize>,
level: &'a str,
priority: Option<i64>,
}

impl LintConfig {
fn level(&self) -> &str {
match self {
LintConfig::Level(level) => level,
LintConfig::Table(table) => &table.level,
}
}

impl<'a> LintConfig<'a> {
fn priority(&self) -> i64 {
match self {
LintConfig::Level(_) => 0,
LintConfig::Table(table) => table.priority.unwrap_or(0),
}
self.priority.unwrap_or(0)
}

fn is_implicit(&self) -> bool {
if let LintConfig::Table(table) = self {
table.priority.is_none()
} else {
true
}
self.priority.is_none()
}
}

type LintTable = BTreeMap<Spanned<String>, Spanned<LintConfig>>;

#[derive(Deserialize, Debug, Default)]
struct Lints {
#[serde(default)]
rust: LintTable,
#[serde(default)]
clippy: LintTable,
}

#[derive(Deserialize, Debug, Default)]
struct Workspace {
#[serde(default)]
lints: Lints,
}

#[derive(Deserialize, Debug)]
struct CargoToml {
#[serde(default)]
lints: Lints,
#[serde(default)]
workspace: Workspace,
}

fn toml_span(range: Range<usize>, file: &SourceFile) -> Span {
Span::new(
file.start_pos + BytePos::from_usize(range.start),
file.start_pos + BytePos::from_usize(range.end),
SyntaxContext::root(),
None,
)
fn parse(value: &'a Spanned<DeValue<'a>>) -> Option<Self> {
let sp = value.span();
let (level, priority) = match value.get_ref() {
DeValue::String(level) => (&**level, None),
DeValue::Table(tbl) => {
let level = tbl.get("level")?.get_ref().as_str()?;
let priority = if let Some(priority) = tbl.get("priority") {
let priority = priority.get_ref().as_integer()?;
Some(i64::from_str_radix(priority.as_str(), priority.radix()).ok()?)
} else {
None
};
(level, priority)
},
_ => return None,
};
Some(Self { sp, level, priority })
}
}

fn check_table(cx: &LateContext<'_>, table: LintTable, known_groups: &FxHashSet<&str>, file: &SourceFile) {
fn check_table(cx: &LateContext<'_>, table: &DeTable<'_>, known_groups: &FxHashSet<&str>, file: &SourceFile) {
let mut lints = Vec::new();
let mut groups = Vec::new();
for (name, config) in table {
if name.get_ref() == "warnings" {
continue;
}

if known_groups.contains(name.get_ref().as_str()) {
groups.push((name, config));
} else {
lints.push((name, config.into_inner()));
if name.get_ref() != "warnings"
&& let Some(config) = LintConfig::parse(config)
{
if known_groups.contains(&**name.get_ref()) {
groups.push((name, config));
} else {
lints.push((name, config));
}
}
}

for (group, group_config) in groups {
let priority = group_config.get_ref().priority();
let level = group_config.get_ref().level();
if let Some((conflict, _)) = lints
.iter()
.rfind(|(_, lint_config)| lint_config.priority() == priority && lint_config.level() != level)
{
if let Some((conflict, _)) = lints.iter().rfind(|(_, lint_config)| {
lint_config.priority() == group_config.priority() && lint_config.level != group_config.level
}) {
span_lint_and_then(
cx,
LINT_GROUPS_PRIORITY,
toml_span(group.span(), file),
format!(
"lint group `{}` has the same priority ({priority}) as a lint",
group.as_ref()
"lint group `{}` has the same priority ({}) as a lint",
group.as_ref(),
group_config.priority(),
),
|diag| {
let config_span = toml_span(group_config.span(), file);
let config_span = toml_span(group_config.sp.clone(), file);

if group_config.as_ref().is_implicit() {
if group_config.is_implicit() {
diag.span_label(config_span, "has an implicit priority of 0");
}
diag.span_label(toml_span(conflict.span(), file), "has the same priority as this lint");
diag.note("the order of the lints in the table is ignored by Cargo");

let mut suggestion = String::new();
let low_priority = lints
.iter()
.map(|(_, config)| config.priority().saturating_sub(1))
.map(|(_, lint_config)| lint_config.priority().saturating_sub(1))
.min()
.unwrap_or(-1);
Serialize::serialize(
&LintConfigTable {
level: level.into(),
priority: Some(low_priority),
},
toml::ser::ValueSerializer::new(&mut suggestion),
)
.unwrap();
diag.span_suggestion_verbose(
config_span,
format!(
"to have lints override the group set `{}` to a lower priority",
group.as_ref()
),
suggestion,
format!("{{ level = {:?}, priority = {low_priority} }}", group_config.level,),
Applicability::MaybeIncorrect,
);
},
Expand All @@ -148,10 +109,29 @@ fn check_table(cx: &LateContext<'_>, table: LintTable, known_groups: &FxHashSet<
}
}

struct LintTbls<'a> {
rust: Option<&'a DeTable<'a>>,
clippy: Option<&'a DeTable<'a>>,
}
fn get_lint_tbls<'a>(tbl: &'a DeTable<'a>) -> LintTbls<'a> {
if let Some(lints) = tbl.get("lints")
&& let Some(lints) = lints.get_ref().as_table()
{
let rust = lints.get("rust").and_then(|x| x.get_ref().as_table());
let clippy = lints.get("clippy").and_then(|x| x.get_ref().as_table());
LintTbls { rust, clippy }
} else {
LintTbls {
rust: None,
clippy: None,
}
}
}

pub fn check(cx: &LateContext<'_>) {
if let Ok(file) = cx.tcx.sess.source_map().load_file(Path::new("Cargo.toml"))
&& let Some(src) = file.src.as_deref()
&& let Ok(cargo_toml) = toml::from_str::<CargoToml>(src)
&& let Ok(cargo_toml) = DeTable::parse(src)
{
let mut rustc_groups = FxHashSet::default();
let mut clippy_groups = FxHashSet::default();
Expand All @@ -167,9 +147,23 @@ pub fn check(cx: &LateContext<'_>) {
}
}

check_table(cx, cargo_toml.lints.rust, &rustc_groups, &file);
check_table(cx, cargo_toml.lints.clippy, &clippy_groups, &file);
check_table(cx, cargo_toml.workspace.lints.rust, &rustc_groups, &file);
check_table(cx, cargo_toml.workspace.lints.clippy, &clippy_groups, &file);
let lints = get_lint_tbls(cargo_toml.get_ref());
if let Some(lints) = lints.rust {
check_table(cx, lints, &rustc_groups, &file);
}
if let Some(lints) = lints.clippy {
check_table(cx, lints, &clippy_groups, &file);
}
if let Some(tbl) = cargo_toml.get_ref().get("workspace")
&& let Some(tbl) = tbl.get_ref().as_table()
{
let lints = get_lint_tbls(tbl);
if let Some(lints) = lints.rust {
check_table(cx, lints, &rustc_groups, &file);
}
if let Some(lints) = lints.clippy {
check_table(cx, lints, &clippy_groups, &file);
}
}
}
}
2 changes: 1 addition & 1 deletion lintcheck/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,6 @@ serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0.85"
strip-ansi-escapes = "0.2.0"
tar = "0.4"
toml = "0.7.3"
toml = "0.9.7"
ureq = { version = "2.2", features = ["json"] }
walkdir = "2.3"
22 changes: 16 additions & 6 deletions tests/compile-test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -405,11 +405,18 @@ fn ui_cargo_toml_metadata() {
continue;
}

let toml = fs::read_to_string(path).unwrap().parse::<toml::Value>().unwrap();

let package = toml.as_table().unwrap().get("package").unwrap().as_table().unwrap();

let name = package.get("name").unwrap().as_str().unwrap().replace('-', "_");
let toml = fs::read_to_string(path).unwrap();
let toml = toml::de::DeTable::parse(&toml).unwrap();

let package = toml.get_ref().get("package").unwrap().get_ref().as_table().unwrap();

let name = package
.get("name")
.unwrap()
.as_ref()
.as_str()
.unwrap()
.replace('-', "_");
assert!(
path.parent()
.unwrap()
Expand All @@ -421,7 +428,10 @@ fn ui_cargo_toml_metadata() {
path.display(),
);

let publish = package.get("publish").and_then(toml::Value::as_bool).unwrap_or(true);
let publish = package
.get("publish")
.and_then(|x| x.get_ref().as_bool())
.unwrap_or(true);
assert!(
!publish || publish_exceptions.contains(&path.parent().unwrap().to_path_buf()),
"`{}` lacks `publish = false`",
Expand Down
Loading