Incorrect indentation using Atom Beautify with PHP_CodeSniffer and WordPress-Coding-Standards

vizzale commented 5 years ago

Bug Description

I’m developing a WordPress theme, as a editor I use Atom with Atom Beautify. I have also installed as default beautifier “PHP_CodeSniffer” with the “WordPress-Coding-Standards “as ruleset (WordPress-VIP,WordPress,WordPress-Extra,WordPress-Docs,WordPress-Core).

Both, the “PHP_CodeSniffer” beautifier and the “WordPress-Coding-Standards” configuration file, work correctly, but they create files that are a mess, and in which some spaces become tabs instead others remain such spaces. Furthermore, the structure of the file itself is also incorrect (at least for what I would like to get).

I don't know if the code shows the problem so I'm going to integrate the issue also with images.

Input Before Beautification

This is what the code looked like before. As you can see in the file, spaces are used and there is an initial indent that should not exist.

Expected Output

The beautified code should have looked like this.

Actual Output (image)

The beautified code actually looked like this. This is what happens after starting Atom Beautify. The initial indent has not been corrected, the general structure is incorrect, and there are both tabs and spaces.

Actual Output (code)

<?php
/**
 * The template for displaying 404 pages (not found)
 *
 * @link https://codex.wordpress.org/Creating_an_Error_404_Page
 *
 * @package help
 */

get_header(); ?>

  <!-- Main -->
  <main>
    <section id="error">
      <div class="one">
        <div class="two">
          <p class="three">Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
        </div>

        <div class="two">
          <p class="three">Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
        </div>
      </div>

      <div class="one">
        <div class="two">
          <p class="three">Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
        </div>
      </div>
    </section>
  </main>

<?php get_footer(); ?>

Error Code

Environment

Question	Answer
PHP version	5.6.30
PHP_CodeSniffer version	3.2.3
WPCS version	0.14.1 (I took this information from the file "CHANGELOG.md")
WPCS install type	Standalone
IDE (if relevant)	Atom 1.38.2

Additional Context (optional)

Here instead you find my personal configuration file phpcs.xml.dist:

<?xml version="1.0" encoding="UTF-8"?>

<ruleset name="WordPress Custom">
  <description>Custom PHP_CodeSniffer ruleset</description>

  <arg name="tab-width" value="2"/>

  <rule ref="WordPress">
    <exclude name="Generic.WhiteSpace.DisallowSpaceIndent"/>
  </rule>

  <rule ref="Generic.WhiteSpace.ScopeIndent">
    <properties>
      <property name="indent" value="2"/>
      <property name="exact" value="true"/>
      <property name="tabIndent" value="false"/>
    </properties>
  </rule>

  <rule ref="Generic.WhiteSpace.DisallowTabIndent"/>

  <rule ref="PEAR.Functions.FunctionCallSignature">
    <properties>
      <property name="indent" value="2"/>
    </properties>
  </rule>
</ruleset>

Tested Against `develop` branch?

[ ] I have verified the issue still exists in the develop branch of WPCS.

jrfnl commented 5 years ago

Hi @vizzale Before I start looking into this I have two questions:

Is there any particular reason why you are using an old version - 0.14.1 - of WPCS ? The last release of WPCS is 2.1.1. A lot of improvements have been made to WPCS between then and now, so your problem may already have been addressed. Please test with the current version of WPCS to see if the problem still exists.
To understand your use-case: why do you want to use spaces, and then 2 instead of 4, for your indentation ?

vizzale commented 5 years ago

Hi @jrfnl I answer you questions:

There is no particular reason, I simply installed both PHPCS and WPCS in May 2018 and I no longer updated WPCS. Also every time for this type of application I always have the fear that something could go wrong, in fact I updated WPCS a few moments ago and now it doesn't work anymore neither Atom Beautify nor linter-phpcs.
I've always used an indent of two spaces for HTML, and CSS and many style guides suggest this type of indentation so it simply became part of my workflow. But regardless of whether to use tabs or spaces for indentation, I would like to be able to do it in both ways as long as it is correctly set by my configuration file.

