Merge pull request #9311 from weirdan/flag-direct-constructor-calls

Flag direct constructor calls

config.xsd

@@ -218,6 +218,7 @@
             <xs:element name="DeprecatedMethod" type="MethodIssueHandlerType" minOccurs="0" />
             <xs:element name="DeprecatedProperty" type="PropertyIssueHandlerType" minOccurs="0" />
             <xs:element name="DeprecatedTrait" type="IssueHandlerType" minOccurs="0" />
+            <xs:element name="DirectConstructorCall" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="DocblockTypeContradiction" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="DuplicateArrayKey" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="DuplicateClass" type="IssueHandlerType" minOccurs="0" />

docs/running_psalm/error_levels.md

@@ -226,6 +226,7 @@ Level 5 and above allows a more non-verifiable code, and higher levels are even
 - [DeprecatedMethod](issues/DeprecatedMethod.md)
 - [DeprecatedProperty](issues/DeprecatedProperty.md)
 - [DeprecatedTrait](issues/DeprecatedTrait.md)
+- [DirectConstructorCall](issues/DirectConstructorCall.md)
 - [DocblockTypeContradiction](issues/DocblockTypeContradiction.md)
 - [InvalidDocblockParamName](issues/InvalidDocblockParamName.md)
 - [InvalidFalsableReturnType](issues/InvalidFalsableReturnType.md)

docs/running_psalm/issues.md

@@ -21,6 +21,7 @@
  - [DeprecatedMethod](issues/DeprecatedMethod.md)
  - [DeprecatedProperty](issues/DeprecatedProperty.md)
  - [DeprecatedTrait](issues/DeprecatedTrait.md)
+ - [DirectConstructorCall](issues/DirectConstructorCall.md)
  - [DocblockTypeContradiction](issues/DocblockTypeContradiction.md)
  - [DuplicateArrayKey](issues/DuplicateArrayKey.md)
  - [DuplicateClass](issues/DuplicateClass.md)

docs/running_psalm/issues/DirectConstructorCall.md

@@ -0,0 +1,12 @@
+# DirectConstructorCall
+
+Emitted when `__construct()` is called directly as a method. Constructors are supposed to be called implicitely, as a result of `new ClassName` statement.
+
+```php
+<?php
+class A {
+    public function __construct() {}
+}
+$a = new A;
+$a->__construct(); // wrong
+```

src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php

@@ -13,6 +13,7 @@ use Psalm\Internal\Analyzer\Statements\Expression\ExpressionIdentifier;
 use Psalm\Internal\Analyzer\Statements\ExpressionAnalyzer;
 use Psalm\Internal\Analyzer\StatementsAnalyzer;
 use Psalm\Internal\Type\TemplateResult;
+use Psalm\Issue\DirectConstructorCall;
 use Psalm\Issue\InvalidMethodCall;
 use Psalm\Issue\InvalidScope;
 use Psalm\Issue\NullReference;
@@ -90,6 +91,18 @@ class MethodCallAnalyzer extends CallAnalyzer
                     return false;
                 }
             }
+
+            if ($stmt->name instanceof PhpParser\Node\Identifier
+                && strtolower($stmt->name->name) === '__construct'
+            ) {
+                IssueBuffer::maybeAdd(
+                    new DirectConstructorCall(
+                        'Constructors should not be called directly',
+                        new CodeLocation($statements_analyzer->getSource(), $stmt),
+                    ),
+                    $statements_analyzer->getSuppressedIssues(),
+                );
+            }
         }
 
         $lhs_var_id = ExpressionIdentifier::getExtendedVarId(

src/Psalm/Issue/DirectConstructorCall.php

@@ -0,0 +1,9 @@
+<?php
+
+namespace Psalm\Issue;
+
+final class DirectConstructorCall extends CodeIssue
+{
+    public const ERROR_LEVEL = 2;
+    public const SHORTCODE = 318;
+}

tests/ClassTest.php

@@ -1210,6 +1210,27 @@ class ClassTest extends TestCase
                 'ignored_issues' => [],
                 'php_version' => '7.0',
             ],
+            'directConstructorCall' => [
+                'code' => '<?php
+                    class A {
+                        public function __construct() {}
+                    }
+                    $a = new A;
+                    $a->__construct();
+                ',
+                'error_message' => 'DirectConstructorCall',
+            ],
+            'directConstructorCallOnThis' => [
+                'code' => '<?php
+                    class A {
+                        public function __construct() {}
+                        public function f(): void { $this->__construct(); }
+                    }
+                    $a = new A;
+                    $a->f();
+                ',
+                'error_message' => 'DirectConstructorCall',
+            ],
         ];
     }
 }