Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FunctionSignature and ScopeIndent sniffs don't detect indents correctly when PHP open tag is not on a line by itself #645

Closed
paulschreiber opened this issue Jul 20, 2015 · 9 comments

Comments

@paulschreiber
Copy link

When inspecting co-authors-plus.php with the WordPress-VIP ruleset, phpcbf takes a long time to finish and fails to fix any problems:

The file in question:
https://github.com/Automattic/Co-Authors-Plus/blob/57236fc6856b715d5e27248f6eea5681153645aa/co-authors-plus.php

Here's the output of phpcbf:

$ phpcbf --standard=WordPress-VIP co-authors-plus.php 
Changing into directory /Users/paul/Co-Authors-Plus
Processing co-authors-plus.php [PHP => 13684 tokens in 1592 lines]... DONE in 1.39 secs (30 fixable violations)
        => Fixing file: 2/30 violations remaining [made 50 passes]... ERROR in 165.13 secs
No fixable errors were found
Time: 2 mins, 46.55 secs; Memory: 54.75Mb

Here's the output of phpcs:

$ phpcs --standard=WordPress-VIP co-authors-plus.php 

FILE: /Users/paul/co-authors-plus.php
----------------------------------------------------------------------
FOUND 63 ERRORS AND 22 WARNINGS AFFECTING 55 LINES
----------------------------------------------------------------------
   38 | ERROR   | [ ] Class name "coauthors_plus" is not in camel
      |         |     caps format
  335 | ERROR   | [ ] Use Yoda Condition checks, you must
  369 | ERROR   | [ ] Expected next thing to be a escaping function,
      |         |     not '$count'
  381 | ERROR   | [ ] Expected next thing to be an escaping function
      |         |     (like esc_html_e() or esc_attr_e()), not '_e'
  388 | ERROR   | [ ] Expected next thing to be an escaping function
      |         |     (like esc_html_e() or esc_attr_e()), not '_e'
  421 | ERROR   | [ ] Use Yoda Condition checks, you must
  425 | ERROR   | [ ] Use Yoda Condition checks, you must
  438 | ERROR   | [ ] Use Yoda Condition checks, you must
  510 | ERROR   | [ ] Expected next thing to be an escaping function
      |         |     (like esc_html_e() or esc_attr_e()), not '_e'
  512 | ERROR   | [ ] Expected next thing to be an escaping function
      |         |     (like esc_html_e() or esc_attr_e()), not '_e'
  526 | WARNING | [ ] Usage of a direct database call is discouraged.
  526 | ERROR   | [ ] Usage of a direct database call without caching
      |         |     is prohibited. Use wp_cache_get / wp_cache_set.
  529 | ERROR   | [ ] get_term_by is prohibited, please use
      |         |     wpcom_vip_get_term_by() instead.
  550 | WARNING | [ ] Usage of a direct database call is discouraged.
  550 | ERROR   | [ ] Usage of a direct database call without caching
      |         |     is prohibited. Use wp_cache_get / wp_cache_set.
  552 | ERROR   | [ ] get_term_by is prohibited, please use
      |         |     wpcom_vip_get_term_by() instead.
  594 | WARNING | [ ] Usage of a direct database call is discouraged.
  594 | ERROR   | [ ] Usage of a direct database call without caching
      |         |     is prohibited. Use wp_cache_get / wp_cache_set.
  595 | WARNING | [ ] Usage of a direct database call is discouraged.
  621 | ERROR   | [x] String "INNER JOIN" does not require double
      |         |     quotes; use single quotes instead
  621 | ERROR   | [x] String "LEFT JOIN" does not require double
      |         |     quotes; use single quotes instead
  624 | ERROR   | [x] String "INNER JOIN" does not require double
      |         |     quotes; use single quotes instead
  624 | ERROR   | [x] String "LEFT JOIN" does not require double
      |         |     quotes; use single quotes instead
  729 | WARNING | [ ] Detected access of super global var $_POST,
      |         |     probably need manual inspection.
  771 | WARNING | [ ] Detected access of super global var $_POST,
      |         |     probably need manual inspection.
  771 | WARNING | [ ] Detected access of super global var $_POST,
      |         |     probably need manual inspection.
  774 | WARNING | [ ] Detected access of super global var $_POST,
      |         |     probably need manual inspection.
  774 | ERROR   | [ ] Detected usage of a non-sanitized input
      |         |     variable: $_POST
  822 | ERROR   | [x] Expected 1 space after closing parenthesis;
      |         |     found 0
  847 | WARNING | [ ] Usage of a direct database call is discouraged.
  863 | WARNING | [ ] Detected access of super global var $_POST,
      |         |     probably need manual inspection.
  863 | WARNING | [ ] Detected access of super global var $_POST,
      |         |     probably need manual inspection.
  871 | WARNING | [ ] Usage of a direct database call is discouraged.
  871 | ERROR   | [ ] Usage of a direct database call without caching
      |         |     is prohibited. Use wp_cache_get / wp_cache_set.
  896 | WARNING | [ ] Detected access of super global var $_REQUEST,
      |         |     probably need manual inspection.
  896 | ERROR   | [ ] Use Yoda Condition checks, you must
  905 | WARNING | [ ] Usage of a direct database call is discouraged.
  905 | ERROR   | [ ] Usage of a direct database call without caching
      |         |     is prohibited. Use wp_cache_get / wp_cache_set.
 1042 | WARNING | [ ] Detected access of super global var $_REQUEST,
      |         |     probably need manual inspection.
 1042 | WARNING | [ ] Detected access of super global var $_REQUEST,
      |         |     probably need manual inspection.
 1042 | ERROR   | [ ] Detected usage of a non-sanitized input
      |         |     variable: $_REQUEST
 1046 | WARNING | [ ] Detected access of super global var $_REQUEST,
      |         |     probably need manual inspection.
 1050 | WARNING | [ ] Detected access of super global var $_REQUEST,
      |         |     probably need manual inspection.
 1050 | ERROR   | [ ] Detected usage of a non-sanitized input
      |         |     variable: $_REQUEST
 1051 | WARNING | [ ] Detected access of super global var $_REQUEST,
      |         |     probably need manual inspection.
 1051 | ERROR   | [ ] Detected usage of a non-validated input
      |         |     variable: $_REQUEST
 1051 | ERROR   | [ ] Detected usage of a non-sanitized input
      |         |     variable: $_REQUEST
 1056 | ERROR   | [ ] Expected next thing to be a escaping function,
      |         |     not '$author'
 1056 | ERROR   | [x] String " | " does not require double quotes;
      |         |     use single quotes instead
 1056 | ERROR   | [ ] Expected next thing to be a escaping function,
      |         |     not '$author'
 1056 | ERROR   | [x] String " | " does not require double quotes;
      |         |     use single quotes instead
 1056 | ERROR   | [ ] Expected next thing to be a escaping function,
      |         |     not '$author'
 1056 | ERROR   | [x] String " | " does not require double quotes;
      |         |     use single quotes instead
 1056 | ERROR   | [ ] Expected next thing to be a escaping function,
      |         |     not '$author'
 1056 | ERROR   | [x] String " | " does not require double quotes;
      |         |     use single quotes instead
 1056 | ERROR   | [ ] Expected next thing to be a escaping function,
      |         |     not '$author'
 1136 | ERROR   | [x] String "user_nicename LIKE" does not require
      |         |     double quotes; use single quotes instead
 1136 | ERROR   | [x] String "display_name LIKE" does not require
      |         |     double quotes; use single quotes instead
 1233 | ERROR   | [ ] Expected a sanitizing function (see Codex for
      |         |     'Data Validation'), but instead saw
      |         |     'add_query_arg'
 1234 | ERROR   | [x] Multi-line function call not indented
      |         |     correctly; expected 4 spaces but found 20
 1238 | ERROR   | [x] Multi-line function call not indented
      |         |     correctly; expected 4 spaces but found 20
 1239 | ERROR   | [x] Multi-line function call not indented
      |         |     correctly; expected 0 spaces but found 16
 1317 | ERROR   | [ ] get_term_by is prohibited, please use
      |         |     wpcom_vip_get_term_by() instead.
 1319 | ERROR   | [ ] get_term_by is prohibited, please use
      |         |     wpcom_vip_get_term_by() instead.
 1457 | ERROR   | [x] Line indented incorrectly; expected at least 1
      |         |     tabs, found 0
 1468 | ERROR   | [x] Line indented incorrectly; expected 1 tabs,
      |         |     found 0
 1469 | ERROR   | [x] Line indented incorrectly; expected at least 2
      |         |     tabs, found 1
 1470 | ERROR   | [x] Line indented incorrectly; expected at least 2
      |         |     tabs, found 1
 1471 | ERROR   | [x] Line indented incorrectly; expected at least 2
      |         |     tabs, found 1
 1472 | ERROR   | [x] Line indented incorrectly; expected 2 tabs,
      |         |     found 1
 1474 | ERROR   | [x] Line indented incorrectly; expected at least 3
      |         |     tabs, found 2
 1475 | ERROR   | [x] Line indented incorrectly; expected 3 tabs,
      |         |     found 2
 1476 | ERROR   | [x] Line indented incorrectly; expected at least 4
      |         |     tabs, found 3
 1489 | WARNING | [ ] Silencing errors is discouraged
 1538 | WARNING | [ ] Detected access of super global var $_SERVER,
      |         |     probably need manual inspection.
 1538 | ERROR   | [ ] Detected usage of a non-validated input
      |         |     variable: $_SERVER
 1538 | ERROR   | [ ] Detected usage of a non-sanitized input
      |         |     variable: $_SERVER
 1553 | ERROR   | [x] String "Content-Type: text/plain; charset=\""
      |         |     does not require double quotes; use single
      |         |     quotes instead
 1555 | ERROR   | [x] Expected 1 spaces after opening bracket; 0
      |         |     found
 1555 | ERROR   | [x] Expected 1 spaces before closing bracket; 0
      |         |     found
 1563 | WARNING | [ ] Silencing errors is discouraged
 1581 | ERROR   | [x] Expected 1 spaces after opening bracket; 0
      |         |     found
 1581 | ERROR   | [x] Expected 1 spaces before closing bracket; 0
      |         |     found
 1584 | ERROR   | [x] Expected 1 spaces after opening bracket; 0
      |         |     found
 1584 | ERROR   | [x] Expected 1 spaces before closing bracket; 0
      |         |     found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 30 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

Time: 1.39 secs; Memory: 25.75Mb
@gsherwood
Copy link
Member

This looks to be something specific to this WordPress coding standard and the combinations of sniffs it is using as I can't replicate it on included standards. So I'll need a bit of time to look into it.

The conflicting fixes are from the Generic scope indent sniff though, so I'll need to see how these settings have been overridden in the WordPress standard:

E: [Line 1234] Multi-line function call not indented correctly; expected 4 spaces but found 16 (PEAR.Functions.FunctionCallSignature.Indent)
PEAR_Sniffs_Functions_FunctionCallSignatureSniff (line 420) replaced token 10286 (T_WHITESPACE) "\t\t\t\tarray" => "····array"
E: [Line 1238] Multi-line function call not indented correctly; expected 4 spaces but found 16 (PEAR.Functions.FunctionCallSignature.Indent)
PEAR_Sniffs_Functions_FunctionCallSignatureSniff (line 420) replaced token 10317 (T_WHITESPACE) "\t\t\t\twp_nonce_url" => "····wp_nonce_url"
* fixed 2 violations, starting loop 5 *
E: [Line 1234] Tabs must be used to indent lines; spaces are not allowed (Generic.WhiteSpace.DisallowSpaceIndent.SpacesUsed)
Generic_Sniffs_WhiteSpace_DisallowSpaceIndentSniff (line 127) replaced token 10286 (T_WHITESPACE) "····array" => "\tarray"
E: [Line 1238] Tabs must be used to indent lines; spaces are not allowed (Generic.WhiteSpace.DisallowSpaceIndent.SpacesUsed)
Generic_Sniffs_WhiteSpace_DisallowSpaceIndentSniff (line 127) replaced token 10317 (T_WHITESPACE) "····wp_nonce_url" => "\twp_nonce_url"
E: [Line 1234] Line indented incorrectly; expected at least 4 tabs, found 1 (Generic.WhiteSpace.ScopeIndent.Incorrect)
* token 10286 has already been modified, skipping *
* fixed 2 violations, starting loop 6 *
E: [Line 1234] Line indented incorrectly; expected at least 4 tabs, found 1 (Generic.WhiteSpace.ScopeIndent.Incorrect)
Generic_Sniffs_WhiteSpace_ScopeIndentSniff (line 660) replaced token 10286 (T_WHITESPACE) "\tarray" => "\t\t\t\tarray"
E: [Line 1238] Line indented incorrectly; expected at least 4 tabs, found 1 (Generic.WhiteSpace.ScopeIndent.Incorrect)
Generic_Sniffs_WhiteSpace_ScopeIndentSniff (line 660) replaced token 10317 (T_WHITESPACE) "\twp_nonce_url" => "\t\t\t\twp_nonce_url"
* fixed 2 violations, starting loop 7 *

Using a tab-width of 4, which is recommended, gives these conflicts instead (still related):

E: [Line 1234] Line indented incorrectly; expected at least 4 tabs, found 1 (Generic.WhiteSpace.ScopeIndent.Incorrect)
Generic_Sniffs_WhiteSpace_ScopeIndentSniff (line 660) replaced token 10286 (T_WHITESPACE) "\tarray" => "\t\t\t\tarray"
E: [Line 1238] Line indented incorrectly; expected at least 4 tabs, found 1 (Generic.WhiteSpace.ScopeIndent.Incorrect)
Generic_Sniffs_WhiteSpace_ScopeIndentSniff (line 660) replaced token 10317 (T_WHITESPACE) "\twp_nonce_url" => "\t\t\t\twp_nonce_url"
* fixed 2 violations, starting loop 4 *
E: [Line 1234] Multi-line function call not indented correctly; expected 4 spaces but found 16 (PEAR.Functions.FunctionCallSignature.Indent)
PEAR_Sniffs_Functions_FunctionCallSignatureSniff (line 420) replaced token 10286 (T_WHITESPACE) "\t\t\t\tarray" => "····array"
E: [Line 1238] Multi-line function call not indented correctly; expected 4 spaces but found 16 (PEAR.Functions.FunctionCallSignature.Indent)
PEAR_Sniffs_Functions_FunctionCallSignatureSniff (line 420) replaced token 10317 (T_WHITESPACE) "\t\t\t\twp_nonce_url" => "····wp_nonce_url"
* fixed 2 violations, starting loop 5 *
E: [Line 1234] Tabs must be used to indent lines; spaces are not allowed (Generic.WhiteSpace.DisallowSpaceIndent.SpacesUsed)
Generic_Sniffs_WhiteSpace_DisallowSpaceIndentSniff (line 127) replaced token 10286 (T_WHITESPACE) "····array" => "\tarray"
E: [Line 1238] Tabs must be used to indent lines; spaces are not allowed (Generic.WhiteSpace.DisallowSpaceIndent.SpacesUsed)
Generic_Sniffs_WhiteSpace_DisallowSpaceIndentSniff (line 127) replaced token 10317 (T_WHITESPACE) "····wp_nonce_url" => "\twp_nonce_url"
E: [Line 1234] Line indented incorrectly; expected at least 4 tabs, found 1 (Generic.WhiteSpace.ScopeIndent.Incorrect)
* token 10286 has already been modified, skipping *
* fixed 2 violations, starting loop 6 *
E: [Line 1234] Line indented incorrectly; expected at least 4 tabs, found 1 (Generic.WhiteSpace.ScopeIndent.Incorrect)
Generic_Sniffs_WhiteSpace_ScopeIndentSniff (line 660) replaced token 10286 (T_WHITESPACE) "\tarray" => "\t\t\t\tarray"
E: [Line 1238] Line indented incorrectly; expected at least 4 tabs, found 1 (Generic.WhiteSpace.ScopeIndent.Incorrect)
Generic_Sniffs_WhiteSpace_ScopeIndentSniff (line 660) replaced token 10317 (T_WHITESPACE) "\twp_nonce_url" => "\t\t\t\twp_nonce_url"
* fixed 2 violations, starting loop 7 *

