Strip partially typed attributes from resolvecontext when suggesting attributes

When completions aren't found this is probably the result of a bug, but errors
in corfu's hooks are a pain for users so it is better to handle them gracefully

phpinspect-autoload.el

@@ -139,12 +139,13 @@ bareword typenames."))
          (prefix (phpinspect-psr4-prefix strat))
          type-fqn)
     (dolist (dir (phpinspect-psr4-directories strat))
-      (dolist (file (phpinspect-fs-directory-files-recursively fs dir "\\.php$"))
-        (setq type-fqn (phpinspect-filename-to-typename dir file prefix))
-        (phpinspect-autoloader-put-type-bag al type-fqn)
-        (puthash type-fqn file typehash)
-        (when own
-          (puthash type-fqn file own-typehash))))))
+      (when (phpinspect-fs-file-directory-p fs dir)
+        (dolist (file (phpinspect-fs-directory-files-recursively fs dir "\\.php$"))
+          (setq type-fqn (phpinspect-filename-to-typename dir file prefix))
+          (phpinspect-autoloader-put-type-bag al type-fqn)
+          (puthash type-fqn file typehash)
+          (when own
+            (puthash type-fqn file own-typehash)))))))
 
 (cl-defmethod phpinspect-al-strategy-execute ((strat phpinspect-psr0))
   (let* ((fs (phpinspect-psr0-fs strat))
@@ -172,10 +173,13 @@ bareword typenames."))
                      (car (last (split-string (phpinspect-name-string type-fqn) "\\\\")))))
          (bag (gethash type-name (phpinspect-autoloader-type-name-fqn-bags al))))
     (if bag
-        (push type-fqn bag)
+        (setcdr bag (cons type-fqn (cdr bag)))
       (push type-fqn bag)
       (puthash type-name bag (phpinspect-autoloader-type-name-fqn-bags al)))))
 
+(cl-defmethod phpinspect-autoloader-get-type-bag ((al phpinspect-autoloader) (type-name (head phpinspect-name)))
+  (gethash type-name (phpinspect-autoloader-type-name-fqn-bags al)))
+
 (cl-defmethod phpinspect-iterate-composer-jsons
   ((al phpinspect-autoloader) file)
   (let* ((fs (phpinspect-autoloader-fs  al))

phpinspect-completion.el

@@ -69,11 +69,14 @@ candidate. Candidates can be indexed functions and variables.")
 (cl-defmethod phpinspect--completion-list-add
   ((comp-list phpinspect--completion-list) (completion phpinspect--completion))
   (setf (phpinspect--completion-list-has-candidates comp-list) t)
-  (unless (intern-soft (phpinspect--completion-value completion)
-                      (phpinspect--completion-list-completions comp-list))
-    (set (intern (phpinspect--completion-value completion)
-                 (phpinspect--completion-list-completions comp-list))
-         completion)))
+
+  ;; Ignore completions in an invalid state (nil values)
+  (when (phpinspect--completion-value completion)
+    (unless (intern-soft (phpinspect--completion-value completion)
+                         (phpinspect--completion-list-completions comp-list))
+      (set (intern (phpinspect--completion-value completion)
+                   (phpinspect--completion-list-completions comp-list))
+           completion))))
 
 (cl-defmethod phpinspect--completion-list-get-metadata
   ((comp-list phpinspect--completion-list) (completion-name string))
@@ -304,9 +307,13 @@ Returns list of `phpinspect--completion'."
                                            (phpinspect--completion-target comp))))
                          (insert ")")))))
                  :company-kind (lambda (comp-name)
-                                 (phpinspect--completion-kind
-                                  (phpinspect--completion-list-get-metadata
-                                   phpinspect--last-completion-list
-                                   comp-name))))))))
+                                 (let ((comp
+                                        (phpinspect--completion-list-get-metadata
+                                         phpinspect--last-completion-list
+                                         comp-name)))
+                                   (if comp
+                                       (phpinspect--completion-kind comp)
+                                     (phpinspect--log  "Unable to find matching completion for name %s" comp-name)
+                                     nil))))))))
 
 (provide 'phpinspect-completion)

phpinspect-imports.el

@@ -96,15 +96,13 @@ buffer position to insert the use statement at."
 
 (defun phpinspect-add-use-interactive (typename buffer project &optional namespace-token)
   (let* ((autoloader (phpinspect-project-autoload project))
-         (fqn-bags (phpinspect-autoloader-type-name-fqn-bags autoloader)))
-
-    (let ((fqns (gethash typename fqn-bags)))
-      (cond ((= 1 (length fqns))
-             (phpinspect-add-use (phpinspect-name-string (car fqns)) buffer namespace-token))
-            ((> (length fqns) 1)
-             (phpinspect-add-use (completing-read "Class: " fqns)
-                                 buffer namespace-token))
-            (t (phpinspect-message "No import found for type %s" typename))))))
+         (fqns (phpinspect-autoloader-get-type-bag autoloader typename)))
+    (cond ((= 1 (length fqns))
+           (phpinspect-add-use (phpinspect-name-string (car fqns)) buffer namespace-token))
+          ((> (length fqns) 1)
+           (phpinspect-add-use (completing-read "Class: " (phpinspect-names-to-alist fqns))
+                               buffer namespace-token))
+          (t (phpinspect-message "No import found for type %s" typename)))))
 
 (defun phpinspect-namespace-part-of-typename (typename)
   (string-trim-right typename "\\\\?[^\\]+"))

phpinspect-index.el

@@ -160,10 +160,8 @@ function (think \"new\" statements, return types etc.)."
 (defun phpinspect--var-annotations-from-token (token)
   (seq-filter #'phpinspect-var-annotation-p token))
 
-(defun phpinspect--index-variable-from-scope (type-resolver scope comment-before &optional static)
-  "Index the variable inside `scope`."
-  (let* ((var-annotations (phpinspect--var-annotations-from-token comment-before))
-         (variable-name (cadr (cadr scope)))
+(defun phpinspect--variable-type-string-from-comment (comment variable-name)
+  (let* ((var-annotations (phpinspect--var-annotations-from-token comment))
          (type (if var-annotations
                    ;; Find the right annotation by variable name
                    (or (cadr (cadr (seq-find (lambda (annotation)
@@ -171,6 +169,15 @@ function (think \"new\" statements, return types etc.)."
                                              var-annotations)))
                        ;; Give up and just use the last one encountered
                        (cadr (cadr (car (last var-annotations))))))))
+    ;; If type is not a string, the annotation is probably invalid and we should
+    ;; return nil.
+    (when (stringp type) type)))
+
+(defun phpinspect--index-variable-from-scope (type-resolver scope comment-before &optional static)
+  "Index the variable inside `scope`."
+  (let* ((variable-name (cadr (cadr scope)))
+         (type
+          (phpinspect--variable-type-string-from-comment comment-before variable-name)))
     (phpinspect--log "calling resolver from index-variable-from-scope")
     (phpinspect--make-variable
      ;; Static class variables are always prefixed with dollar signs when

phpinspect-splayt.el

@@ -438,7 +438,7 @@ The PLACE is assigned the value of each node.
 
         (phpinspect-splayt-node-traverse-lr (sibling (phpinspect-splayt-node-right first))
           (when (>= (phpinspect-splayt-node-key sibling) key-max)
-            (throw 'return all))
+            (throw 'return (cdr all)))
           (setq all-rear (setcdr all-rear (cons (phpinspect-splayt-node-value sibling) nil))))
 
         (if (and (phpinspect-splayt-node-parent first)

phpinspect-suggest.el

@@ -113,6 +113,19 @@ resolved to provide completion candidates.
 
 If STATIC is non-nil, candidates are provided for constants,
 static variables and static methods."
+  ;; Strip away the existing (incomplete) attribute token. Otherwise, resolving
+  ;; a type from this context while the user has already typed part of an
+  ;; attribute name could return the type of an existing attribute that matches
+  ;; the incomplete name. (this could for example result in methods of the type
+  ;; of $this->entity to be suggested when we really want more suggestions for
+  ;; attributes of the type $this like $this->entityRepository). Essentially, we
+  ;; convert the subject $this->entity into $this so that only the type of $this
+  ;; (or whatever comes before the attribute accessor token (-> or ::)) is
+  ;; actually resolved.
+  (when (phpinspect-attrib-p (car (last (phpinspect--resolvecontext-subject resolvecontext))))
+    (setf (phpinspect--resolvecontext-subject resolvecontext)
+          (butlast (phpinspect--resolvecontext-subject resolvecontext))))
+
   (let* ((type-resolver (phpinspect--make-type-resolver-for-resolvecontext
                          resolvecontext))
          (method-lister (phpinspect--make-method-lister

phpinspect-util.el

@@ -25,7 +25,7 @@
 
 (defvar phpinspect-names (make-hash-table :test #'equal :size 5000 :rehash-size 1.2)
   "An hash-table containing cons cells representing encountered names in
-PHP code. Used to optimize string comparison. See also `phpinspect-indern-name'")
+PHP code. Used to optimize string comparison. See also `phpinspect-intern-name'")
 
 (defun phpinspect-make-name-hash ()
   (make-hash-table :test #'equal :size 5000 :rehash-size 1.2))
@@ -70,8 +70,6 @@ PHP code. Used to optimize string comparison. See also `phpinspect-indern-name'"
               (eq group (cdr cons)))
             phpinspect-enabled-log-groups))
 
-(phpinspect--declare-log-group 'bam)
-
 (defmacro phpinspect--log (&rest args)
   (let ((log-group (alist-get (macroexp-file-name)
                               phpinspect-log-groups nil nil #'string=)))
@@ -124,9 +122,14 @@ level of START-FILE in stead of `default-directory`."
               (phpinspect--find-project-root parent-without-vendor))))))))
 
 (defun phpinspect-intern-name (name)
-  (setq name (cons 'phpinspect-name name))
   (or (gethash name phpinspect-names)
-      (puthash name name phpinspect-names)))
+      (puthash name (cons 'phpinspect-name name) phpinspect-names)))
+
+(defun phpinspect-names-to-alist (names)
+  (let ((alist))
+    (dolist (name names)
+      (push (cons (phpinspect-name-string name) name) alist))
+    alist))
 
 (defsubst phpinspect--wrap-plist-name-in-symbol (property-list)
   (let ((new-plist)

test/test-autoload.el

@@ -100,6 +100,15 @@
     (should (= 2 (length (seq-filter #'phpinspect-psr0-p result))))
     (should (= 2 (length (seq-filter #'phpinspect-psr4-p result))))))
 
+(ert-deftest phpinspect-al-put-type-bag ()
+  (let ((al (phpinspect-make-autoloader)))
+    (phpinspect-autoloader-put-type-bag al (phpinspect-intern-name "\\App\\Place\\Mountain"))
+    (phpinspect-autoloader-put-type-bag al (phpinspect-intern-name "\\App\\Earth\\Mountain"))
+
+    (should (equal `(,(phpinspect-intern-name "\\App\\Place\\Mountain")
+                     ,(phpinspect-intern-name "\\App\\Earth\\Mountain"))
+                   (phpinspect-autoloader-get-type-bag al (phpinspect-intern-name "Mountain"))))))
+
 (ert-deftest phpinspect-al-strategy-execute ()
   (let* ((fs (phpinspect-make-virtual-fs))
          (project (phpinspect--make-project :root "/project/root" :fs fs))

test/test-buffer.el

@@ -467,3 +467,27 @@ class YYY {
                                  (phpinspect-buffer-project buffer)
                                  (phpinspect--make-type :name "\\TestClass"))
                                 "banana"))))))
+
+(ert-deftest phpinspect-buffer-map-imports ()
+  (with-temp-buffer
+    (let ((buffer (phpinspect-make-buffer :buffer (current-buffer))))
+      (insert "<?php
+declare(strict_types=1);
+
+namespace App\\Controller\\Api\\V1;
+
+use Illuminate\\Database\\Eloquent\\Model;
+use Illuminate\\Database\\Eloquent\\Relations\\Relation;
+use Illuminate\\Support\\Facades\\Auth;
+
+class AccountStatisticsController {
+
+    function __construct(){}
+}")
+      (let ((bmap (phpinspect-buffer-parse-map buffer)))
+        (should (equal
+                 `((:use (:word "Illuminate\\Database\\Eloquent\\Model") (:terminator ";"))
+                   (:use (:word "Illuminate\\Database\\Eloquent\\Relations\\Relation") (:terminator ";"))
+                   (:use (:word "Illuminate\\Support\\Facades\\Auth") (:terminator ";")))
+                 (mapcar #'phpinspect-meta-token
+                         (phpinspect-splayt-to-list (phpinspect-bmap-imports bmap)))))))))

test/test-splayt.el

@@ -172,3 +172,15 @@
 
 
     (should (equal '("one" "two" "three") (phpinspect-splayt-to-list tree)))))
+
+(ert-deftest phpinspect-splayt-find-all-between ()
+  (let ((tree (phpinspect-make-splayt)))
+    (phpinspect-splayt-insert tree 9 "nine")
+    (phpinspect-splayt-insert tree 3 "three")
+    (phpinspect-splayt-insert tree 11 "eleven")
+    (phpinspect-splayt-insert tree 8 "eight")
+    (phpinspect-splayt-insert tree 12 "twelve")
+    (phpinspect-splayt-insert tree 4 "four")
+    (phpinspect-splayt-insert tree 1 "one")
+
+    (should (equal '("three" "four") (phpinspect-splayt-find-all-between tree 1 5)))))

test/test-toc.el

@@ -48,4 +48,10 @@
       (should (= 1 (length result-deleted)))
 
       (should (eq tok1 (car result-deleted)))
-      (should (eq tok4 (car result-new))))))
+      (should (eq tok4 (car result-new))))
+
+    (should (equal '(token2 token3)
+                   (mapcar #'phpinspect-meta-token (phpinspect-toc-tokens-in-region toc 0 70))))
+
+    (should (equal '(token2 token3 token4)
+                   (mapcar #'phpinspect-meta-token (phpinspect-toc-tokens-in-region toc 0 91))))))