rust-lang/rustfmt

Incorrect indentation for multiline logical expression

jdm opened this issue · 3 comments

jdm commented

Before:

     // http://tools.ietf.org/html/rfc6265#section-5.1.4
     pub fn path_match(request_path: &str, cookie_path: &str) -> bool {
         // A request-path path-matches a given cookie-path if at least one of
         // the following conditions holds:
         // The cookie-path and the request-path are identical.
         request_path == cookie_path ||

         (request_path.starts_with(cookie_path) && (
             // The cookie-path is a prefix of the request-path, and the last
             // character of the cookie-path is %x2F ("/").
             cookie_path.ends_with("/") ||
             // The cookie-path is a prefix of the request-path, and the first
             // character of the request-path that is not included in the cookie-
             // path is a %x2F ("/") character.
             request_path[cookie_path.len()..].starts_with("/")
         ))
    }

After:

     // http://tools.ietf.org/html/rfc6265#section-5.1.4
     pub fn path_match(request_path: &str, cookie_path: &str) -> bool {
         // A request-path path-matches a given cookie-path if at least one of
         // the following conditions holds:
         // The cookie-path and the request-path are identical.
         request_path == cookie_path ||
            (request_path.starts_with(cookie_path) &&
                (
                    // The cookie-path is a prefix of the request-path, and the last
                    // character of the cookie-path is %x2F ("/").
                    cookie_path.ends_with("/") ||
            // The cookie-path is a prefix of the request-path, and the first
            // character of the request-path that is not included in the cookie-
            // path is a %x2F ("/") character.
            request_path[cookie_path.len()..].starts_with("/")
               ))
    }

It seems like knowledge about the parentheses (opened, closed, count, ...) would be needed to have a nice formatting in this case, which incidentally would also be needed for #3108 as per @nrc on discord.

For example, a formatting like below (mostly wrt closing parentheses) could be ouputted:

     // http://tools.ietf.org/html/rfc6265#section-5.1.4
     pub fn path_match(request_path: &str, cookie_path: &str) -> bool {
         // A request-path path-matches a given cookie-path if at least one of
         // the following conditions holds:
         // The cookie-path and the request-path are identical.
         request_path == cookie_path ||
            (request_path.starts_with(cookie_path) &&
                (
                    // The cookie-path is a prefix of the request-path, and the last
                    // character of the cookie-path is %x2F ("/").
                    cookie_path.ends_with("/") ||
                    // The cookie-path is a prefix of the request-path, and the first
                    // character of the request-path that is not included in the cookie-
                    // path is a %x2F ("/") character.
                    request_path[cookie_path.len()..].starts_with("/")
                )
            )
    }

Maybe that output is possible without making the rewrite-logic aware of parentheses, but I wanted to highlight another use case for it.

nrc commented

It seems like knowledge about the parentheses (opened, closed, count, ...)

I think that knowledge is implicit in the call stack and the Shape used for layout.

This is because of a binary expression with comment in between:

            cookie_path.ends_with("/") ||
            // The cookie-path is a prefix of the request-path, and the first
            // character of the request-path that is not included in the cookie-
            // path is a %x2F ("/") character.
            request_path[cookie_path.len()..].starts_with("/")

rustfmt currently cannot handle this.