From 8734c9418d34df9125863c6c24003d1d85a5fe3c Mon Sep 17 00:00:00 2001 From: Hugo Thunnissen Date: Sat, 17 Aug 2024 09:57:48 +0200 Subject: [PATCH] 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 | 113 +++++++++++++++++++++++++++++++++++------- phpinspect-index.el | 25 +++++++--- phpinspect-parser.el | 40 ++++++++++++--- phpinspect-type.el | 5 +- test/test-imports.el | 113 +++++++++++++++++++++++++++++++++++++++++- test/test-index.el | 15 +++--- 6 files changed, 270 insertions(+), 41 deletions(-) diff --git a/phpinspect-imports.el b/phpinspect-imports.el index 83b658c..de679c4 100644 --- a/phpinspect-imports.el +++ b/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 - 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))) (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) - (let* ((class-imports (alist-get 'imports class)) - (used-types (alist-get 'used-types class)) - (class-name (alist-get 'class-name class)) - (region (alist-get 'location class)) - token-meta) - (setq token-meta (phpinspect-meta-find-parent-matching-token - (phpinspect-bmap-last-token-before-point - (phpinspect-buffer-map buffer) - (+ (phpinspect-region-start region) 1)) - #'phpinspect-class-p)) + (phpinspect-format-use-statements + buffer (phpinspect-find-first-use (phpinspect-buffer-root-meta buffer))) + + (phpinspect-buffer-parse buffer 'no-interrupt) + + (dolist (cell namespace-tokens) + (let* ((namespace (cdr cell)) + (namespace-name (car namespace)) + (token-meta (car cell)) + (used-types (alist-get 'used-types namespace)) + (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) diff --git a/phpinspect-index.el b/phpinspect-index.el index 9e4eddc..6484c5f 100644 --- a/phpinspect-index.el +++ b/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) - (alist-get 'used-types namespace-index)) + (nconc (alist-get 'namespaces indexed) + (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 diff --git a/phpinspect-parser.el b/phpinspect-parser.el index 26cc97c..8b4f79d 100644 --- a/phpinspect-parser.el +++ b/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))) - nil))) + (if delimiter-condition + `(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 diff --git a/phpinspect-type.el b/phpinspect-type.el index 3f71637..5df2478 100644 --- a/phpinspect-type.el +++ b/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)))) diff --git a/test/test-imports.el b/test/test-imports.el index 8e0e364..e6d4809 100644 --- a/test/test-imports.el +++ b/test/test-imports.el @@ -37,7 +37,7 @@ (let* ((buffer (phpinspect-make-buffer :buffer (current-buffer) :-project project))) - (insert "