squizlabs / PHP_CodeSniffer

PHP_CodeSniffer tokenizes PHP files and detects violations of a defined set of coding standards.
BSD 3-Clause "New" or "Revised" License
10.67k stars 1.48k forks source link

prettier with php code sniferer Expected 1 space before "as"; 20 found #3673

Closed snake-py closed 2 years ago

snake-py commented 2 years ago

Describe the bug Hello I am using prettier with the php plugin to format my code and then I am using PHP_CodeSniffer to sniff out non PSR-12 standards. So The issue is that prettier will break below line:

foreach ($client->getContractsByCustomerId($customer["id"]) as $contract) {

into this:

                foreach (
                    $client->getContractsByCustomerId($customer["id"])
                    as $contract
                ) {

Then PHP_CodeSniffer will throw me:

FILE: /app/app/Services/AuthService.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 388 | ERROR | [x] Expected 1 space before "as"; 20 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

I thought this might be a bug of prettier, but they told me that it is not violating the PSR-12 standards to break the line here. https://github.com/prettier/plugin-php/issues/2060 So I wanted to report this here, since this seems to be a bug in the rule set that it is not properly detecting a line break.

gsherwood commented 2 years ago

PSR-12 is pretty light on detail for foreach: https://www.php-fig.org/psr/psr-12/#55-foreach

Noting the "placement of spaces" is why this sniff tries to ensure there is only a single space before the as statement. This part of the standard does not indicate that the expression can be split over multiple lines, as it does for IF/SWITCH/WHILE/FOR.

But the sniff isn't picking up the right indent regardless of PSR-12 because it isn't reading the newline correctly, so there is still a bug here.

snake-py commented 2 years ago

@gsherwood thank you for your feedback. So, WHat do you think how I can solve this issue? Will you build in a flag for foreach to allow line breaks?

TimWolla commented 2 years ago

@snake-py I suggest you raise an issue, seeking clarification in the coding style PER repository: https://github.com/php-fig/per-coding-style/issues

snake-py commented 2 years ago

Okay so it is not allowed to break foreach into multiple lines

jrfnl commented 2 years ago

@snake-py Thanks for reporting back. In that case, I suggest you open an issue for Prettier and the issue here can be closed.

snake-py commented 2 years ago

I had opened the issue over there before I opened here the issue in the first place https://github.com/prettier/plugin-php/issues/2060

It seems, that they are disagreeing and saying prettier does not support formatting to a specific standard. Is there a way to disable this particular rule?

jrfnl commented 2 years ago

@snake-py Well, you could also consider using the build-in phpcbf functionality to format the code (instead of prettier) and then you'd not have any conflicts....

snake-py commented 2 years ago

@jrfnl

I tried phpcbf before I even considered using prettier, however I ran into more trouble than it was helping me. I had often the problem of hitting some odd infinity loops, and some parts not getting really formatted even though they looked really off.

For instance, phpcbf will not format arrays in a proper was and will leave them in states like this:


[
  "test"
   =>
   "test"
,
]

I coded the above from memory, it might be slightly different.

here is how I ran it:

#! /usr/bin/env bash

run_sniffer(){
files=$1  
 if [ "$files" != "" ]
then
  # execute code sniffer on staged files
  echo "Running Code Sniffer..."
  /root/.config/composer/vendor/bin/phpcs --runtime-set ignore_warnings_on_exit true --colors --no-cache --standard=PSR12 --encoding=utf-8  $files
  if [ $? != 0 ]
  then
    echo "Fix the error before commit."
    exit 1
  fi
fi
}

formate_php_code(){
  files=$1  
 if [ "$files" != "" ]
then
  echo "Creating Report ..."
  echo $files
  /root/.config/composer/vendor/bin/phpcs --standard=PSR12 --report-diff=/tmp/changes.diff $files
  contents=$(cat /tmp/changes.diff)
  if [ ${#contents} -gt 0 ]
    then
      echo "Fixing the code..."
      patch -p0 -ui /tmp/changes.diff
      if [ $? != 0 ]
      then
        echo "Something seemed wrong please contact responsible person Fabio"
        exit 1
        else
        # Add back the modified/prettier files to staging
        echo "$files" | xargs git add
        rm /tmp/changes.diff
    fi
  fi
fi
}
jrfnl commented 2 years ago

I tried phpcbf before I even considered using prettier, however I ran into more trouble than it was helping me. I had often the problem of hitting some odd infinity loops, and some parts not getting really formatted even though they looked really off.

PHPCBF will format code based on your ruleset. You mention above that you use PSR12, but PSR12 doesn't contain any rules about the formatting of arrays, which explains why those aren't being formatted.

PHPCS does have some sniffs available to handle array formatting, but you'd need to add those to your ruleset if you want to use them.

If you'd want PSR12 to have rules about array formatting, I suggest you open an issue in the FIG PER repo to discuss that: https://github.com/php-fig/per-coding-style/issues

Re: infinity loops - please always report those if you run into them as those are bugs which need to be fixed. Having said that, I don't know when you first tried using PHPCBF, but bug fixes are continuously made, so those issues may have been solved already.

snake-py commented 2 years ago

Re: infinity loops - please always report those if you run into them as those are bugs which need to be fixed. Having said that, I don't know when you first tried using PHPCBF, but bug fixes are continuously made, so those issues may have been solved already.

I will try to report it when I retry PHPCBF again. For now, we do have a working solution with prettier. I don't think that we are going to switch again.