From 4e859671844036ac937f1b41aae62443af8c8b60 Mon Sep 17 00:00:00 2001
From: Matthew Brown <github@muglug.com>
Date: Sun, 5 Jan 2020 21:58:18 -0500
Subject: [PATCH] Fix tests

---
 config.xsd                                    |  2 ++
 docs/running_psalm/issues.md                  | 33 +++++++++++++++++
 .../Assignment/PropertyAssignmentAnalyzer.php | 34 +++++++++++++-----
 .../Fetch/PropertyFetchAnalyzer.php           | 35 ++++++++++++++-----
 tests/MagicPropertyTest.php                   |  4 +--
 5 files changed, 88 insertions(+), 20 deletions(-)

diff --git a/config.xsd b/config.xsd
index 53918684e4c..eb42a124e38 100644
--- a/config.xsd
+++ b/config.xsd
@@ -341,6 +341,8 @@
             <xs:element name="UndefinedFunction" type="FunctionIssueHandlerType" minOccurs="0" />
             <xs:element name="UndefinedInterface" type="ClassIssueHandlerType" minOccurs="0" />
             <xs:element name="UndefinedInterfaceMethod" type="MethodIssueHandlerType" minOccurs="0" />
+            <xs:element name="UndefinedMagicPropertyAssignment" type="PropertyIssueHandlerType" minOccurs="0" />
+            <xs:element name="UndefinedMagicPropertyFetch" type="PropertyIssueHandlerType" minOccurs="0" />
             <xs:element name="UndefinedMethod" type="MethodIssueHandlerType" minOccurs="0" />
             <xs:element name="UndefinedPropertyAssignment" type="PropertyIssueHandlerType" minOccurs="0" />
             <xs:element name="UndefinedPropertyFetch" type="PropertyIssueHandlerType" minOccurs="0" />
