Skip to content
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

macro fails to be wrapped if it is exactly one character beyond max_width #3805

Closed
AaronKutch opened this issue Sep 23, 2019 · 2 comments · Fixed by #5582
Closed

macro fails to be wrapped if it is exactly one character beyond max_width #3805

AaronKutch opened this issue Sep 23, 2019 · 2 comments · Fixed by #5582
Labels
a-macros help wanted only-with-option requires a non-default option value to reproduce

Comments

@AaronKutch
Copy link

macro_rules! test {
    ($asdfghj:expr, $qwertyuiop:expr, $zxcvbnmasdfghjkl:expr, $aeiouaeiouaeiou:expr, $add:expr) => {{
        return;
    }};
}

Using rustfmt 1.4.8 nightly with a .rustfmt.toml with the following:

unstable_features = true
error_on_line_overflow = true
error_on_unformatted = true
#format_macro_matchers = true

If this is manually formatted, overflow will not happen. However, if the format_macro_matchers = true line is uncommented, the line will always be reformatted to overflow until another character is added.

@AaronKutch AaronKutch changed the title macro fails to be wrapped if it is one character beyond max_width macro fails to be wrapped if it is exactly one character beyond max_width May 2, 2020
@ytmimi ytmimi added the only-with-option requires a non-default option value to reproduce label Jul 20, 2022
@ytmimi
Copy link
Contributor

ytmimi commented Jul 20, 2022

Confirming I can reproduce the issue with rustfmt 1.5.1-nightly (f2c31ba0 2022-07-19). rustfmt does not format the given input when format_macro_matchers is true even though it exceeds the max_width of 100.

input (length of the macro matcher line including indentation is 101)

macro_rules! test {
    ($asdfghj:expr, $qwertyuiop:expr, $zxcvbnmasdfghjkl:expr, $aeiouaeiouaeiou:expr, $add:expr) => {{
        return;
    }};
}

output format_macro_matchers=true (lunchanged)

macro_rules! test {  
    ($asdfghj:expr, $qwertyuiop:expr, $zxcvbnmasdfghjkl:expr, $aeiouaeiouaeiou:expr, $add:expr) => {{
        return;
    }};
}

When the macro_matcher is 102 characters long, then it wraps. I assume we're not taking the length of the 2nd { into account.

input (length of the macro matcher line including indentation is 102)

macro_rules! test {
    ($aasdfghj:expr, $qwertyuiop:expr, $zxcvbnmasdfghjkl:expr, $aeiouaeiouaeiou:expr, $add:expr) => {{
        return;
    }};
}

output format_macro_matchers=true (wrapped)

macro_rules! test {
    (
        $aasdfghj:expr, $qwertyuiop:expr, $zxcvbnmasdfghjkl:expr, $aeiouaeiouaeiou:expr, $add:expr
    ) => {{
        return;
    }};
}

For anyone who wants to take a look at this I'd start in format_macro_args, and then move on to wrap_macro_args

rustfmt/src/macros.rs

Lines 966 to 987 in f2c31ba

// This is a bit sketchy. The token rules probably need tweaking, but it works
// for some common cases. I hope the basic logic is sufficient. Note that the
// meaning of some tokens is a bit different here from usual Rust, e.g., `*`
// and `(`/`)` have special meaning.
//
// We always try and format on one line.
// FIXME: Use multi-line when every thing does not fit on one line.
fn format_macro_args(
context: &RewriteContext<'_>,
token_stream: TokenStream,
shape: Shape,
) -> Option<String> {
if !context.config.format_macro_matchers() {
let span = span_for_token_stream(&token_stream);
return Some(match span {
Some(span) => context.snippet(span).to_owned(),
None => String::new(),
});
}
let parsed_args = MacroArgParser::new().parse(token_stream)?;
wrap_macro_args(context, &parsed_args, shape)
}

Linking the format_macro_matchers tracking issue #3354

@davidBar-On
Copy link
Contributor

Submitted PR #5582 with a suggested fix.

davidBar-On added a commit to davidBar-On/rustfmt that referenced this issue Feb 6, 2023
ytmimi pushed a commit to davidBar-On/rustfmt that referenced this issue Jan 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-macros help wanted only-with-option requires a non-default option value to reproduce
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants