"Copyright is missing or has wrong format" warning on custom modules

[hostep]

Preconditions

Hi folks, I recently tried upgrading the coding standards library in my custom module from version 6 to version 10 and it suddenly starts complaining about missing copyright headers in files in my module?

This is something I should decide, not something you should decide right?

This coding standards library is still meant for 3rd party modules as well, right? Not only for checking core Magento files?

Steps to reproduce

1. Have a custom module
2. Have a file etc/di.xml in that module that contains no copyright header

Expected result

1. No warnings

Actual result

1. Getting a warning:

FILE: /path/to/custom/module/etc/di.xml
----------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------
  | WARNING | Copyright is missing or has wrong format
----------------------------------------------------------------------------------------------------

[m2-assistant]

Hi @hostep. Thank you for your report.
To help us process this issue please make sure that you provided sufficient information.

Please, add a comment to assign the issue: @magento I am working on this

---

• Join Magento Community Engineering Slack and ask your questions in #github channel.

[hostep]

Maybe the copyright check should only happen on files that use Magento as first part of their namespace?

[svera]

@hostep thank you for the report. You could also exclude this specific sniff through the command line using the --exclude=Magento2.Legacy.CopyrightAnotherExtensionsFiles flag, or create another ruleset based on the Magento one that excludes the sniffs you don't want.

@fascinosum do you think we should restrict this sniff to files in the magento namespace only?

[hostep]

I've thought about that namespace a bit more, but that only applies to PHP files, while the copyright is also required in other files like xml files, so that would be tricky to get right probably.

I'm aware of the exclude flag and I was using it to filter that warning out, but thanks for putting it out here in case other people run into the same problem! 🙂

[sivaschenko]

@hostep @svera what do you think about introducing a new ruleset dividing the tests designed for Adobe projects specifically and less strict rules for 3rd party projects. That might be related to internal ticket AC-206

[hostep]

@sivaschenko: that would probably be the best solution!

Other good candidates that could be added to the Adobe ruleset are:

• Magento2.Annotation.MethodAnnotationStructure.MethodAnnotation
• Magento2.Annotation.MethodAnnotationStructure.MethodArguments
• Magento2.Annotation.MethodArguments.MethodArguments
• Magento2.Commenting.ClassPropertyPHPDocFormatting.Missing
• Magento2.Legacy.Copyright.FoundCopyrightMissingOrWrongFormat
• Magento2.Legacy.CopyrightAnotherExtensionsFiles.FoundCopyrightMissingOrWrongFormat

Which are all new warnings I saw after upgrading from v6 to v10 and Adobe shouldn't try to push for us to add phpdocs to every single little thing where it's really not worth to add docs.

[fascinosum]

@svera I agree with @sivaschenko, we should have a separate more advanced ruleset for Adobe repositories and a general basic ruleset

[hostep]

@svera: this seems to be fixed in v11 of this module. Should we close this issue or is there anything else you want to do in scope of this issue?

[fredden]

It looks like these sniffs were disabled in f8cab4a, but I can't tell if that was intended to be temporary or not. (If it was intended to be permanent, removing the sniffs would have been a better choice.) There's mention of AC-1335 and AC-1314 in the commit, but I have no way of knowing what they refer to (other than something private to Adobe staff). No context for the commit was given in #284.

[sivaschenko]

@fredden @hostep @ihor-sviziev we are planning to introduce several rulesets to the magento-conding-standard:

• Ruleset including all the tests for Adobe projects
• Ruleset including all the tests (and including code style) for 3rd-party extension developers
• Ruleset including only tests checking for functional, compatibility and security issues for system integrators

Copyright test is going to be included in the first ruleset only.

[ihor-sviziev]

@sivaschenko TBH, I think 2nd, and 3rd options might be merged into one. But I don't mind against it

[sivaschenko]

@hostep @fredden @ihor-sviziev A separate standard has been introduced for framework-specific checks and Copyright sniffs have been moved to this standard in #313

[ihor-sviziev]

@sivaschenko Great! Thx for the update. Waiting for the release

[sivaschenko]

@ihor-sviziev new version is avaiable https://github.com/magento/magento-coding-standard/releases/tag/v14

[ihor-sviziev]

@hostep, could you confirm that issue is fixed in version 14?

[hostep]

@ihor-sviziev: as mentioned above, this was already fixed in v11 of this module (just re-tested this right now to be sure, and it's fixed in versions 11, 12, 13 and 14)

So, should we close this issue, or is something still not finished yet?

[hostep]

I do still believe some other checks shouldn't be included in the Magento2 coding standard (requiring documentation on top of every single method, class or member for example), I don't see what the benefit of this is and why it should be required for non-Magento core code?

[ihor-sviziev]

@hostep thank you for confirmation!
I'm closing this issue

[ihor-sviziev]

I do still believe some other checks shouldn't be included in the Magento2 coding standard (requiring documentation on top of every single method, class or member for example), I don't see what the benefit of this is and why it should be required for non-Magento core code?

@hostep, as to me - it's a good idea. Could you create a separate issue for that with a list of suggestions?

[fredden]

There's an existing list here: #275 (comment)

[sivaschenko]

@hostep @fredden @ihor-sviziev it is our recommendation to use the annotation sniffs for extensions and customizations, so they will be kept in the Magento2 coding standard for now. I do understand that each project is unique and can have a specific preference for the code style.

There is an ability to adjust the ruleset provided by magento-coding-standard for your project needs, utilising the rules that are important for you and excluding the rules that are less important. A custom ruleset extending the ruleset from Magento2 standard can be created for this purpose.

Example:

<?xml version="1.0"?>
<ruleset name="MyProject">
    <description>Custom coding standard of my project.</description>
    <rule ref="./../../../vendor/magento/magento-coding-standard/Magento2">
        <exclude name="Magento2.Annotation.MethodAnnotationStructure.MethodAnnotation"/>
        <exclude name="Magento2.Annotation.MethodAnnotationStructure.MethodArguments"/>
        <exclude-pattern>*/_files/*</exclude-pattern>
    </rule>
</ruleset>