I hope that we will be able to solve both problems. Thank you very much.

vizzale commented 5 years ago

However @jrfnl I add this information, it could be useful. Yesterday I immediately proceeded to redo the standalone procedure to set the new version: 1) PHP_CodeSniffer is already installed; 2) git clone -b master https://github.com/WordPress/WordPress-Coding-Standards.git wpcs; 3) phpcs --config-set installed_paths / path / to / wpcs.

Unfortunately however, as I told you, Atom Beautify and the linter-phpcs stopped working. Returning instead to the 0.14.1 version, it started working again. Looking forward to receiving your valuable advice, I thank you.

jrfnl commented 5 years ago

@vizzale Well, getting your setup working properly for Atom should probably be the first priority.

So, for Atom, could you please verify that things are set up correctly based on this article in the wiki ? https://github.com/WordPress/WordPress-Coding-Standards/wiki/Setting-up-WPCS-to-work-in-Atom

Also: do you only use WPCS with Atom or also on the command-line ? If so, could you test on the command-line if things work correctly with the latest version of WPCS when you use it there ?

A good indication of whether or not you are using WPCS 2+ or older is to check phpcs -i. If WordPress-VIP is no longer listed, you are using version 2.0 or higher.

vizzale commented 5 years ago

I'm trying to do things right so that we can easily find the problem.

These are all the steps done so far: 1) first of all, through Homebrew brew upgrade php-code-sniffer, I updated PHPCS and checked that it was the last version with the command: phpcs --version 2) I have completely updated Homebrew brew update 3) I cloned WPCS via the command git clone -b master https://github.com/WordPress/WordPress-Coding-Standards.git wpcs 4) I set the folder as default phpcs --config-set installed_paths / path / to / wpcs 5) I checked if everything went well phpcs -i (WordPress-VIP is missing) 6) I followed the steps of the link you posted

So now, in summary, the situation is this:

fully updated Homebrew
PHP_CodeSniffer updated to version 3.4.2
WordPress-Coding-Standards updated to version 2.1.1
linter-phpcs works well
atom-beautify still does not perform a correct indentation
run the phpcs command line: phpcs --standard=WordPress 404.php and then phpcbf --standard=WordPress 404.php and the result is the same, an incorrect indentation

jrfnl commented 5 years ago

@vizzale If I understand you correctly, the first problem of using the up-to-date version in Atom is now solved. Is that correct ?

Now for me to try and reproduce the issue with the tabs vs spaces and to figure out what's going on, I need the original code before your ran the beautifier, not the output. I seem to remember that we do actually ask for that in the issue template... ?

Anyway, I'm not going to type it over from the screenshot, so would you please either post a link to the code or post the original code in a reply ?

jrfnl commented 5 years ago

Oh and also, you say you ran PHPCBF with this command: phpcbf --standard=WordPress 404.php. That would use the WordPress standard as is, not your custom ruleset. So if that's the case, it's no surprise that your space vs tabs settings aren't being respected as you're not using your own ruleset.

Have you tried running it with just phpcbf 404.php (from the project root presuming your phpcs.xml.dist file is also in the project root) or phpcbf --standard=/path/to/phpcs.xml.dist 404.php if your custom ruleset would be located elsewhere ?

vizzale commented 5 years ago

First of all, thanks for the patience @jrfnl.

As recommended by you I used the command phpcbf 404.php (my file is in the project folder). If I use this command finally the tabs are converted, as per custom ruleset, into spaces. So, first question: how can I make this work via Atom Beautify without having to use the command line each time?

Second question: the indentation still does not meet my expectations, I try to explain to you what I expect, to understand if it is possible to get it. In the image attached here there is no space or tab:

Whether you launch Atom Beautify, or run the phpcbf 404.php command, the code is not correctly spelled, that is:

jrfnl commented 5 years ago

So, first question: how can I make this work via Atom Beautify without having to use the command line each time?

I honestly have no idea. I don't use Atom myself and that's an Atom support question, not a WPCS question. I don't know if any of the other WPCS maintainers/watchers use Atom, if so: please jump in and share your wisdom :smile:

