Skip to content

Metadata collection monster searching for Clippy's configuration options #7214

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

Merged
merged 5 commits into from
May 16, 2021
Merged
Show file tree
Hide file tree
Changes from 4 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 clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1009,7 +1009,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
#[cfg(feature = "metadata-collector-lint")]
{
if std::env::var("ENABLE_METADATA_COLLECTION").eq(&Ok("1".to_string())) {
store.register_late_pass(|| box utils::internal_lints::metadata_collector::MetadataCollector::default());
store.register_late_pass(|| box utils::internal_lints::metadata_collector::MetadataCollector::new());
}
}

Expand Down
31 changes: 29 additions & 2 deletions clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@ impl TryConf {

macro_rules! define_Conf {
($(
#[$doc:meta]
#[doc = $doc:literal]
$(#[conf_deprecated($dep:literal)])?
($name:ident: $ty:ty = $default:expr),
)*) => {
/// Clippy lint configuration
pub struct Conf {
$(#[$doc] pub $name: $ty,)*
$(#[doc = $doc] pub $name: $ty,)*
}

mod defaults {
Expand Down Expand Up @@ -89,6 +89,33 @@ macro_rules! define_Conf {
Ok(TryConf { conf, errors })
}
}

#[cfg(feature = "metadata-collector-lint")]
pub mod metadata {
use crate::utils::internal_lints::metadata_collector::ClippyConfigurationBasicInfo;

pub(crate) fn get_configuration_metadata() -> Vec<ClippyConfigurationBasicInfo> {
vec![
$(
{
#[allow(unused_mut, unused_assignments)]
let mut deprecation_reason = None;

// only set if a deprecation reason was set
$(deprecation_reason = Some(stringify!($dep));)?

ClippyConfigurationBasicInfo {
name: stringify!($name),
config_type: stringify!($ty),
default: stringify!($default),
doc_comment: $doc,
deprecation_reason,
}
},
)+
]
}
}
};
}

Expand Down
153 changes: 151 additions & 2 deletions clippy_lints/src/utils/internal_lints/metadata_collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::{sym, Loc, Span, Symbol};
use serde::{ser::SerializeStruct, Serialize, Serializer};
use std::collections::BinaryHeap;
use std::fmt;
use std::fs::{self, OpenOptions};
use std::io::prelude::*;
use std::path::Path;
Expand All @@ -41,6 +42,30 @@ const EXCLUDED_LINT_GROUPS: [&str; 1] = ["clippy::internal"];
/// Collected deprecated lint will be assigned to this group in the JSON output
const DEPRECATED_LINT_GROUP_STR: &str = "DEPRECATED";

/// This template will be used to format the configuration section in the lint documentation.
/// The `configurations` parameter will be replaced with one or multiple formatted
/// `ClippyConfiguration` instances. See `CONFIGURATION_VALUE_TEMPLATE` for further customizations
macro_rules! CONFIGURATION_SECTION_TEMPLATE {
() => {
r#"
**Configuration**
This lint has the following configuration variables:

{configurations}
"#
};
}
/// This template will be used to format an individual `ClippyConfiguration` instance in the
/// lint documentation.
///
/// The format function will provide strings for the following parameters: `name`, `ty`, `doc` and
/// `default`
macro_rules! CONFIGURATION_VALUE_TEMPLATE {
() => {
"* {name}: {ty}: {doc} (defaults to `{default}`)\n"
};
}

const LINT_EMISSION_FUNCTIONS: [&[&str]; 7] = [
&["clippy_utils", "diagnostics", "span_lint"],
&["clippy_utils", "diagnostics", "span_lint_and_help"],
Expand Down Expand Up @@ -102,13 +127,32 @@ declare_clippy_lint! {
impl_lint_pass!(MetadataCollector => [INTERNAL_METADATA_COLLECTOR]);

#[allow(clippy::module_name_repetitions)]
#[derive(Debug, Clone, Default)]
#[derive(Debug, Clone)]
pub struct MetadataCollector {
/// All collected lints
///
/// We use a Heap here to have the lints added in alphabetic order in the export
lints: BinaryHeap<LintMetadata>,
applicability_info: FxHashMap<String, ApplicabilityInfo>,
config: Vec<ClippyConfiguration>,
}

impl MetadataCollector {
pub fn new() -> Self {
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this trigger our new_without_default lint?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's the first time that I've heard of that lint... so, I guess not 🤔. Unless it got merged after this branch was crated from master. I would also expect it not to trigger as new() does more than just assign default values but that might be just my opinion 🙃 .

It didn't trigger during testing. We'll see if bors complains about it 🙃

Self {
lints: BinaryHeap::<LintMetadata>::default(),
applicability_info: FxHashMap::<String, ApplicabilityInfo>::default(),
config: collect_configs(),
}
}

fn get_lint_configs(&self, lint_name: &str) -> Option<String> {
self.config
.iter()
.filter_map(|x| x.lints.iter().any(|x| x == lint_name).then(|| format!("{}", x)))
.reduce(|acc, x| acc + &x)
.map(|configurations| format!(CONFIGURATION_SECTION_TEMPLATE!(), configurations = configurations))
}
}

impl Drop for MetadataCollector {
Expand Down Expand Up @@ -214,6 +258,107 @@ impl Serialize for ApplicabilityInfo {
}
}

// ==================================================================
// Configuration
// ==================================================================
#[derive(Debug)]
pub(crate) struct ClippyConfigurationBasicInfo {
pub name: &'static str,
pub config_type: &'static str,
pub default: &'static str,
pub doc_comment: &'static str,
pub deprecation_reason: Option<&'static str>,
}

#[derive(Debug, Clone, Default)]
struct ClippyConfiguration {
name: String,
lints: Vec<String>,
doc: String,
config_type: &'static str,
default: String,
deprecation_reason: Option<&'static str>,
}

fn collect_configs() -> Vec<ClippyConfiguration> {
let cons = crate::utils::conf::metadata::get_configuration_metadata();
cons.iter()
.map(move |x| {
let (lints, doc) = parse_config_field_doc(x.doc_comment)
.unwrap_or_else(|| (vec![], "[ERROR] MALFORMED DOC COMMENT".to_string()));

ClippyConfiguration {
name: to_kebab(x.name),
lints,
doc,
config_type: x.config_type,
default: clarify_default(x.default),
deprecation_reason: x.deprecation_reason,
}
})
.collect()
}

fn clarify_default(default: &'static str) -> String {
if let Some((_start, init)) = default.split_once('[') {
if let Some((init, _end)) = init.split_once(']') {
return format!("[{}]", init);
}
}

default.to_string()
}

/// This parses the field documentation of the config struct.
///
/// ```rust, ignore
/// parse_config_field_doc(cx, "Lint: LINT_NAME_1, LINT_NAME_2. Papa penguin, papa penguin")
/// ```
///
/// Would yield:
/// ```rust, ignore
/// Some(["lint_name_1", "lint_name_2"], "Papa penguin, papa penguin")
/// ```
fn parse_config_field_doc(doc_comment: &str) -> Option<(Vec<String>, String)> {
const DOC_START: &str = " Lint: ";
if_chain! {
if doc_comment.starts_with(DOC_START);
if let Some(split_pos) = doc_comment.find('.');
then {
let mut doc_comment = doc_comment.to_string();
let documentation = doc_comment.split_off(split_pos);

doc_comment.make_ascii_lowercase();
let lints: Vec<String> = doc_comment.split_off(DOC_START.len()).split(", ").map(str::to_string).collect();

Some((lints, documentation))
} else {
None
}
}
}

/// Transforms a given `snake_case_string` to a tasty `kebab-case-string`
fn to_kebab(config_name: &str) -> String {
config_name.replace('_', "-")
}

impl fmt::Display for ClippyConfiguration {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> fmt::Result {
write!(
f,
CONFIGURATION_VALUE_TEMPLATE!(),
name = self.name,
ty = self.config_type,
doc = self.doc,
default = self.default
)
}
}

// ==================================================================
// Lint pass
// ==================================================================
impl<'hir> LateLintPass<'hir> for MetadataCollector {
/// Collecting lint declarations like:
/// ```rust, ignore
Expand All @@ -235,8 +380,12 @@ impl<'hir> LateLintPass<'hir> for MetadataCollector {
if !BLACK_LISTED_LINTS.contains(&lint_name.as_str());
// metadata extraction
if let Some(group) = get_lint_group_or_lint(cx, &lint_name, item);
if let Some(docs) = extract_attr_docs_or_lint(cx, item);
if let Some(mut docs) = extract_attr_docs_or_lint(cx, item);
then {
if let Some(configuration_section) = self.get_lint_configs(&lint_name) {
docs.push_str(&configuration_section);
}

self.lints.push(LintMetadata::new(
lint_name,
SerializableSpan::from_item(cx, item),
Expand Down