backdrop-ops / phpcs

Backdrop PHP coding standards.
GNU General Public License v2.0
2 stars 1 forks source link

"no blank lines before the file comment" error is not reported for namespaced classes #11

Open avpaderno opened 1 year ago

avpaderno commented 1 year ago

A file containing the following code causes a There must be no blank lines before the file comment error when running phpcs --standard="$HOME/.config/phpcs_rulesets/Backdrop" ./myclass.class.inc.

<?php

/**
 * @file
 * Contains \MyModule\MyClass.
 */

/**
 * Class description.
 */
class MyClass implements MyClassInterface {

  /**
   * A protected property.
   *
   * @var int
   */
  protected $property;

}

The full report is the following. (I just changed the full filename to relative and made the separation lines shorter.)

FILE: ./myclass.class.inc
-----------------
FOUND 1 ERROR AFFECTING 1 LINE
-----------------
 1 | ERROR | There must be no blank lines before the file comment
   |       | (Backdrop.Commenting.FileComment.SpacingAfterOpen)
-----------------

Adding a namespace, as in the following example code, that error is not anymore reported.

<?php

/**
 * @file
 * Contains \MyModule\MyClass.
 */

namespace MyModule;

/**
 * Class description.
 */
class MyClass implements MyClassInterface {

  /**
   * A protected property.
   *
   * @var int
   */
  protected $property;

}
FILE: ./myclass.class.inc
-----------------
FOUND 1 ERROR AFFECTING 1 LINE
-----------------
 3 | ERROR | [x] Namespaced classes, interfaces and traits should not begin with a file doc comment
   |       |     (Backdrop.Commenting.FileComment.NamespaceNoFileDoc)
-----------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------

I tried changing what follow Contains (just the class name, the class name and the namespace without the \ at the beginning), but phpcs still does not report that error, when a namespace is used.

I edited the code to show a more complete class definition.

avpaderno commented 1 year ago

The code showing that error is in the FileCommentSniff.php file, line 203 and following.

// No blank line between the open tag and the file comment.
if ($tokens[$commentStart]['line'] > ($tokens[$stackPtr]['line'] + 1)) {
    $error = 'There must be no blank lines before the file comment';
    $phpcsFile->addError($error, $stackPtr, 'SpacingAfterOpen');
}
avpaderno commented 1 year ago

I ran the following code through phpcs --standard=Drupal ./MyClass.php.

<?php

/**
 * @file
 * Contains \MyModule\MyClass.
 */

/**
 * Class description.
 */
class MyClass implements MyClassInterface {

  /**
   * A protected property.
   *
   * @var int
   */
  protected $property;

}

It does not report a There must be no blank lines before the file comment error. Adding a namespace Mymodule; line does not cause that error either, although it causes an error about the @file line, which is expected, since Drupal does not allow them in files containing a class.

avpaderno commented 1 year ago

I apologize for the noise.

I copied the FileCommentSniff.php file from https://raw.githubusercontent.com/pfrenssen/coder/8.3.x/coder_sniffer/Drupal/Sniffs/Commenting/FileCommentSniff.php, starting from line 14, into Backdrop/Sniffs/Commenting/FileCommentSniff.php.

Running phpcs --standard=$HOME/.config/phpcs_rulesets/Backdrop ./MyClass.php does not show anymore the There must be no blank lines before the file comment error. If I add a namespace, it shows the other error, as expected.

avpaderno commented 1 year ago

To make it clearer: I copied these lines and the following ones, until the end of the file.

use PHP_CodeSniffer\Files\File;
use PHP_CodeSniffer\Sniffs\Sniff;
avpaderno commented 1 year ago

The difference between the the FileCommentSniff.php file from pfrenssen/coder and the same file in this repository are the following line, apart the lines at the start of the file.

// No blank line between the open tag and the file comment.
if ($tokens[$commentStart]['line'] > ($tokens[$stackPtr]['line'] + 1)) {
    $error = 'There must be no blank lines before the file comment';
    $phpcsFile->addError($error, $stackPtr, 'SpacingAfterOpen');
}

In this repository, those lines have been added in the initial commit. I cannot find in the original file when those lines have been removed and why.

indigoxela commented 1 year ago

@kiamlaluno out of curiosity: why did you open yet another issue re namespace declaration. Isn't that a dup of #10?

As for the sniff forked from Drupal: I adapted the part that conflicted with the Backdrop standards. What's exactly the problem you have with that?

Your comment seems to suggest that I have to justify for starting a Backdrop-specific ruleset. :grin: Here's the related core discussion. https://github.com/backdrop/backdrop-issues/issues/5245

avpaderno commented 1 year ago

Actually, these are two different issues. If there should not be any empty line before the @file comment, that empty line there should not be whether the file defines a namespace or not. This is different from a @file comment that is not allowed in a file that defines a namespace.

What's exactly the problem you have with that?

I do not have any problem. I just reported an error that is shown in a case, but not in another case. That is all.