From 33254c43cc947f428d51fa7ab26abedda5da70f8 Mon Sep 17 00:00:00 2001
From: Brown <github@muglug.com>
Date: Wed, 3 Oct 2018 13:58:32 -0400
Subject: [PATCH] Fix #1010 - track changes to traits and trait uses

---
 src/Psalm/Checker/ClassChecker.php           |   4 +-
 src/Psalm/Codebase/Analyzer.php              |  22 +-
 src/Psalm/Diff/ClassStatementsDiffer.php     |   8 +
 src/Psalm/Diff/FileStatementsDiffer.php      |  11 +-
 src/Psalm/Diff/NamespaceStatementsDiffer.php |   9 +-
 tests/FileDiffTest.php                       |  54 ++++
 tests/FileUpdateTest.php                     | 254 +++++++++++++++++++
 7 files changed, 352 insertions(+), 10 deletions(-)

diff --git a/src/Psalm/Checker/ClassChecker.php b/src/Psalm/Checker/ClassChecker.php
index abdac5a2d..3a5e62c6e 100644
--- a/src/Psalm/Checker/ClassChecker.php
+++ b/src/Psalm/Checker/ClassChecker.php
@@ -914,6 +914,8 @@ class ClassChecker extends ClassLikeChecker
             return $method_checker;
         }
 
+        //echo 'Analysing ' . $analyzed_method_id . "\n";
+
         $existing_error_count = IssueBuffer::getErrorCount();
 
         $method_context = clone $class_context;
@@ -1000,7 +1002,7 @@ class ClassChecker extends ClassLikeChecker
             && !$class_context->collect_mutations
             && !$is_fake
         ) {
-            $codebase->analyzer->setCorrectMethod($source->getFilePath(), strtolower($analyzed_method_id));
+            $codebase->analyzer->setCorrectMethod($source->getFilePath(), strtolower($actual_method_id));
         }
 
         return $method_checker;
diff --git a/src/Psalm/Codebase/Analyzer.php b/src/Psalm/Codebase/Analyzer.php
index 346fd9e68..fb1d201a9 100644
--- a/src/Psalm/Codebase/Analyzer.php
+++ b/src/Psalm/Codebase/Analyzer.php
@@ -308,24 +308,42 @@ class Analyzer
 
         $all_referencing_methods = $project_checker->file_reference_provider->getMethodsReferencing();
 
+        foreach ($all_referencing_methods as $member_id => $referencing_method_ids) {
+            $member_stub = preg_replace('/::.*$/', '::*', $member_id);
+
+            if (!isset($all_referencing_methods[$member_stub])) {
+                $all_referencing_methods[$member_stub] = $referencing_method_ids;
+            } else {
+                $all_referencing_methods[$member_stub] += $referencing_method_ids;
+            }
+        }
+
         $newly_invalidated_methods = [];
 
         foreach ($changed_members as $file_changed_members) {
             foreach ($file_changed_members as $member_id => $_) {
                 if (isset($all_referencing_methods[$member_id])) {
-                    $newly_invalidated_methods = array_merge($all_referencing_methods[$member_id]);
+                    $newly_invalidated_methods = array_merge(
+                        $all_referencing_methods[$member_id],
+                        $newly_invalidated_methods
+                    );
                 }
             }
         }
 
         foreach ($this->correct_methods as $file_path => $correct_methods) {
             foreach ($correct_methods as $correct_method_id => $_) {
-                if (isset($newly_invalidated_methods[$correct_method_id])) {
+                $correct_method_stub = preg_replace('/::.*$/', '::*', $correct_method_id);
+
+                if (isset($newly_invalidated_methods[$correct_method_id])
+                    || isset($newly_invalidated_methods[$correct_method_stub])
+                ) {
                     unset($this->correct_methods[$file_path][$correct_method_id]);
                 }
 
                 if (isset($unchanged_members[$file_path])
                     && !isset($unchanged_members[$file_path][$correct_method_id])
+                    && !isset($unchanged_members[$file_path][$correct_method_stub])
                 ) {
                     unset($this->correct_methods[$file_path][$correct_method_id]);
                 }
diff --git a/src/Psalm/Diff/ClassStatementsDiffer.php b/src/Psalm/Diff/ClassStatementsDiffer.php
index 1da39ecce..688d5d7ee 100644
--- a/src/Psalm/Diff/ClassStatementsDiffer.php
+++ b/src/Psalm/Diff/ClassStatementsDiffer.php
@@ -154,6 +154,10 @@ class ClassStatementsDiffer extends Differ
                     foreach ($diff_elem->old->consts as $const) {
                         $keep[] = strtolower($name) . '::' . $const->name;
                     }
+                } elseif ($diff_elem->old instanceof PhpParser\Node\Stmt\TraitUse) {
+                    foreach ($diff_elem->old->traits as $trait) {
+                        $keep[] = strtolower((string) $trait->getAttribute('resolvedName')) . '::*';
+                    }
                 }
             } elseif ($diff_elem->type === DiffElem::TYPE_KEEP_SIGNATURE) {
                 if ($diff_elem->old instanceof PhpParser\Node\Stmt\ClassMethod) {
@@ -174,6 +178,10 @@ class ClassStatementsDiffer extends Differ
                     foreach ($diff_elem->old->consts as $const) {
                         $delete[] = strtolower($name) . '::' . $const->name;
                     }
+                } elseif ($diff_elem->old instanceof PhpParser\Node\Stmt\TraitUse) {
+                    foreach ($diff_elem->old->traits as $trait) {
+                        $delete[] = strtolower((string) $trait->getAttribute('resolvedName')) . '::*';
+                    }
                 }
             }
         };
