Merge pull request #10265 from tuqqu/introduce-duplicate-property-issue

Introduce `DuplicateProperty` issue
vimeo · Oct 9, 2023 · 9b00ac0 · 9b00ac0
2 parents c71f0ab + 8edb886
commit 9b00ac0
Show file tree

Hide file tree

Showing 8 changed files with 95 additions and 0 deletions.
diff --git a/config.xsd b/config.xsd
@@ -230,6 +230,7 @@
             <xs:element name="DuplicateFunction" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="DuplicateMethod" type="MethodIssueHandlerType" minOccurs="0" />
             <xs:element name="DuplicateParam" type="IssueHandlerType" minOccurs="0" />
+            <xs:element name="DuplicateProperty" type="PropertyIssueHandlerType" minOccurs="0" />
             <xs:element name="EmptyArrayAccess" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="ExtensionRequirementViolation" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="FalsableReturnStatement" type="IssueHandlerType" minOccurs="0" />

diff --git a/docs/running_psalm/error_levels.md b/docs/running_psalm/error_levels.md
@@ -29,6 +29,7 @@ Level 5 and above allows a more non-verifiable code, and higher levels are even
  - [DuplicateFunction](issues/DuplicateFunction.md)
  - [DuplicateMethod](issues/DuplicateMethod.md)
  - [DuplicateParam](issues/DuplicateParam.md)
+ - [DuplicateProperty](issues/DuplicateProperty.md)
  - [EmptyArrayAccess](issues/EmptyArrayAccess.md)
  - [ExtensionRequirementViolation](issues/ExtensionRequirementViolation.md)
  - [ImplementationRequirementViolation](issues/ImplementationRequirementViolation.md)

diff --git a/docs/running_psalm/issues.md b/docs/running_psalm/issues.md
@@ -31,6 +31,7 @@
  - [DuplicateFunction](issues/DuplicateFunction.md)
  - [DuplicateMethod](issues/DuplicateMethod.md)
  - [DuplicateParam](issues/DuplicateParam.md)
+ - [DuplicateProperty](issues/DuplicateProperty.md)
  - [EmptyArrayAccess](issues/EmptyArrayAccess.md)
  - [ExtensionRequirementViolation](issues/ExtensionRequirementViolation.md)
  - [FalsableReturnStatement](issues/FalsableReturnStatement.md)

diff --git a/docs/running_psalm/issues/DuplicateProperty.md b/docs/running_psalm/issues/DuplicateProperty.md
@@ -0,0 +1,19 @@
+# DuplicateProperty
+
+Emitted when a class property is defined twice
+
+```php
+<?php
+
+class Foo
+{
+    public int $foo;
+    public string $foo;
+}
+
+class Bar
+{
+    public int $bar;
+    public static string $bar;
+}
+```
diff --git a/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php b/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php
@@ -42,6 +42,7 @@
 use Psalm\Issue\DuplicateClass;
 use Psalm\Issue\DuplicateConstant;
 use Psalm\Issue\DuplicateEnumCase;
+use Psalm\Issue\DuplicateProperty;
 use Psalm\Issue\InvalidAttribute;
 use Psalm\Issue\InvalidDocblock;
 use Psalm\Issue\InvalidEnumBackingType;
@@ -1613,6 +1614,16 @@ private function visitPropertyDeclaration(
         foreach ($stmt->props as $property) {
             $doc_var_location = null;
 
+            if (isset($storage->properties[$property->name->name])) {
+                IssueBuffer::maybeAdd(
+                    new DuplicateProperty(
+                        'Property ' . $fq_classlike_name . '::$' . $property->name->name . ' has already been defined',
+                        new CodeLocation($this->file_scanner, $stmt, null, true),
+                        $fq_classlike_name . '::$' . $property->name->name,
+                    ),
+                );
+            }
+
             $property_storage = $storage->properties[$property->name->name] = new PropertyStorage();
             $property_storage->is_static = $stmt->isStatic();
             $property_storage->type = $signature_type;

diff --git a/src/Psalm/Issue/DuplicateProperty.php b/src/Psalm/Issue/DuplicateProperty.php
@@ -0,0 +1,9 @@
+<?php
+
+namespace Psalm\Issue;
+
+final class DuplicateProperty extends PropertyIssue
+{
+    public const ERROR_LEVEL = -1;
+    public const SHORTCODE = 325;
+}
diff --git a/tests/ClassTest.php b/tests/ClassTest.php
@@ -1436,6 +1436,50 @@ class BazClass implements InterFaceA, InterFaceB {}
                 'error_message' => 'InheritorViolation',
                 'ignored_issues' => [],
             ],
+            'duplicateInstanceProperties' => [
+                'code' => <<<'PHP'
+                    <?php
+                    class Foo {
+                        public mixed $bar;
+                        public int $bar;
+                    }
+                    PHP,
+                'error_message' => 'DuplicateProperty',
+                'ignored_issues' => [],
+            ],
+            'duplicateStaticProperties' => [
+                'code' => <<<'PHP'
+                    <?php
+                    class Foo {
+                        public static mixed $bar = null;
+                        public static string $bar = 'bar';
+                    }
+                    PHP,
+                'error_message' => 'DuplicateProperty',
+                'ignored_issues' => [],
+            ],
+            'duplicateMixedProperties' => [
+                'code' => <<<'PHP'
+                    <?php
+                    class Foo {
+                        public bool $bar = true;
+                        public static bool $bar = false;
+                    }
+                    PHP,
+                'error_message' => 'DuplicateProperty',
+                'ignored_issues' => [],
+            ],
+            'duplicatePropertiesDifferentVisibility' => [
+                'code' => <<<'PHP'
+                    <?php
+                    class Foo {
+                        public bool $bar;
+                        private string $bar;
+                    }
+                    PHP,
+                'error_message' => 'DuplicateProperty',
+                'ignored_issues' => [],
+            ],
         ];
     }
 }
diff --git a/tests/TraitTest.php b/tests/TraitTest.php
@@ -1236,6 +1236,15 @@ trait A { const B = 0; }
                 'ignored_issues' => [],
                 'php_version' => '8.1',
             ],
+            'duplicateTraitProperty' => [
+                'code' => '<?php
+                    trait T {
+                        public mixed $foo = 5;
+                        protected static mixed $foo;
+                    }
+                    ',
+                'error_message' => 'DuplicateProperty',
+            ],
         ];
     }
 }