Warn unknown atom #1265
Warn unknown atom #1265
Conversation
richcarl
force-pushed
the
warn-unknown-atom
branch
from
8ef2af7
to
ec70847
Compare
|
Did you notice "All checks have failed"? Not sure why, but your changes in erl_lint breaks dialyzer. |
|
It may well be that this is a Travis hiccup. Perhaps it's a good idea for somebody (with proper authority) to press the |
|
No, one of my changes triggered an error for recursively defined records. Fixing... |
Also need to ensure that recursive record definitions are accepted.
richcarl
force-pushed
the
warn-unknown-atom
branch
from
ec70847
to
895bf55
Compare
|
Had to make some larger changes to how record fields are processed. Might interest Kostis. |
|
Kostis has recently made sure that dialyzer is enabled on Travis to catch even logical errors such as the one that the test failure currently indicates. (I believe that the More interesting to commenting on the code, to me at least, would be to see the effect that this change has to various Erlang/OTP files when enabled. I would be willing to take it for a test drive to HiPE and Dialyzer (but also to PropEr and CutEr) files once this gets merged to the repository. Hope this happens soon. |
This option is off by default. If enabled, a warning will be generated for
each atom in a module that does not occur in some type declaration; for
example -type foobar() :: foo | {bar, any()} declares atoms foo and bar.
The compiler knows about standard atoms that occur in most Erlang/OTP
modules, such as `true', `false', `ok', `undefined', etc.
richcarl
force-pushed
the
warn-unknown-atom
branch
from
895bf55
to
48c3fc5
Compare
| @@ -112,6 +112,10 @@ value_option(Flag, Default, On, OnVal, Off, OffVal, Opts) -> | |||
| on_load=[] :: [fa()], %On-load function | |||
| on_load_line=erl_anno:new(0) %Line for on_load | |||
| :: erl_anno:anno(), | |||
| atoms=undefined %Declared atoms | |||
| :: undefined | |||
There was a problem hiding this comment.
Please do not do that. It's fundamentally not OK to mix an opaque type with a structured one because it requires that you pattern match on a structure to distinguish between them (which breaks the opacity of the opaque type). It's probably the case that the current version of dialyzer does not disallow this, but it's quite likely that a future version will. It's much better that the code is ready for that time.
|
For the same reason as I don't think checking of remote calls should be done in the linter I don't think this check should be in the linter even if I suggested that in an old post. A possible step in the direction where checking like this could span over module borders would be an introduction of a "compile_application" function which takes all modules in an application an handles them together possibly producing an archive as output and maybe as input as well. |
|
@KennethL Glad to note that you see where I'm going with this. :-) |
Reworked an old suggestion of mine to have an option warn_unknown_atom, which can be useful for avoiding misspelled atoms, or just ensuring all used atoms are documented (such as options/flags, or tagged tuples). This version uses Kenneth's suggestion to rely on the -type specifications rather than adding a new kind of declaration. See http://erlang.org/pipermail/erlang-questions/2012-February/064117.html for the discussion.