diff --git a/src/Psalm/Diff/FileStatementsDiffer.php b/src/Psalm/Diff/FileStatementsDiffer.php
index 4f8d208d1..1e0a8494e 100644
--- a/src/Psalm/Diff/FileStatementsDiffer.php
+++ b/src/Psalm/Diff/FileStatementsDiffer.php
@@ -42,6 +42,7 @@ class FileStatementsDiffer extends Differ
                 if (($a instanceof PhpParser\Node\Stmt\Namespace_ && $b instanceof PhpParser\Node\Stmt\Namespace_)
                     || ($a instanceof PhpParser\Node\Stmt\Class_ && $b instanceof PhpParser\Node\Stmt\Class_)
                     || ($a instanceof PhpParser\Node\Stmt\Interface_ && $b instanceof PhpParser\Node\Stmt\Interface_)
+                    || ($a instanceof PhpParser\Node\Stmt\Trait_ && $b instanceof PhpParser\Node\Stmt\Trait_)
                 ) {
                     return (string)$a->name === (string)$b->name;
                 }
@@ -58,7 +59,7 @@ class FileStatementsDiffer extends Differ
 
         $keep = [];
         $keep_signature = [];
-        $delete = [];
+        $add_or_delete = [];
         $diff_map = [];
 
         foreach ($diff as $diff_elem) {
@@ -76,12 +77,14 @@ class FileStatementsDiffer extends Differ
 
                     $keep = array_merge($keep, $namespace_keep[0]);
                     $keep_signature = array_merge($keep_signature, $namespace_keep[1]);
-                    $delete = array_merge($delete, $namespace_keep[2]);
+                    $add_or_delete = array_merge($add_or_delete, $namespace_keep[2]);
                     $diff_map = array_merge($diff_map, $namespace_keep[3]);
                 } elseif (($diff_elem->old instanceof PhpParser\Node\Stmt\Class_
                         && $diff_elem->new instanceof PhpParser\Node\Stmt\Class_)
                     || ($diff_elem->old instanceof PhpParser\Node\Stmt\Interface_
                         && $diff_elem->new instanceof PhpParser\Node\Stmt\Interface_)
+                    || ($diff_elem->old instanceof PhpParser\Node\Stmt\Trait_
+                        && $diff_elem->new instanceof PhpParser\Node\Stmt\Trait_)
                 ) {
                     $class_keep = ClassStatementsDiffer::diff(
                         (string) $diff_elem->old->name,
@@ -93,12 +96,12 @@ class FileStatementsDiffer extends Differ
 
                     $keep = array_merge($keep, $class_keep[0]);
                     $keep_signature = array_merge($keep_signature, $class_keep[1]);
-                    $delete = array_merge($delete, $class_keep[2]);
+                    $add_or_delete = array_merge($add_or_delete, $class_keep[2]);
                     $diff_map = array_merge($diff_map, $class_keep[3]);
                 }
             }
         }
 
