Skip to content

Conversation

@ericmj
Copy link
Member

@ericmj ericmj commented Sep 12, 2020

No longer needs to compile modules to test type warnings. Reduces type tests execution time from 1.0s to 0.6s.

@ericmj ericmj requested a review from josevalim September 12, 2020 16:19
No longer needs to compile modules to test type warnings. Reduces type tests execution time from 1.0s to 0.6s.
@ericmj ericmj force-pushed the emj/move-type-warnings branch from e6ef0f2 to 023db08 Compare September 12, 2020 16:23
expected one of the following fields: foo
where "map" was given the type map() in:
Copy link
Member Author

Choose a reason for hiding this comment

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

How come we show map() instead of %{foo: integer()} here? Since it failed because of map.bar we should show the actual type instead of simplifying to map() which is a compatible type.

where "foo" was given the type integer() in:
# types_test.ex:1
<<foo::integer()>>
Copy link
Member Author

Choose a reason for hiding this comment

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

How come we print as <<foo::integer()>> instead of <<foo::integer>>?

Copy link
Member

Choose a reason for hiding this comment

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

We need to fix Macro.to_string to skip parens on the right side of :: inside <<>> for the built-in identifiers.

Copy link
Member

@josevalim josevalim left a comment

Choose a reason for hiding this comment

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

LGTM! We should move some of the function call warnings too but we can do it next.

@ericmj ericmj merged commit a59fe07 into master Sep 13, 2020
@ericmj ericmj deleted the emj/move-type-warnings branch September 13, 2020 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants