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

Implement type inferences from guard expressions #239

Merged

Conversation

Goose97
Copy link
Contributor

@Goose97 Goose97 commented Jul 15, 2023

Potential improvements for future:

  1. Better handle and operator in guards: current implementation picks one and discards the other
  2. Better handle or operator in guards: current implementation discards both sides. We can represent them as union types. In real use cases, this should happen rarely, so I'll leave it like this for now

Closes #204

Potential improvements for future:
1. Better handle `and` operator in guards: current implementation picks one and discards the other
2. Better handle `or` operator in guards: current implementation discards both sides. We can represent them as union types. In real use cases, this should happen rarely, so I'll leave it like this for now

Closes elixir-lsp#204
@Goose97 Goose97 force-pushed the feature/type-inferences-from-guard branch from 952f837 to 3fad3c5 Compare July 15, 2023 12:13

test "list guards" do
assert %VarInfo{name: :x, type: :list} = var_with_guards("is_list(x)")
assert %VarInfo{name: :x, type: :list} = var_with_guards("hd(x) == 1")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it be easy to get {:list, :integer} here? We'd need to parse {:==, _, _} expression

Copy link
Contributor Author

@Goose97 Goose97 Aug 30, 2023

Choose a reason for hiding this comment

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

I'm not sure if we should do this. List can have different type values, so situations like this can provide misleading type information:

list = [1, :atom, "binary"]
hd(list) == 1
# infer type of list as {:list, :integer} -> incorrect

Copy link
Collaborator

Choose a reason for hiding this comment

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

I made the assumption elsewhere just to simplify things. The type system here is used mostly for completions and I never aimed for perfectly expressing every type possible

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for explaining this. Fixed in fe7b2be


test "tuple guards" do
assert %VarInfo{name: :x, type: :tuple} = var_with_guards("is_tuple(x)")
assert %VarInfo{name: :x, type: :tuple} = var_with_guards("tuple_size(x) == 1")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Similarly with list example above it would be nice to get {:tuple, 1, [nil]} here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in fe7b2be

For unknown type tuple (i.e. when tuple_size(x) == 2), the type will be:

{:tuple, <size>, []}

@lukaszsamson
Copy link
Collaborator

Nice work @Goose97 and sorry for the delay with review. Can you address the comments?

@Goose97 Goose97 force-pushed the feature/type-inferences-from-guard branch from de4e73d to ea32947 Compare August 30, 2023 14:17
@Goose97 Goose97 force-pushed the feature/type-inferences-from-guard branch from 7287a49 to 11428b9 Compare August 30, 2023 15:18
@Goose97 Goose97 force-pushed the feature/type-inferences-from-guard branch from e266041 to 5ef6393 Compare September 1, 2023 01:24
@Goose97
Copy link
Contributor Author

Goose97 commented Oct 31, 2023

Sorry for the delay. I've complete forgot about this PR 🙇

The logic get pretty long so I extracted it into another module 6b4c0d0

@lukaszsamson lukaszsamson merged commit 5583feb into elixir-lsp:master Nov 27, 2023
23 checks passed
@lukaszsamson
Copy link
Collaborator

Awesome @Goose97

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.

Use guards when determining types of variables
2 participants