prettier / plugin-php

Prettier PHP Plugin
https://loilo.github.io/prettier-php-playground/
MIT License
1.75k stars 129 forks source link

PSR-12 compliant function call wrapping #26

Closed czosel closed 6 years ago

czosel commented 6 years ago

Quoting @nicoder:

As far as I can tell, both are PSR-2-compliant, and PSR-12 adds precisions because there was debate about that:

https://github.com/php-fig/fig-standards/blob/master/proposed/extended-coding-style-guide.md#47-method-and-function-calls

for example PSR-12 lists this is valid:

$app->get('/hello/{name}', function ($name) use ($app) {
    return 'Hello ' . $app->escape($name);
});

but this would not be :

$app->get('/hello/{name}',
    function ($name) use ($app) {
        return 'Hello ' . $app->escape($name);
    }
);

I understand it this way:

  • if one argument is defined starting on a new line, then all of the arguments must start on their own line
  • but arguments may take up more than one line (independently of whether they start on their own line or not)

For example it may be nicer to see:

foo([
   $longExpression,
   $prettyLongExpression,   
]);

than:

foo(
    [
       $longExpression,
       $prettyLongExpression,   
    ]
);

Or nicer to see:

$db->Execute($sql, [
    $foo,
    $somewhatLongParameter,
]);

than:

$db->Execute(
    $sql,
    [
        $foo,
        $somewhatLongParameter,
    ]
);
MichaelDeBoey commented 6 years ago

So the example in #23:

$this->something->method($argument, $this->more->stuff($this->even->more->things->complicatedMethod()));

Should become

$this->something->method($argument, $this->more->stuff(
  $this->even->more->things->complicatedMethod()
));

I think 🙂

czosel commented 6 years ago

@MichaelDeBoey yes, i think so too! Do you want to give this a shot? 😉

MichaelDeBoey commented 6 years ago

@czosel I'm not really into the codebase 😕

I'm just here because I'd like to use prettier for both my front- and backend in a Laravel project actually... 🙂

czosel commented 6 years ago

Alright, no worries 🙂 If you change your mind, i'd recomment to take a look here and otherwise you can also always ask me!

mathieutu commented 6 years ago

An other example here. I think it can be related to this issue:

    {
-        return str_replace_array('#', [
-            $this->budget->name,
-            $this->requester->name,
-            'justif',
-        ], '# - # - #');
+        return str_replace_array(
+            '#',
+            [$this->budget->name, $this->requester->name, 'justif'],
+            '# - # - #'
+        );
     }
mgrip commented 6 years ago

So I started looking into this but some of the examples outline above get pretty tricky. For reference, this is how prettier (js) handles it

https://prettier.io/playground/#N4Igxg9gdgLgprEAuEBDADugdAczjACgHIiAaAAgIBIpUBbOASnIF4A+c4cgJ3wFduUckQAScADbiIwgNzkAvoxkAdKKozY8hEhWq0Gzdpx79BwsZOlE58ikXgBnGESWr1mXPmK9UkgJ4+-oHiAXC+IcF+UlA4ZJQ09EysHFy8MAJCohJSsgquarQeWgSOMBQA2vZwTnFVNXaltY0N1c4t9cLNna1NPQC6+e6aXqUVdW3dHeO9U13T7RPzk859FKWDhcOEo+SVc-s9CzOLB7OHy8dEq+TrMiCkIBDoMACW0A7IoKjc3BAA7gAFb4ID4oVAANwgLwAJvc0E5kAAzXwOOAPABG3FQYAA1vgAMrobEvGLIGDcPhokDQiBgJEoqkk1HcGAArE4OioeniVEPABWDgAHgAhLG4gmJAAyJLg3N5jz4MHQioATHKqUTuMzkCB0ah0VFoHD0NwSTAAOowmAAC2QAA4AAwPE0QVHmrHoHUm6pwbjg2UPXgARz4L14bNQHK5SGRPKpqLoLzJFPjJJw4jgAEU+BB4OqHjB9ZboTbkCqC1iXuI0wBhCB0Tk6qDQAMgPiogAq+tBsdR8nkQA

mathieutu commented 6 years ago

So, for me, your examples are:

Do you confirm?

czosel commented 6 years ago

@mgrip i also tried to implement this in #123 based on the JS implementation, but i didn't arrive at a proper solution yet. You might want to take a look :smile:

mgrip commented 6 years ago

Ahhh I was wondering if we would have to use that conditionalGroup - the docs say to use it as a last resort which scared me off haha

mgrip commented 6 years ago

@mathieutu yes I agree - I'm wondering if its actually going to be possible to pretty-print in the way that PSR12 is dictating. Wanted to just give examples of how prettier-js does it for reference

czosel commented 6 years ago

Most of the examples listed in this issue should be fixed by #123 - let's open new issues for anything that might be missing.