As of version 2.0.0 of PHP CodeSniffer, we can now fix errors as well as highlight them, using PHP Code Beautifier and Fixer (PHPCBF). I first noticed the phpcbf command mentioned on the Installing Coder Sniffer documentation page for the Coder module.

I have not yet tested it, but the home page of the Coder module claims that it supports phpcbf as well as phpcs.

I am not sure how to integrate with vim. If phpcbf supports it, people might like to filter a few lines through it, replacing them in the current buffer.

A simpler option is to filter the entire file through phpcbf and then use git and fugitive to compare using vim's diff mode.

A variant on that idea is to create a temporary file using phpcbf and then compare that to the current file using vim's diff mode.

I think that this is going to be awesome!

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rodrigoaguilera’s picture

I like all the ideas:

Provide functions so you can filter the whole file or just a highlighted area just like with '='.
But if you want to know what are the changes applied you need some kind of diff. The ideal I think is to use fugitive

benjifisher’s picture

@rodrigoaguilera:

Provide functions so you can filter the whole file or just a highlighted area just like with '='.

I have been experimenting with phpcbf, and it is designed to work with the whole file. It is also designed to work with a file on disk, overwriting it: that is, it does not work as a filter. There are ways around this: write a few lines to a temp file, apply phpcbf with some sniffs disabled (so that it does not automatically add a file doc block), then read in the result. But all that seems like a high-maintenance kluge. Better to wait for a version of phpcbf that is designed to work that way.

My first experiment was to overwrite the file, then use fugitive's :Gdiff command to compare to the original. It works pretty well, but I prefer not to depend on git + fugitive. Also, I prefer a conservative approach, where you have to review and accept the changes. Especially since phpcbf corrects

/**
 * hook_foo()
 */

to

/**
 * Hook_foo().
 */

Maybe when phpcbf (and the Drupal sniffs designed for it) gets better, I will not want to be so conservative.

As a proof of concept, I added the following lines to my vimrc file:

command! PHPcbf call PHPcbf()
fun! PHPcbf()
  let copy = tempname()
  execute 'w' copy
  call system('/Users/bfisher/codesniffer/vendor/bin/phpcbf --standard=Drupal ' . copy)
  execute 'vertical diffsplit' copy
  wincmd w
endfun

Before you try it, you will have to install PHP CodeSniffer and adjust the path to the phpcbf function, and register the Drupal standards: see https://www.drupal.org/node/1419988#cs-composer. Then use the ]c and [c motion commands to move between diffs, and use do to pull in chunks from the other file, or edit as you will. It works pretty well.

benjifisher’s picture

Status: Active » Needs review
FileSize
3.05 KB

Moving beyond proof of concept, the attached patch should find the phpcbf executable as reliably as we already find the phpcs executable. It then works pretty much as described in #2. Just try editing someone else's messy code and

:PHPcbf

rodrigoaguilera’s picture

First, the hook_foo example correction that you mention I don't regard it as bad. those kind of docblocks should start with "Implements hook...

Adding empty docblocks is bad though.

I did a brief test of your patch and is great, Thank you.

I found that when I'm finished accepting changes I close the left window and the syntastic functionality that checks code standars with phpcs is gone. If I had bad lines the marker stays the same and It doesn't mark bad lines anymore.

Is it because of the way I close the diffsplit? can you reproduce this?

rodrigoaguilera’s picture

Status: Needs review » Needs work
FileSize
616 bytes
3.14 KB

I tested this patch on a linux machine and I wasn't getting any fixes, I debbugged a little bit seeing that It was executing this command

phpcbf --standard=/home/rodrigoaguilera/.composer/vendor/drupal/coder/coder_sniffer/Drupal /tmp/vIKbVJe/8

I executed the command in a terminal and got this

Changing into directory /tmp/vIKbVJe
Processing 8 [PHP => 310 tokens in 44 lines]... DONE in 20ms (1 fixable violations)
        => Fixing file: 0/1 violations remaining [made 2 passes]... DONE in 64ms
Array
(
    [0] => Ignoring potentially dangerous file name /tmp/vIKbVJe/8
    [1] => can't find file to patch at input line 3
    [2] => Perhaps you used the wrong -p or --strip option?
    [3] => The text leading up to this was:
    [4] => --------------------------
    [5] => |--- /tmp/vIKbVJe/8
    [6] => |+++ PHP_CodeSniffer
    [7] => --------------------------
    [8] => File to patch:
    [9] => Skip this patch? [y]
    [10] => Skipping patch.
    [11] => 1 out of 1 hunk ignored
)
Returned: 1
Time: 125ms; Memory: 6Mb

Is a problem with the patch program so my suggestion is that we use the --no-patch option. Patch attached

Another improvement might be not to do the operation at all if there is nothing to fix with phpcbf, maybe the command gives some interesting output.

Anyway I still don't know how to exit this mode cleanly so phpcs works again when I save the file.