wunderio / code-quality

List of tools that aims to help with static code quality inspection.
GNU General Public License v3.0
2 stars 5 forks source link

Non Drupal 8 projects checking fails if PHP doesn't have iconv enabled #33

Open hkirsman opened 5 years ago

hkirsman commented 5 years ago

I added code-quality to my Drupal 7 project but it fails when commiting:

git commit                                                         
GrumPHP detected a pre-commit command.
GrumPHP is sniffing your code!
Running task 1/8: EcsTask... ✘
Aborted ...
PHP Warning:  Use of undefined constant ICONV_IMPL - assumed 'ICONV_IMPL' (this will throw an Error in a future version of PHP) in /home/hannes/Sites/tekla/campus/vendor/nette/utils/src/Utils/Strings.php on line 154
PHP Fatal error:  Uncaught Error: Call to undefined function Nette\Utils\iconv() in /home/hannes/Sites/tekla/campus/vendor/nette/utils/src/Utils/Strings.php:168
Stack trace:
#0 /home/hannes/Sites/tekla/campus/vendor/nette/utils/src/Utils/Strings.php(180): Nette\Utils\Strings::toAscii('/home/hannes/Si...')
#1 /home/hannes/Sites/tekla/campus/vendor/symplify/easy-coding-standard/packages/ChangedFilesDetector/src/Cache/Simple/FilesystemCacheFactory.php(22): Nette\Utils\Strings::webalize('/home/hannes/Si...')
#2 /tmp/easy_coding_standard/ContainerZdGlkCq/getFilesystemCacheService.php(9): Symplify\EasyCodingStandard\ChangedFilesDetector\Cache\Simple\FilesystemCacheFactory->create()
#3 /tmp/easy_coding_standard/ContainerZdGlkCq/HttpKernelSymplify_EasyCodingStandard_HttpKernel_EasyCodingStandardKernelProdd90fcfcbe4265705e72551ee048dbb9063446Container.php(152): require('/tmp/easy_codin...')
#4 /tmp/easy_coding_standard/ContainerZdGlkCq/getSimpleCacheAdapterService.php(9): ContainerZdGlkCq\HttpKernelSymplify_EasyCodingStandard_HttpKerne in /home/hannes/Sites/tekla/campus/vendor/nette/utils/src/Utils/Strings.php on line 168
To skip commit checks, add -n or --no-verify flag to commit command

My system doesn't seem to have php module iconv enabled and configured. But then again it works fine for Drupal 8. When checking deeper, then I found that Drupal 8 has symfony/polyfill-iconv package installed. This solved my issue:

 composer require symfony/polyfill-iconv:^1 

It's missing --dev intentionally because Drupal 8 web/core/composer.json file has the dependency also in require section as:

"symfony/polyfill-iconv": "^1.0",
hkirsman commented 4 years ago

1 more comment on the issue that it in case you have for example PHP 7.3 installed but not the iconv module enabled, then it will fail silently as it will install 1.0.3 version of Code Quality tool. You'll then don't have the phpstan package installed and the line from documentation to copy it's config will fail:

cp vendor/wunderio/code-quality/config/phpstan.neon ./phpstan.neon
hkirsman commented 4 years ago

I was not able to solve it from composer.json.

Tried:

  1. Install the polyfill but for Composer this is not the same as ext-iconv
"require": {
    "symfony/polyfill-iconv": "^1",
  1. Created fake iconv that has provide entry in it, still Composer did not like it.
    "require": {
        "symfony/polyfill-iconv": "^1",
        "wunderio/fake-iconv": "^1",
  1. Tried provide in the package itself. Nothing.
    "provide": {
        "ext-iconv": "*"
    },
  1. Faked the platform but I guess it works from the composer.json thay user has in his system.
    "config": {
        "platform": {
            "ext-iconv": "0"
        }
    },

All in all, it's advised to use the PHP iconv library as it's faster. Some more options to propose here

  1. Still add the polyfill and update readme install section to ignore the requirements with --ignore-platform-reqs
  2. Add note to readme about the issue and say why it is the way it is.

Something else?

@guncha25 @mgalang

hkirsman commented 4 years ago

Ok, here's my last thought on this. Here's the situation that I would like to avoid. I've installed the latest code-quality in lando using:

lando composer require wunderio/code-quality --dev

Everything goes fine but now when trying to commit it yells about the iconv library:

alt text

Wouldn't hurt to add this safe fallback for projects like Drupal 7 (it's in core drupal 8 composer.json)?