Drupal Version
9.4.x.
Domain module version
8.x-1.x
Expected Behavior
Run drupal-check without errors.
Actual Behavior
Returns " \Drupal calls should be avoided in classes, use dependency injection instead "
Steps to reproduce
Run phpstan without suppressing "Drupal calls should be avoided in classes, use dependency injection instead"
UPDATE: Implementation details
The README shows what tests we want to run. The relevant parts are below.
### Code linting
We use (and recommend) [PHPCBF](https://phpqa.io/projects/phpcbf.html), [PHP Codesniffer](https://github.com/squizlabs/PHP_CodeSniffer) and [phpstan](https://phpstan.org/) for code quality review.
The following commands are run before commit:
* `vendor/bin/phpcbf web/modules/contrib/domain --standard="Drupal,DrupalPractice" -n --extensions="php,module,inc,install,test,profile,theme"`
* `vendor/bin/phpcs web/modules/contrib/domain --standard="Drupal,DrupalPractice" -n --extensions="php,module,inc,install,test,profile,theme"`
* `vendor/bin/phpstan analyse web/modules/contrib/domain`
### Testing tools
We are using the following composer dev dependencies for local testing:
```
"drupal/coder": "^8.3",
"mglaman/drupal-check": "^1.4",
"squizlabs/php_codesniffer": "^3.6",
```
Note that PHPCBF is installed as part of php_codesniffer.
### phpstan config
We use the following phpstan.neon file:
```
parameters:
level: 2
ignoreErrors:
# new static() is a best practice in Drupal, so we cannot fix that.
- "#^Unsafe usage of new static#"
# Ignore common errors for now.
- "#Drupal calls should be avoided in classes, use dependency injection instead#"
drupal:
entityMapping:
domain:
class: Drupal\domain\Entity\Domain
storage: Drupal\domain\DomainStorage
domain_alias:
class: Drupal\domain_alias\Entity\DomainAlias
storage: Drupal\domain_alias\DomainAliasStorage
```
The drupal entityMapping is also provided by `entity_mapping.neon` in the project root, for use with other tests.
For this issue, we want to remove this part of phpstan.neon:
# Ignore common errors for now.
- "#Drupal calls should be avoided in classes, use dependency injection instead#"
Comments
Comment #3
akshaydalvi212 commentedi will work on this issue.
Comment #5
akshaydalvi212 commentedFix all the dependency injection in the module.
kindly review and merge the MR.
Comment #6
agentrickardFailing tests. And the tests should be against Drupal 9.4 probably using PHP 8.0 -- I am not aware that tests will pass on 8.1, but re-running.
See https://www.drupal.org/node/99990/qa
Comment #7
agentrickardInteresting....Drupal core now passes on 8.1
Comment #8
viniciuscosta commentedHi, I'll work on int
Comment #9
viniciuscosta commentedComment #10
gabrieldv commentedI'll be reviewing it =)
Comment #11
agentrickardComment #12
gabrieldv commentedI was not able to test it, but i'll put it in "Needs Work" because the Drupal tests didn't pass in the #8 change.
Comment #13
gabrieldv commentedI'll try to work on it again.
Comment #15
gabrieldv commentedFixed all dependency injection on the code, except from the tests.
SInce the other commits were not passing the CI tests, i've analyzed it and found in the changes, parts that was not in the issue scope that was generating a lot of errors. Because of this, i created another MR with only the scope fixes.
I've tried to talk with the maintainer regarding doing DI on the tests, but didn't got a response, so i'm keeping this on "Needs work".
(Very good automated tests in this module btw)
Comment #16
christyanpaim commentedComment #17
christyanpaim commentedComment #18
Neeraj333 commentedComment #19
Neeraj333 commentedComment #20
Neeraj333 commentedComment #21
Neeraj333 commentedComment #23
agentrickardI don't really intend to do another 8.x-1.x release. We should fix this in 2.0.
See #3363211: Drupal 10.0.x Updates for the latest code.
Comment #24
roshni27 commentedExecute the command: phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig domain/
I have share text file in which 2.0.x phpcs error and warning.
Since this issue is focus only on dependency injection i will solve only that.
I am working on it.
Comment #25
roshni27 commentedBranch :2.0.x
Some issue remain due to circular reference. Please review the patch.
Comment #28
chaitanyadessai commentedPlease review MR #27
Comment #29
idebr commentedThis was fixed in #3458055: Configure and fix PHPStan issues