@gsherwood
Copy link
Member

Smallest bit of sample code I can get to reproduce the error:

<script>
    var foo = <?php echo bar(
        'baz'
    ); ?>;
</script>

@gsherwood gsherwood changed the title phpcbf gets stuck, fails to fix this file (co-authors-plus.php) FunctionSignature and ScopeIndent sniffs don't detect indents correctly when PHP open tag is not on a line by itself Jul 21, 2015
gsherwood added a commit that referenced this issue Jul 21, 2015
…t indents correctly when PHP open tag is not on a line by itself
@gsherwood
Copy link
Member

This is now working for me on the master branch.

@alexander-akait
Copy link

@gsherwood bump, when the code in the master branch?

@gsherwood
Copy link
Member

Already in the master branch, will be released soon, when I get some time.

Forgot to close this report. Closing now.

@alexander-akait
Copy link

@gsherwood use last version
but get error this code

<div>
    <?php getTemplatePart(
        'partials/web-page/carousel-slick/item-slide/header',
        [
            'class' => $class
        ]
    ); ?>
</div>

but use this code, not contain error

<div>
    <?php 
        getTemplatePart(
            'partials/web-page/carousel-slick/item-slide/header',
            [
                'class' => $class
            ]
        ); 
    ?>
</div>

gsherwood added a commit that referenced this issue Oct 6, 2015
@gsherwood
Copy link
Member

@evilebottnawi I've fixed this now. Will be in the next release (no date set).

@warrickhill
Copy link

I'm using version 2.5.1 and i'm also having the same problem. has there been a regression?

@gsherwood
Copy link
Member

I'm using version 2.5.1 and i'm also having the same problem. has there been a regression?

The tests are all still passing, so there are no known regressions.

Do you have some sample code you can post to explain the error you are seeing? And the command you are running, coding standard you are using, that sort of thing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants