Test + improve `phpinspect-fix-imports' (and parse enums as classes)

- `phpinspect-fix-imports' now sorts the imports alphabetically
- `phpinspect-fix-imports' removes excess trailing newlines
- "enum" keywords are now regarded like "class" keywords. (enum cases not yet
   supported)
- Namespaces were added to token index

phpinspect-imports.el

@@ -134,7 +134,9 @@ NAMESPACE-META itself is returned without alterations."
   (dolist (import imports)
     ;; Namespace must be inferred within the loop, see comments in
     ;; `phpinspect-add-use-statements-for-missing-types' for context.
-    (let ((namespace (phpinspect-meta-find-parent-matching-token parent-token #'phpinspect-namespace-p)))
+    (let ((namespace (if (phpinspect-namespace-p (phpinspect-meta-token parent-token))
+                         parent-token
+                       (phpinspect-meta-find-parent-matching-token parent-token #'phpinspect-namespace-p))))
       (unless (member (car import) types)
         (when-let ((use-meta (phpinspect-find-use-statement-for-import namespace (cdr import))))
           (let ((start-point (phpinspect-meta-start use-meta))
@@ -161,8 +163,10 @@ determine the scope of the imports (global or local namespace)."
     ;; Namespace token must be inferred within the loop, as the ancestors of
     ;; PARENT-TOKEN may change after a buffer reparse (which happens after each
     ;; insert)
-    (let* ((namespace (phpinspect-meta-find-parent-matching-token
+    (let* ((namespace (if (phpinspect-namespace-p (phpinspect-meta-token parent-token))
-                       parent-token #'phpinspect-namespace-p))
+                          parent-token
+                        (phpinspect-meta-find-parent-matching-token
+                         parent-token #'phpinspect-namespace-p)))
            (namespace-name (if namespace
                                (phpinspect-namespace-name (phpinspect-meta-token namespace))
                              "")))
@@ -179,6 +183,48 @@ determine the scope of the imports (global or local namespace)."
           (phpinspect-add-use-interactive type buffer project namespace)
           (phpinspect-buffer-parse buffer 'no-interrupt))))))
 
+(defun phpinspect-format-use-statements (buffer first-meta)
+  "Format a group of use statements to be sorted in alphabetical
+order and have the right amount of whitespace.
+
+BUFFER should be the buffer (`phpinspect-buffer-p') the use
+statements are located in.
+
+FIRST-META should be the metadata of the first use token of the
+group."
+  (when first-meta
+    (let ((statements
+           (list (cons (phpinspect-use-name-string (phpinspect-meta-token first-meta))
+                       first-meta)))
+          (start (phpinspect-meta-start first-meta))
+          (end (phpinspect-meta-end first-meta))
+          (current first-meta))
+      (while (and (setq current (phpinspect-meta-find-right-sibling current))
+                  (phpinspect-use-p (phpinspect-meta-token current)))
+        (setq end (phpinspect-meta-end current))
+        (push (cons (phpinspect-use-name-string (phpinspect-meta-token current))
+                    current)
+              statements))
+
+      (sort statements (lambda (a b) (string< (car a) (car b))))
+
+      ;; Re-insert use statements
+      (with-current-buffer (phpinspect-buffer-buffer buffer)
+        (goto-char start)
+        (delete-region start end)
+        (dolist (statement statements)
+          (insert (format "use %s;%c" (car statement) ?\n)))
+
+        (if (and (looking-at "[[:blank:]\n]+"))
+            ;; Delete excess trailing whitespace (there's more than 2 between the
+            ;; last use statement and the next token)
+            (when (< 1 (- (match-end 0) (match-beginning 0)))
+              (delete-region (match-beginning 0) (match-end 0))
+              (insert-char ?\n))
+          ;; Insert an extra newline (there's only one between the last use
+          ;; statement and the next token)
+          (insert-char ?\n))))))
+
 (defun phpinspect-fix-imports ()
   "Find types that are used in the current buffer and make sure
 that there are import (\"use\") statements for them."
@@ -198,9 +244,34 @@ that there are import (\"use\") statements for them."
              (index (phpinspect--index-tokens
                      tree nil (phpinspect-buffer-location-resolver buffer)))
              (classes (alist-get 'classes index))
+             (namespaces (alist-get 'namespaces index))
              (imports (alist-get 'imports index))
              (project (phpinspect-buffer-project buffer))
-             (used-types (alist-get 'used-types index)))
+             (used-types (alist-get 'used-types index))
+             class-tokens
+             namespace-tokens)
+
+        ;; First collect tokens in the buffer via which the namespace tokens can
+        ;; be found. The edits we do to add imports will invalidate the class
+        ;; region bounds, making this hard to do during the loop that adds them.
+        (dolist (namespace namespaces)
+          (let ((region (alist-get 'location namespace)))
+            ;; Use the first child of the namespace token. The namespace token
+            ;; itself will be tainted and reparsed after every edit (inserted
+            ;; use statement), making its reference useless as it will be in an
+            ;; overlayed tree. The first token in the namespace, however will be
+            ;; adopted into the new tree as long as it isn't altered. This
+            ;; allows a lookup of the new namespace token by getting this
+            ;; token's (newly assigned) parent after every edit.
+            (push (cons (phpinspect-meta-find-first-child-matching-token
+                         (phpinspect-meta-find-parent-matching-token
+                          (phpinspect-bmap-last-token-before-point
+                           (phpinspect-buffer-map buffer)
+                           (+ (phpinspect-region-start region) 1))
+                          #'phpinspect-namespace-p)
+                         #'phpinspect-word-p)
+                        namespace)
+                  namespace-tokens)))
 
         (phpinspect-add-use-statements-for-missing-types
          used-types buffer imports project (phpinspect-buffer-root-meta buffer))
@@ -208,24 +279,30 @@ that there are import (\"use\") statements for them."
         (phpinspect-remove-unneeded-use-statements
          used-types buffer imports (phpinspect-buffer-root-meta buffer))
 
-        (dolist (class classes)
+        (phpinspect-format-use-statements
-          (let* ((class-imports (alist-get 'imports class))
+         buffer (phpinspect-find-first-use (phpinspect-buffer-root-meta buffer)))
-                 (used-types (alist-get 'used-types class))
+
-                 (class-name (alist-get 'class-name class))
+        (phpinspect-buffer-parse buffer 'no-interrupt)
-                 (region (alist-get 'location class))
+
-                 token-meta)
+        (dolist (cell namespace-tokens)
-            (setq token-meta (phpinspect-meta-find-parent-matching-token
+          (let* ((namespace (cdr cell))
-                              (phpinspect-bmap-last-token-before-point
+                 (namespace-name (car namespace))
-                               (phpinspect-buffer-map buffer)
+                 (token-meta (car cell))
-                               (+ (phpinspect-region-start region) 1))
+                 (used-types (alist-get 'used-types namespace))
-                              #'phpinspect-class-p))
+                 (namespace-imports (alist-get 'imports namespace)))
+
             (unless token-meta
-              (error "Unable to find token for class %s" class-name))
+              (error "Unable to find token for namespace %s" namespace-name))
 
             (phpinspect-add-use-statements-for-missing-types
-             used-types buffer (append imports class-imports) project token-meta)
+             used-types buffer (append imports namespace-imports) project token-meta)
 
             (phpinspect-remove-unneeded-use-statements
-             used-types buffer class-imports token-meta))))))
+             used-types buffer namespace-imports token-meta)
+
+            (let ((parent (phpinspect-meta-find-parent-matching-token
+                           token-meta #'phpinspect-namespace-or-root-p)))
+              (phpinspect-format-use-statements buffer (phpinspect-find-first-use parent))
+              (phpinspect-buffer-parse buffer 'no-interrupt)))))))
 
 (provide 'phpinspect-imports)

phpinspect-index.el

@@ -451,9 +451,10 @@ NAMESPACE will be assumed the root namespace if not provided"
 
 (defun phpinspect--index-namespace (namespace type-resolver-factory location-resolver)
   (let* (used-types
+         (imports (phpinspect--uses-to-types (seq-filter #'phpinspect-use-p namespace)))
          (index
           `((classes . ,(phpinspect--index-classes-in-tokens
-                         (phpinspect--uses-to-types (seq-filter #'phpinspect-use-p namespace))
+                         imports
                          namespace
                          type-resolver-factory location-resolver (cadadr namespace)))
             (functions . ,(phpinspect--index-functions-in-tokens
@@ -461,9 +462,21 @@ NAMESPACE will be assumed the root namespace if not provided"
                            type-resolver-factory
                            (phpinspect--uses-to-types (seq-filter #'phpinspect-use-p namespace))
                            (cadadr namespace)
-                           (lambda (types) (setq used-types (nconc used-types types))))))))
+                           (lambda (types) (setq used-types (nconc used-types (mapcar #'phpinspect-intern-name types)))))))))
 
-    (push `(used-types . ,used-types) index)))
+    (let (class-names)
+      (dolist (class (alist-get 'classes index))
+        (push (car class) class-names)
+        (setq used-types (nconc used-types (alist-get 'used-types class))))
+
+      (setq used-types (seq-uniq used-types #'eq))
+
+      (push `(namespaces . ((,(cadadr namespace) . ((location  . ,(funcall location-resolver namespace))
+                                                    (imports . ,imports)
+                                                    (used-types . ,used-types)
+                                                    (classes . ,class-names)))))
+            index)
+    index)))
 
 (defun phpinspect--index-namespaces
     (namespaces type-resolver-factory location-resolver &optional indexed)
@@ -474,8 +487,8 @@ NAMESPACE will be assumed the root namespace if not provided"
 
         (if indexed
             (progn
-              (nconc (alist-get 'used-types indexed)
+              (nconc (alist-get 'namespaces indexed)
-                     (alist-get 'used-types namespace-index))
+                     (alist-get 'namespaces namespace-index))
               (nconc (alist-get 'classes indexed)
                      (alist-get 'classes namespace-index))
               (nconc (alist-get 'functions indexed)
@@ -581,6 +594,7 @@ Returns a list of type name strings."
                                                location-resolver)))
            `(phpinspect--root-index
              (imports . ,imports)
+             (namespaces . ,(alist-get 'namespaces namespace-index))
              (classes ,@(append
                          (alist-get 'classes namespace-index)
                          (phpinspect--index-classes-in-tokens
@@ -588,7 +602,6 @@ Returns a list of type name strings."
              (used-types ,@(mapcar #'phpinspect-intern-name
                                    (seq-uniq
                                     (append
-                                     (alist-get 'used-types namespace-index)
                                      (phpinspect--find-used-types-in-tokens tokens))
                                     #'string=)))
              (functions . ,(append

phpinspect-parser.el

@@ -132,7 +132,7 @@ You can purge the parser cache with \\[phpinspect-purge-parser-cache]."
        (put (quote ,inline-name) 'phpinspect--handler t))))
 
 (eval-when-compile
-  (defun phpinspect-make-parser-function (name tree-type handlers &optional delimiter-predicate)
+  (defun phpinspect-make-parser-function (name tree-type handlers &optional delimiter-predicate delimiter-condition)
     "Create a parser function using the handlers by names defined in HANDLER-LIST.
 
 See also `phpinspect-defhandler`.
@@ -160,8 +160,11 @@ token is \";\", which marks the end of a statement in PHP."
            (while (and (< (point) max-point)
                        (if continue-condition (funcall continue-condition) t)
                        (not ,(if delimiter-predicate
-                                   `(,delimiter-predicate (car (last tokens)))
+                                 (if delimiter-condition
-                                 nil)))
+                                     `(and (,delimiter-condition tokens)
+                                           (,delimiter-predicate (car (last tokens))))
+                                   `(,delimiter-predicate (car (last tokens))))
+                               nil)))
              (cond ,@(mapcar
                       (lambda (handler)
                         `((looking-at (,(phpinspect-handler-regexp-func-name handler)))
@@ -175,7 +178,7 @@ token is \";\", which marks the end of a statement in PHP."
            tokens))))
 
 
-  (defun phpinspect-make-incremental-parser-function (name tree-type handlers &optional delimiter-predicate)
+  (defun phpinspect-make-incremental-parser-function (name tree-type handlers &optional delimiter-predicate delimiter-condition)
     "Like `phpinspect-make-parser-function', but returned function
 is able to reuse an already parsed tree."
     (cl-assert (symbolp delimiter-predicate))
@@ -202,7 +205,10 @@ is able to reuse an already parsed tree."
              (while (and (< (point) max-point)
                          (if continue-condition (funcall continue-condition) t)
                          (not ,(if delimiter-predicate
-                                   `(,delimiter-predicate (car (last tokens)))
+                                   (if delimiter-condition
+                                       `(and (,delimiter-condition tokens)
+                                             (,delimiter-predicate (car (last tokens))))
+                                     `(,delimiter-predicate (car (last tokens))))
                                  nil)))
                (when check-interrupt
                  (phpinspect-pctx-check-interrupt context))
@@ -274,6 +280,13 @@ handlers that this parser uses.")
                          :documentation "A predicate function that is passed each
 parsed token. When the predicate returns a non-nil value, the parser stops
 executing.")
+    (delimiter-condition nil
+                         :type function
+                         :read-only t
+                         :documentation "A predicate function
+ that is passed the list of parsed tokens after each parsed
+ token. If it returns a non-nil value, the delimiter-predicate is
+ invoked and checked.")
     (func nil
           :type function
           :documentation "The parser function.")
@@ -289,7 +302,8 @@ executing.")
                (phpinspect-parser-name parser)
                (intern (concat ":" (phpinspect-parser-tree-keyword parser)))
                (phpinspect-parser-handlers parser)
-               (phpinspect-parser-delimiter-predicate parser)))))
+               (phpinspect-parser-delimiter-predicate parser)
+               (phpinspect-parser-delimiter-condition parser)))))
 
   (cl-defmethod phpinspect-parser-compile-incremental ((parser phpinspect-parser))
     "Like `phpinspect-parser-compile', but for an incremental
@@ -300,7 +314,8 @@ version of the parser function."
                (phpinspect-parser-name parser)
                (intern (concat ":" (phpinspect-parser-tree-keyword parser)))
                (phpinspect-parser-handlers parser)
-               (phpinspect-parser-delimiter-predicate parser)))))
+               (phpinspect-parser-delimiter-predicate parser)
+               (phpinspect-parser-delimiter-condition parser)))))
 
   (cl-defmethod phpinspect-parser-compile-entry ((parser phpinspect-parser))
     (let ((func-name (phpinspect-parser-func-name (phpinspect-parser-name parser)))
@@ -586,8 +601,12 @@ nature like argument lists"
      ((string= start-token "->")
       (list :object-attrib name)))))
 
+(define-inline phpinspect--namespace-should-end-at-block-p (tokens)
+  (>= 4 (length tokens)))
+
 (phpinspect-defparser namespace
   :tree-keyword "namespace"
+  :delimiter-condition #'phpinspect--namespace-should-end-at-block-p
   :delimiter-predicate #'phpinspect-block-p)
 
 (phpinspect-defhandler namespace (start-token max-point)
@@ -833,7 +852,12 @@ Returns the consumed text string without face properties."
 (phpinspect-defhandler class-keyword (start-token max-point)
   "Handler for the class keyword, and tokens that follow to define
 the properties of the class"
-  ((regexp . (concat "\\(abstract\\|final\\|class\\|interface\\|trait\\)"
+  ;; FIXME: "case" keyworded enum cases are not recognized/parsed into anything
+  ;; other than "word" tokens. Enums might require a different (specialized)
+  ;; handler to parse into an indexable tree. For now, this works to get basic
+  ;; functionality (enum methods) as enum case support hasn't been implemented
+  ;; yet.
+  ((regexp . (concat "\\(abstract\\|final\\|class\\|interface\\|trait\\|enum\\)"
                      (phpinspect--word-end-regex))))
   (setq start-token (phpinspect--strip-word-end-space start-token))
   `(:class ,(phpinspect-parse-declaration

phpinspect-type.el

@@ -316,8 +316,11 @@ mutability of the variable")
                                  (concat "\\" fqn-string))
                          :fully-qualified t))
 
+(define-inline phpinspect-use-name-string (use)
+  (inline-quote (cadr (cadr ,use))))
+
 (defun phpinspect--use-to-type-cons (use)
-  (let* ((fqn (cadr (cadr use)))
+  (let* ((fqn (phpinspect-use-name-string use))
          (type (phpinspect-use-name-to-type fqn))
          (type-name (if (and (phpinspect-word-p (caddr use))
                              (string= "as" (cadr (caddr use))))

test/test-imports.el

@@ -37,7 +37,7 @@
       (let* ((buffer (phpinspect-make-buffer :buffer (current-buffer)
                                              :-project project)))
 
-        (insert "<php
+        (insert "<?php
 
 namespace Not\\App;
 
@@ -50,16 +50,125 @@ class Baz {
         (add-hook 'after-change-functions #'phpinspect-after-change-function)
 
         (phpinspect-fix-imports)
-        (should (string= "<php
+        (should (string= "<?php
 
 namespace Not\\App;
 
 use App\\Bar;
 use App\\Foo;
 
+class Baz {
+    private Foo $foo;
+    public Bar $bar;
+}"
+                         (buffer-string)))))))
+
+(ert-deftest phpinspect-fix-imports-multiple-namespaced-classes ()
+  (let ((project (phpinspect--make-dummy-composer-project)))
+    (with-temp-buffer
+      (let* ((buffer (phpinspect-make-buffer :buffer (current-buffer)
+                                             :-project project)))
+
+        (insert "<?php
+
+namespace Not\\App;
+
+class Bee {
+    public Bar $bar;
+}
 
 class Baz {
     private Foo $foo;
+}")
+        ;; Ensure buffer is made aware of changes
+        (setq phpinspect-current-buffer buffer)
+        (add-hook 'after-change-functions #'phpinspect-after-change-function)
+
+        (phpinspect-fix-imports)
+        (should (string= "<?php
+
+namespace Not\\App;
+
+use App\\Bar;
+use App\\Foo;
+
+class Bee {
     public Bar $bar;
+}
+
+class Baz {
+    private Foo $foo;
+}"
+                         (buffer-string)))))))
+
+(ert-deftest phpinspect-fix-imports-namespaced-class-and-enum ()
+  (let ((project (phpinspect--make-dummy-composer-project)))
+    (with-temp-buffer
+      (let* ((buffer (phpinspect-make-buffer :buffer (current-buffer)
+                                             :-project project)))
+
+        (insert "<?php
+
+namespace Not\\App;
+
+enum Bee: string {
+    pubic function Bar():  Bar {}
+}
+
+class Baz {
+    private Foo $foo;
+}")
+        ;; Ensure buffer is made aware of changes
+        (setq phpinspect-current-buffer buffer)
+        (add-hook 'after-change-functions #'phpinspect-after-change-function)
+
+        (phpinspect-fix-imports)
+        (should (string= "<?php
+
+namespace Not\\App;
+
+use App\\Bar;
+use App\\Foo;
+
+enum Bee: string {
+    pubic function Bar():  Bar {}
+}
+
+class Baz {
+    private Foo $foo;
+}"
+                         (buffer-string)))))))
+
+(ert-deftest phpinspect-fix-imports-namespaced-class-and-function ()
+  (let ((project (phpinspect--make-dummy-composer-project)))
+    (with-temp-buffer
+      (let* ((buffer (phpinspect-make-buffer :buffer (current-buffer)
+                                             :-project project)))
+
+        (insert "<?php
+
+namespace Not\\App;
+
+function bar(): Bar {}
+
+class Baz {
+    private Foo $foo;
+}")
+        ;; Ensure buffer is made aware of changes
+        (setq phpinspect-current-buffer buffer)
+        (add-hook 'after-change-functions #'phpinspect-after-change-function)
+
+        (phpinspect-fix-imports)
+        (should (string= "<?php
+
+namespace Not\\App;
+
+use App\\Bar;
+use App\\Foo;
+
+function bar(): Bar {}
+
+class Baz {
+    private Foo $foo;
 }"
                          (buffer-string)))))))

test/test-index.el

@@ -50,6 +50,7 @@
          (expected-index
           `(phpinspect--root-index
             (imports)
+            (namespaces)
             (classes
              (,(phpinspect--make-type :name "\\Potato" :fully-qualified t)
               phpinspect--indexed-class
@@ -285,8 +286,11 @@ function test_func(): array {}
 function example(Firewall $wall): Thing {}")
          (tokens (phpinspect-parse-string code))
          (index (phpinspect--index-tokens tokens))
+         (namespace (alist-get "Local" (alist-get 'namespaces index) nil nil #'string=))
          functions)
 
+    (should namespace)
+
     (should (setq functions (alist-get 'functions index)))
     (should (= 2 (length functions)))
     (should (string= "Local\\test_func" (phpinspect--function-name (cadr functions))))
@@ -296,12 +300,11 @@ function example(Firewall $wall): Thing {}")
                                (phpinspect--function-return-type (cadr functions))))
     (should (phpinspect--type= (phpinspect--make-type :name "\\Example\\Thing")
                                (phpinspect--function-return-type (car functions))))
-    (should (= 3 (length (alist-get 'used-types index))))
+    (should (alist-get 'used-types namespace))
-    (should (member (phpinspect-intern-name "Firewall") (alist-get 'used-types index)))
+    (should (= 3 (length (alist-get 'used-types namespace))))
-    (should (member (phpinspect-intern-name "array") (alist-get 'used-types index)))
+    (should (member (phpinspect-intern-name "Firewall") (alist-get 'used-types namespace)))
-    (should (member (phpinspect-intern-name "Thing") (alist-get 'used-types index)))
+    (should (member (phpinspect-intern-name "array") (alist-get 'used-types namespace)))
-
+    (should (member (phpinspect-intern-name "Thing") (alist-get 'used-types namespace)))))
-    (should (alist-get 'used-types index))))
 
 (ert-deftest phpinspect-index-typehinted-variables ()
   (with-temp-buffer