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 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 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
32 changes: 30 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,34 @@ macro_rules! define_Conf {
Ok(TryConf { conf, errors })
}
}

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

macro_rules! wrap_option {
() => (None);
($x:literal) => (Some($x));
}

pub(crate) fn get_configuration_metadata() -> Vec<ClippyConfiguration> {
vec![
$(
{
let deprecation_reason = wrap_option!($($dep)?);

ClippyConfiguration::new(
stringify!($name),
stringify!($ty),
format!("{:?}", super::defaults::$name()),
$doc,
deprecation_reason,
)
},
)+
]
}
}
};
}

Expand Down
142 changes: 140 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,33 @@ 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(|config| config.lints.iter().any(|lint| lint == lint_name))
.map(ToString::to_string)
.reduce(|acc, x| acc + &x)
.map(|configurations| format!(CONFIGURATION_SECTION_TEMPLATE!(), configurations = configurations))
}
}

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

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

impl ClippyConfiguration {
pub fn new(
name: &'static str,
config_type: &'static str,
default: String,
doc_comment: &'static str,
deprecation_reason: Option<&'static str>,
) -> Self {
let (lints, doc) = parse_config_field_doc(doc_comment)
.unwrap_or_else(|| (vec![], "[ERROR] MALFORMED DOC COMMENT".to_string()));

Self {
name: to_kebab(name),
lints,
doc,
config_type,
default,
deprecation_reason,
}
}
}

fn collect_configs() -> Vec<ClippyConfiguration> {
crate::utils::conf::metadata::get_configuration_metadata()
}

/// 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 +369,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