Fix some bugs introduced by the incremental parsing feature

Among other things:

- use-keyword parser handler result being registered for two positions due to
wrong use of "root" parser parameter.
- phpinspect-fix-imports was broken

phpinspect-bmap.el

@@ -93,6 +93,9 @@
 (defsubst phpinspect-meta-end (meta)
   (cadddr meta))
 
+(defsubst phpinspect-meta-whitespace-before (meta)
+  (car (cddddr meta)))
+
 (defsubst phpinspect-meta-width (meta)
   (- (phpinspect-meta-end meta) (phpinspect-meta-start meta)))
 

phpinspect-buffer.el

@@ -90,7 +90,17 @@ linked with."
 (cl-defmethod phpinspect-buffer-token-meta ((buffer phpinspect-buffer) token)
   (phpinspect-bmap-token-meta (phpinspect-buffer-map buffer) token))
 
-(cl-defmethod phpinspect-buffer-location-resover ((buffer phpinspect-buffer))
-  (phpinspect-bmap-make-location-resolver (phpinspect-buffer-map buffer)))
+(cl-defmethod phpinspect-buffer-location-resolver ((buffer phpinspect-buffer))
+  "Derive location resolver from BUFFER's buffer map. Guarantees to
+retrieve the lastest available map of BUFFER upon first
+invocation, but subsequent invocations will not update the used
+map afterwards, so don't keep the resolver around for long term
+use."
+  (let ((bmap-resolver))
+    (lambda (token)
+      (funcall (with-memoization bmap-resolver
+                 (phpinspect-bmap-make-location-resolver (phpinspect-buffer-map buffer)))
+               token))))
+
 
 (provide 'phpinspect-buffer)

phpinspect-edtrack.el

@@ -59,11 +59,10 @@
   (or (caar edit) 0))
 
 (defsubst phpinspect-edit-end (edit)
-  (if edit
-      (let ((end (or (caar edit) 0))
-            (delta 0)
-            (previous-edit (cdr edit)))
-        (+ end (phpinspect-edit-delta previous-edit)))))
+  (let ((end (or (caar edit) 0))
+        (delta 0)
+        (previous-edit (cdr edit)))
+    (+ end (phpinspect-edit-delta previous-edit))))
 
 (defsubst phpinspect-edit-delta (edit)
   (let ((delta (or (cdar edit) 0))
@@ -73,29 +72,48 @@
     delta))
 
 (defsubst phpinspect-edtrack-original-position-at-point (track point)
-  (let ((edit (phpinspect-edtrack-edits track)))
+  (let ((edit (phpinspect-edtrack-edits track))
+        (encroached)
+        (pos))
     (while (and edit (< point (phpinspect-edit-end edit)))
       (setq edit (cdr edit)))
 
-    (- point (phpinspect-edit-delta edit))))
+    (setq pos (- point (phpinspect-edit-delta edit)))
+
+    ;; When point is within the edit delta's range, correct the delta by the
+    ;; amount of encroachment.
+    (if (< 0 (setq encroached (- (+ (phpinspect-edit-end edit) (or (cdar edit) 0)) point)))
+        (+ pos encroached)
+      pos)))
 
 (defsubst phpinspect-edtrack-current-position-at-point (track point)
-  (let ((edit (phpinspect-edtrack-edits track)))
+  (let ((edit (phpinspect-edtrack-edits track))
+        (encroached)
+        (pos))
     (while (and edit (< point (phpinspect-edit-original-end edit)))
       (setq edit (cdr edit)))
 
-    (+ point (phpinspect-edit-delta edit))))
+    (setq pos (+ point (phpinspect-edit-delta edit)))
+
+    (if (< 0 (setq encroached (- (+ (phpinspect-edit-original-end edit) (or (cdar edit) 0)) point)))
+        (- pos encroached)
+      pos)))
+
 
 (defsubst phpinspect-edtrack-register-edit (track start end pre-change-length)
+  (phpinspect--log
+   "Edtrack registered change: [start: %d, end: %d, pre-change-length: %d]"
+   start end pre-change-length)
+
+  (phpinspect-edtrack-register-taint
+   track
+   (phpinspect-edtrack-original-position-at-point track start)
+   (phpinspect-edtrack-original-position-at-point track (+ start pre-change-length)))
+
   (let ((edit-before (phpinspect-edtrack-edits track)))
     (while (and edit-before (< end (phpinspect-edit-end edit-before)))
       (setq edit-before (cdr edit-before)))
 
-    (phpinspect-edtrack-register-taint
-     track
-     (phpinspect-edtrack-original-position-at-point track start)
-     (phpinspect-edtrack-original-position-at-point track end))
-
     (let* ((new-edit (cons
                       ;; The end location of the edited region, before being
                       ;; edited, with the delta edits that happened at preceding

phpinspect-imports.el

@@ -62,7 +62,7 @@ buffer position to insert the use statement at."
                (format "%c%cuse %s;%c" ?\n ?\n fqn ?\n))
             (phpinspect-insert-at-point
              (phpinspect-meta-end
-                   (phpinspect-buffer-token-meta
+              (phpinspect-buffer-token-meta
                     buffer (seq-find #'phpinspect-terminator-p namespace-token)))
              (format "%c%cuse %s;%c" ?\n ?\n fqn ?\n)))))
     ;; else
@@ -131,14 +131,12 @@ that there are import (\"use\") statements for them."
                                (phpinspect-index-get-class
                                 index class-name)))
 
-
-
               (let ((namespace
-                     (seq-find #'phpinspect-namespace-p
+                     (seq-find (lambda (meta) (phpinspect-namespace-p (phpinspect-meta-token meta)))
                                (phpinspect-buffer-tokens-enclosing-point
-                                phpinspect-current-buffer (phpinspect-meta-start meta)))))
-                ;; Add use statements for types that aren't imported.
+                                phpinspect-current-buffer (phpinspect-region-start region)))))
 
+                ;; Add use statements for types that aren't imported.
                 (unless (or (or (alist-get type class-imports)
                                 (alist-get type imports))
                             (gethash (phpinspect-intern-name
@@ -149,7 +147,7 @@ that there are import (\"use\") statements for them."
                                      (phpinspect-autoloader-types
                                       (phpinspect-project-autoload project))))
                   (phpinspect-add-use-interactive
-                   type phpinspect-current-buffer project namespace)
+                   type phpinspect-current-buffer project (phpinspect-meta-token namespace))
                   ;; Buffer has been modified by adding type, update tree +
                   ;; location map. This is not optimal but will have to do until
                   ;; partial parsing is implemented.

phpinspect-index.el

@@ -463,7 +463,7 @@ Return value is a list of the types that are \"newed\"."
 
 
 (cl-defmethod phpinspect-index-get-class
-  ((index (head phpinspect--root-index) (class-name phpinspect--type)))
+  ((index (head phpinspect--root-index)) (class-name phpinspect--type))
   (alist-get class-name (alist-get 'classes index)
              nil nil #'phpinspect--type=))
 

phpinspect-parser.el

@@ -385,7 +385,10 @@ during runtime.  Parsers are implemented with macros, so changing
 handler functions without calling this function will often not
 have any effect."
   (interactive)
-  (obarray-map #'fmakunbound phpinspect-parser-obarray))
+  (obarray-map (lambda (parser-symbol)
+                 (fmakunbound parser-symbol)
+                 (setf (phpinspect-parser-incremental-func (symbol-value parser-symbol)) nil))
+               phpinspect-parser-obarray))
 
 (defmacro phpinspect-pctx-save-whitespace (pctx &rest body)
   (declare (indent 1))
@@ -492,83 +495,6 @@ parsing. Usually used in combination with
     (setf (phpinspect-pctx-whitespace-before pctx) "")
     whitespace))
 
-(defun phpinspect-make-bmap-parser-function (tree-type handler-list &optional delimiter-predicate)
-  "Like `phpinspect-make-parser-function', but returned function is able to reuse an already parsed tree."
-  (let ((handlers (mapcar
-                   (lambda (handler-name)
-                     (let* ((handler-name (symbol-name handler-name))
-                            (handler (intern-soft handler-name phpinspect-handler-obarray)))
-                       (if handler
-                           handler
-                         (error "No handler found by name \"%s\"" handler-name))))
-                   handler-list))
-        (delimiter-predicate (if (symbolp delimiter-predicate)
-                                 `(quote ,delimiter-predicate)
-                               delimiter-predicate)))
-    `(lambda (context buffer max-point &optional continue-condition root)
-       (with-current-buffer buffer
-         (let* ((tokens)
-                (root-start (point))
-                (bmap (phpinspect-pctx-bmap context))
-                (previous-bmap (phpinspect-pctx-previous-bmap context))
-                (edtrack (phpinspect-pctx-edtrack context))
-                (taint-iterator (when edtrack (phpinspect-edtrack-make-taint-iterator edtrack)))
-                (delimiter-predicate (when (functionp ,delimiter-predicate) ,delimiter-predicate)))
-           (phpinspect-pctx-save-whitespace context
-            (while (and (< (point) max-point)
-                        (if continue-condition (funcall continue-condition) t)
-                        (not (if delimiter-predicate
-                                 (funcall delimiter-predicate (car (last tokens)))
-                               nil)))
-              (cond ,@(mapcar
-                       (lambda (handler)
-                         `((looking-at ,(plist-get (symbol-value handler) 'regexp))
-                           (let* ((match (match-string 0))
-                                  (start-position (point))
-                                  (original-position
-                                   (when (and previous-bmap edtrack)
-                                     (phpinspect-edtrack-original-position-at-point edtrack start-position)))
-                                  (existing-meta)
-                                  (current-end-position)
-                                  (token))
-                             (when (and previous-bmap edtrack)
-                               (setq existing-meta (phpinspect-bmap-token-starting-at previous-bmap original-position))
-
-                               (when existing-meta
-                                 (setq current-end-position (phpinspect-edtrack-current-position-at-point
-                                                             edtrack (phpinspect-meta-end existing-meta)))))
-
-                             (if (and existing-meta
-                                      (not (or (phpinspect-root-p (phpinspect-meta-token existing-meta))
-                                               (phpinspect-taint-iterator-token-is-tainted-p taint-iterator existing-meta))))
-                                 (progn
-                                   (setq token (phpinspect-meta-token existing-meta))
-
-                                   ;; Re-register existing token
-                                   (let ((delta (- start-position original-position)))
-                                     (phpinspect-bmap-overlay
-                                      bmap previous-bmap existing-meta delta
-                                      (phpinspect-pctx-consume-whitespace context)))
-
-                                   (goto-char current-end-position))
-                               (progn
-                                 (setq token (funcall ,(symbol-function handler) match max-point))
-                                 (when token
-                                   (phpinspect-pctx-register-token context token start-position (point)))))
-                             (when token
-                               (if (null tokens)
-                                   (setq tokens (list token))
-                                 (progn
-                                   (nconc tokens (list token))))))))
-                       handlers)
-                    (t (forward-char)))))
-           (push ,tree-type tokens)
-           (when root
-             (phpinspect-pctx-register-token context tokens root-start (point)))
-
-           ;; Return
-           tokens)))))
-
 (defun phpinspect-make-incremental-parser-function (tree-type handler-list &optional delimiter-predicate)
   "Like `phpinspect-make-parser-function', but returned function is able to reuse an already parsed tree."
   (let ((handlers (mapcar
@@ -596,6 +522,7 @@ parsing. Usually used in combination with
                 (original-position)
                 (current-end-position)
                 (existing-meta)
+                (delta)
                 (token)
                 (delimiter-predicate (when (functionp ,delimiter-predicate) ,delimiter-predicate)))
            (phpinspect-pctx-save-whitespace context
@@ -605,7 +532,6 @@ parsing. Usually used in combination with
                                  (funcall delimiter-predicate (car (last tokens)))
                                nil)))
               (setq start-position (point))
-
               (cond ((and previous-bmap edtrack
                           (setq existing-meta
                                 (phpinspect-bmap-token-starting-at
@@ -614,32 +540,29 @@ parsing. Usually used in combination with
                                        (phpinspect-edtrack-original-position-at-point edtrack start-position))))
                           (not (or (phpinspect-root-p (phpinspect-meta-token existing-meta))
                                    (phpinspect-taint-iterator-token-is-tainted-p taint-iterator existing-meta))))
-                     (setq current-end-position (phpinspect-edtrack-current-position-at-point
-                                                 edtrack (phpinspect-meta-end existing-meta)))
-
-                     (setq token (phpinspect-meta-token existing-meta))
-                     ;;(message "reusing token %s" token)
-
+                     (setq delta (- start-position original-position)
+                           current-end-position (+ (phpinspect-meta-end existing-meta) delta)
+                           token (phpinspect-meta-token existing-meta))
 
                      ;; Re-register existing token
                      (phpinspect-bmap-overlay
-                      bmap previous-bmap existing-meta (- start-position original-position)
+                      bmap previous-bmap existing-meta delta
                       (phpinspect-pctx-consume-whitespace context))
 
                      ;; Check if we can fast-forward to more siblings
-                     (when (phpinspect-meta-right-siblings existing-meta)
-                       (dolist (sibling (phpinspect-meta-right-siblings existing-meta))
-                         (setq existing-meta (phpinspect-bmap-token-meta previous-bmap sibling))
-                         (unless (phpinspect-taint-iterator-region-is-tainted-p
-                                  taint-iterator current-end-position (phpinspect-meta-end existing-meta))
-                           (nconc tokens (list token))
-                           (setq token (phpinspect-meta-token existing-meta))
-                           (phpinspect-bmap-overlay
-                            bmap previous-bmap existing-meta (- start-position original-position)
-                            (phpinspect-pctx-consume-whitespace context))
-
-                           (setq current-end-position (phpinspect-edtrack-current-position-at-point
-                                                       edtrack (phpinspect-meta-end existing-meta))))))
+                     ;; (when (phpinspect-meta-right-siblings existing-meta)
+                     ;;   (dolist (sibling (phpinspect-meta-right-siblings existing-meta))
+                     ;;     (setq existing-meta (phpinspect-bmap-token-meta previous-bmap sibling))
+                     ;;     (unless (phpinspect-taint-iterator-region-is-tainted-p
+                     ;;              taint-iterator current-end-position (phpinspect-meta-end existing-meta))
+                     ;;       (nconc tokens (list token))
+                     ;;       (setq token (phpinspect-meta-token existing-meta))
+                     ;;       (phpinspect-bmap-overlay
+                     ;;        bmap previous-bmap existing-meta (- start-position original-position)
+                     ;;        (phpinspect-meta-whitespace-before existing-meta))
+
+                     ;;       (setq current-end-position (phpinspect-edtrack-current-position-at-point
+                     ;;                                   edtrack (phpinspect-meta-end existing-meta))))))
 
                      ;;(message "Current pos: %d, end pos: %d" (point) current-end-position)
                      (goto-char current-end-position)
@@ -825,13 +748,13 @@ executing.")
                 (doc-block (save-restriction
                              (goto-char region-start)
                              (narrow-to-region region-start region-end)
-                             (funcall parser (current-buffer) (point-max) nil 'root))))
+                             (funcall parser (current-buffer) (point-max) nil))))
            (forward-char 2)
            doc-block))
         (t
          (let ((parser (phpinspect-get-parser-func 'comment))
                (end-position (line-end-position)))
-           (funcall parser (current-buffer) end-position nil 'root)))))
+           (funcall parser (current-buffer) end-position nil)))))
 
 (phpinspect-defhandler variable (start-token &rest _ignored)
   "Handler for tokens indicating reference to a variable"
@@ -875,7 +798,7 @@ executing.")
   (forward-char (length start-token))
 
   (let ((parser (phpinspect-get-parser-func 'use)))
-    (funcall parser (current-buffer) max-point nil 'root)))
+    (funcall parser (current-buffer) max-point nil)))
 
 (phpinspect-defhandler attribute-reference (start-token &rest _ignored)
   "Handler for references to object attributes, or static class attributes."
@@ -921,7 +844,7 @@ executing.")
   (setq start-token (phpinspect--strip-word-end-space start-token))
   (forward-char (length start-token))
   (let* ((parser (phpinspect-get-parser-func 'const))
-         (token (funcall parser (current-buffer) max-point nil 'root)))
+         (token (funcall parser (current-buffer) max-point nil)))
     (when (phpinspect-incomplete-token-p (car (last token)))
       (setcar token :incomplete-const))
     token))
@@ -969,7 +892,7 @@ static keywords with the same meaning as in a class block."
          (continue-condition (lambda ()
                                (not (and (char-equal (char-after) ?})
                                          (setq complete-block t)))))
-         (parsed (funcall parser (current-buffer) max-point continue-condition 'root)))
+         (parsed (funcall parser (current-buffer) max-point continue-condition)))
     (if complete-block
         (forward-char)
       (setcar parsed :incomplete-block))

test/test-bmap.el

@@ -29,6 +29,14 @@
     (should (eq 'token (phpinspect-meta-token
                         (phpinspect-bmap-token-starting-at bmap2 7))))
 
+    ;; Nesting for token-starting-at
+    (should (eq 'token3 (phpinspect-meta-token
+                         (phpinspect-bmap-token-starting-at bmap 50))))
+
+    (should (eq 'token3 (phpinspect-meta-token
+                         (phpinspect-bmap-token-starting-at bmap2 55))))
+
+
     (should (phpinspect-bmap-token-meta bmap 'token))
     (should (phpinspect-bmap-token-meta bmap2 'token2))
     (should (phpinspect-bmap-token-meta bmap2 'token))

test/test-buffer.el

@@ -124,7 +124,8 @@
       (let ((function (phpinspect-meta-token (phpinspect-meta-parent (phpinspect-meta-parent bello1)))))
         (should (= 2 (length function)))
         (should (phpinspect-declaration-p (cadr function)))
-        (should (member '(:word "Bello") (cadr function))))
+        (should (member '(:word "Bello") (cadr function)))
+        (should (member '(:word "echo") (cadr function))))
 
       (phpinspect-document-apply-edit document 24 25 1 "{")
       (should (string= "<?php function Bello() { echo 'Hello World!'; if ($name) { echo 'Hello ' . $name . '!';} }"
@@ -134,3 +135,74 @@
       (should parsed)
       (setq bello2 (car (phpinspect-buffer-tokens-enclosing-point buffer 18)))
       (should (eq (phpinspect-meta-token bello) (phpinspect-meta-token bello2))))))
+
+(ert-deftest phpinspect-buffer-parse-incrementally-position-change ()
+  (with-temp-buffer
+    (let ((buffer (phpinspect-make-buffer :buffer (current-buffer))))
+      (insert "<?php
+declare(strict_types=1);
+
+namespace App\\Controller\\Api\\V1;
+
+class AccountStatisticsController {
+
+    function __construct(){}
+}")
+
+      (setq-local phpinspect-test-buffer t)
+      (add-to-list 'after-change-functions
+                   (lambda (start end pre-change-length)
+                     (when (boundp 'phpinspect-test-buffer)
+                       (phpinspect-buffer-register-edit buffer start end pre-change-length))))
+
+      (let* ((bmap (phpinspect-buffer-parse-map buffer))
+             (class-location 67)
+             (class (phpinspect-bmap-token-starting-at bmap class-location))
+
+        (should class)
+        (should (phpinspect-class-p (phpinspect-meta-token class)))
+        (should (= class-location (phpinspect-meta-start class))))
+
+      (goto-char 65)
+      (let ((edit-string "use Symfony\\Component\\HttpFoundation\\JsonResponse;\n")
+            bmap class tokens-enclosing use-statement)
+        (insert edit-string)
+
+        (setq bmap (phpinspect-buffer-parse-map buffer)
+              class (phpinspect-bmap-token-starting-at bmap (+ 67 (length edit-string))))
+
+        (setq class-location (+ class-location (length edit-string)))
+        (should class)
+        (should (phpinspect-class-p (phpinspect-meta-token class)))
+        (should (= class-location (phpinspect-meta-start class)))
+
+        (setq tokens-enclosing (phpinspect-bmap-tokens-overlapping bmap class-location))
+        (setq class (seq-find (lambda (meta) (phpinspect-class-p (phpinspect-meta-token meta)))
+                              tokens-enclosing))
+        (should  class)
+        (should (= class-location (phpinspect-meta-start class)))
+        (should (phpinspect-class-p (phpinspect-meta-token class)))
+
+        (setq use-statement (phpinspect-bmap-token-starting-at bmap 65))
+        (should use-statement)
+        (should (phpinspect-use-p (phpinspect-meta-token use-statement)))
+        (should (seq-find #'phpinspect-use-p (seq-find #'phpinspect-namespace-p (phpinspect-buffer-tree buffer))))
+
+        (let ((second-use))
+          (goto-char 65)
+          (setq edit-string "use Another\\Use\\Statement;\n")
+          (insert edit-string)
+
+          (setq class-location (+ class-location (length edit-string)))
+          (setq bmap (phpinspect-buffer-parse-map buffer)
+                class (phpinspect-bmap-token-starting-at bmap class-location))
+
+          (should class)
+
+          (setq second-use (phpinspect-bmap-token-starting-at bmap 65))
+          (should second-use)
+
+          (setq class (phpinspect-bmap-token-starting-at bmap class-location))
+          (should  class)
+          (should (= class-location (phpinspect-meta-start class)))
+          (should (phpinspect-class-p (phpinspect-meta-token class)))))))))

test/test-edtrack.el

@@ -12,13 +12,15 @@
          (edit2 (phpinspect-edtrack-register-edit edtrack 15 22 7)))
 
     (should (equal `((255 . -50) (27 . 0) (15 . -5)) (phpinspect-edtrack-edits edtrack)))))
-    ;; (pp (phpinspect-edtrack-edits edtrack))
-    ;; (should (= 10 (phpinspect-edit-end edit1)))
-    ;; (should (= 22 (phpinspect-edit-end edit2)))
 
-    ;; (should (= 30 (phpinspect-edtrack-original-position-at-point edtrack 25)))
-    ;; (should (= 4 (phpinspect-edtrack-original-position-at-point edtrack 4)))
-    ;; (should (= 260 (phpinspect-edtrack-original-position-at-point edtrack 205)))))
+(ert-deftest phpinspect-edtrack-orginal-position-at-point ()
+  (let ((track (phpinspect-make-edtrack)))
+    (phpinspect-edtrack-register-edit track 10 20 0)
+    (should (= 10 (phpinspect-edtrack-original-position-at-point track 20)))
+    (should (= 10 (phpinspect-edtrack-original-position-at-point track 15)))
+    (phpinspect-edtrack-register-edit track 30 40 5)
+    (should (= 35 (phpinspect-edtrack-original-position-at-point track 50)))
+    (should (= 25 (phpinspect-edtrack-original-position-at-point track 39)))))
 
 (ert-deftest phpinsepct-edtrack-register-multi-edits ()
   (let ((track (phpinspect-make-edtrack)))