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#"

Issue fork domain-3291185

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

agentrickard created an issue. See original summary.

akshaydalvi212 made their first commit to this issue’s fork.

akshaydalvi212’s picture

Assigned: Unassigned » akshaydalvi212

i will work on this issue.

akshaydalvi212’s picture

Assigned: akshaydalvi212 » Unassigned
Status: Active » Needs review

Fix all the dependency injection in the module.
kindly review and merge the MR.

agentrickard’s picture

Status: Needs review » Needs work

Failing 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

agentrickard’s picture

Interesting....Drupal core now passes on 8.1

viniciuscosta’s picture

Assigned: Unassigned » viniciuscosta

Hi, I'll work on int

viniciuscosta’s picture

Assigned: viniciuscosta » Unassigned
Status: Needs work » Needs review
gabrieldv’s picture

Assigned: Unassigned » gabrieldv

I'll be reviewing it =)

agentrickard’s picture

Issue summary: View changes
gabrieldv’s picture

Assigned: gabrieldv » Unassigned
Status: Needs review » Needs work

I 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.

gabrieldv’s picture

Assigned: Unassigned » gabrieldv

I'll try to work on it again.

gabrieldv’s picture

Assigned: gabrieldv » Unassigned

Fixed 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)

christyanpaim’s picture

Assigned: Unassigned » christyanpaim
christyanpaim’s picture

Assigned: christyanpaim » Unassigned
Neeraj333’s picture

Assigned: Unassigned » Neeraj333
Neeraj333’s picture

Status: Needs work » Needs review
StatusFileSize
new68.69 KB
Neeraj333’s picture

Assigned: Neeraj333 » Unassigned
Neeraj333’s picture

StatusFileSize
new68.77 KB

Status: Needs review » Needs work

The last submitted patch, 21: fix_dependency_injection-3291185-20.patch, failed testing. View results

agentrickard’s picture

Version: 8.x-1.x-dev » 2.0.x-dev

I 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.

roshni27’s picture

Assigned: Unassigned » roshni27
Issue tags: +Coding standards
StatusFileSize
new109.97 KB

Execute 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.

FILE: /home/lenovo/d7to9/web/modules/contrib/domain/domain_content/src/Controller/DomainContentController.php
-----------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERROR AND 4 WARNINGS AFFECTING 0 LINES
-----------------------------------------------------------------------------------------------------------------------------------
  40 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
  41 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
 106 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
 122 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
-----------------------------------------------------------------------------------------------------------------------------------
FILE: /home/lenovo/d7to9/web/modules/contrib/domain/domain_source/src/Form/DomainSourceSettingsForm.php
-----------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNINGS AFFECTING 1 LINES
-----------------------------------------------------------------------------------------------------------------
 33 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
-----------------------------------------------------------------------------------------------------------------

FILE: /home/lenovo/d7to9/web/modules/contrib/domain/domain_config/src/DomainConfigOverrider.php
---------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERROR AND 2 WARNINGS AFFECTING 2 LINES
---------------------------------------------------------------------------------------------------------------------------------
 236 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
 240 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
---------------------------------------------------------------------------------------------------------------------------------

FILE: /home/lenovo/d7to9/web/modules/contrib/domain/domain_config_ui/src/Form/DeleteForm.php
----------------------------------------------------------------------------------------------------------------------
FOUND 0 ERROR AND 4 WARNINGS AFFECTING 4 LINES
----------------------------------------------------------------------------------------------------------------------
 34 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
 94 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
 95 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
 96 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
----------------------------------------------------------------------------------------------------------------------

FILE: /home/lenovo/d7to9/web/modules/contrib/domain/domain/src/Commands/DomainCommands.php
-----------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERROR AND 7 WARNINGS AFFECTING 7 LINES
-----------------------------------------------------------------------------------------------------------------------------------------------------------------
  951 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
  993 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
 1140 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
 1157 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
 1250 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
 1282 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
 1317 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
-----------------------------------------------------------------------------------------------------------------------------------------------------------------

FILE: /home/lenovo/d7to9/web/modules/contrib/domain/domain/src/Form/DomainForm.php
--------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
--------------------------------------------------------------------------------------------------
 221 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
 224 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
--------------------------------------------------------------------------------------------------

FILE: /home/lenovo/d7to9/web/modules/contrib/domain/domain/src/Form/DomainSettingsForm.php
-----------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNINGS AFFECTING 1 LINES
-----------------------------------------------------------------------------------------------------------------
 48 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
-----------------------------------------------------------------------------------------------------------------

FILE: /home/lenovo/d7to9/web/modules/contrib/domain/domain_alias/src/Form/DomainAliasForm.php
--------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
--------------------------------------------------------------------------------------------------
 162 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
 224 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
 231 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
--------------------------------------------------------------------------------------------------

I am working on it.

roshni27’s picture

Assigned: roshni27 » Unassigned
Status: Needs work » Needs review
Issue tags: -Coding standards +Dependency Injection (DI)
StatusFileSize
new24.67 KB

Branch :2.0.x
Some issue remain due to circular reference. Please review the patch.

Status: Needs review » Needs work

The last submitted patch, 25: dependency-injection-3291185-25.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

chaitanyadessai made their first commit to this issue’s fork.

chaitanyadessai’s picture

Status: Needs work » Needs review

Please review MR #27

idebr’s picture

Status: Needs review » Closed (duplicate)