-        return [$keep, $keep_signature, $delete, $diff_map];
+        return [$keep, $keep_signature, $add_or_delete, $diff_map];
     }
 }
diff --git a/src/Psalm/Diff/NamespaceStatementsDiffer.php b/src/Psalm/Diff/NamespaceStatementsDiffer.php
index 82ceb90e7..1ab491690 100644
--- a/src/Psalm/Diff/NamespaceStatementsDiffer.php
+++ b/src/Psalm/Diff/NamespaceStatementsDiffer.php
@@ -38,6 +38,7 @@ class NamespaceStatementsDiffer extends Differ
             function (PhpParser\Node\Stmt $a, PhpParser\Node\Stmt $b, $a_code, $b_code) {
                 if (($a instanceof PhpParser\Node\Stmt\Class_ && $b instanceof PhpParser\Node\Stmt\Class_)
                     || ($a instanceof PhpParser\Node\Stmt\Interface_ && $b instanceof PhpParser\Node\Stmt\Interface_)
+                    || ($a instanceof PhpParser\Node\Stmt\Trait_ && $b instanceof PhpParser\Node\Stmt\Trait_)
                 ) {
                     // @todo add check for comments comparison
 
@@ -56,7 +57,7 @@ class NamespaceStatementsDiffer extends Differ
 
         $keep = [];
         $keep_signature = [];
-        $delete = [];
+        $add_or_delete = [];
         $diff_map = [];
 
         foreach ($diff as $diff_elem) {
@@ -65,6 +66,8 @@ class NamespaceStatementsDiffer extends Differ
                         && $diff_elem->new instanceof PhpParser\Node\Stmt\Class_)
                     || ($diff_elem->old instanceof PhpParser\Node\Stmt\Interface_
                         && $diff_elem->new instanceof PhpParser\Node\Stmt\Interface_)
+                    || ($diff_elem->old instanceof PhpParser\Node\Stmt\Trait_
+                        && $diff_elem->new instanceof PhpParser\Node\Stmt\Trait_)
                 ) {
                     $class_keep = ClassStatementsDiffer::diff(
                         ($name ? $name . '\\' : '') . $diff_elem->old->name,
@@ -76,12 +79,12 @@ class NamespaceStatementsDiffer extends Differ
 
                     $keep = array_merge($keep, $class_keep[0]);
                     $keep_signature = array_merge($keep_signature, $class_keep[1]);
-                    $delete = array_merge($delete, $class_keep[2]);
+                    $add_or_delete = array_merge($add_or_delete, $class_keep[2]);
                     $diff_map = array_merge($diff_map, $class_keep[3]);
                 }
             }
         }
 
-        return [$keep, $keep_signature, $delete, $diff_map];
+        return [$keep, $keep_signature, $add_or_delete, $diff_map];
     }
 }
diff --git a/tests/FileDiffTest.php b/tests/FileDiffTest.php
index 52671c29a..23d4cc63e 100644
--- a/tests/FileDiffTest.php
+++ b/tests/FileDiffTest.php
@@ -767,6 +767,60 @@ class FileDiffTest extends TestCase
                 [],
                 [[0, 0], [0, 0], [120, 2]]
             ],
+            'sameTrait' => [
+                '<?php
+                namespace Foo;
+
+                trait T {
+                    public $aB = 5;
+
+                    const F = 1;
+
+                    public function foo() {
+                        $a = 1;
+                    }
+                    public function bar() {
+                        $b = 1;
+                    }
+                }',
+                '<?php
+                namespace Foo;
+
+                trait T {
+                    public $aB = 5;
+
+                    const F = 1;
+
+                    public function foo() {
+                        $a = 1;
+                    }
+                    public function bar() {
+                        $b = 1;
+                    }
+                }',
+                ['foo\t::$aB', 'foo\t::F', 'foo\t::foo', 'foo\t::bar'],
+                [],
+                [],
+                [[0, 0], [0, 0], [0, 0], [0, 0]]
+            ],
+            'traitPropertyChange' => [
+                '<?php
+                namespace Foo;
+
+                trait T {
+                    public $a;
+                }',
+                '<?php
+                namespace Foo;
+
+                trait T {
+                    public $b;
+                }',
+                [],
+                [],
+                ['foo\t::$a'],
+                []
+            ],
         ];
     }
 }
