Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Variable configure file #4

Merged
merged 25 commits into from
Apr 15, 2018
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
7bf0f23
composer require --dev phpstan/phpstan:dev-master
zonuexe Apr 11, 2018
ba7dfc8
Add dependency php-mode in development
zonuexe Apr 11, 2018
526f0b7
Add file local variables and getter functions
zonuexe Apr 11, 2018
968b8fe
Consider the existence of "phpstan.neon.dist"
zonuexe Apr 11, 2018
c48251b
Refactor using cl-loop
zonuexe Apr 11, 2018
a5f1711
Modify flycheck checker
zonuexe Apr 11, 2018
45ba779
Rename flycheck checker
zonuexe Apr 11, 2018
bc56031
Add test files
zonuexe Apr 11, 2018
0825ac8
Use config-file instead of configure-file
zonuexe Apr 11, 2018
6fa077b
Remove unused variable
zonuexe Apr 12, 2018
6840f6c
Fix phpstan executable
zonuexe Apr 12, 2018
a198897
Fix pattern of level (accepts 1 digit number)
zonuexe Apr 12, 2018
63fd9eb
Add defgroup phpstan and custom variable phpstan-flycheck-auto-set-ex…
zonuexe Apr 12, 2018
4555592
Modify phpstan-executable specification
zonuexe Apr 12, 2018
d559ee1
Add phpstan-get-config-file-and-set-flycheck-variable as side effect
zonuexe Apr 12, 2018
05c9f76
Add phpstan-normalize-path and support Docker
zonuexe Apr 12, 2018
70cb256
Add 'docker keyword
zonuexe Apr 12, 2018
23d1860
Normalize phpstan-config-file for Docker
zonuexe Apr 12, 2018
dffcad7
Add 'max keyword to phpstan-level
zonuexe Apr 12, 2018
3748533
Add phpstan-working-dir for working directory of PHPStan
zonuexe Apr 12, 2018
4d53e60
Rename root-directory local variable
zonuexe Apr 12, 2018
56355b0
Search phpstan.neon from (phpstan-get-working-dir)
zonuexe Apr 12, 2018
21cf970
Modify doc comment and README
zonuexe Apr 12, 2018
8d63798
Modify phpstan-enabled-and-set-flycheck-variable
zonuexe Apr 13, 2018
0508372
phpstan-get-executable return always executable with arguments
zonuexe Apr 13, 2018
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
/.cask
/composer.lock
/vendor
3 changes: 2 additions & 1 deletion Cask
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@

(package-file "phpstan.el")
(development
(depends-on "flycheck"))
(depends-on "flycheck")
(depends-on "php-mode"))
8 changes: 8 additions & 0 deletions composer.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "emacs-php/phpstan.el",
"description": "Emacs interface to PHPStan",
"license": "GPL-3.0-or-later",
"require-dev": {
"phpstan/phpstan": "dev-master"
}
}
78 changes: 72 additions & 6 deletions phpstan.el
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,85 @@
;; https://github.com/phpstan/phpstan

;;; Code:
(require 'php-project)
(require 'flycheck nil)


;; Variables:

;;;###autoload
(progn
(defvar phpstan-config-file nil)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason of having these as variables instead of flycheck options? The flycheck macro expands to a defcustom so we can still change it manually just as a defvar but it probides good documentation and listing of options for users.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flycheck also provides the flycheck-def-config-file-var macro for this

(make-variable-buffer-local 'phpstan-config-file)
(put 'phpstan-config-file 'safe-local-variable
#'(lambda (v) (if (consp v)
(and (eq 'root (car v)) (stringp (cdr v)))
(null v) (stringp v)))))

;;;###autoload
(progn
(defvar phpstan-level "0")
(make-variable-buffer-local 'phpstan-level)
(put 'phpstan-level 'safe-local-variable
#'(lambda (v) (or (null v)
(integerp v)
(and (stringp v)
(string-match-p "\\`[1-9][0-9]*\\'" v))))))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not match "0", why?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. It fixed.


;;;###autoload
(progn
(defvar phpstan-executable nil)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flycheck defines a variable for this automatically. I guess the question is the same as for the variable above.

(make-variable-buffer-local 'phpstan-executable)
(put 'phpstan-executable 'safe-local-variable
#'(lambda (v) (if (consp v)
(and (eq 'root (car v)) (stringp (cdr v)))
(null v) (stringp v)))))

;; Functions:
(defun phpstan-get-config-file ()
"Return path to phpstan configure file or `NIL'."
(if phpstan-config-file
(if (and (consp phpstan-config-file)
(eq 'root (car phpstan-config-file)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not clear what the structure of phpstan-config-file should be. I think user options should at least be defined as defcustoms with a type annotation.

(expand-file-name (cdr phpstan-config-file) (php-project-get-root-dir))
phpstan-config-file)
(cl-loop for name in '("phpstan.neon" "phpstan.neon.dist")
for file = nil
for dir = (locate-dominating-file default-directory name)
if dir
return (expand-file-name name dir))))

(defun phpstan-get-level ()
"Return path to phpstan configure file or `NIL'."
(cond
((null phpstan-level) "0")
((integerp phpstan-level) (int-to-string phpstan-level))
(t phpstan-level)))

(defun phpstan-get-executable ()
"Return PHPStan excutable file."
(let ((executable (or phpstan-executable '(root . "vendor/bin/phpstan"))))
(when (and (consp executable)
(eq 'root (car executable)))
(setq executable
(expand-file-name (cdr executable) (php-project-get-root-dir))))
(if (file-exists-p executable)
executable
(if (executable-find "phpstan")
"phpstan"
(error "PHPStan executable not found")))))

;;;###autoload
(when (featurep 'flycheck)
(flycheck-define-checker phpstan-checker
(flycheck-define-checker phpstan
"PHP static analyzer based on PHPStan."
:command ("phpstan"
"analyze"
"--no-progress"
"--errorFormat=raw"
:command ("php" (eval (phpstan-get-executable))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since phpstan is generally a PHP script (or Phar archive), this command works fine. Therefore, it is not possible to specify shell scripts that are not PHP or "real" executables.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a feature though? I run phpstan through docker so this would not work for me for example. And there are many people who do so as well: https://github.com/phpstan/docker-image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. This problem was solved with the hack of phpstan-enabled-and-set-flycheck-variable. It is an evil side effect.

"analyze" "--errorFormat=raw" "--no-progress" "--no-interaction"
"-c" (eval (phpstan-get-config-file))
"-l" (eval (phpstan-get-level))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if these options eval to nil, then the flag has no value and it errors out. The (option ..) expansion flycheck uses takes care of that.

source)
:working-directory (lambda (_) (php-project-get-root-dir))
:enabled (lambda () (locate-dominating-file "phpstan.neon" default-directory))
:enabled (lambda () (phpstan-get-config-file))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this still doesn't work if I don't use any configuration file at all.

:error-patterns
((error line-start (1+ (not (any ":"))) ":" line ":" (message) line-end))
:modes (php-mode)
Expand Down
1 change: 1 addition & 0 deletions phpstan.neon.dist
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# dummy
13 changes: 13 additions & 0 deletions test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?php

foo();
f();
foo();

echo Fooo;
echo FOO;

// Local Variables:
// phpstan-config-file: (root . "tests/phpstan.neon")
// phpstan-level: 7
// End:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is not automated, but we can check flycheck's behavior manually.

5 changes: 5 additions & 0 deletions tests/bootstrap.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<?php

const FOO = 'Foo';

function foo() {}
2 changes: 2 additions & 0 deletions tests/phpstan.neon
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
parameters:
bootstrap: %rootDir%/../../../tests/bootstrap.php