PHPCheckstyle / phpcheckstyle

PHPCheckstyle is an open-source tool that helps PHP programmers adhere to certain coding conventions.
https://github.com/PHPCheckstyle/phpcheckstyle
GNU Lesser General Public License v3.0
164 stars 31 forks source link

PHP 5.5 Support #9

Closed jbrooksuk closed 7 years ago

jbrooksuk commented 10 years ago

Copied from the GoogleCode issues.

jbrooksuk commented 10 years ago

I've added support for T_FINALLY in #12.

rr- commented 10 years ago

There's also this:

Class Name Resolution As Scalar Via "class" Keyword

I use phpcheckstyle version 0.14.1 and it breaks on files like this:

<?php
namespace Szurubooru\Dao;

final class TokenDao extends AbstractDao implements ICrudDao
{
    public function __construct(\Szurubooru\DatabaseConnection $databaseConnection)
    {
        parent::__construct($databaseConnection, \Szurubooru\Entities\Post::class);
    }
}

with message:

File "/srv/www/booru-dev/src/Dao/PostDao.php" warning, line 8 - File /srv/www/booru-dev/src/Dao/PostDao.php must not have multiple class declarations.

Moreover, if my files contain multiple constructs like that, phpcheckstyle seems to freeze and I need to kill it with ^C. I haven't been able to figure out the rules behind the freezing, but it depends strictly on file content, not on external factors - i.e. if I run it again, it will freeze again, unless I change my source code somehow.

tchule commented 10 years ago

Thanks for the report, will have a look as soon as I find some time.

popojargo commented 7 years ago

The short array syntax is generating indentation errors. Your parser is looking for T_ARRAY token array() but doesn't handle the [ ]. Any feasible way we could support the short array syntax?

tchule commented 7 years ago

I need to have a look. We use the internal PHP tokenizer, so if PHP detects the short array syntax, we should be able to do it too.

tchule commented 7 years ago

See https://github.com/PHPCheckstyle/phpcheckstyle/issues/62

tchule commented 7 years ago

Could you provide a sample for the indentation problem ? I think I have a fix but i'd like to test it first.

popojargo commented 7 years ago

Let's say I have a multiline array like this :

protected static $codeSubtypes = [
        401 => 'CouchUnauthorizedException',
        403 => 'CouchForbiddenException',
        404 => 'CouchNotFoundException',
        417 => 'CouchExpectationException'
    ];
tchule commented 7 years ago

I hav'nt been able to produce the indentation errors, do you check to indentation with tabs or spaces ?

    <!-- Tests to make sure that a line does not contain the tab character. -->
    <test name="indentation">  <!-- noTabs -->
        <property name="type" value="tabs" /> <!-- tabs or spaces -->
        <property name="number" value="4" /> <!-- number of spaces if type = spaces -->
    </test>
popojargo commented 7 years ago

Spaces My full config can be found here

tchule commented 7 years ago

OK, i'll have a look tomorrow, thx.

tchule commented 7 years ago

OK, i've tested with your configuration and the multiline array example you provided, and I have some "Tab indentation must not be used." warnings. This seems consistent to me, it is not linked to the fact that it is a short array syntax.

But maybe this is not the indentation errors you where seeing, don' t hesitate to complete the example.

popojargo commented 7 years ago

I am having this warning : The indentation level must be 4 but was 8. With the following file : CouchException.php

If I change the indentation according to the warning, it's just very awful and I have never seen any language with such formatting.

Also, from what I have seen from the code, the multiline array is possible with the Array token. Therefore, the short array syntax [] is not recognize as array token I think.

tchule commented 7 years ago

OK, I understand now.

With your file (and copy/paste from GitHub doesn't work because spaces are replaced by tabs, but downloading the raw file is OK), i've been able to reproduce your problem.

I've commited something that should solve the problem (https://github.com/PHPCheckstyle/phpcheckstyle/commit/5ddeac37c51d1eeb46df224a058a65aa82588c4f).

I will wait the see if there is some regression before to release a new version, i'd like to complete my unit tests too.