Skip to content

config_type: add unstable_variant attribute #5379

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
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 Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ unicode-segmentation = "1.9"
unicode-width = "0.1"
unicode_categories = "0.1"

rustfmt-config_proc_macro = { version = "0.2", path = "config_proc_macro" }
rustfmt-config_proc_macro = { version = "0.3", path = "config_proc_macro" }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm pretty sure this needs a version bump, however I don't know if this has knock on impacts anywhere else? AFAIK this is an internal only crate that we can break without fear?


# A noop dependency that changes in the Rust repository, it's a bit of a hack.
# See the `src/tools/rustc-workspace-hack/README.md` file in `rust-lang/rust`
Expand Down
2 changes: 1 addition & 1 deletion config_proc_macro/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion config_proc_macro/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "rustfmt-config_proc_macro"
version = "0.2.0"
version = "0.3.0"
edition = "2018"
description = "A collection of procedural macros for rustfmt"
license = "Apache-2.0/MIT"
Expand Down
27 changes: 23 additions & 4 deletions config_proc_macro/src/attrs.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
//! This module provides utilities for handling attributes on variants
//! of `config_type` enum. Currently there are two types of attributes
//! that could appear on the variants of `config_type` enum: `doc_hint`
//! and `value`. Both comes in the form of name-value pair whose value
//! is string literal.
//! of `config_type` enum. Currently there are the following attributes
//! that could appear on the variants of `config_type` enum:
//!
//! - `doc_hint`: name-value pair whose value is string literal
//! - `value`: name-value pair whose value is string literal
//! - `unstable_variant`: name only
Copy link
Contributor

Choose a reason for hiding this comment

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

An idea for a follow up PR: I wonder if we could optionally include the tracking issue number in the unstable_variant annotation. It might be nice to include a link to the tracking issue in the warning message. Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like this idea, but agree it should be in followup


/// Returns the value of the first `doc_hint` attribute in the given slice or
/// `None` if `doc_hint` attribute is not available.
Expand All @@ -27,6 +29,11 @@ pub fn find_config_value(attrs: &[syn::Attribute]) -> Option<String> {
attrs.iter().filter_map(config_value).next()
}

/// Returns `true` if the there is at least one `unstable` attribute in the given slice.
pub fn any_unstable_variant(attrs: &[syn::Attribute]) -> bool {
attrs.iter().any(is_unstable_variant)
}