Second question: the indentation still does not meet my expectations, I try to explain to you what I expect, to understand if it is possible to get it.

Well, as I asked before, it would be helpful for me to have the original code to run some tests with.

vizzale commented 5 years ago

I apologize, as a fool I only read your last message: D Here's to you:

<?php
/**
 * The template for displaying 404 pages (not found)
 *
 * @link https://codex.wordpress.org/Creating_an_Error_404_Page
 *
 * @package help
 */

get_header(); ?>

<!-- Main -->
<main>
<section id="error">
<div class="one">
<div class="two">
<p class="three">Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
</div>

<div class="two">
<p class="three">Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
</div>
</div>

<div class="one">
<div class="two">
<p class="three">Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
</div>
</div>
</section>
</main>

<?php get_footer(); ?>

jrfnl commented 5 years ago

@vizzale Thanks for that, but that doesn't look like the original input from the screenshot at the top ?

vizzale commented 5 years ago

I've done various tests these days. This version is structurally similar to the initial one, only the indentation that is totally missing here changes.

jrfnl commented 5 years ago

@vizzale Ok, well I've ran some tests and I'll try and explain what is happening, but am a bit short of time, so if anything is unclear, please ask about it and I'll try and explain better when I have a little more time.

The WordPress-Core ruleset, which is included in the WordPress ruleset sets a number of properties for the Generic.WhiteSpace.ScopeIndent sniff. You overrule most in your custom ruleset, but not all. The ignoreIndentationTokens property set by WPCS - which you don't overrule - actually excludes T_INLINE_HTML from being handled by the scope indent sniff which is why WPCS won't touch the indentation of the HTML. If you want to change this, you need to overrule that property too in your custom ruleset, like so:
```
<rule ref="Generic.WhiteSpace.ScopeIndent">
  <properties>
    <property name="indent" value="2"/>
    <property name="exact" value="true"/>
    <property name="tabIndent" value="false"/>
    <property name="ignoreIndentationTokens" type="array">
      <element value="T_HEREDOC"/>
      <element value="T_NOWDOC"/>
    </property>
  </properties>
</rule>
```
To make the ruleset completely 2-space-based, you will also need to set another property:
```
<rule ref="PSR2.ControlStructures.SwitchDeclaration">
  <properties>
    <property name="indent" value="2"/>
  </properties>
</rule>
```
That sniff has a default of 4 spaces which WPCS doesn't overrule as 4 spaces suits WPCS just fine, but for your 2-space default, you would want to change this.

Now the ruleset is set up properly for 2-space indents, you'll find that the HTML indentation is still not being fixed (correctly).

The short of that, is that PHPCS only deals with PHP, JS and CSS. PHPCS does not parse HTML, nor does it reformat it. You may now say, "Hang on, but it did make changes initially...". Yes, it did, but it only interchanged tabs vs spaces, it didn't make any changes to the actual indentation as it has no concept of, nor keeps track of HTML open/close tags.

So what I suspect is happening here, is that Atom runs PHPCS and then runs some Atom-native HTML indentation formatting over the code.

To test this:

