emacs-elsa / Elsa

Emacs Lisp Static Analyzer and gradual type system.
GNU General Public License v3.0
644 stars 27 forks source link

Elsa doesn't like it when I (require 's) #159

Closed DarwinAwardWinner closed 5 years ago

DarwinAwardWinner commented 5 years ago

I'm trying to add elsa checking to my package ido-completing-read+. However, it doesn't seem to run successfully on my code. I've set up a branch here: https://github.com/DarwinAwardWinner/ido-completing-read-plus/tree/elsa. You should be able to clone, cask install, cask exec elsa ... on that.

When I run cask exec elsa ido-completing-read+.el, I get:

$ cask exec elsa ido-completing-read+.el
analyzer: updating variable ido-completing-read+-version, old type nil
Other must be ‘elsa-type-child-p’

and the exit code is 255. Flycheck-elsa gives the corresponding warning message:

Suspicious state from syntax checker emacs-lisp-elsa: Flycheck checker emacs-lisp-elsa returned non-zero exit code 255, but its output contained no errors: analyzer: updating variable ido-completing-read+-version, old type nil
Other must be ‘elsa-type-child-p’

Try installing a more recent version of emacs-lisp-elsa, and please open a bug report if the issue persists in the latest release.  Thanks!

However, if I create a file called temp.el containing the obvious error (+ 1 "oops"), elsa runs just fine on it, reporting the error as expected (and flycheck-elsa works as well):

$ cask exec elsa temp.el
temp.el:1:1:error:Argument 2 accepts type Number | Marker but received Const "oops"
DarwinAwardWinner commented 5 years ago

Upon further experimentation, the offending line of code seems to be (require 's). Putting just that line in a file by itself causes the same error.

Fuco1 commented 5 years ago

Thanks for narrowing it down. There seems to be some issue when Elsa is byte-compiled and tries to follow requires (does not actually load the file, only inspects it for function declarations). There is a caching mechanism and it broke one some releases of Emacs. I've been quite unsuccessful in tracking it down.

Can you try to remove all the .elc files from Elsa package and try again?

Also this is probably an Elsa issue, not flycheck. I will transfer the issue once you acknowledge so it doesn't disappear for you.

DarwinAwardWinner commented 5 years ago

After deleting all of Elsa's elc files, I'm seeing no change in behavior.

Fuco1 commented 5 years ago

@DarwinAwardWinner What's your Emacs version? Any other environment info that might be relevant?

DarwinAwardWinner commented 5 years ago

GNU Emacs 26.2 (build 1, x86_64-apple-darwin18.2.0, NS appkit-1671.20 Version 10.14.3 (Build 18D109)) of 2019-04-12

Installed via Homebrew on Mac OS 10.14.5

I don't think my Emacs config matters since cask doesn't load it.

DarwinAwardWinner commented 5 years ago

Is there anything else I can to do debug this? I've tried deleting every elc file in in the package's directory, including every elc file in .cask, and I get the same error.

Fuco1 commented 5 years ago

@DarwinAwardWinner I think not, except just digging into the code and finding the problem. Which admittedly might take disproportionate amount of your time given your probable unfamiliarity with it.

I very much want to push this project forward, but finding the time is very difficult for me now :( This is however one of the first issues to work on, since it's pretty fundamentally broken.

DarwinAwardWinner commented 5 years ago

For what it's worth, the issue doesn't seem to be specific to the (require 's) line. I removed the dependency of my package on s (it was only using 2 functions from it, both of which were essentially aliases for builtins), and it still has the same error for a different line. Figuring out which line would probably be harder this time, and I'm not sure it's worth the effort given what you've said here, since the problem presumably isn't with that line anyway.

Fuco1 commented 5 years ago

@DarwinAwardWinner Can you just for completeness paste the offending file to a gist or add a permalink here (such that the content of it won't change) so I can narrow it down later.

DarwinAwardWinner commented 5 years ago

Like I said, just (require 's) by itself in a file is sufficient to trigger the error.

DarwinAwardWinner commented 5 years ago

Anyway, if you want to run it on my full code, you can clone the elsa branch here: https://github.com/DarwinAwardWinner/ido-completing-read-plus/tree/elsa

The current commit is f858690b18e630ab01216c24701817b3e5fe5145. Then run cask install, and cask exec elsa ido-completing-read+.el. For getting a full error backtrace, I've been running

cask exec elsa -l debug -f toggle-debug-on-error -l cl-print --eval "(setq debugger-batch-max-lines 1000)" ido-completing-read+.el

For some reason -l debug and -l cl-print are required or else it throws errors about being unable to load those files.

Alexander-Miller commented 5 years ago

just (require 's) by itself in a file is sufficient to trigger the error.

So is (require 'f) and (require 'dash).

Fuco1 commented 5 years ago

This was a classic XY problem. Turns out the analyzer didn't like a specific combination of arguments where the thing matched-against was a mixed type.

The problem actually occured when analyzing the content of s.el or any other library which contains a setq.

DarwinAwardWinner commented 5 years ago

I can confirm that elsa now runs on my code. Thanks!

contrapunctus-1 commented 5 years ago

Running into the same issue with Elsa v20190825.1513 (MELPA), both with my init (the warning in Messages comes up upon opening any Elisp buffer, even when creating a new blank buffer and enabling Emacs Lisp mode)...

...and when running cask exec elsa chronometrist.el with my Elisp program (which doesn't have any Elsa declarations yet; permalink to commit) -

Other must be ‘elsa-type-child-p’

GNU Emacs 26.1 (build 2, i686-pc-linux-gnu, GTK+ Version 3.24.4) of 2019-02-04, modified by Debian

Fuco1 commented 5 years ago

@contrapunctus-1 Thanks for the report, I'm on vacation now and coming back next wednesday, will have a look. I'm going to re-open as it seems to be a similar issue.

Fuco1 commented 5 years ago

@contrapunctus-1 I tried to run it on the commit you provided and it went through. This is the report. We should probably open another issue if you still can't run it.

chronometrist.el:63:25:error:Function `gethash' expects at least 2 arguments but received 1
chronometrist.el:64:25:error:Function `last' expects at least 1 argument but received 0
chronometrist.el:65:25:error:Function `car' expects at least 1 argument but received 0
chronometrist.el:83:16:error:Reference to free variable `string-lessp'.
chronometrist.el:83:8:error:Function `-sort' expects at least 2 arguments but received 1
chronometrist.el:86:44:error:Reference to free variable `it-index'.
chronometrist.el:86:41:error:Argument 1 accepts type Number | Marker but received Unbound
chronometrist.el:84:8:error:Function `--map-indexed' expects at least 2 arguments but received 1
chronometrist.el:111:21:error:Reference to free variable `timeclock-last-project'.
chronometrist.el:122:29:error:Reference to free variable `chronometrist-mode-map'.
chronometrist.el:121:12:error:Argument 2 accepts type Mixed but received Unbound
chronometrist.el:132:47:error:Reference to free variable `chronometrist-mode-map'.
chronometrist.el:131:30:error:Argument 2 accepts type Mixed but received Unbound
chronometrist.el:134:47:error:Reference to free variable `chronometrist-mode-map'.
chronometrist.el:133:30:error:Argument 2 accepts type Mixed but received Unbound
chronometrist.el:141:8:error:Argument 1 accepts type String | Int but received Mixed
chronometrist.el:149:36:error:Reference to free variable `chronometrist-add-new-project-button'.
chronometrist.el:165:36:error:Reference to free variable `chronometrist-report'.
chronometrist.el:170:36:error:Reference to free variable `chronometrist-open-file'.
chronometrist.el:221:38:error:Variable save-next expects (), got Nil
chronometrist.el:216:32:warning:Condition always evaluates to nil.
chronometrist.el:228:10:error:Function `reverse' expects at least 1 argument but received 0
chronometrist.el:242:66:error:Reference to free variable `timeclock-last-project'.
chronometrist.el:242:40:error:Argument 1 accepts type Mixed but received Unbound
chronometrist.el:298:36:error:Reference to free variable `chronometrist-toggle-project'.
chronometrist.el:299:36:error:Reference to free variable `chronometrist-toggle-project-no-reason'.
chronometrist.el:300:36:error:Reference to free variable `chronometrist-open-file'.
chronometrist.el:301:36:error:Reference to free variable `chronometrist-report'.
chronometrist.el:302:36:error:Reference to free variable `chronometrist-toggle-project'.
chronometrist.el:303:36:error:Reference to free variable `chronometrist-toggle-project-no-reason'.
chronometrist.el:304:36:error:Reference to free variable `chronometrist-add-new-project'.
chronometrist.el:308:21:error:Reference to free variable `chronometrist-mode'.
chronometrist.el:308:40:error:Reference to free variable `tabulated-list-mode'.
chronometrist.el:312:8:warning:Assigning to free variable tabulated-list-format
chronometrist.el:317:8:warning:Assigning to free variable tabulated-list-entries
chronometrist.el:319:8:warning:Assigning to free variable tabulated-list-sort-key
chronometrist.el:321:33:error:Reference to free variable `chronometrist-refresh'.
chronometrist.el:321:8:warning:Assigning to free variable revert-buffer-function
chronometrist.el:322:40:error:Reference to free variable `chronometrist-ask-for-reason'.
chronometrist.el:322:8:warning:Assigning to free variable timeclock-get-reason-function
chronometrist.el:429:49:error:Reference to free variable `chronometrist-add-new-project-button'.
chronometrist.el:436:27:warning:Assigning to free variable cursor-type
chronometrist.el:434:25:warning:Condition always evaluates to nil.
chronometrist.el:442:23:warning:Condition always evaluates to nil.
chronometrist.el:449:43:error:Reference to free variable `chronometrist-refresh-file'.
chronometrist.el:445:18:warning:Condition always evaluates to nil.