branch: externals/phpinspect
commit af623fe4692b197ddff419de49636a21ad840257
Author: Hugo Thunnissen <de...@hugot.nl>
Commit: Hugo Thunnissen <de...@hugot.nl>
Fix endless recursion bug in resolving of self-referential variable type
---
phpinspect-resolve.el | 54 ++++++++++++++++++++++-------------
test/phpinspect-test.el | 23 +++++++++++++--
test/test-buffer-resolve.el | 68 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 124 insertions(+), 21 deletions(-)
diff --git a/phpinspect-resolve.el b/phpinspect-resolve.el
index 5c9e0c5ae8..efe69adc02 100644
--- a/phpinspect-resolve.el
+++ b/phpinspect-resolve.el
@@ -64,6 +64,19 @@
(preceding-assignments nil
:type list))
+(defun phpi--assignment-statement-p (statement)
+ (when (length> statement 2)
+ (let (assignment-found)
+ (catch 'phpi--return
+ (dolist (token statement)
+ (when (and assignment-found
+ (phpinspect-not-comment-p token)
+ (not (phpinspect-terminator-p token)))
+ (throw 'phpi--return t))
+
+ (when (phpinspect-maybe-assignment-p token)
+ (setq assignment-found t)))))))
+
(defun phpinspect--find-assignment-ctxs-in-token (token &optional
assignments-before var-annotations)
(when (keywordp (car token))
(setq token (cdr token)))
@@ -74,8 +87,9 @@
assignments blocks-or-lists)
(dolist (statement statements)
(phpinspect--log "Finding assignment in statement '%s'" statement)
- (when (seq-find #'phpinspect-maybe-assignment-p statement)
- (phpinspect--log "Found assignment statement")
+
+ ;; Only add assignments which have more than 2 tokens, meaning
+ (when (phpi--assignment-statement-p statement)
(push (phpinspect--make-assignment-context
:annotations var-annotations
:tokens statement
@@ -95,16 +109,16 @@
(when (setq blocks-or-lists (seq-filter #'phpinspect-block-or-list-p
statement))
(dolist (block-or-list blocks-or-lists)
(phpinspect--log "Found block or list %s" block-or-list)
- (let ((local-assignments
- (phpinspect--find-assignment-ctxs-in-token
- block-or-list assignments-before var-annotations)))
- (dolist (local-assignment (nreverse local-assignments))
- (push local-assignment assignments))
+ (when-let ((local-assignments
+ (phpinspect--find-assignment-ctxs-in-token
+ block-or-list assignments-before var-annotations)))
+ (setq assignments (nconc local-assignments assignments))
(setq assignments-before assignments)))))
;; return
(phpinspect--log "Found assignments in token: %s" assignments)
(phpinspect--log "Found statements in token: %s" statements)
+
assignments))
(defun phpinspect--find-assignment-by-predicate (assignment-ctxs predicate)
@@ -351,13 +365,14 @@ resolve types of function argument variables."
pattern-code (length assignments))
(if (not last-assignment)
- (when (and (= (length pattern-code) 2) (phpinspect-variable-p (cadr
pattern-code)))
- (let ((variable-name (cadadr pattern-code)))
- (progn
- (phpinspect--log "No assignments found for variable %s, checking
function arguments: %s"
- variable-name function-arg-list)
- (setq result (phpinspect-get-variable-type-in-function-arg-list
- variable-name type-resolver function-arg-list)))))
+ (progn
+ (when (and (= (length pattern-code) 2) (phpinspect-variable-p (cadr
pattern-code)))
+ (let ((variable-name (cadadr pattern-code)))
+ (progn
+ (phpinspect--log "No assignments found for variable %s,
checking function arguments: %s"
+ variable-name function-arg-list)
+ (setq result (phpinspect-get-variable-type-in-function-arg-list
+ variable-name type-resolver
function-arg-list))))))
(setq result
(phpinspect--interpret-expression-type-in-context
@@ -536,7 +551,9 @@ value/type."
(cadar expression)
type-resolver function-arg-list))
(phpinspect--get-variable-type-in-block
- resolvecontext (cadar expression) php-block assignments
type-resolver function-arg-list)))))
+ resolvecontext (cadar expression) php-block
+ assignments type-resolver function-arg-list)))))
+
(defun phpinspect-resolve-type-from-context (resolvecontext &optional
type-resolver assume-derived)
"Resolve the type that RESOLVECONTEXT's subject evaluates to.
@@ -564,10 +581,8 @@ type. (`phpinspect--type-p')"
(let ((enclosing-tokens (seq-filter #'phpinspect-not-class-p
(phpinspect--resolvecontext-enclosing-tokens
resolvecontext)))
- (enclosing-token)
- (type))
+ enclosing-token type)
(while (and enclosing-tokens (not type))
- ;;(phpinspect--log "Trying to find type in %s" enclosing-token)
(setq enclosing-token (pop enclosing-tokens))
(let ((subject (phpinspect--resolvecontext-subject resolvecontext)))
@@ -579,7 +594,8 @@ type. (`phpinspect--type-p')"
((phpinspect-namespace-p enclosing-token)
(phpinspect-interpret-expression-type-in-context
resolvecontext
- (or (phpinspect-namespace-block enclosing-token)
enclosing-token)
+ (or (phpinspect-namespace-block enclosing-token)
+ enclosing-token)
type-resolver subject))
((or (phpinspect-block-p enclosing-token)
diff --git a/test/phpinspect-test.el b/test/phpinspect-test.el
index 8f88a3779a..aa838302bd 100644
--- a/test/phpinspect-test.el
+++ b/test/phpinspect-test.el
@@ -123,9 +123,20 @@
(should (phpinspect--type= (phpinspect--make-type :name "\\Thing")
result)))))
+(ert-deftest phpinspect--find-assignment-ctxs-in-token-no-self-reference ()
+
+ (let* ((tokens (cadr
+ (phpinspect-parse-string "{ $a = $b; $foo = 'abc'; $a = 'c';
if ($a = $b) { /* comment */ $a = $b; $something->doSomething(); $bla = $foo; }
if (true) { $x = 34; $y = 99; $foo = $nr7; }}")))
+ (assignments (phpinspect--find-assignment-ctxs-in-token tokens)))
+ (dolist (assignment (cdr assignments))
+ (should-not (seq-find (lambda (actx)
+ (eq (phpinspect--actx-tokens assignment)
+ (phpinspect--actx-tokens actx)))
+ (phpinspect--actx-preceding-assignments
assignment))))))
+
(ert-deftest phpinspect--find-assignment-ctxs-in-token ()
(let* ((tokens (cadr
- (phpinspect-parse-string "{ $foo = ['nr 1']; $bar = $nr2; if
(true === ($foo = $nr3)) { $foo = $nr4; $notfoo = $nr5; if ([] === ($foo = [
$nr6 ])){ $foo = [ $nr7 ];}}}")))
+ (phpinspect-parse-string "{ $foo = ['nr 1']; $bar = $nr2; if
(true === ($foo = $nr3)) { $foo = $nr4; $notfoo = $nr5; if ([] === ($foo = [
$nr6 ])){ $foobar = /* comment */; $foo = [ $nr7 ]; $bar = }}}")))
(expected '(((:variable "foo")
(:assignment "=")
(:array
@@ -152,7 +163,14 @@
(:string "nr 1")))))
(assignments (phpinspect--find-assignment-ctxs-in-token tokens)))
- (should (equal expected (mapcar #'phpinspect--actx-tokens assignments)))))
+ (should (equal expected (mapcar #'phpinspect--actx-tokens assignments)))
+
+ (dolist (assignment (cdr assignments))
+ (should-not (seq-find (lambda (actx)
+ (eq (phpinspect--actx-tokens assignment)
+ (phpinspect--actx-tokens actx)))
+ (phpinspect--actx-preceding-assignments
assignment))))))
+
(ert-deftest phpinspect-resolve-type-from-context ()
@@ -340,6 +358,7 @@ class FlufferUpper
(load-file (concat phpinspect-test-directory "/test-completion.el"))
(load-file (concat phpinspect-test-directory "/test-change.el"))
(load-file (concat phpinspect-test-directory "/test-token-predicates.el"))
+(load-file (concat phpinspect-test-directory "/test-buffer-resolve.el"))
(provide 'phpinspect-test)
diff --git a/test/test-buffer-resolve.el b/test/test-buffer-resolve.el
new file mode 100644
index 0000000000..a8c604c8ce
--- /dev/null
+++ b/test/test-buffer-resolve.el
@@ -0,0 +1,68 @@
+;;; test-buffer-resolve.el --- Test resolving types in live edited buffers
-*- lexical-binding: t; -*-
+
+;; Copyright (C) 2025 Free Software Foundation
+
+;; Author: Hugo Thunnissen <de...@hugot.nl>
+
+;; This program is free software; you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; This program is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with this program. If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;;
+
+;;; Code:
+
+(require 'ert)
+(require 'phpinspect-buffer)
+(require 'phpinspect-test-env
+ (expand-file-name "phpinspect-test-env.el"
+ (file-name-directory (macroexp-file-name))))
+
+(ert-deftest phpinspect-buffer-test-resolve-self-referential-variable ()
+ (with-temp-buffer
+ (let ((buffer (phpinspect-claim-buffer
+ (current-buffer)
(phpinspect--make-dummy-composer-project-with-code))))
+ (insert "<?php
+
+namespace App\\Testerino;
+
+class Test
+{
+ public function test(bool $flipflop = false): void
+ {
+ $value = new \\App\\Bar();
+ if (true) {
+ $b = $flipflop;
+ // for some reason this bug only triggers when there is at least
+ // one line of code before the self-referential assignment
+ $value = $value->foo();
+ }
+
+ $this->value = $value->
+ }
+}")
+
+ (phpinspect-buffer-parse buffer)
+
+ (backward-char 7)
+ (insert "$value->")
+
+ (let* ((rctx (phpinspect-buffer-get-resolvecontext buffer (point)))
+ (type (phpinspect-resolve-type-from-context rctx)))
+
+ (should type)
+ (should (phpinspect--type= (phpinspect--make-type :name "\\App\\Foo"
:fully-qualified t) type ))))))
+
+(provide 'test-buffer-resolve)
+;;; test-buffer-resolve.el ends here
