axelerant / drupal-quality-checker Goto Github PK

After the discussion here, I am reconsidering the decision to only output .dist files and overwrite them with every install. Does it feel very confusing and potentially fragile? What do you think of writing non-dist files and then having users own them. We could build tooling to let users diff and update their config once we roll out updates (it's tricky to do that but we can discuss this later).

Coder recently added a sniff to check if root namespace classes (or interfaces or traits) are fully namespaced. As a consequence, the following code does not pass the sniff.

use Twig_Extension;
use Twig_SimpleFilter;

new Twig_Extension();

The new sniffs check that the code references classes in the root namespace use the full name and not get imported.

new \Twig_Extension();

However, this causes an issue with phpmd: the above example will not pass phpmd's cleancode rules.

There is no workaround to this situation except bypassing checks (git commit -n).

We should adhere to Drupal's coding standards, or at least, coder's interpretation of Drupal's coding standards in this case. The fix would be to exclude this specific rule from phpmd.

Hello,

It is not possible to instantiate or use dependency injection to retrieve a site settings.
So it is a good idea to add an exclusion for this class in the phpmd.xml.dist file

We require the following dependencies:

        "dealerdirect/phpcodesniffer-composer-installer": "*",
        "drupal/coder": "^8.3.7",
        "friendsoftwig/twigcs": "^4.0 || ^5.0 || ^6.0",
        "php-parallel-lint/php-parallel-lint": "^1.2",
        "phpcompatibility/php-compatibility": "^9.0",
        "phpmd/phpmd": "^2.8",
        "sebastian/phpcpd": "^3.0 || ^4.0 || ^5.0 || ^6.0"

There is always a possibility that these dependencies could introduce conflicts in some projects. Consider moving them to composer.json suggested rather than require so that the dependencies are not locked.

The problem is if we remove all the dependencies, then our default grumphp config will fail. We will need to make it clear in the documentation that the user is responsible for installing the relevant packages.

Some of the tools we use require conflicting dependencies which makes it difficult to use the latest or even supported versions of these tools. For example, the latest release of PHPCPD is version 6 but only PHPCPD 4 may be installed on a Drupal 8 site (because of Symfony dependencies). This is even more of a problem because GrumPHP has broken support for PHPCPD 4. This means that the version of PHPCPD that gets installed for Drupal 8 does not work at all. We can't change the constraints or this entire package will stop working for Drupal 8. The only options are to ignore the error or remove the dependency on PHPCPD.

Phive lets us avoid all of these problems by installing the PHAR versions of the tools rather than through composer. This keeps the project free of these dependencies. The problem is that this requires users to install another tool (Phive) and also an extra step to run phive install after installing dependencies through composer.

GrumPHP supports TwigCS, so we don't have to do much. We should just put it in our default template.

Hi,

It is not possible to instantiate or use dependency injection to use Checkboxes methods (e.g. Checkboxes::detectEmptyCheckboxes())
So it is a good idea to add an exclusion for this class in the phpmd.xml.dist file

Look at all supported GrumPHP tasks and adopt those that make sense. https://github.com/phpro/grumphp/blob/v2.x/doc/tasks.md

It turns out that there are various issues with using drupal/core-composer-scaffold to copy the files. The most obvious one is that someone has to remember to add this package to composer.json extra section. It's documented, but it's not typical and people are likely to forget.

Another problem is that GrumPHP generates a configuration file on installation as well and there is no clear way to disable this. Fortunately, it checks if a configuration file is already present before generating one. So, if we generate our configuration file before, GrumPHP won't create one. However, the drupal/core-composer-scaffold plugin copies files at the very end which means we can't create the file before.

We should figure out a way to copy the file before events in GrumPHP are fired. The current solution in my mind is to write our own composer plugin which executes before GrumPHP and copies the files. The benefit with this approach is that we no longer have to rely on drupal/core-composer-scaffold to copy files either. So, it's a win-win except that we now have to maintain a plugin just for this.

Using composer for development tools have problems with conflicting dependencies (#12) and phpcpd makes it clear that they don't recommend composer. More here: https://twitter.com/s_bergmann/status/999635212723212288 and sebastianbergmann/phpcpd@dac917d.

Update static calls exceptions list.

Hi,
Maybe I'm missing something but PHPCompatibility last release (9.3.5) is three years old and doesn't handle properly PHP 8.0 or 8.1 compatibility:

• PHPCompatibility/PHPCompatibility#809
• PHPCompatibility/PHPCompatibility#1299

I think drupal-quality-checker should use 'dev-develop' or '*' in order to be able to be use this standard properly with modern PHP versions.

I have noticed that when paths in ignore_patterns have a / at the end, it generates the following warning while running GrumPHP:

PHP Warning: preg_match(): Unknown modifier 't' in phar:///var/www/html/vendor/phpro/grumphp-shim/grumphp.phar/vendor/symfony/finder/Iterator/MultiplePcreFilterIterator.php on line 53

Solutions:
Fix paths for ignore_patterns in grumphp.yml.dist

The default files are the same which is confusing when using a non color term.

There is a 1.0 release of GrumPHP. We should determine if we need to change our config file and then update to the new version.

One issue I noticed so far is that GrumPHP is now prompting for creating a config on installation. We should find an option to disable that.

As per the changelog, there is no change in usage of GrumPHP. However, the task extension system has changed but this does not affect this repository.

Problem Statement:

I'm not able to use it on Drupal 10 project. The problem is coming because of friendsoftwig/twigcs
https://github.com/axelerant/drupal-quality-checker/blob/v1.2.0/composer.json#L16

Possible Solution:

Update the dependencies from:
"friendsoftwig/twigcs": "^3.2 || ^4.0 || ^5.0",
to
"friendsoftwig/twigcs": "^3.2 || ^4.0 || ^5.0 || ^ 6.0",