plugin-php icon indicating copy to clipboard operation
plugin-php copied to clipboard

false, true and null case collision with phpcs

Open alechko opened this issue 5 years ago • 7 comments

phpcs using Drupal coding standards require for FALSE, TRUE and NULL to be uppercase but prettier formats them to lowercase. phpcs error message: TRUE, FALSE and NULL must be uppercase; expected "FALSE" but found "false"

[email protected]

prettier/[email protected]

Input:

  $vars = [
    'account' => $account,
    'manage_access' => FALSE,
  ];

Output:

  $vars = [
    'account' => $account,
    'manage_access' => false,
  ];

Expected behavior:

  $vars = [
    'account' => $account,
    'manage_access' => FALSE,
  ];

Prettier output:

["INFO" - 9:51:20 AM] Detected local configuration (i.e. .prettierrc or .editorconfig), VS Code configuration will not be used
["INFO" - 9:51:20 AM] Using config file at '.../.prettierrc'
["INFO" - 9:51:20 AM] Prettier Options:
{
  "filepath": "...",
  "parser": "php",
  "useTabs": false,
  "tabWidth": 2,
  "endOfLine": "lf",
  "braceStyle": "1tbs",
}
["INFO" - 9:51:20 AM] Formatting completed in 238.064488ms.

alechko avatar Oct 07 '20 06:10 alechko

IIRC prettier-php mostly follows the PSR-12 coding standard which asks for PHP reserved keywords to be lower case: https://www.php-fig.org/psr/psr-12/#25-keywords-and-types

maybe you could use a "cs-fixer" that would be applied after prettier?

nicoder avatar Oct 07 '20 17:10 nicoder

I think you are correct about the PSR-12, and I think I've tried using multiple formatters once, but the code formatting was a bit chaotic, there was no consistency and the extensions were "fighting" one another. That's why I've actually decided to go with prettier, to avoid having multiple extensions.

I think that if there was an option to specify a specific cs like PSR-2 or Drupal or whatever, or for example to have an uppercase setting for these - the same way we have the inline opening brace for a function:

"braceStyle": "1tbs" to "uppercaseConstants": "true"

This could be much better solution then trying to install multiple cs formatters. Otherwise you cannot really use prettier-php for some projects, which is a shame...

alechko avatar Oct 07 '20 17:10 alechko

Hi @alechko, we're trying to follow the prettier option philosophy which means that we only consider adding options for the most critical cases. I didn't do any analysis on this, but my feeling is that uppercase constants is something that is not very common in the PHP community.

czosel avatar Oct 07 '20 21:10 czosel

@czosel I totally get it, however, if you could consider making prettier-php follow a different coding standard for a project, that would be great. I really love that I have only one code formatter for any language out there, and I believe that code standards are changing constantly, lots of projects using different cs and adapting their own cs, so that might be handy in the future. However, I totally understand if that's not possible for you guys, and I will use a different formatter for this project.

thanks you either way! :)

alechko avatar Oct 08 '20 03:10 alechko

Much better to lobby Drupal to adopt PSR-12 as the base for their standard. All-uppercase booleans make me think of my PHP3 days. This is not an option that should be added to Prettier.

hackel avatar Dec 15 '20 17:12 hackel

to be honest, i agree

alexander-akait avatar Dec 15 '20 18:12 alexander-akait

@hackel While personally I agree with you about this (uppercase variables etc.) being ugly and old, I was simply trying to prevent from working with multiple code formatting plugins, prettier is the best of any I've used and that's why I tried to see if it's possible to take cs configuration into consideration. I was explained that it's not, and that's perfectly fine!

If anyone else bothered with this issue, take a look at PHP Sniffer & Beautifier which works OK, but is customizable by any cs configuration. It's nowhere near as good as prettier, but it gets the job done :)

alechko avatar Dec 19 '20 08:12 alechko