diff --git a/Makefile b/Makefile index 4dce40a..1dd88af 100644 --- a/Makefile +++ b/Makefile @@ -49,4 +49,4 @@ compile-native: ./.deps .PHONY: test test: deps - $(RUN_EMACS) -L ./test -l ./test/phpinspect-test e -f ert-run-tests-batch + $(RUN_EMACS) -L ./test -l ./test/phpinspect-test -f ert-run-tests-batch diff --git a/phpinspect-buffer.el b/phpinspect-buffer.el index 3bdaa5c..354ec5e 100644 --- a/phpinspect-buffer.el +++ b/phpinspect-buffer.el @@ -339,27 +339,38 @@ linked with." (setf (phpinspect-buffer-class-variables buffer) (phpinspect-make-toc class-variables))) (dolist (var to-be-indexed) + ;; We have left the class we were previously indexing properties + ;; for. (when (and class (> (phpinspect-meta-start var) (phpinspect-meta-end class))) (setq class nil)) + ;; Retrieve the class that the property belongs to. In a lot of cases only + ;; one class will be in the current buffer. But PHP allows the definition + ;; of multiple classes in the same file so this should be supported. (unless class (setq class (phpinspect-toc-token-at-point classes (phpinspect-meta-start var)))) + ;; Fetch the index for the current class (setq class-obj (phpinspect-buffer-get-index-for-token buffer (phpinspect-meta-token class))) (let (scope static indexed index-env comment-before) (if (phpinspect-static-p (phpinspect-meta-token (phpinspect-meta-parent var))) + ;; Variable is defined as [scope?] static [type?] $variable (progn (setq static (phpinspect-meta-parent var)) (when (phpinspect-scope-p (phpinspect-meta-token (phpinspect-meta-parent static))) + ;; Variable is defined as scope static [type?] $variable (setq scope `(,(car (phpinspect-meta-token (phpinspect-meta-parent static))) - ,(phpinspect-meta-token var)) + ,@(phpinspect-meta-token-with-left-siblings var)) comment-before (phpinspect-meta-find-left-sibling (phpinspect-meta-parent static))))) (when (phpinspect-scope-p (phpinspect-meta-token (phpinspect-meta-parent var))) + ;; Variable is defined as scope [type?] $variable (setq scope (phpinspect-meta-token (phpinspect-meta-parent var)) comment-before (phpinspect-meta-find-left-sibling (phpinspect-meta-parent var))))) - (unless scope (setq scope `(:public ,(phpinspect-meta-token var)))) + (unless scope + (setq scope `(:public ,@(mapcar #'phpinspect-meta-token (phpinspect-meta-left-siblings var)) + ,(phpinspect-meta-token var)))) (unless (setq index-env (alist-get class class-environments nil nil #'eq)) (setq index-env (phpinspect-get-token-index-context namespaces imports class)) @@ -419,6 +430,16 @@ linked with." (phpinspect-buffer-parse buffer 'no-interrupt)) (cl-defmethod phpinspect-buffer-update-project-index ((buffer phpinspect-buffer)) + "Update project index using the last parsed token map of this +buffer. When `phpinspect-buffer-parse' has been executed before +and a map is available from the previous parse, this is used. If +none is available `phpinspect-buffer-parse' is called before +continuing execution." + + ;; Parse buffer if no map is available. + (unless (phpinspect-buffer-map buffer) + (phpinspect-buffer-parse buffer)) + ;; Use inhibit-quit to prevent index corruption though partial index ;; application. (let ((inhibit-quit t)) diff --git a/phpinspect-index.el b/phpinspect-index.el index 6c999d0..a786861 100644 --- a/phpinspect-index.el +++ b/phpinspect-index.el @@ -174,10 +174,29 @@ function (think \"new\" statements, return types etc.)." (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))) + "Index the variable inside SCOPE, use doc in COMMENT-BEFORE if missing typehint. + +Provide STATIC if the variable was defined as static. + +SCOPE should be a scope token (`phpinspect-scope-p')." + (setq scope (take 5 (seq-filter #'phpinspect-not-comment-p scope))) + (let (variable-name type) + (cond + ((phpinspect--match-sequence (take 3 scope) + ;; Sequence: scope-type, typehint, variable [ = value ] + :m * :f #'phpinspect-word-p :f #'phpinspect-variable-p) + (setq variable-name (cadr (nth 2 scope))) + (setq type (cadr (nth 1 scope)))) + ((phpinspect--match-sequence (take 2 scope) + ;; Sequence: variable [ = value ] + :m * :f #'phpinspect-variable-p) + (setq variable-name (cadr (nth 1 scope)) + ;; Since no typehint is available, attempt extracting one from a + ;; docstring. + type (phpinspect--variable-type-string-from-comment + comment-before variable-name))) + (t (error (format "Failed to determine variable from scope %s" scope)))) + (phpinspect--log "calling resolver from index-variable-from-scope") (phpinspect--make-variable ;; Static class variables are always prefixed with dollar signs when @@ -260,7 +279,7 @@ function (think \"new\" statements, return types etc.)." (cond ((phpinspect-const-p (cadr token)) (push (phpinspect--index-const-from-scope token) constants)) - ((phpinspect-variable-p (cadr token)) + ((seq-find #'phpinspect-variable-p token) (push (phpinspect--index-variable-from-scope type-resolver token comment-before) @@ -275,10 +294,9 @@ function (think \"new\" statements, return types etc.)." add-used-types) static-methods)) - ((phpinspect-variable-p (cadadr token)) + ((seq-find #'phpinspect-variable-p (cdadr token)) (push (phpinspect--index-variable-from-scope type-resolver - (list (car token) - (cadadr token)) + `(,(car token) ,@(cdadr token)) comment-before 'static) static-variables)))) @@ -298,11 +316,11 @@ function (think \"new\" statements, return types etc.)." add-used-types) static-methods)) - ((phpinspect-variable-p (cadr token)) + ((seq-find #'phpinspect-variable-p (cdr token)) (push (phpinspect--index-variable-from-scope type-resolver - `(:public - ,(cadr token)) - comment-before) + `(:public ,@(cdr token)) + comment-before + 'static) static-variables)))) ((phpinspect-const-p token) ;; Bare constants are always public diff --git a/phpinspect-meta.el b/phpinspect-meta.el index 098b9ed..d6590af 100644 --- a/phpinspect-meta.el +++ b/phpinspect-meta.el @@ -149,6 +149,22 @@ (phpinspect-meta-children (phpinspect-meta-parent meta)) (phpinspect-meta-parent-offset meta)) #'phpinspect-meta-sort-start)) +(defun phpinspect-meta-left-sibling-tokens (meta) + (let* ((tokens (cons nil nil)) + (rear tokens)) + (dolist (sibling (phpinspect-meta-left-siblings meta)) + (setq rear (setcdr rear (cons (phpinspect-meta-token sibling) nil)))) + (cdr tokens))) + +(defun phpinspect-meta-token-with-left-siblings (meta) + (nconc (phpinspect-meta-left-sibling-tokens meta) (list (phpinspect-meta-token meta)))) + +(defun phpinspect-meta-left-siblings (meta) + (sort + (phpinspect-splayt-find-all-before + (phpinspect-meta-children (phpinspect-meta-parent meta)) (phpinspect-meta-parent-offset meta)) + #'phpinspect-meta-sort-start)) + (defun phpinspect-meta-wrap-token-pred (predicate) (lambda (meta) (funcall predicate (phpinspect-meta-token meta)))) diff --git a/phpinspect-parser.el b/phpinspect-parser.el index 61f41f6..26cc97c 100644 --- a/phpinspect-parser.el +++ b/phpinspect-parser.el @@ -760,19 +760,19 @@ Returns the consumed text string without face properties." (phpinspect-defparser scope-public :tree-keyword "public" :handlers '(function-keyword static-keyword const-keyword class-variable here-doc - string terminator tag comment) + string terminator tag comment word) :delimiter-predicate #'phpinspect--scope-terminator-p) (phpinspect-defparser scope-private :tree-keyword "private" :handlers '(function-keyword static-keyword const-keyword class-variable here-doc - string terminator tag comment) + string terminator tag comment word) :delimiter-predicate #'phpinspect--scope-terminator-p) (phpinspect-defparser scope-protected :tree-keyword "protected" :handlers '(function-keyword static-keyword const-keyword class-variable here-doc - string terminator tag comment) + string terminator tag comment word) :delimiter-predicate #'phpinspect--scope-terminator-p) (phpinspect-defhandler scope-keyword (start-token max-point) diff --git a/phpinspect-resolve.el b/phpinspect-resolve.el index cae5fa8..9dfdb87 100644 --- a/phpinspect-resolve.el +++ b/phpinspect-resolve.el @@ -46,6 +46,11 @@ (or (phpinspect-assignment-p token) (equal '(:word "as") token))) +(define-inline phpinspect-not-assignment-p (token) + "Inverse of applying `phpinspect-assignment-p to TOKEN." + (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" (when (keywordp (car token)) @@ -71,13 +76,6 @@ (phpinspect--log "Found statements in token: %s" statements) assignments)) -(defsubst phpinspect-not-assignment-p (token) - "Inverse of applying `phpinspect-assignment-p to TOKEN." - (not (phpinspect-maybe-assignment-p token))) - -(defsubst phpinspect-not-comment-p (token) - (not (phpinspect-comment-p token))) - (defun phpinspect--find-assignments-by-predicate (token predicate) (let ((variable-assignments) (all-assignments (phpinspect--find-assignments-in-token token))) diff --git a/phpinspect-token-predicates.el b/phpinspect-token-predicates.el index 27dce56..4724c39 100644 --- a/phpinspect-token-predicates.el +++ b/phpinspect-token-predicates.el @@ -251,4 +251,9 @@ Type can be any of the token types returned by (and (listp token) (keywordp (car token)))) +(define-inline phpinspect-not-comment-p (token) + (inline-quote + (not (phpinspect-comment-p ,token)))) + + (provide 'phpinspect-token-predicates) diff --git a/test/fixtures/IndexClass1.eld b/test/fixtures/IndexClass1.eld index 8ccf155..88f6f3f 100644 --- a/test/fixtures/IndexClass1.eld +++ b/test/fixtures/IndexClass1.eld @@ -1 +1 @@ -(:root (:word "declare") (:list (:word "strict_types") (:assignment "=")) (:terminator ";") (:namespace (:word "App\\Entity") (:terminator ";") (:use (:word "Doctrine\\ORM\\Mapping") (:word "as") (:word "ORM") (:terminator ";")) (:doc-block (:annotation "ORM\\Entity")) (:class (:declaration (:word "class") (:word "AuthToken")) (:block (:private (:class-variable "token") (:terminator ";")) (:doc-block (:var-annotation (:word "App\\\\Entity\\\\User"))) (:private (:class-variable "user") (:terminator ";")) (:doc-block (:var-annotation (:word "bool"))) (:private (:class-variable "valid") (:terminator ";")) (:doc-block (:var-annotation (:word "\\DateTime"))) (:private (:class-variable "creation_time") (:terminator ";")) (:public (:function (:declaration (:word "function") (:word "__construct") (:list (:word "string") (:variable "token") (:comma ",") (:word "User") (:variable "user") (:comma ",") (:word "bool") (:variable "valid") (:assignment "=") (:word "false") (:comma ",") (:word "\\DateTime") (:variable "creation_time") (:assignment "=") (:word "null"))) (:block (:variable "this") (:object-attrib (:word "token")) (:assignment "=") (:variable "token") (:terminator ";") (:variable "this") (:object-attrib (:word "user")) (:assignment "=") (:variable "user") (:terminator ";") (:variable "this") (:object-attrib (:word "valid")) (:assignment "=") (:variable "valid") (:terminator ";") (:variable "this") (:object-attrib (:word "creation_time")) (:assignment "=") (:variable "creation_time") (:word "new") (:word "\\DateTime") (:list) (:terminator ";")))) (:public (:function (:declaration (:word "function") (:word "getToken") (:list) (:word "string")) (:block (:word "return") (:variable "this") (:object-attrib (:word "token")) (:terminator ";")))) (:public (:function (:declaration (:word "function") (:word "getUser") (:list) (:word "User")) (:block (:word "return") (:variable "this") (:object-attrib (:word "user")) (:terminator ";")))) (:public (:function (:declaration (:word "function") (:word "hasStudentRole") (:list) (:word "bool")) (:block (:word "return") (:variable "this") (:object-attrib (:word "role_student")) (:terminator ";")))) (:public (:function (:declaration (:word "function") (:word "isValid") (:list) (:word "bool")) (:block (:word "return") (:variable "this") (:object-attrib (:word "valid")) (:terminator ";")))) (:public (:function (:declaration (:word "function") (:word "getCreationTime") (:list) (:word "\\DateTime")) (:block (:word "return") (:variable "this") (:object-attrib (:word "creation_time")) (:terminator ";")))) (:doc-block (:return-annotation (:word "DateTime[]"))) (:public (:function (:declaration (:word "function") (:word "arrayReturn") (:list) (:word "array")) (:block (:word "return") (:array (:word "new") (:word "\\DateTime") (:list)) (:terminator ";")))))))) +(:root (:word "declare") (:list (:word "strict_types") (:assignment "=")) (:terminator ";") (:namespace (:word "App\\Entity") (:terminator ";") (:use (:word "Doctrine\\ORM\\Mapping") (:word "as") (:word "ORM") (:terminator ";")) (:doc-block (:annotation "ORM\\Entity")) (:class (:declaration (:word "class") (:word "AuthToken")) (:block (:private (:class-variable "token") (:terminator ";")) (:doc-block (:var-annotation (:word "App\\\\Entity\\\\User"))) (:private (:class-variable "user") (:terminator ";")) (:doc-block (:var-annotation (:word "bool"))) (:private (:class-variable "valid") (:word "false") (:terminator ";")) (:doc-block (:var-annotation (:word "\\DateTime"))) (:private (:class-variable "creation_time") (:terminator ";")) (:public (:function (:declaration (:word "function") (:word "__construct") (:list (:word "string") (:variable "token") (:comma ",") (:word "User") (:variable "user") (:comma ",") (:word "bool") (:variable "valid") (:assignment "=") (:word "false") (:comma ",") (:word "\\DateTime") (:variable "creation_time") (:assignment "=") (:word "null"))) (:block (:variable "this") (:object-attrib (:word "token")) (:assignment "=") (:variable "token") (:terminator ";") (:variable "this") (:object-attrib (:word "user")) (:assignment "=") (:variable "user") (:terminator ";") (:variable "this") (:object-attrib (:word "valid")) (:assignment "=") (:variable "valid") (:terminator ";") (:variable "this") (:object-attrib (:word "creation_time")) (:assignment "=") (:variable "creation_time") (:word "new") (:word "\\DateTime") (:list) (:terminator ";")))) (:public (:function (:declaration (:word "function") (:word "getToken") (:list) (:word "string")) (:block (:word "return") (:variable "this") (:object-attrib (:word "token")) (:terminator ";")))) (:public (:function (:declaration (:word "function") (:word "getUser") (:list) (:word "User")) (:block (:word "return") (:variable "this") (:object-attrib (:word "user")) (:terminator ";")))) (:public (:function (:declaration (:word "function") (:word "hasStudentRole") (:list) (:word "bool")) (:block (:word "return") (:variable "this") (:object-attrib (:word "role_student")) (:terminator ";")))) (:public (:function (:declaration (:word "function") (:word "isValid") (:list) (:word "bool")) (:block (:word "return") (:variable "this") (:object-attrib (:word "valid")) (:terminator ";")))) (:public (:function (:declaration (:word "function") (:word "getCreationTime") (:list) (:word "\\DateTime")) (:block (:word "return") (:variable "this") (:object-attrib (:word "creation_time")) (:terminator ";")))) (:doc-block (:return-annotation (:word "DateTime[]"))) (:public (:function (:declaration (:word "function") (:word "arrayReturn") (:list) (:word "array")) (:block (:word "return") (:array (:word "new") (:word "\\DateTime") (:list)) (:terminator ";")))))))) diff --git a/test/fixtures/IndexClass2.eld b/test/fixtures/IndexClass2.eld index 691272b..ce6fcde 100644 --- a/test/fixtures/IndexClass2.eld +++ b/test/fixtures/IndexClass2.eld @@ -1 +1 @@ -(:root (:word "declare") (:list (:word "strict_types") (:assignment "=")) (:terminator ";") (:namespace (:word "App\\Entity") (:terminator ";") (:use (:word "Doctrine\\ORM\\Mapping") (:word "as") (:word "ORM") (:terminator ";")) (:doc-block (:annotation "ORM\\Entity")) (:class (:declaration (:word "class") (:word "AuthToken")) (:block (:private (:class-variable "token") (:terminator ";")) (:private (:class-variable "extra") (:terminator ";")) (:doc-block (:var-annotation (:word "App\\\\Entity\\\\User"))) (:private (:class-variable "user") (:terminator ";")) (:doc-block (:var-annotation (:word "bool"))) (:private (:class-variable "valid") (:terminator ";")) (:doc-block (:var-annotation (:word "\\DateTime"))) (:private (:class-variable "creation_time") (:terminator ";")) (:public (:function (:declaration (:word "function") (:word "__construct") (:list (:word "string") (:variable "token") (:comma ",") (:word "User") (:variable "user") (:comma ",") (:word "bool") (:variable "valid") (:assignment "=") (:word "false") (:comma ",") (:word "\\DateTime") (:variable "creation_time") (:assignment "=") (:word "null"))) (:block (:variable "this") (:object-attrib (:word "token")) (:assignment "=") (:variable "token") (:terminator ";") (:variable "this") (:object-attrib (:word "user")) (:assignment "=") (:variable "user") (:terminator ";") (:variable "this") (:object-attrib (:word "valid")) (:assignment "=") (:variable "valid") (:terminator ";") (:variable "this") (:object-attrib (:word "creation_time")) (:assignment "=") (:variable "creation_time") (:word "new") (:word "\\DateTime") (:list) (:terminator ";")))) (:public (:function (:declaration (:word "function") (:word "getToken") (:list) (:word "bool")) (:block (:word "return") (:variable "this") (:object-attrib (:word "token")) (:terminator ";")))) (:public (:function (:declaration (:word "function") (:word "getUser") (:list) (:word "User")) (:block (:word "return") (:variable "this") (:object-attrib (:word "user")) (:terminator ";")))) (:public (:function (:declaration (:word "function") (:word "hasStudentRole") (:list) (:word "bool")) (:block (:word "return") (:variable "this") (:object-attrib (:word "role_student")) (:terminator ";")))) (:public (:function (:declaration (:word "function") (:word "isValid") (:list) (:word "bool")) (:block (:word "return") (:variable "this") (:object-attrib (:word "valid")) (:terminator ";")))) (:public (:function (:declaration (:word "function") (:word "anAddedFunction") (:list)) (:block (:word "return") (:variable "this") (:object-attrib (:word "extra")) (:terminator ";")))) (:public (:function (:declaration (:word "function") (:word "getCreationTime") (:list) (:word "\\DateTime")) (:block (:word "return") (:variable "this") (:object-attrib (:word "creation_time")) (:terminator ";")))))))) +(:root (:word "declare") (:list (:word "strict_types") (:assignment "=")) (:terminator ";") (:namespace (:word "App\\Entity") (:terminator ";") (:use (:word "Doctrine\\ORM\\Mapping") (:word "as") (:word "ORM") (:terminator ";")) (:doc-block (:annotation "ORM\\Entity")) (:class (:declaration (:word "class") (:word "AuthToken")) (:block (:private (:class-variable "token") (:terminator ";")) (:private (:class-variable "extra") (:terminator ";")) (:doc-block (:var-annotation (:word "App\\\\Entity\\\\User"))) (:private (:class-variable "user") (:terminator ";")) (:doc-block (:var-annotation (:word "bool"))) (:private (:class-variable "valid") (:word "false") (:terminator ";")) (:doc-block (:var-annotation (:word "\\DateTime"))) (:private (:class-variable "creation_time") (:terminator ";")) (:public (:function (:declaration (:word "function") (:word "__construct") (:list (:word "string") (:variable "token") (:comma ",") (:word "User") (:variable "user") (:comma ",") (:word "bool") (:variable "valid") (:assignment "=") (:word "false") (:comma ",") (:word "\\DateTime") (:variable "creation_time") (:assignment "=") (:word "null"))) (:block (:variable "this") (:object-attrib (:word "token")) (:assignment "=") (:variable "token") (:terminator ";") (:variable "this") (:object-attrib (:word "user")) (:assignment "=") (:variable "user") (:terminator ";") (:variable "this") (:object-attrib (:word "valid")) (:assignment "=") (:variable "valid") (:terminator ";") (:variable "this") (:object-attrib (:word "creation_time")) (:assignment "=") (:variable "creation_time") (:word "new") (:word "\\DateTime") (:list) (:terminator ";")))) (:public (:function (:declaration (:word "function") (:word "getToken") (:list) (:word "bool")) (:block (:word "return") (:variable "this") (:object-attrib (:word "token")) (:terminator ";")))) (:public (:function (:declaration (:word "function") (:word "getUser") (:list) (:word "User")) (:block (:word "return") (:variable "this") (:object-attrib (:word "user")) (:terminator ";")))) (:public (:function (:declaration (:word "function") (:word "hasStudentRole") (:list) (:word "bool")) (:block (:word "return") (:variable "this") (:object-attrib (:word "role_student")) (:terminator ";")))) (:public (:function (:declaration (:word "function") (:word "isValid") (:list) (:word "bool")) (:block (:word "return") (:variable "this") (:object-attrib (:word "valid")) (:terminator ";")))) (:public (:function (:declaration (:word "function") (:word "anAddedFunction") (:list)) (:block (:word "return") (:variable "this") (:object-attrib (:word "extra")) (:terminator ";")))) (:public (:function (:declaration (:word "function") (:word "getCreationTime") (:list) (:word "\\DateTime")) (:block (:word "return") (:variable "this") (:object-attrib (:word "creation_time")) (:terminator ";")))))))) diff --git a/test/test-buffer.el b/test/test-buffer.el index 9df7b2a..689e7cb 100644 --- a/test/test-buffer.el +++ b/test/test-buffer.el @@ -491,3 +491,61 @@ class AccountStatisticsController { (:use (:word "Illuminate\\Support\\Facades\\Auth") (:terminator ";"))) (mapcar #'phpinspect-meta-token (phpinspect-splayt-to-list (phpinspect-bmap-imports bmap))))))))) + + +(ert-deftest phpinspect-buffer-index-typehinted-class-variables () + (with-temp-buffer + (let ((buffer (phpinspect-make-buffer + :buffer (current-buffer) + :-project (phpinspect--make-project :autoload (phpinspect-make-autoloader))))) + (insert "