/// Returns a string literal value if the given attribute is `value`
/// attribute or `None` otherwise.
pub fn config_value(attr: &syn::Attribute) -> Option<String> {
Expand All @@ -38,13 +45,25 @@ pub fn is_config_value(attr: &syn::Attribute) -> bool {
is_attr_name_value(attr, "value")
}

/// Returns `true` if the given attribute is an `unstable` attribute.
pub fn is_unstable_variant(attr: &syn::Attribute) -> bool {
is_attr_path(attr, "unstable_variant")
}

fn is_attr_name_value(attr: &syn::Attribute, name: &str) -> bool {
attr.parse_meta().ok().map_or(false, |meta| match meta {
syn::Meta::NameValue(syn::MetaNameValue { ref path, .. }) if path.is_ident(name) => true,
_ => false,
})
}

fn is_attr_path(attr: &syn::Attribute, name: &str) -> bool {
attr.parse_meta().ok().map_or(false, |meta| match meta {
syn::Meta::Path(path) if path.is_ident(name) => true,
_ => false,
})
}

fn get_name_value_str_lit(attr: &syn::Attribute, name: &str) -> Option<String> {
attr.parse_meta().ok().and_then(|meta| match meta {
syn::Meta::NameValue(syn::MetaNameValue {
Expand Down
40 changes: 37 additions & 3 deletions config_proc_macro/src/item_enum.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use proc_macro2::TokenStream;
use quote::quote;
use quote::{quote, quote_spanned};
use syn::spanned::Spanned;

use crate::attrs::*;
use crate::utils::*;
Expand Down Expand Up @@ -47,25 +48,50 @@ fn process_variant(variant: &syn::Variant) -> TokenStream {
let metas = variant
.attrs
.iter()
.filter(|attr| !is_doc_hint(attr) && !is_config_value(attr));
.filter(|attr| !is_doc_hint(attr) && !is_config_value(attr) && !is_unstable_variant(attr));
let attrs = fold_quote(metas, |meta| quote!(#meta));
let syn::Variant { ident, fields, .. } = variant;
quote!(#attrs #ident #fields)
}

/// Return the correct syntax to pattern match on the enum variant, discarding all
/// internal field data.
fn fields_in_variant(variant: &syn::Variant) -> TokenStream {
// With thanks to https://stackoverflow.com/a/65182902
match &variant.fields {
syn::Fields::Unnamed(_) => quote_spanned! { variant.span() => (..) },
syn::Fields::Unit => quote_spanned! { variant.span() => },
syn::Fields::Named(_) => quote_spanned! { variant.span() => {..} },
}
}

fn impl_doc_hint(ident: &syn::Ident, variants: &Variants) -> TokenStream {
let doc_hint = variants
.iter()
.map(doc_hint_of_variant)
.collect::<Vec<_>>()
.join("|");
let doc_hint = format!("[{}]", doc_hint);

let variant_stables = variants
.iter()
.map(|v| (&v.ident, fields_in_variant(&v), !unstable_of_variant(v)));
let match_patterns = fold_quote(variant_stables, |(v, fields, stable)| {
quote! {
#ident::#v #fields => #stable,
}
});
quote! {
use crate::config::ConfigType;
impl ConfigType for #ident {
fn doc_hint() -> String {
#doc_hint.to_owned()
}
fn stable_variant(&self) -> bool {
match self {
#match_patterns
}
}
}
}
}
Expand Down Expand Up @@ -123,13 +149,21 @@ fn impl_from_str(ident: &syn::Ident, variants: &Variants) -> TokenStream {
}

fn doc_hint_of_variant(variant: &syn::Variant) -> String {
find_doc_hint(&variant.attrs).unwrap_or(variant.ident.to_string())
let mut text = find_doc_hint(&variant.attrs).unwrap_or(variant.ident.to_string());
if unstable_of_variant(&variant) {
text.push_str(" (unstable)")
};
text
}

fn config_value_of_variant(variant: &syn::Variant) -> String {
find_config_value(&variant.attrs).unwrap_or(variant.ident.to_string())
}

fn unstable_of_variant(variant: &syn::Variant) -> bool {
any_unstable_variant(&variant.attrs)
}

fn impl_serde(ident: &syn::Ident, variants: &Variants) -> TokenStream {
let arms = fold_quote(variants.iter(), |v| {
let v_ident = &v.ident;
Expand Down
1 change: 1 addition & 0 deletions config_proc_macro/tests/smoke.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
pub mod config {
pub trait ConfigType: Sized {
fn doc_hint() -> String;
fn stable_variant(&self) -> bool;
}
}

Expand Down
90 changes: 74 additions & 16 deletions src/config/config_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@ pub(crate) trait ConfigType: Sized {
/// Returns hint text for use in `Config::print_docs()`. For enum types, this is a
/// pipe-separated list of variants; for other types it returns "<type>".
fn doc_hint() -> String;

/// Return `true` if the variant (i.e. value of this type) is stable.
///
/// By default, return true for all values. Enums annotated with `#[config_type]`
/// are automatically implemented, based on the `#[unstable_variant]` annotation.
fn stable_variant(&self) -> bool {
true
}
}

impl ConfigType for bool {
Expand Down Expand Up @@ -51,6 +59,13 @@ impl ConfigType for IgnoreList {
}

macro_rules! create_config {
// Options passed in to the macro.
//
// - $i: the ident name of the option
// - $ty: the type of the option value
// - $def: the default value of the option
// - $stb: true if the option is stable
// - $dstring: description of the option
($($i:ident: $ty:ty, $def:expr, $stb:expr, $( $dstring:expr ),+ );+ $(;)*) => (
#[cfg(test)]
use std::collections::HashSet;
Expand All @@ -61,9 +76,12 @@ macro_rules! create_config {
#[derive(Clone)]
#[allow(unreachable_pub)]
pub struct Config {
// For each config item, we store a bool indicating whether it has
// been accessed and the value, and a bool whether the option was
// manually initialised, or taken from the default,
// For each config item, we store:
//
// - 0: true if the value has been access
// - 1: true if the option was manually initialized
// - 2: the option value
// - 3: true if the option is unstable
$($i: (Cell<bool>, bool, $ty, bool)),+
}

Expand Down Expand Up @@ -143,18 +161,13 @@ macro_rules! create_config {

fn fill_from_parsed_config(mut self, parsed: PartialConfig, dir: &Path) -> Config {
$(
if let Some(val) = parsed.$i {
if self.$i.3 {
if let Some(option_value) = parsed.$i {
let option_stable = self.$i.3;
if $crate::config::config_type::is_stable_option_and_value(
stringify!($i), option_stable, &option_value
) {
self.$i.1 = true;
self.$i.2 = val;
} else {
if crate::is_nightly_channel!() {
self.$i.1 = true;
self.$i.2 = val;
} else {
eprintln!("Warning: can't set `{} = {:?}`, unstable features are only \
available in nightly channel.", stringify!($i), val);
}
self.$i.2 = option_value;
}
}
)+
Expand Down Expand Up @@ -221,12 +234,22 @@ macro_rules! create_config {
match key {
$(
stringify!($i) => {
self.$i.1 = true;
self.$i.2 = val.parse::<$ty>()
let option_value = val.parse::<$ty>()
.expect(&format!("Failed to parse override for {} (\"{}\") as a {}",
stringify!($i),
val,
stringify!($ty)));

// Users are currently allowed to set unstable
// options/variants via the `--config` options override.
//
// There is ongoing discussion about how to move forward here:
// https://github.com/rust-lang/rustfmt/pull/5379
//
// For now, do not validate whether the option or value is stable,
// just always set it.
self.$i.1 = true;
self.$i.2 = option_value;
}
)+
_ => panic!("Unknown config key in override: {}", key)
Expand Down Expand Up @@ -424,3 +447,38 @@ macro_rules! create_config {
}
)
}

pub(crate) fn is_stable_option_and_value<T>(
option_name: &str,
option_stable: bool,
option_value: &T,
) -> bool
where
T: PartialEq + std::fmt::Debug + ConfigType,
{
let nightly = crate::is_nightly_channel!();
let variant_stable = option_value.stable_variant();
match (nightly, option_stable, variant_stable) {
// Stable with an unstable option
(false, false, _) => {
eprintln!(
"Warning: can't set `{} = {:?}`, unstable features are only \
available in nightly channel.",
option_name, option_value
);
false
}
// Stable with a stable option, but an unstable variant
(false, true, false) => {
eprintln!(
"Warning: can't set `{} = {:?}`, unstable variants are only \
available in nightly channel.",
option_name, option_value
);
false
}
// Nightly: everything allowed
// Stable with stable option and variant: allowed
(true, _, _) | (false, true, true) => true,
}
}
Loading