~/.composer/vendor/bin/phpcs --standard=/home/peter/.composer/vendor/drupal/coder/coder_sniffer/Drupal --sniffs=Drupal.Classes.ClassCreateInstance core/lib/Drupal/Core/Controller/ControllerResolver.php 

FILE: ...bdata/www/core/lib/Drupal/Core/Controller/ControllerResolver.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 76 | ERROR | Calling class constructors must always include
    |       | parentheses
----------------------------------------------------------------------

Offending line: return new $controller;

CommentFileSizeAuthor
#5 diff-2573195-3-5.txt5.31 KBanoopjohn
#5 coder-auto-fix-class-create-instance-parenthesis-2573195.patch4.79 KBanoopjohn
#3 interdiff.txt581 bytespfrenssen
#3 2573195-3.patch2.25 KBpfrenssen
#2 i2573195-2.patch1.82 KBattiks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

attiks created an issue. See original summary.

attiks’s picture

attiks CreditAttribution: attiks at Attiks commented
Status: Needs work » Needs review
Related issues: +#2572601: Fix 'Drupal.Classes.ClassCreateInstance' coding standard
FileSize
i2573195-2.patch1.82 KB

Patch to mark them as fixable, and fix them

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen commented
FileSize
2573195-3.patch2.25 KB
interdiff.txt581 bytes

Added a test.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented
Status: Needs review » Needs work 
  1. +++ b/coder_sniffer/Drupal/Sniffs/Classes/ClassCreateInstanceSniff.php
@@ -55,9 +55,17 @@ class Drupal_Sniffs_Classes_ClassCreateInstanceSniff implements PHP_CodeSniffer_
+            // Detect and fix new \DOMDocument;

    Should say "Detect and fix constructor calls with namespaces, example "new \DOMDocument;". 

  2. +++ b/coder_sniffer/Drupal/Sniffs/Classes/ClassCreateInstanceSniff.php
@@ -55,9 +55,17 @@ class Drupal_Sniffs_Classes_ClassCreateInstanceSniff implements PHP_CodeSniffer_
+              $constructor = $phpcsFile->findNext(PHP_CodeSniffer_Tokens::$emptyTokens, ($stackPtr + 2), null, true, null, true);

    indentation should be 4 spaces, per PHPCS coding standards that we use in sniffs. 

  3. +++ b/coder_sniffer/Drupal/Sniffs/Classes/ClassCreateInstanceSniff.php
@@ -55,9 +55,17 @@ class Drupal_Sniffs_Classes_ClassCreateInstanceSniff implements PHP_CodeSniffer_
+              if ($tokens[$stackPtr + 3]['code'] === T_STRING && $tokens[$stackPtr + 4]['code'] === T_SEMICOLON) {

    I don't think this is the correct fix. We should just search for T_STRING and T_NS_SEPARATOR sequences and insert he parenthesis after that. 

  4. +++ b/coder_sniffer/Drupal/Sniffs/Classes/ClassCreateInstanceSniff.php
--- /dev/null
+++ b/coder_sniffer/Drupal/Test/Classes/ClassCreateInstanceUnitTest.inc.fixed

    The test case should include a namespaces example like \DOMDocument and a longer example such as \Drupal\Core\Whatever.

anoopjohn’s picture

anoopjohn CreditAttribution: anoopjohn at Zyxware Technologies commented
Assigned: attiks » Unassigned
Status: Needs work » Needs review
FileSize
coder-auto-fix-class-create-instance-parenthesis-2573195.patch4.79 KB
diff-2573195-3-5.txt5.31 KB

I believe that part of the fix (test cases) had already gotten into coder. The auto fixing was not coded for the specific case listed in the issue line 70 in core/lib/Drupal/Core/Controller/ControllerResolver.php

I have added the logic to auto fix cases where classname is in a variable and also address cases where the name is in an array. Have updated the existing test cases for the automatic fixing.

Please find the patch attached. Interdiff was failing so attaching diff as well.

  • klausi committed d35c162 on 8.x-2.x authored by anoopjohn 
    Issue #2573195 by pfrenssen, anoopjohn, attiks: Not all Drupal.Classes....
klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented
Status: Needs review » Fixed

Committed, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.