Adjust your custom ruleset with the suggestions I made above.
Using the sample code below (yes, I know horribly formatted, but that's the point), run phpcs --standard=phpcs.xml.dist ./testfile.php over the test file (pointing to your custom ruleset as adjusted). You will see errors/warnings about precision alignment and end of line whitespace, but no errors reported about (scope) indentation.
Next run phpcbf --standard=phpcs.xml.dist ./testfile.php --suffix=.fixed over the same test file (pointing to your custom ruleset as adjusted). This will output the result to a separate file with a .fixed suffix. Take note that the HTML indentation in the .fixed file is unchanged when compared to the original .php file.
Now run the Atom Beautifier over the same code in testfile.php (which should still be unchanged up to now).
Compare the testfile.php.fixed file (the PHPCBF output) with the file beautified by Atom to see the difference. Any differences you see are Atom doing something extra outside of the scope/rules set by WPCS.

I hope that explains it.

You may then say, but shouldn't WPCS deal with HTML indentation too ? And the answer would be "no". Your template is quite "clean" in that it has the open/close tags for each HTML element in the same block of inline HTML. For most PHP code, this is not the case. The open tags may be in one method and the close tags in another. There can be blocks of PHP in between blocks of inline HTML, open and close tags may be in different files (think header.php/footer.php) etc etc. In short, it would be impossible to reliably keep track of the HTML indentation needed, so there is no intention to add any checks of that kind to WPCS. (which is part of the reason why WPCS excludes inline HTML from the scope indent sniff in the first place)

Probably not the answer you were hoping for, but I hope it helps to understand how these things work anyhow.

Horribly formatted HTML sample code

```php

Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.

```

vizzale commented 5 years ago

Thanks @jrfnl you have been more than exhaustive and I appreciate your availability. I followed all your suggestions and I corrected my custom ruleset with your instructions. I have only one last question: based on your experience, are there any "more suitable" beautifiers that I could use, perhaps continuing to use PHP_CodeSniffer only for linter-phpcs, to process this type of code?

I have also tried to solve the problem with Atom Beautify by doing several attempts, but this doesn't take anything at all considering my custom ruleset. If anyone has any ideas, whistle.

vizzale commented 5 years ago

Hi @jrfnl I'm going to bother you again because this morning I tried to run the command phpcbf --standard=phpcs.xml.dist 404.php on a file and now PHP_CodeSniffer converts the tabs into 4 spaces instead of 2.

My custom ruleset is identical to yesterday's. By the way, I also removed the last rule you suggested but nothing, it converts the tabs into 4 spaces. I can't understand what could have happened.

This is currently my custom ruleset with your directions:

<?xml version="1.0" encoding="UTF-8"?>

<ruleset name="WordPress Custom">
  <description>PHP_CodeSniffer custom ruleset: indentation with spaces</description>

  <arg name="tab-width" value="2"/>

  <rule ref="WordPress">
    <exclude name="Generic.WhiteSpace.DisallowSpaceIndent"/>
  </rule>

  <rule ref="Generic.WhiteSpace.ScopeIndent">
    <properties>
      <property name="indent" value="2"/>
      <property name="exact" value="true"/>
      <property name="tabIndent" value="false"/>
      <property name="ignoreIndentationTokens" type="array">
        <element value="T_HEREDOC"/>
        <element value="T_NOWDOC"/>
      </property>
    </properties>
  </rule>

  <rule ref="Generic.WhiteSpace.DisallowTabIndent"/>

  <rule ref="PEAR.Functions.FunctionCallSignature">
    <properties>
      <property name="indent" value="2"/>
    </properties>
  </rule>

  <rule ref="PSR2.ControlStructures.SwitchDeclaration">
    <properties>
      <property name="indent" value="2"/>
    </properties>
  </rule>
</ruleset>

vizzale commented 5 years ago

Hi @jrfnl, no idea what may depend on or what can be done to understand what is creating this incongruity of the 4 spaces?

vizzale commented 5 years ago

Hi @jrfnl I'm trying in every way to solve the problem but I'm not able to do it. Although my custom ruleset is the one we set together this converts the tab into 4 spaces instead of 2 spaces.

It seems therefore that my custom ruleset is not complete. Could you help me understand what's still missing?

Thanks a lot.

jrfnl commented 4 years ago

@vizzale Sorry for my slow response. I only just saw that I hadn't replied to your last questions.

Try setting <arg name="tab-width" value="2"/> after the inclusion of the WordPress ruleset.

The WordPress ruleset sets the tab-width to 4, so your custom setting may be overruled.

The order of directives in a ruleset is important as the rules are read top to bottom.

Please let me know if that helps.

vizzale commented 4 years ago

Hi Juliette, I too am writing to you very late. I do it because since then, despite your suggestion, I have not been able to solve the problem.

Can we carry out some type of investigation to understand why the configuration file cannot convert a tab into two spaces? Tell me everything you need to help you understand what the wrong operation depends on.

I hope we can finally understand the problem. Thanks in advance for your patience.

WordPress / WordPress-Coding-Standards