otp icon indicating copy to clipboard operation
otp copied to clipboard

Dialyzer: Report opaqueness violation, but don't assume no return

Open michallepicki opened this issue 4 years ago • 2 comments

Currently whenever a opaqueness type violation happens, e.g. when constructing an opaque function argument through a literal instead of a dedicated function, this function is assumed to fail. Then, this infamous no_return warning proceeds to propagate up to all calling functions, resulting in no_return warnings being reported in many places. This happens even when the constructed function argument adheres to the internal opaque type structure. I believe the same thing happens when pattern matching on the structure of variables of the opaque type.

This may not seem like a problem when owning all the code of a project. It becomes more frustrating when opaqueness violation happens in a dependency (e.g. a library from hex), when even after heavy testing, the clearly working code keeps reporting no_return.

This also happens in more situations in a language with arbitrary compile-time code evaluation, like Elixir, where even despite respecting opaqueness rules, opaqueness violation is reported. Consider a module that constructs a value of opaque type (from another module) at compile time, and bakes it into a module attribute. Because Dialyzer has no way of knowing where the value of that module attribute came from, whenever it is used, it reports an opaqueness mismatch resulting in propagating no_return warnings.

I would propose that the opaqueness violation warnings should be reported, but if the value (or pattern) is compatible with the opaque type they should not result in functions being assumed to fail. (In other words: I propose that opaqueness rules should not affect regular Dialyzer analysis in that regard)

Screenshot using Erlang/OTP 24.1.7, Elixir 1.13.0-otp-24, VSCode with Elixir-LS 0.9.0 . image Opaqueness violations are reported at line 20 and 26. "No local return" is reported at lines 25 and 29, even though the code will work at run-time.

michallepicki avatar Dec 09 '21 09:12 michallepicki

I like how this suggestion will make it so there are fewer unhelpful "cascading" no_returns. Along those lines, something similar could be done for illegal record construction, which doesn't need to cause cascading errors. Here's a proof of concept: https://github.com/mheiber/otp/pull/2

mheiber avatar Dec 09 '21 11:12 mheiber

Thanks for bringing this up! We don't think we'll have the time to have a deep look at it before OTP 25, so we'll try to do it for OTP 26 instead.

jhogberg avatar Dec 13 '21 14:12 jhogberg

Looks like this is how Dialyzer will behave on OTP 28 🎉

michallepicki avatar Dec 03 '24 18:12 michallepicki