-
Notifications
You must be signed in to change notification settings - Fork 924
feat: add skip_macro_invocations option #5347
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
Changes from all commits
d2422de
cc9fb29
342925a
3b60f3e
c4f84a8
a88ec63
89ec6d1
bbce6ee
4bca420
d4d5381
7fd8be2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,118 @@ | ||
//! This module contains types and functions to support formatting specific macros. | ||
|
||
use itertools::Itertools; | ||
use std::{fmt, str}; | ||
|
||
use serde::{Deserialize, Serialize}; | ||
use serde_json as json; | ||
use thiserror::Error; | ||
|
||
/// Defines the name of a macro. | ||
#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd, Deserialize, Serialize)] | ||
pub struct MacroName(String); | ||
|
||
impl MacroName { | ||
pub fn new(other: String) -> Self { | ||
Self(other) | ||
} | ||
} | ||
|
||
impl fmt::Display for MacroName { | ||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
self.0.fmt(f) | ||
} | ||
} | ||
|
||
impl From<MacroName> for String { | ||
fn from(other: MacroName) -> Self { | ||
other.0 | ||
} | ||
} | ||
|
||
/// Defines a selector to match against a macro. | ||
#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd, Deserialize, Serialize)] | ||
pub enum MacroSelector { | ||
Name(MacroName), | ||
All, | ||
} | ||
|
||
impl fmt::Display for MacroSelector { | ||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
match self { | ||
Self::Name(name) => name.fmt(f), | ||
Self::All => write!(f, "*"), | ||
} | ||
} | ||
} | ||
|
||
impl str::FromStr for MacroSelector { | ||
type Err = std::convert::Infallible; | ||
|
||
fn from_str(s: &str) -> Result<Self, Self::Err> { | ||
Ok(match s { | ||
"*" => MacroSelector::All, | ||
name => MacroSelector::Name(MacroName(name.to_owned())), | ||
}) | ||
} | ||
} | ||
|
||
/// A set of macro selectors. | ||
#[derive(Clone, Debug, Default, PartialEq, Deserialize, Serialize)] | ||
pub struct MacroSelectors(pub Vec<MacroSelector>); | ||
|
||
impl fmt::Display for MacroSelectors { | ||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
write!(f, "{}", self.0.iter().format(", ")) | ||
} | ||
} | ||
|
||
#[derive(Error, Debug)] | ||
pub enum MacroSelectorsError { | ||
#[error("{0}")] | ||
Json(json::Error), | ||
} | ||
|
||
// This impl is needed for `Config::override_value` to work for use in tests. | ||
impl str::FromStr for MacroSelectors { | ||
type Err = MacroSelectorsError; | ||
|
||
fn from_str(s: &str) -> Result<Self, Self::Err> { | ||
let raw: Vec<&str> = json::from_str(s).map_err(MacroSelectorsError::Json)?; | ||
Ok(Self( | ||
raw.into_iter() | ||
.map(|raw| { | ||
MacroSelector::from_str(raw).expect("MacroSelector from_str is infallible") | ||
}) | ||
.collect(), | ||
)) | ||
} | ||
} | ||
|
||
#[cfg(test)] | ||
mod test { | ||
use super::*; | ||
use std::str::FromStr; | ||
|
||
#[test] | ||
fn macro_names_from_str() { | ||
let macro_names = MacroSelectors::from_str(r#"["foo", "*", "bar"]"#).unwrap(); | ||
assert_eq!( | ||
macro_names, | ||
MacroSelectors( | ||
[ | ||
MacroSelector::Name(MacroName("foo".to_owned())), | ||
MacroSelector::All, | ||
MacroSelector::Name(MacroName("bar".to_owned())) | ||
] | ||
.into_iter() | ||
.collect() | ||
) | ||
); | ||
} | ||
|
||
#[test] | ||
fn macro_names_display() { | ||
let macro_names = MacroSelectors::from_str(r#"["foo", "*", "bar"]"#).unwrap(); | ||
assert_eq!(format!("{}", macro_names), "foo, *, bar"); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ use rustc_ast_pretty::pprust; | |
/// by other context. Query this context to know if you need skip a block. | ||
#[derive(Default, Clone)] | ||
pub(crate) struct SkipContext { | ||
pub(crate) all_macros: bool, | ||
macros: Vec<String>, | ||
Comment on lines
+10
to
11
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't really like this structure, I think it should be refactored into something like enum SkipMacroContext {
All,
Named(HashSet<MacroName>),
}
pub(crate) struct SkipContext {
macros: SkipMacroContext,
attributes: HashSet<String>,
} but thought that was probably out of scope for this PR. Happy to open a followup afterwards, or include as an additional commit if you think it's okay to add here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the PR is in a pretty good state and I'm leaning towards making this change in a follow up PR. If you feel strongly about it though and don't think it will add a lot of extra work to the current PR, then I'll leave that call up to you |
||
attributes: Vec<String>, | ||
} | ||
|
@@ -23,8 +24,15 @@ impl SkipContext { | |
self.attributes.append(&mut other.attributes); | ||
} | ||
|
||
pub(crate) fn update_macros<T>(&mut self, other: T) | ||
where | ||
T: IntoIterator<Item = String>, | ||
{ | ||
self.macros.extend(other.into_iter()); | ||
} | ||
|
||
pub(crate) fn skip_macro(&self, name: &str) -> bool { | ||
self.macros.iter().any(|n| n == name) | ||
self.all_macros || self.macros.iter().any(|n| n == name) | ||
} | ||
|
||
pub(crate) fn skip_attribute(&self, name: &str) -> bool { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
// rustfmt-skip_macro_invocations: ["*"] | ||
|
||
// Should skip this invocation | ||
items!( | ||
const _: u8 = 0; | ||
); | ||
|
||
// Should skip this invocation | ||
renamed_items!( | ||
const _: u8 = 0; | ||
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
// rustfmt-skip_macro_invocations: ["*","items"] | ||
|
||
// Should skip this invocation | ||
items!( | ||
const _: u8 = 0; | ||
); | ||
|
||
// Should also skip this invocation, as the wildcard covers it | ||
renamed_items!( | ||
const _: u8 = 0; | ||
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
// rustfmt-skip_macro_invocations: [] | ||
|
||
// Should not skip this invocation | ||
items!( | ||
const _: u8 = 0; | ||
); | ||
|
||
// Should not skip this invocation | ||
renamed_items!( | ||
const _: u8 = 0; | ||
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
// rustfmt-skip_macro_invocations: ["items"] | ||
|
||
// Should skip this invocation | ||
items!( | ||
const _: u8 = 0; | ||
); | ||
|
||
// Should not skip this invocation | ||
renamed_items!( | ||
const _: u8 = 0; | ||
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
// rustfmt-skip_macro_invocations: ["unknown"] | ||
|
||
// Should not skip this invocation | ||
items!( | ||
const _: u8 = 0; | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also wondering if we could update the help text here to be a little more descriptive. Something like:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps this for the opening line?