Automattic / phpcs-neutron-standard

A set of phpcs sniffs for PHP >7 development
MIT License
94 stars 7 forks source link

`RequireImports.Import` with `use const` #72

Closed Hywan closed 5 years ago

Hywan commented 5 years ago

Hello,

The following code:

use A\{B, C};

echo C;

will throw an error from ImportDetection.Imports.RequireImports.Import saying the constant C is unused, while it is :-).

Thanks for the tool!

sirbrillig commented 5 years ago

Thanks for the report!

Note: this is an issue for https://github.com/sirbrillig/phpcs-import-detection

However, I've been unable to reproduce the issue. I created a branch here to try to find it: https://github.com/sirbrillig/phpcs-import-detection/pull/23

Using that as a base, can you create a test case that fails?

sirbrillig commented 5 years ago

I also tried a simple manual test, and it looks correct to me (it only finds that B is unused):

user@dev~/public_html$ cat testConstants.php
<?php
use A\{B, C};

echo C;
user@dev:~/public_html$ phpcs --standard=ImportDetection testConstants.php

FILE: ~/public_html/testConstants.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 2 | WARNING | Found unused symbol 'A\B'.
----------------------------------------------------------------------
Hywan commented 5 years ago

Then, can you check D21704-code please?

sirbrillig commented 5 years ago

In that example, the code actually looks like this:

use const A\{
  B, 
  C,
};

echo C;

That is failing because in PHP, grouped import statements cannot have a trailing comma on the final symbol. If you run php -l on that file, you'll get an error: Parse error: syntax error, unexpected '}', expecting identifier (T_STRING).

If you remove the trailing comma, so it looks like the following, it should work.

use const A\{
  B, 
  C
};

echo C;
sirbrillig commented 5 years ago

Oh, It looks like PHP 7.2 might have added support for a trailing comma. I created an issue to track that, but in any case, the PHP we can use in the diff you mentioned is PHP 7.1 only.