Skip to content

Conversation

@ymtszw
Copy link
Contributor

@ymtszw ymtszw commented Jan 22, 2018

It looks like %{} stands for only "empty" maps in typespecs (whereas it can match for "any" maps on pattern matches).

So, currently codes like below are warned by dialyzer:

  defp ensure_admin_domain(%OAuth2.AccessToken{other_params: %{"id_token" => id_token}}) do
    # Parse id_token of OpenID Connect, check authorizing user's domain
    # ...
  end
  defp ensure_admin_domain(otherwise) do
    {:error, :id_token_not_found}
  end
$ mix dialyze # Using https://github.com/fishcakez/dialyze
Finding applications for analysis
Finding suitable PLTs
Looking up modules in dialyze_erlang-20.0_elixir-1.5.3_deps-dev.plt
Finding applications for dialyze_erlang-20.0_elixir-1.5.3_deps-dev.plt
Finding modules for dialyze_erlang-20.0_elixir-1.5.3_deps-dev.plt
Checking 1725 modules in dialyze_erlang-20.0_elixir-1.5.3_deps-dev.plt
Finding modules for analysis
Analysing 33 modules with dialyze_erlang-20.0_elixir-1.5.3_deps-dev.plt
web/repo/admin_token.ex:50: The pattern #{'other_params':=#{#{#<105>(8, 1, 'integer', ['unsigned', 'big']), #<100>(8, 1, 'integer', ['unsigned', 'big']), #<95>(8, 1, 'integer', ['unsigned', 'big']), #<116>(8, 1, 'integer', ['unsigned', 'big']), #<111>(8, 1, 'integer', ['unsigned', 'big']), #<107>(8, 1, 'integer', ['unsigned', 'big']), #<101>(8, 1, 'integer', ['unsigned', 'big']), #<110>(8, 1, 'integer', ['unsigned', 'big'])}#:=_id_token@1}, '__struct__':='Elixir.OAuth2.AccessToken'} can never match the type #{'__struct__':='Elixir.OAuth2.AccessToken', 'access_token':=binary(), 'expires_at':=integer(), 'other_params':=#{}, 'refresh_token':='nil' | binary(), 'token_type':=binary()}

** (Mix) Dialyzer reported 1 warnings

This PR fixes other_params's typespec so that related codes will be correctly success-typed.

Also fixing body type as well, though it looks like body type is currently unused so it may well be removed. I can do that in this branch too, if requested.

And thanks for developing/maintaining this library! Looking for your reviews ❤️

@coveralls
Copy link

coveralls commented Jan 22, 2018

Coverage Status

Coverage increased (+1.0%) to 94.581% when pulling ee93b95 on ymtszw:fix_incorrect_type into f2362ed on scrogson:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+1.0%) to 94.581% when pulling ee93b95 on ymtszw:fix_incorrect_type into f2362ed on scrogson:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.0%) to 94.581% when pulling ee93b95 on ymtszw:fix_incorrect_type into f2362ed on scrogson:master.

Copy link
Member

@scrogson scrogson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

@scrogson scrogson merged commit 4a1b244 into ueberauth:master Feb 5, 2018
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

Successfully merging this pull request may close these issues.

3 participants