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

Formatting for calls with multiline string arguments #2876

Open
RReverser opened this issue Jul 30, 2018 · 8 comments
Open

Formatting for calls with multiline string arguments #2876

RReverser opened this issue Jul 30, 2018 · 8 comments

Comments

@RReverser
Copy link
Contributor

Currently code passing a multiline string argument to a function will be reformatted to a multiline call no matter whether it needs it or not. Example using unindent crate:

fn main() {
    assert_eq!(code, unindent(r#"
        def hello():
            print("Hello, world!")
        
        hello()
    "#));
}

->

fn main() {
    assert_eq!(
        code,
        unindent(
            r#"
        def hello():
            print("Hello, world!")
        
        hello()
    "#
        )
    );
}

You can see how it becomes hard to track the beginning / end of the string literal and surrounding call due to extra line breaks with varying indentation.

Same happens when using an indoc! macro instead:

fn main() {
    assert_eq!(code, indoc!(r#"
        def hello():
            print("Hello, world!")
        
        hello()
    "#));
}

->

fn main() {
    assert_eq!(
        code,
        indoc!(
            r#"
        def hello():
            print("Hello, world!")
        
        hello()
    "#
        )
    );
}

Reformatting that takes place in these examples kind of defeats the purpose of using indoc! in the first place which is to prettify multiline string literals in code while preserving indentation.

I think it would be possible to special-case indoc! as Rustfmt already does for few other well-known macros, but maybe instead it would be better to fix this issue in general? For example, I like what happens to "multiline" struct literals in same argument position much more:

fn main() {
    assert_eq!(s, wrap(A {
        x: 10,
        y: 20,
        z: 30,
    }));
}

->

fn main() {
    assert_eq!(
        s,
        wrap(A {
            x: 10,
            y: 20,
            z: 30,
        })
    );
}

I understand that their handling is different, as structs can be also reformatted to fit on same line, but still I wonder if it would be possible / make sense to apply same logic to strings and keep beginning and end quotes near their corresponding parentheses of the surrounding call?

cc @dtolnay as author of mentioned crates

@nrc
Copy link
Member

nrc commented Jul 30, 2018

This is kind of intentional, but not really - string literals are not 'combining' items (like, e.g., struct literals) so won't be put on the same line as the surrounding function call/macro use. We also refuse to reformat string literals because whitespace can be significant. When formatting function calls, we use the multiline form if any argument is multiline unless there is a single argument and it 'combine's.

It might be possible to reformat the string literal after formatting to make this look better? It might be possible to make string literals 'combine' - I'm not sure if this would have any negative effects without trying it. If none of that works, then we could special-case the macro, but we don't have a policy for when to do that (I think we currently only do that with std macros or at least those from the rust-lang org) and special-casing every macro that needs it is not a long-term solution (I don't know what the long-term solution is, but we'll be thinking about that post-1.0).

@RReverser
Copy link
Contributor Author

Changing indentation of string literal after formatting doesn't really make it look better IMO, and I'm definitely not suggesting reenabling string formatting, but rather interested in the latter option - treating it as 'combine' so that it sticks to the surrounding parentheses like other multiline items.

I'm not sure either what negative effects of that could be if any, and whether such combine treatment would need to be limited just to multiline strings or any, but from the first glance it doesn't seem to be too risky and worth trying under a flag?

@RReverser
Copy link
Contributor Author

Given that it's also not just macro but also function calls, I suggested special casing mostly as a fallback option rather than preferred treatment, and ideally would rather see formatting fixed consistently in different positions if feasible.

@RReverser
Copy link
Contributor Author

It might be possible to reformat the string literal after formatting to make this look better?

Sorry, I just got what you meant, and yeah, it does seem possible, although rather inconvenient to keep in sync with surrounding formatting in long-term.

@thomcc
Copy link
Member

thomcc commented Mar 22, 2019

I've hit this a lot in code that uses e.g. rusqlite, and would prefer an option that didn't involve special casing one crate's macros (it happens on normal function calls too when using that crate).

Making them work more like struct literals, even if only applied to raw string literals, seems ideal to me.

@jhpratt
Copy link
Member

jhpratt commented Jun 3, 2020

Running into this myself with regard to SQL queries.

Depending on how difficult this is, I'd be interested in a PR. FWIW I don't think this should only be kept on the same line in macro invocations, though.

@matklad
Copy link
Member

matklad commented Jun 10, 2020

Horrible work-around: allow leading comma in the macro definition:

#[macro_export]
macro_rules! expect {
    [$(,)?$data:literal] => {
        $crate::Expectation {
            file: $crate::__rt::file!(), line: $crate::__rt::line!(), data: $data
        }
    };
}

#[test]
fn foo() {
    let expectation = expect![,r#"
        64..84 '{     ...1 }; }': ()
        70..81 'SS { x: 1 }': S<u32>
        78..79 '1': u32
    "#];

    eprintln!("{:?}", expectation);
}

@ytmimi
Copy link
Contributor

ytmimi commented Jul 19, 2022

This is still reproducible with rustfmt 1.5.1-nightly (a7bf0090 2022-07-17)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants