Wrong indentation in argument of method when followed by chained method (promises)

[Lakitna]

Description

When dealing with chaining there are a lot of opinions. But I, and the folks on Gitter, think that this is a bug.

Input

The code looked like this before beautification:

new Promise((resolve, reject) => {
    setTimeout(resolve, 100);
})
    .then(() => {});

Expected Output

The code should have looked like this after beautification (unchanged):

new Promise((resolve, reject) => {
    setTimeout(resolve, 100);
})
    .then(() => {});

Actual Output

The code actually looked like this after beautification (extra indentation on line 2 & 3):

new Promise((resolve, reject) => {
        setTimeout(resolve, 100);
    })
    .then(() => {});

Steps to Reproduce

Run Beautify on the code above

Environment

OS: Windows 10
Runtime env: VSCode

Settings

{
    "brace_style": "collapse",
    "break_chained_methods": true,
    "e4x": false,
    "end_with_newline": true,
    "indent_char": " ",
    "indent_level": 0,
    "indent_size": 4,
    "indent_with_tabs": false,
    "jslint_happy": false,
    "keep_array_indentation": false,
    "keep_function_indentation": false,
    "max_preserve_newlines": 0,
    "preserve_newlines": true,
    "space_after_anon_function": false,
    "space_before_conditional": true,
    "space_in_empty_paren": false,
    "space_in_paren": false,
    "unescape_strings": false,
    "wrap_line_length": 80
}

[bitwiseman]

This is an interesting problem.

Note, the following works as one would expect:

new Promise((resolve, reject) => {
    setTimeout(resolve, 100);
}).then(() => {});

The newline between the }) and .then( is making the beautifier treat the contents inside }) as multiline. It's not clear if that is wrong.

Technically, the expression is continuing across multiple lines, so the indenting as it stands makes sense. But I can also see where adding a newline after the }) should not change the indentation inside the parenthesis.

I'd forgotten, but back in #621 someone requested something similar to this.

The more common request is:

new Promise((resolve, reject) => {
    setTimeout(resolve, 100);
})
.then(() => {});

I'd expect unindent_chained_methods to produce the above, but instead it currently does this:

new Promise((resolve, reject) => {
        setTimeout(resolve, 100);
    })
.then(() => {});

[bitwiseman]

@HanabishiRecca said:

@bitwiseman, sorry for no explanation about this. The change is not related only to the dot. Problem is really deeper. I've traced behavior and found a bunch of problems with deindention, not with dot only. There is wrong behavior with not closed block statements within an expression.

For example:
(unindent_chained_methods is ON)

getNum(() => {
    return 10;
}) -
5;

Expected output:

getNum(() => {
    return 10;
}) -
5;

Actual output:

getNum(() => {
        return 10;
    }) -
    5;

So yes, my fix is global and works in all cases, not only for dots. I can understand your nervous because this is pretty not obvious and complicated :)

Yes, there is a general bug but it is a little hard to nail down.

Let's expand this slightly.

Similar to what I said above this remains unchanged:

getNum(() => {
    return 15;
}) + getNum(() => {
    return 10;
}) - 5;

So does this:

getNum(() => {
    return 15;
}) + getNum(() => {
    return 10;
}) - (
    5
);

But (for the same reason as in this issue) the following indents:

// A1
getNum(() => {
        return 15;
    }) + getNum(() => {
        return 10;
    }) - 
    5;

The is because the Statement getNum() .... ; became multi-liine and needs to be indented for clarity.

To be clear, the argument for the following indentation also has validity:

// A2
getNum(() => {
    return 15;
}) + getNum(() => {
    return 10;
}) - 
    5;

But i don't see how this is valid:

// A3
getNum(() => {
    return 15;
}) + getNum(() => {
    return 10;
}) - 
5;

What should this look like?

// B1
getNum(() => {
        return 15;
    }) + getNum(() => {
        return 10;
    }) - 
    getNum(() => {
        return 15;
    }) + getNum(() => {
        return 10;
    }) - 
    5;

// B2 ?
getNum(() => {
    return 15;
}) + getNum(() => {
    return 10;
}) - 
    getNum(() => {
        return 15;
    }) + getNum(() => {
        return 10;
    }) -
    5;

[HanabishiRecca]

I can just suggest variants:
A) Rename unindent_chained_methods to something like unindent_multiline_statements and apply my previous fix. (Pretty legit solution I think + user can control it.)
B) Force }) to be recognized as the end of statement. (Is there any possible conflicts?)

[bitwiseman]

Similar if not duplicate of #886.