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

CLI flag to define/override specs #88

Closed
erszcz opened this issue Nov 16, 2018 · 5 comments
Closed

CLI flag to define/override specs #88

erszcz opened this issue Nov 16, 2018 · 5 comments

Comments

@erszcz
Copy link
Collaborator

erszcz commented Nov 16, 2018

Lager is probably the most popular logging framework used in the Erlang ecosystem. However, it's API is somewhat peculiar, because calling it is done with pseudo-calls, i.e. hooks which look like regular function calls, but are actually rewritten by lager_transform parse transformation. This results in, for example, Call to undefined function lager:debug/2 on line 204 messages.

One obvious way of solving this would be for a library using such a parse transformation to export properly typed function stubs. What's the Gradualizer team's stance on that? Do you suggest sending PRs adding suitable stubs e.g. to lager?

@zuiderkwast
Copy link
Collaborator

Hm.. I don't know. Do you check your beam files or your erl files? The beam files (compiled with debug_info) should contain the AST after parse transform. The erl files contain the code before parse transform.

It is possible to inform gradualizer about such functions, or override specs with other specs, by loading a module with spec declarations such as -spec lager:debug(_, _) -> ok. using the gradualizer_db functions import_module/1, import_erl_files/1 and import_beam_files/1. These are internal but you can use them to check if it solves the problem. We should add an option to include such files for the cli command and for the gradualizer:type_check_* functions where options are used.

@erszcz
Copy link
Collaborator Author

erszcz commented Dec 4, 2018

I've finally had time to check this. Indeed, typechecking .beam files works without an issue:

$ cat src/lager_tracing.erl
-module(lager_tracing).

-compile([export_all]).

-include_lib("lager/include/lager.hrl").

log_event_with_tag() ->
    lager:warning([{tag, true}], "warning with tag ~p", ["{tag, true}"]).

log_event_no_tag() ->
    lager:warning("warning without tags", []).

$ gradualizer src/lager_tracing.erl
src/lager_tracing.erl: Call to undefined function lager:warning/3 on line 8
src/lager_tracing.erl: Call to undefined function lager:warning/2 on line 11

$ gradualizer -pa _build/default/lib/lager/ebin _build/default/lib/lager_tracing/ebin/lager_tracing.beam
$

So does importing spec stubs with gradualizer_db.

I think the CLI flag would be a huge plus. Thanks for the response. I'll leave this issue open for you to decide if it's a good reminder for the flag feature or should just be closed.

@zuiderkwast
Copy link
Collaborator

Thanks for verifying this!

@josefs
Copy link
Owner

josefs commented Dec 5, 2018

I think it would make sense to have a CLI flag in the long term. But given that we have a work-around that works well I don't consider it a high priority.

@zuiderkwast zuiderkwast changed the title Typing parse tranformation hooks (lager) CLI flag to define/override specs Dec 6, 2018
@erszcz
Copy link
Collaborator Author

erszcz commented Nov 13, 2019

For the record, it's been a while since I created this issue and it turns out the original problem - false warnings about lager - is now easily solvable.

The --specs_override_dir fits the purpose of overriding specs. Used together with the following lager.specs.erl file, the Lager warnings are gone:

-module(lager).

-spec lager:debug(string(), list()) -> ok.
-spec lager:info(string(), list()) -> ok.
-spec lager:warning(string(), list()) -> ok.
-spec lager:error(string(), list()) -> ok.
-spec lager:critical(string(), list()) -> ok.

Since I use Vim, I made the Neomake gradualizer maker (https://github.com/erszcz/neomake/tree/erlang-gradualizer-wip @ 285c6232) use --specs_override_dir. Coupled together with a project-local .vimrc:

let g:neomake_erlang_gradualizer_specs_override_dir = '/Users/erszcz/work/my_project/priv/gradualizer_overrides'

the warnings are gone for the whole project. Works great!

I'm closing the issue, since there's a CLI flag now, and the original problem is solved.

@erszcz erszcz closed this as completed Nov 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants