Open dbarnett opened 9 years ago
I actually wrote a very simple version of this (throws
propagation with call-graph analysis) for google/vim-maktaba#110 (see corp CL 83011828 for generators and output). Some examples of the problems I ran into:
maktaba#error#InvalidArguments()
calls maktaba#error#Exception()
, which will throw WrongType
or BadValue
if the exception name is invalid. However, maktaba#error#InvalidArguments()
uses a constant string literal, and so those are impossible.autoload/plugin.vim
's s:ApplySettings()
calls maktaba#error#Split()
, which will throw BadValue
if the exception is of the wrong type. However, s:ApplySettings()
only catches certain types of exception in the first place, so this is impossible.maktaba#command#GetOutput()
calls maktaba#ensure#IsTrue()
with something that's known to be a boolean value, making the WrongType
exception impossible. More interestingly, though, is that maktaba#ensure#IsTrue()
will raise Failure
if the condition is not met, and yet in this case it doesn't make sense for maktaba#command#GetOutput()
to propagate Failure
to the callers, because, while it's possible that it'll be thrown, it's being used to check an assertion about Vim's behaviour itself, and so callers shouldn't try to catch it (or even really document it).maktaba#flags#Callback()
calls maktaba#function#Apply()
, which can throw BadValue
and WrongType
if the argument isn't a funcdict; however, the collection that maktaba#flags#Callback()
iterates was populated by maktaba#flags#AddCallback()
, which has already performed this validation.In my implementation, I avoided this by manually annotating callgraph edges to remove exceptions that I verified were handled or impossible (i.e. "The call edge from maktaba#error#InvalidArguments()
to maktaba#error#Exception()
should not propagate WrongType
or BadValue
".) This seemed to be good enough for that use case.
Another thing to note on the original callgraph production is that Vimscript is hard to do static analysis on: things like funcdict calls are tricky to resolve, of course (and I didn't even attempt to do so), but also constructs like call map(l:items, 'maktaba#list#RemoveAll(l:Target, v:val)')
, where the function name is in a string.
I avoided this by adopting a very simple approach: I extracted all the function names (the strings that appear after function!
— and I was lucky that in Maktaba's case the script-local function names are also unique), and then simply looked for occurrences of those strings (as "\b<FUNCTION NAME>\b
") in the non-comment text of a calling function. However, this did cause some problems, so I did have to do some manual editing to the resulting graph: for example, maktaba#flags#Create()
doesn't actually call any methods, it just sets up a funcdict with references to them.
For vimdoc, being able to warn (and also suppress warnings for) direct undocumented throws
statements certainly sounds reasonable, but given the above, I'd suggest that building a general callgraph might be too much work (for both vimdoc and for the user running it) just for this feature.
Wow, I knew it would be tricky, but hadn't considered some of these bright red gotchas.
Particularly the cases where we already know an arg value is safe, to always avoid false positives we'd need a theorem prover and a complete programmatic model of the entire vimscript implementation (e.g., to verify that a literal string value passes a certain regex used in the implementation).
Another possibility is to have a standalone vimdoc lint command to detect possible issues. But probably nobody would use it besides (occasionally) us.
At any rate, could you attach your script here for the curious?
Vimdoc should detect undeclared thrown errors and either error out of doc generation or automatically document them. This would include explicit
throw
invocations and/or errors thrown by detected function calls, but not any caught errors.Note: There will be a lot of corner cases and we don't have legitimate static analysis, so we'd want to be careful about annoying false positives.