diff --git a/docs/running_psalm/issues.md b/docs/running_psalm/issues.md
index 6f7903fa615..1b462ca75c7 100644
--- a/docs/running_psalm/issues.md
+++ b/docs/running_psalm/issues.md
@@ -2304,6 +2304,39 @@ class C {}
 interface I extends C {}
 ```
 
+### UndefinedMagicPropertyAssignment
+
+Emitted when assigning a property on an object that doesn’t have that magic property defined
+
+```php
+/**
+ * @property string $bar
+ */
+class A {
+    /** @param mixed $value */
+    public function __set(string $name, $value) {}
+}
+$a = new A();
+$a->foo = "bar";
+```
+
+### UndefinedMagicPropertyFetch
+
+Emitted when getting a property on an object that doesn’t have that magic property defined
+
+```php
+/**
+ * @property string $bar
+ */
+class A {
+    public function __get(string $name) {
+        return "cool";
+    }
+}
+$a = new A();
+echo $a->foo;
+```
+
 ### UndefinedMethod
 
 Emitted when calling a method that doesn’t exist
diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/PropertyAssignmentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/PropertyAssignmentAnalyzer.php
index 437a2211ad5..7694bf1c16a 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/PropertyAssignmentAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/PropertyAssignmentAnalyzer.php
@@ -341,6 +341,8 @@ public static function analyzeInstance(
                 $property_id = $fq_class_name . '::$' . $prop_name;
                 $property_ids[] = $property_id;
 
+                $has_magic_setter = false;
+
                 if ($codebase->methodExists($fq_class_name . '::__set')
                     && (!$codebase->properties->propertyExists($property_id, false, $statements_analyzer, $context)
                         || ($lhs_var_id !== '$this'
@@ -355,6 +357,7 @@ public static function analyzeInstance(
                             ) !== true)
                     )
                 ) {
+                    $has_magic_setter = true;
                     $class_storage = $codebase->classlike_storage_provider->get($fq_class_name);
 
                     if ($var_id) {
@@ -495,15 +498,28 @@ public static function analyzeInstance(
                             // fall through
                         }
                     } else {
-                        if (IssueBuffer::accepts(
-                            new UndefinedPropertyAssignment(
-                                'Instance property ' . $property_id . ' is not defined',
-                                new CodeLocation($statements_analyzer->getSource(), $stmt),
-                                $property_id
-                            ),
-                            $statements_analyzer->getSuppressedIssues()
-                        )) {
-                            // fall through
+                        if ($has_magic_setter) {
+                            if (IssueBuffer::accepts(
+                                new UndefinedMagicPropertyAssignment(
+                                    'Magic instance property ' . $property_id . ' is not defined',
+                                    new CodeLocation($statements_analyzer->getSource(), $stmt),
+                                    $property_id
+                                ),
+                                $statements_analyzer->getSuppressedIssues()
+                            )) {
+                                // fall through
+                            }
+                        } else {
+                            if (IssueBuffer::accepts(
+                                new UndefinedPropertyAssignment(
+                                    'Instance property ' . $property_id . ' is not defined',
+                                    new CodeLocation($statements_analyzer->getSource(), $stmt),
+                                    $property_id
+                                ),
+                                $statements_analyzer->getSuppressedIssues()
+                            )) {
+                                // fall through
+                            }
                         }
                     }
 
diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/PropertyFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/PropertyFetchAnalyzer.php
index 8f2c959ef7b..18810e512e1 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/PropertyFetchAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/PropertyFetchAnalyzer.php
@@ -413,6 +413,8 @@ public static function analyzeInstance(
 
             $override_property_visibility = false;
 
+            $has_magic_getter = false;
+
             $class_exists = false;
             $interface_exists = false;
 
@@ -498,6 +500,8 @@ public static function analyzeInstance(
                         ) !== true)
                 )
             ) {
+                $has_magic_getter = true;
+
                 $class_storage = $codebase->classlike_storage_provider->get($fq_class_name);
 
                 if (isset($class_storage->pseudo_property_get_types['$' . $prop_name])) {
@@ -636,15 +640,28 @@ public static function analyzeInstance(
                             // fall through
                         }
                     } else {
-                        if (IssueBuffer::accepts(
-                            new UndefinedPropertyFetch(
-                                'Instance property ' . $property_id . ' is not defined',
-                                new CodeLocation($statements_analyzer->getSource(), $stmt),
-                                $property_id
-                            ),
-                            $statements_analyzer->getSuppressedIssues()
-                        )) {
-                            // fall through
+                        if ($has_magic_getter) {
+                            if (IssueBuffer::accepts(
+                                new UndefinedMagicPropertyFetch(
+                                    'Magic instance property ' . $property_id . ' is not defined',
+                                    new CodeLocation($statements_analyzer->getSource(), $stmt),
+                                    $property_id
+                                ),
+                                $statements_analyzer->getSuppressedIssues()
+                            )) {
+                                // fall through
+                            }
+                        } else {
+                            if (IssueBuffer::accepts(
+                                new UndefinedPropertyFetch(
+                                    'Instance property ' . $property_id . ' is not defined',
+                                    new CodeLocation($statements_analyzer->getSource(), $stmt),
+                                    $property_id
+                                ),
+                                $statements_analyzer->getSuppressedIssues()
+                            )) {
+                                // fall through
+                            }
                         }
                     }
 
diff --git a/tests/MagicPropertyTest.php b/tests/MagicPropertyTest.php
index 40ae6004738..b55d0873298 100644
--- a/tests/MagicPropertyTest.php
+++ b/tests/MagicPropertyTest.php
@@ -653,7 +653,7 @@ public function __set(string $name, $value): void {
 
                     $a = new A();
                     $a->bar = 5;',
-                'error_message' => 'UndefinedPropertyAssignment',
+                'error_message' => 'UndefinedMagicPropertyAssignment',
             ],
             'propertySealedDocblockDefinedPropertyAssignment' => [
                 '<?php
@@ -719,7 +719,7 @@ public function __set(string $name, $value): void {
 
                     $a = new A();
                     echo $a->bar;',
-                'error_message' => 'UndefinedPropertyFetch',
+                'error_message' => 'UndefinedMagicPropertyFetch',
             ],
             /**
              * The property $foo is not defined on the object, but accessed with the magic setter.