diff --git a/tests/FileUpdateTest.php b/tests/FileUpdateTest.php
index 5d08dedaf..3af6d52c0 100644
--- a/tests/FileUpdateTest.php
+++ b/tests/FileUpdateTest.php
@@ -467,6 +467,260 @@ class FileUpdateTest extends TestCase
                     ],
                 ]
             ],
+            'dontInvalidateTraitMethods' => [
+                'start_files' => [
+                    getcwd() . DIRECTORY_SEPARATOR . 'A.php' => '<?php
+                        namespace Foo;
+
+                        class A {
+                            use T;
+
+                            public function fooFoo(): void { }
+                        }',
+                    getcwd() . DIRECTORY_SEPARATOR . 'B.php' => '<?php
+                        namespace Foo;
+
+                        class B {
+                            public function foo(): void {
+                                (new A)->fooFoo();
+                            }
+
+                            public function bar() : void {
+                                echo (new A)->barBar();
+                            }
+
+                            public function noReturnType() {}
+                        }',
+                     getcwd() . DIRECTORY_SEPARATOR . 'T.php' => '<?php
+                        namespace Foo;
+
+                        trait T {
+                            public function barBar(): string {
+                                return "hello";
+                            }
+                        }',
+                ],
+                'end_files' => [
+                    getcwd() . DIRECTORY_SEPARATOR . 'A.php' => '<?php
+                        namespace Foo;
+
+                        class A {
+                            use T;
+
+                            public function fooFoo(?string $foo = null): void { }
+                        }',
+                    getcwd() . DIRECTORY_SEPARATOR . 'B.php' => '<?php
+                        namespace Foo;
+
+                        class B {
+                            public function foo(): void {
+                                (new A)->fooFoo();
+                            }
+
+                            public function bar() : void {
+                                echo (new A)->barBar();
+                            }
+
+                            public function noReturnType() {}
+                        }',
+                     getcwd() . DIRECTORY_SEPARATOR . 'T.php' => '<?php
+                        namespace Foo;
+
+                        trait T {
+                            public function barBar(): string {
+                                return "hello";
+                            }
+                        }',
+                ],
+                'initial_correct_methods' => [
+                    getcwd() . DIRECTORY_SEPARATOR . 'T.php' => [
+                        'foo\t::barbar' => true,
+                    ],
+                    getcwd() . DIRECTORY_SEPARATOR . 'A.php' => [
+                        'foo\a::foofoo' => true,
+                    ],
+                    getcwd() . DIRECTORY_SEPARATOR . 'B.php' => [
+                        'foo\b::foo' => true,
+                        'foo\b::bar' => true,
+                        'foo\b::noreturntype' => true,
+                    ],
+                ],
+                'unaffected_correct_methods' => [
+                    getcwd() . DIRECTORY_SEPARATOR . 'T.php' => [
+                        'foo\t::barbar' => true,
+                    ],
+                    getcwd() . DIRECTORY_SEPARATOR . 'A.php' => [],
+                    getcwd() . DIRECTORY_SEPARATOR . 'B.php' => [
+                        'foo\b::bar' => true,
+                        'foo\b::noreturntype' => true,
+                    ],
+                ],
+                [
+                    'MissingReturnType' => \Psalm\Config::REPORT_INFO,
+                ]
+            ],
+            'invalidateTraitMethodsWhenTraitRemoved' => [
+                'start_files' => [
+                    getcwd() . DIRECTORY_SEPARATOR . 'A.php' => '<?php
+                        namespace Foo;
+
+                        class A {
+                            use T;
+
+                            public function fooFoo(): void { }
+                        }',
+                    getcwd() . DIRECTORY_SEPARATOR . 'B.php' => '<?php
+                        namespace Foo;
+
+                        class B {
+                            public function foo(): void {
+                                (new A)->fooFoo();
+                            }
+
+                            public function bar() : void {
+                                echo (new A)->barBar();
+                            }
+                        }',
+                     getcwd() . DIRECTORY_SEPARATOR . 'T.php' => '<?php
+                        namespace Foo;
+
+                        trait T {
+                            public function barBar(): string {
+                                return "hello";
+                            }
+                        }',
+                ],
+                'end_files' => [
+                    getcwd() . DIRECTORY_SEPARATOR . 'A.php' => '<?php
+                        namespace Foo;
+
+                        class A {
+                            public function fooFoo(?string $foo = null): void { }
+                        }',
+                    getcwd() . DIRECTORY_SEPARATOR . 'B.php' => '<?php
+                        namespace Foo;
+
+                        class B {
+                            public function foo(): void {
+                                (new A)->fooFoo();
+                            }
+
+                            public function bar() : void {
+                                echo (new A)->barBar();
+                            }
+                        }',
+                     getcwd() . DIRECTORY_SEPARATOR . 'T.php' => '<?php
+                        namespace Foo;
+
+                        trait T {
+                            public function barBar(): string {
+                                return "hello";
+                            }
+                        }',
+                ],
+                'initial_correct_methods' => [
+                    getcwd() . DIRECTORY_SEPARATOR . 'T.php' => [
+                        'foo\t::barbar' => true,
+                    ],
+                    getcwd() . DIRECTORY_SEPARATOR . 'A.php' => [
+                        'foo\a::foofoo' => true,
+                    ],
+                    getcwd() . DIRECTORY_SEPARATOR . 'B.php' => [
+                        'foo\b::foo' => true,
+                        'foo\b::bar' => true,
+                    ],
+                ],
+                'unaffected_correct_methods' => [
+                    getcwd() . DIRECTORY_SEPARATOR . 'T.php' => [
+                        'foo\t::barbar' => true,
+                    ],
+                    getcwd() . DIRECTORY_SEPARATOR . 'A.php' => [],
+                    getcwd() . DIRECTORY_SEPARATOR . 'B.php' => [],
+                ]
+            ],
+            'invalidateTraitMethodsWhenTraitReplaced' => [
+                'start_files' => [
+                    getcwd() . DIRECTORY_SEPARATOR . 'A.php' => '<?php
+                        namespace Foo;
+
+                        class A {
+                            use T;
+
+                            public function fooFoo(): void { }
+                        }',
+                    getcwd() . DIRECTORY_SEPARATOR . 'B.php' => '<?php
+                        namespace Foo;
+
+                        class B {
+                            public function foo(): void {
+                                (new A)->fooFoo();
+                            }
+
+                            public function bar() : void {
+                                echo (new A)->barBar();
+                            }
+                        }',
+                     getcwd() . DIRECTORY_SEPARATOR . 'T.php' => '<?php
+                        namespace Foo;
+
+                        trait T {
+                            public function barBar(): string {
+                                return "hello";
+                            }
+                        }',
+                ],
+                'end_files' => [
+                    getcwd() . DIRECTORY_SEPARATOR . 'A.php' => '<?php
+                        namespace Foo;
+
+                        class A {
+                            public function fooFoo(?string $foo = null): void { }
+
+                            public function barBar(): int {
+                                return 5;
+                            }
+                        }',
+                    getcwd() . DIRECTORY_SEPARATOR . 'B.php' => '<?php
+                        namespace Foo;
+
+                        class B {
+                            public function foo(): void {
+                                (new A)->fooFoo();
+                            }
+
+                            public function bar() : void {
+                                echo (new A)->barBar();
+                            }
+                        }',
+                     getcwd() . DIRECTORY_SEPARATOR . 'T.php' => '<?php
+                        namespace Foo;
+
+                        trait T {
+                            public function barBar(): string {
+                                return "hello";
+                            }
+                        }',
+                ],
+                'initial_correct_methods' => [
+                    getcwd() . DIRECTORY_SEPARATOR . 'T.php' => [
+                        'foo\t::barbar' => true,
+                    ],
+                    getcwd() . DIRECTORY_SEPARATOR . 'A.php' => [
+                        'foo\a::foofoo' => true,
+                    ],
+                    getcwd() . DIRECTORY_SEPARATOR . 'B.php' => [
+                        'foo\b::foo' => true,
+                        'foo\b::bar' => true,
+                    ],
+                ],
+                'unaffected_correct_methods' => [
+                    getcwd() . DIRECTORY_SEPARATOR . 'T.php' => [
+                        'foo\t::barbar' => true,
+                    ],
+                    getcwd() . DIRECTORY_SEPARATOR . 'A.php' => [],
+                    getcwd() . DIRECTORY_SEPARATOR . 'B.php' => [],
+                ]
+            ],
         ];
     }