Enable phpcs rule DrupalPractice.InfoFiles.NamespacedDependency

Will review

Patch applies fine. I did a quick search of the core modules folder and found 151 matches in 142 files so there must be similar issues for other modules. Will scan the queue and raise issues as applicable. https://www.drupal.org/node/1935708 should probably be updated in line with this issue too.

If we do this ewe do this all in one go. Doing this per module is against the scoping advice. See https://www.drupal.org/core/scope for guidelines and examples for Drupal core issue scope.

@alexpott: no problem, I am happy to apply this to all core modules. Are you happy with reviewing 142 patch files?

@Andy_D it would be good to have some idea of what we gain. One thing I can see is a good example but when the dependency is inside the same project the project part is not required. This information is primarily for testbot to sort out dependencies.

Re-rolled patch to encompass all relevant modules. Note: I have ignored tests and sub-modules.

Well that didn't go well. SimplyTest.Me couldn't even build due to missing dependencies on profiles. Removed and re-rolled patch.

We'd have to do all the test modules too.

Re-re-rolling as I have all set-up, ready to go.

Try this bad boy!

Hiding previous patches

Patch applies cleanly. Tested by enabling some of the changed modules (forum, content moderation) and they both enabled and configured correctly.

Marking as RTBC again.

@Andy_D can you relate the coder issue to this one. We need to postpone on getting that done and agreed. So if we do this we don't introduce regressions.

@alexpott Issue added as requested. Thanks again for your help on this.

So until #2842706: Add a test for dependencies for *.info.yml files. in added to coder we can't really do this because it will be super simple to regress.

One consideration. *.info.yml files for profiles should be adjusted too.

When I run code sniffer provided in #2842706: Add a test for dependencies for *.info.yml files. I get following issue:

FILE: ...sites/drupal_2821499/core/profiles/standard/standard.info.yml
----------------------------------------------------------------------
FOUND 0 ERRORS AND 33 WARNINGS AFFECTING 33 LINES
----------------------------------------------------------------------
  8 | WARNING | All dependencies must be prefixed with the project
    |         | name, for example "drupal:"
  9 | WARNING | All dependencies must be prefixed with the project
    |         | name, for example "drupal:"

When I try (just for first module) to set prefix I get following error during installation:

Required modules not found.
The following modules are required but were not found. Move them into the appropriate modules subdirectory, such as /modules. Missing modules: Drupal:node

I don't know is there already issue for that or it should be solved here.

#2842706: Add a test for dependencies for *.info.yml files. is marked fixed.

That does not work for profiles, so filed
- #2855026-12: Installation profiles do not support project:module format for dependencies
- #2857856: Allow not namespaced dependencies in theme info YAML files

Rerolled and works perfectly on 8.4.x

I'm not sure how to test this. I applied the patch and reinstalled site and it works. If my testing is ok, this could be marked as RTBC.

I prefer issues in #28 solved first

https://www.drupal.org/project/drupal/issues/2855026 is commited, so we can continue.

Rerolled for 8.7.x

Good to go, only sniffers issue from #28 needs to be committed

I would still argue that we need the coding standards issues to land so we can enable the rule in core in this issue.

Here is re-roll of patch #37

The patch applied evenly and it's working as expected. Moving to reviewed.

#2842706: Add a test for dependencies for *.info.yml files. is marked fixed and added to the 8.x-3.x branch in January of 2017, but that sniff is not present in coder 3.7, released December 2019.

So we can't add it to phpcs.xml.dist, per #40.

Is it maybe a victim of that time someone deleted the github for Coder?

Turns out the sniff was moved in #2854781: NamespacedDependencySniff can have false positives for custom modules.

So we need to add the DrupalPractice.InfoFiles.NamespacedDependency sniff to core.phpcs.yml in order to prevent regressions, per #40.

Edited: Sorry, self-referenced

Duplicate with #3116077: All dependencies must be prefixed with the project name, which one to close?

Just closed #3116077: All dependencies must be prefixed with the project name as this one was filed earlier. Even though both expanded the scope in the middle. Changing the title to meet the scope.

Would be great having a new patch combining the patch from #3116077 and here for further review. For this purpose, adding a Needs reroll tag

I'm working on re-rolling this issue at Drupal Camp Manila 2020.

I could apply the latest patch cleanly against the latest 8.9.x codebase, so no reroll is needed. Removing the Needs reroll tag.

Hope everything is in order.

Thank you!

Re-rolled patch for 9.1.x

Patch #58 LGTM + 1 RTBC

This patch is not correct. Something happened... The patch in #29 was correct. The "reroll" in #37, and all the patches since then, are incorrect.

The project should be "drupal" everywhere, not things like

+++ b/core/modules/config/tests/config_install_fail_test/config_install_fail_test.info.yml
@@ -4,4 +4,4 @@ package: Testing
 version: VERSION
 core: 8.x
 dependencies:
-  - drupal:config_test
+  - config_test:config_test

Let's start with a newer patch from the duplicate issue #3116077, comment #18, by @vsujeetkumar. Just renamed the patch and reuploading here.

The phpcs snifif existed is DrupalPractice.InfoFiles.NamespacedDependency probably.

Rerolled from #61 partially, -- dropped conflicts.

Re-rolled patch for 9.1.x

as per as drupal naming convention https://www.drupal.org/docs/8/creating-custom-modules/let-drupal-8-know-... dependencies should be like {module}:{submodule}.in case of failed test cases "Failed asserting that exception of type "Drupal\Core\Extension\ModuleUninstallValidatorException" is thrown." array return project name and module name for the installation of the module, but test cases work only on the name.

I am not able to understand what's h going on here.
The huge differences between the sizes of the patch. As the title mentioned it should for all info.yml.
Please help me to understand the journey from #29 to #68

Re #70 changing the title that it's enough to make sense to you, I think, and 2821499-71-test-only.patch will show you how many to fix.

I do not think an interdiff is necessary, so ignored on purpose.

Crediting the people who worked on #3116077: All dependencies must be prefixed with the project name

Patch looks good, 21 issues in the test-only patch, all fixed in the green patch.

Saving issue credit for reviewers. #37 and #44 weren't correct -- please make sure to understand the goal of the issue before submitting a patch. :) Thanks to everyone who took the time to try and understand this issue and sorry that it was confusing!

More credits from the other issue.

It looks like most of the non-test modules as well as plenty of test modules were addressed in #2798891: Define project dependencies in core module .info files, and just these few were either missed or have crept in since. It's a good thing we have the coder rule now for that reason.

Committed to 9.1.x and cherry-picked to 9.0.x. I ran composer run phpcs -- -p against both branches to ensure they were passing with the new rule.

The patch did not cherry-pick cleanly to 8.9.x, so needs a backport.

On it, thanks for the RTBC @longwave, and thanks for committing @xjm!

A test only patch first

22 to fix from #86

The sniff is available in 8.8.x, the coder version locked is 8.3.6 -- not the latest, If it's necessary to backport this to 8.8.x, I am happy to make a new patch. But I will not do it without approval.

Another random failure? Requeued.

Fixing the test.

Since Bugfix support for Drupal 8.9.x is restricted to selected low-disruption major and critical bug-fixes and this was comitted to Drupal 9, changing status to Fixed.

Setting parent as this commit added a DrupalPractice sniff to phpcs.xml.dist