google / yapf

A formatter for Python files
Apache License 2.0
13.8k stars 892 forks source link

Bad formatting in long function arguments with conditional expression #730

Open sangwoo-joh opened 5 years ago

sangwoo-joh commented 5 years ago

Hi!

I have a code that calls with complex arguments, and some of them have conditional expression. For example, the code below is the minimal exploit that show this issue:

def foo():
    return call.me.by.dot(
        key1=val1,
        key2=val2,
        key3=self._val_helper(
            other.helper(
                val1,
                keykey1=val2,
                keykey2=val3,
                keykey3=val4,
                keykey4=val5,
                keykeykey=val3) if self._long_long_foo() else ()))

For the exploit, I ran yapf with --style='{based_on_style=pep8, column_limit=50}' option.

Since the self._long_long_foo() exceeds the column limit, I thought that whole self._long_long_foo() should be wrapped after formatting, but it wasn't:

def foo():
    return call.me.by.dot(
        key1=val1,
        key2=val2,
        key3=self._val_helper(
            other.helper(val1,
                         keykey1=val2,
                         keykey2=val3,
                         keykey3=val4,
                         keykey4=val5,
                         keykeykey=val3) if self.
            _long_long_foo() else ()))

I think it is better to format not to wrap before the dot in self., but to wrap before if, so that the reformatted code should look at least like:

def foo():
    return call.me.by.dot(
        key1=val1,
        key2=val2,
        key3=self._val_helper(
            other.helper(
                val1,
                keykey1=val2,
                keykey2=val3,
                keykey3=val4,
                keykey4=val5,
                keykeykey=val3) if
            self._long_long_foo() else ()))

or, for the best,

def foo():
    return call.me.by.dot(
        key1=val1,
        key2=val2,
        key3=self._val_helper(
            other.helper(
                val1,
                keykey1=val2,
                keykey2=val3,
                keykey3=val4,
                keykey4=val5,
                keykeykey=val3)
            if self._long_long_foo() else ()))

What do you think about this? It would be grateful if you consider this issue. Thank you!

priyansh19 commented 5 years ago

I think this error will simply be rectified by adding the "." operator in the traversal function of this code.