From 3a0ce3ac2dad7fbee01010fa8a4464dfef39dbfa Mon Sep 17 00:00:00 2001 From: Hugo Thunnissen Date: Thu, 15 Aug 2024 23:52:38 +0200 Subject: [PATCH] Fix another infinite recursion bug Bug occured in scenario's where array-members were self-referentially assigned in a foreach loop. like so: foreach($things as $thing) { $thing = $this->something; } --- phpinspect-resolve.el | 158 ++++++++++++++++++++++++---------------- test/phpinspect-test.el | 50 ++----------- test/test-resolve.el | 64 ++++++++++++++++ 3 files changed, 167 insertions(+), 105 deletions(-) create mode 100644 test/test-resolve.el diff --git a/phpinspect-resolve.el b/phpinspect-resolve.el index 50000a2..04a9cb5 100644 --- a/phpinspect-resolve.el +++ b/phpinspect-resolve.el @@ -33,6 +33,9 @@ (cl-defstruct (phpinspect--assignment (:constructor phpinspect--make-assignment)) + (ctx nil + :type phpinspect--assignment-context + :documentation "The context that the assignment was found in") (to nil :type phpinspect-variable :documentation "The variable that is assigned to") @@ -54,8 +57,15 @@ (inline-quote (not (phpinspect-maybe-assignment-p ,token)))) -(cl-defgeneric phpinspect--find-assignments-in-token (token) - "Find any assignments that are in TOKEN, at top level or nested in blocks" +(cl-defstruct (phpinspect--assignment-context + (:constructor phpinspect--make-assignment-context) + (:conc-name phpinspect--actx-)) + (tokens nil + :type list) + (preceding-assignments nil + :type list)) + +(defun phpinspect--find-assignment-ctxs-in-token (token &optional assignments-before) (when (keywordp (car token)) (setq token (cdr token))) @@ -65,58 +75,63 @@ (dolist (statement statements) (when (seq-find #'phpinspect-maybe-assignment-p statement) (phpinspect--log "Found assignment statement") - (push statement assignments)) + (push (phpinspect--make-assignment-context + :tokens statement + :preceding-assignments assignments-before) + assignments) + (setq assignments-before assignments)) (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-assignments-in-token block-or-list))) + (let ((local-assignments + (phpinspect--find-assignment-ctxs-in-token block-or-list assignments-before))) (dolist (local-assignment (nreverse local-assignments)) - (push local-assignment assignments)))))) + (push local-assignment 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-tokens predicate) +(defun phpinspect--find-assignment-by-predicate (assignment-ctxs predicate) "Find first assignment in ASSIGNMENT-TOKENS matching PREDICATE. Destructively removes tokens from the end of ASSIGNMENT-TOKENS." (catch 'return - (while-let ((assignment (car (last assignment-tokens)))) - (if (cdr assignment-tokens) - (setcdr assignment-tokens (butlast (cdr assignment-tokens))) - (setcar assignment-tokens nil)) - - (let* ((is-loop-assignment nil) - (left-of-assignment - (seq-filter #'phpinspect-not-comment-p - (seq-take-while #'phpinspect-not-assignment-p assignment))) - (right-of-assignment - (seq-filter - #'phpinspect-not-comment-p - (cdr (seq-drop-while - (lambda (elt) - (if (phpinspect-maybe-assignment-p elt) - (progn - (when (equal '(:word "as") elt) - (phpinspect--log "It's a loop assignment %s" elt) - (setq is-loop-assignment t)) - nil) - t)) - assignment))))) - - (if is-loop-assignment - (when (funcall predicate right-of-assignment) - ;; Masquerade as an array access assignment - (setq left-of-assignment (append left-of-assignment '((:array)))) - (throw 'return (phpinspect--make-assignment :to right-of-assignment - :from left-of-assignment))) - (when (funcall predicate left-of-assignment) - (throw 'return (phpinspect--make-assignment :from right-of-assignment - :to left-of-assignment)))))) - nil)) + (dolist (actx assignment-ctxs) + (let ((assignment (phpinspect--actx-tokens actx))) + (let* ((is-loop-assignment nil) + (left-of-assignment + (seq-filter #'phpinspect-not-comment-p + (seq-take-while #'phpinspect-not-assignment-p assignment))) + (right-of-assignment + (seq-filter + #'phpinspect-not-comment-p + (cdr (seq-drop-while + (lambda (elt) + (if (phpinspect-maybe-assignment-p elt) + (progn + (when (equal '(:word "as") elt) + (phpinspect--log "It's a loop assignment %s" elt) + (setq is-loop-assignment t)) + nil) + t)) + assignment))))) + + (if is-loop-assignment + (when (funcall predicate right-of-assignment) + ;; Masquerade as an array access assignment + (setq left-of-assignment (append left-of-assignment '((:array)))) + (throw 'return (phpinspect--make-assignment :to right-of-assignment + :from left-of-assignment + :ctx actx))) + (when (funcall predicate left-of-assignment) + (throw 'return (phpinspect--make-assignment :from right-of-assignment + :to left-of-assignment + :ctx actx))))))) + nil)) ;; (phpinspect--log "Returning the thing %s" variable-assignments) ;; (nreverse variable-assignments))) @@ -125,7 +140,7 @@ Destructively removes tokens from the end of ASSIGNMENT-TOKENS." (pop statement)) statement) -;; TODO: the use of this function and similar ones should be replaced with code +;; FIXME: the use of this function and similar ones should be replaced with code ;; that uses locally injected project objects in stead of retrieving the project ;; object through global variables. (defsubst phpinspect-get-cached-project-class (project-root class-fqn) @@ -199,8 +214,13 @@ self::method(); ClassName::method(); $variable = ClassName::method(); $variable = $variable->method();" -(setq assignments (or assignments (nreverse (phpinspect--find-assignments-in-token php-block)))) + (phpinspect--get-derived-statement-type-in-block + resolvecontext statement php-block + (or assignments (phpinspect--find-assignment-ctxs-in-token php-block)) + type-resolver function-arg-list)) +(defun phpinspect--get-derived-statement-type-in-block + (resolvecontext statement php-block assignments type-resolver &optional function-arg-list) ;; A derived statement can be an assignment itself. (when (seq-find #'phpinspect-assignment-p statement) (phpinspect--log "Derived statement is an assignment: %s" statement) @@ -227,9 +247,9 @@ $variable = $variable->method();" ;; No bare word, assume we're dealing with a variable. (when (phpinspect-variable-p first-token) - (phpinspect-get-variable-type-in-block - resolvecontext (cadr first-token) php-block - type-resolver function-arg-list assignments))))) + (phpinspect--get-variable-type-in-block + resolvecontext (cadr first-token) php-block assignments + type-resolver function-arg-list))))) (phpinspect--log "Statement: %s" statement) (phpinspect--log "Starting attribute type: %s" previous-attribute-type) @@ -243,7 +263,7 @@ $variable = $variable->method();" (pop statement) (setq previous-attribute-type (or - (phpinspect-get-cached-project-class-method-type + (phpinspect-get-cached-project-class-method-type (phpinspect--resolvecontext-project-root resolvecontext) (funcall type-resolver previous-attribute-type) @@ -286,8 +306,16 @@ $variable = $variable->method();" ;; TODO: since we're passing type-resolver to all of the get-variable-type functions now, ;; we may as well always return FQNs in stead of relative type names. ;;;; + (defun phpinspect-get-variable-type-in-block - (resolvecontext variable-name php-block type-resolver &optional function-arg-list assignments) + (resolvecontext variable-name php-block type-resolver &optional function-arg-list) + (phpinspect--get-variable-type-in-block + resolvecontext variable-name php-block + (phpinspect--find-assignment-ctxs-in-token php-block) + type-resolver function-arg-list)) + +(defun phpinspect--get-variable-type-in-block + (resolvecontext variable-name php-block assignments type-resolver &optional function-arg-list) "Find the type of VARIABLE-NAME in PHP-BLOCK using TYPE-RESOLVER. Returns either a FQN or a relative type name, depending on @@ -296,17 +324,16 @@ side of assignment) can be found in FUNCTION-ARG-LIST. When PHP-BLOCK belongs to a function, supply FUNCTION-ARG-LIST to resolve types of function argument variables." - (setq assignments (or assignments (nreverse (phpinspect--find-assignments-in-token php-block)))) (phpinspect--log "Looking for assignments of variable %s in php block" variable-name) (if (string= variable-name "this") (funcall type-resolver (phpinspect--make-type :name "self")) - (phpinspect-get-pattern-type-in-block + (phpinspect--get-pattern-type-in-block resolvecontext (phpinspect--make-pattern :m `(:variable ,variable-name)) - php-block type-resolver function-arg-list assignments))) + php-block assignments type-resolver function-arg-list))) (defun phpinspect-get-pattern-type-in-block - (resolvecontext pattern php-block type-resolver &optional function-arg-list assignments) + (resolvecontext pattern php-block type-resolver &optional function-arg-list) "Find the type of PATTERN in PHP-BLOCK using TYPE-RESOLVER. PATTERN must be an object of the type `phpinspect--pattern'. @@ -319,12 +346,17 @@ When PHP-BLOCK belongs to a function, supply FUNCTION-ARG-LIST to resolve types of function argument variables." (phpinspect--get-pattern-type-from-assignments resolvecontext pattern php-block - type-resolver function-arg-list (or assignments (nreverse (phpinspect--find-assignments-in-token php-block))))) + (phpinspect--find-assignment-ctxs-in-token php-block) + type-resolver function-arg-list)) + +(defun phpinspect--get-pattern-type-in-block + (resolvecontext pattern php-block assignments type-resolver &optional function-arg-list) + (phpinspect--get-pattern-type-from-assignments + resolvecontext pattern php-block assignments type-resolver function-arg-list)) + (defun phpinspect--get-pattern-type-from-assignments - (resolvecontext pattern php-block type-resolver &optional function-arg-list assignments) - (cl-assert (phpinspect-blocklike-p php-block)) - (setq assignments (or assignments (nreverse (phpinspect--find-assignments-in-token php-block)))) + (resolvecontext pattern php-block assignments type-resolver &optional function-arg-list) (let* ((last-assignment (phpinspect--find-assignment-by-predicate assignments (phpinspect--pattern-matcher pattern))) @@ -346,7 +378,9 @@ resolve types of function argument variables." (setq result (phpinspect--interpret-expression-type-in-context resolvecontext php-block type-resolver - last-assignment-value function-arg-list assignments))) + last-assignment-value function-arg-list + (phpinspect--actx-preceding-assignments + (phpinspect--assignment-ctx last-assignment))))) (phpinspect--log "Type interpreted from last assignment expression of pattern %s: %s" pattern-code result) @@ -363,10 +397,8 @@ resolve types of function argument variables." (phpinspect--log "Inferring type of concatenated pattern %s" (phpinspect--pattern-code concat-pattern)) (setf (phpinspect--type-contains result) - ;; Don't pass assignments, we need to look from scratch as array - ;; assignments for this variable may already have been dropped. (phpinspect--get-pattern-type-from-assignments - resolvecontext concat-pattern php-block + resolvecontext concat-pattern php-block assignments type-resolver function-arg-list)))) ;; return @@ -450,9 +482,9 @@ EXPRESSION." (phpinspect-array-p part))) expression)) (phpinspect--log "Variable was assigned with a derived statement") - (phpinspect-get-derived-statement-type-in-block - resolvecontext expression php-block - type-resolver function-arg-list assignments)) + (phpinspect--get-derived-statement-type-in-block + resolvecontext expression php-block assignments + type-resolver function-arg-list)) ;; If the right of an assignment is just $variable;, we can check if it is a ;; function argument and otherwise recurse to find the type of that variable. @@ -463,8 +495,8 @@ EXPRESSION." (phpinspect-get-variable-type-in-function-arg-list (cadar expression) type-resolver function-arg-list)) - (phpinspect-get-variable-type-in-block - resolvecontext (cadar expression) php-block type-resolver function-arg-list assignments))))) + (phpinspect--get-variable-type-in-block + resolvecontext (cadar expression) php-block assignments type-resolver function-arg-list))))) (defun phpinspect-resolve-type-from-context (resolvecontext &optional type-resolver) diff --git a/test/phpinspect-test.el b/test/phpinspect-test.el index ad2a663..8690fb6 100644 --- a/test/phpinspect-test.el +++ b/test/phpinspect-test.el @@ -98,40 +98,6 @@ (should (= (length (phpinspect--resolvecontext-enclosing-tokens context1)) (length (phpinspect--resolvecontext-enclosing-tokens context2)))))) - - -(ert-deftest phpinspect-get-variable-type-in-block-array-access () - (let* ((code "class Foo { function a(\\Thing $baz) { $foo = []; $foo[] = $baz; $bar = $foo[0]; $bork = [$foo[0]]; $bark = $bork[0]; $borknest = [$bork]; $barknest = $borknest[0][0]; }}") - (tokens (phpinspect-parse-string-to-bmap code)) - (context (phpinspect-get-resolvecontext tokens (- (length code) 4))) - (project-root "could never be a real project root") - (phpinspect-project-root-function - (lambda (&rest _ignored) project-root)) - (project (phpinspect--make-project - :fs (phpinspect-make-virtual-fs) - :root project-root - :worker (phpinspect-make-worker)))) - - (puthash project-root project (phpinspect--cache-projects phpinspect-cache)) - - (let* ((function-token (car (phpinspect--resolvecontext-enclosing-tokens context))) - (result1 (phpinspect-get-variable-type-in-block - context "bar" - (phpinspect-function-block function-token) - (phpinspect--make-type-resolver-for-resolvecontext context) - (phpinspect-function-argument-list function-token))) - (result2 (phpinspect-get-variable-type-in-block - context "bark" - (phpinspect-function-block function-token) - (phpinspect--make-type-resolver-for-resolvecontext context) - (phpinspect-function-argument-list function-token)))) - - (should (phpinspect--type= (phpinspect--make-type :name "\\Thing") - result1)) - (should (phpinspect--type= (phpinspect--make-type :name "\\Thing") - result2))))) - - (ert-deftest phpinspect-get-variable-type-in-block-array-foreach () (let* ((code "class Foo { function a(\\Thing $baz) { $foo = []; $foo[] = $baz; foreach ($foo as $bar) {$bar->") (bmap (phpinspect-parse-string-to-bmap code)) @@ -157,7 +123,6 @@ (should (phpinspect--type= (phpinspect--make-type :name "\\Thing") result))))) - (ert-deftest phpinspect-get-variable-type-in-block-nested-array () (let* ((code "class Foo { function a(\\Thing $baz) { $foo = [[$baz]]; foreach ($foo[0] as $bar) {$bar->") (bmap (phpinspect-parse-string-to-bmap code)) @@ -184,7 +149,7 @@ result))))) -(ert-deftest phpinspect--find-assignments-in-token () +(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 ];}}}"))) (expected '(((:variable "foo") @@ -211,9 +176,9 @@ (:assignment "=") (:array (:string "nr 1"))))) - (assignments (phpinspect--find-assignments-in-token tokens))) + (assignments (phpinspect--find-assignment-ctxs-in-token tokens))) - (should (equal expected assignments)))) + (should (equal expected (mapcar #'phpinspect--actx-tokens assignments))))) (ert-deftest phpinspect-resolve-type-from-context () @@ -405,14 +370,14 @@ class Thing (phpinspect--get-last-statement-in-token (phpinspect-parse-string php-code-bare)))))) -(ert-deftest phpinspect--find-assignments-by-predicate () +(ert-deftest phpinspect--find-assignment-by-predicate () (let* ((token '(:block (:variable "bam") (:object-attrib "boom") (:assignment "=") (:variable "beng") (:terminator) (:variable "notbam") (:word "nonsense") (:assignment "=") (:string) (:terminator) (:variable "bam") (:comment) (:object-attrib "boom") (:assignment "=") (:variable "wat") (:object-attrib "call") (:terminator))) - (assignments (nreverse (phpinspect--find-assignments-in-token token))) + (assignments (phpinspect--find-assignment-ctxs-in-token token)) (result (phpinspect--find-assignment-by-predicate assignments (phpinspect--match-sequence-lambda :m `(:variable "bam") :m `(:object-attrib "boom"))))) @@ -424,9 +389,9 @@ class Thing (should (equal '((:variable "wat") (:object-attrib "call")) (phpinspect--assignment-from result))) - (setq result (phpinspect--find-assignment-by-predicate - assignments + (phpinspect--actx-preceding-assignments + (phpinspect--assignment-ctx result)) (phpinspect--match-sequence-lambda :m `(:variable "bam") :m `(:object-attrib "boom")))) (should result) @@ -466,6 +431,7 @@ class Thing (load-file (concat phpinspect-test-directory "/test-pipeline.el")) (load-file (concat phpinspect-test-directory "/test-toc.el")) (load-file (concat phpinspect-test-directory "/test-meta.el")) +(load-file (concat phpinspect-test-directory "/test-resolve.el")) (provide 'phpinspect-test) diff --git a/test/test-resolve.el b/test/test-resolve.el new file mode 100644 index 0000000..75ef4a6 --- /dev/null +++ b/test/test-resolve.el @@ -0,0 +1,64 @@ + +(require 'ert) +(require 'phpinspect-resolve) +(require 'phpinspect) +(require 'phpinspect-test-env + (expand-file-name "phpinspect-test-env.el" + (file-name-directory (macroexp-file-name)))) + + +(ert-deftest phpinspect-get-variable-type-in-block-array-access () + (let* ((code "class Foo { function a(\\Thing $baz) { $foo = []; $foo[] = $baz; $bar = $foo[0]; $bork = [$foo[0]]; $bark = $bork[0]; $borknest = [$bork]; $barknest = $borknest[0][0]; }}") + (tokens (phpinspect-parse-string-to-bmap code)) + (context (phpinspect-get-resolvecontext tokens (- (length code) 4))) + (project-root "could never be a real project root") + (phpinspect-project-root-function + (lambda (&rest _ignored) project-root)) + (project (phpinspect--make-project + :fs (phpinspect-make-virtual-fs) + :root project-root + :worker (phpinspect-make-worker)))) + + (puthash project-root project (phpinspect--cache-projects phpinspect-cache)) + + (let* ((function-token (car (phpinspect--resolvecontext-enclosing-tokens context))) + (result1 (phpinspect-get-variable-type-in-block + context "bar" + (phpinspect-function-block function-token) + (phpinspect--make-type-resolver-for-resolvecontext context) + (phpinspect-function-argument-list function-token))) + (result2 (phpinspect-get-variable-type-in-block + context "bark" + (phpinspect-function-block function-token) + (phpinspect--make-type-resolver-for-resolvecontext context) + (phpinspect-function-argument-list function-token)))) + + (should (phpinspect--type= (phpinspect--make-type :name "\\Thing") + result1)) + (should (phpinspect--type= (phpinspect--make-type :name "\\Thing") + result2))))) + +(ert-deftest phpinspect-get-variable-type-in-block-array-foreach-self-referential () + (let* ((code "class Foo { function a(\\Thing $baz) { $foo = []; $foo[] = $baz; foreach ($foo as $bar) {$bar = $bar; $bar->") + (bmap (phpinspect-parse-string-to-bmap code)) + (context (phpinspect-get-resolvecontext bmap (length code))) + (project-root "could never be a real project root") + (phpinspect-project-root-function + (lambda (&rest _ignored) project-root)) + (project (phpinspect--make-project + :fs (phpinspect-make-virtual-fs) + :root project-root + :worker (phpinspect-make-worker)))) + + (puthash project-root project (phpinspect--cache-projects phpinspect-cache)) + + (let* ((function-token (seq-find #'phpinspect-function-p + (phpinspect--resolvecontext-enclosing-tokens context))) + (result (phpinspect-get-variable-type-in-block + context "bar" + (phpinspect-function-block function-token) + (phpinspect--make-type-resolver-for-resolvecontext context) + (phpinspect-function-argument-list function-token)))) + + (should (phpinspect--type= (phpinspect--make-type :name "\\Thing") + result)))))