magento-coding-standard: "Copyright is missing or has wrong format" warning on custom modules

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

About this issue

  • Original URL
  • State: closed
  • Created 3 years ago
  • Reactions: 3
  • Comments: 21 (14 by maintainers)

Most upvoted comments

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

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

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 @fredden @ihor-sviziev A separate standard has been introduced for framework-specific checks and Copyright sniffs have been moved to this standard in https://github.com/magento/magento-coding-standard/pull/313

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

@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>

It looks like these sniffs were disabled in https://github.com/magento/magento-coding-standard/commit/f8cab4a2223bf49acd0686a0016517502b9b78c3, 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.

@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?