Skip to content

[Super errors] Tweak missing param msg #2548

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 28, 2018
Merged

[Super errors] Tweak missing param msg #2548

merged 1 commit into from
Feb 28, 2018

Conversation

chenglou
Copy link
Member

@chenglou chenglou commented Feb 27, 2018 •
edited
Loading

No description provided.

@chenglou chenglou requested a review from cristianoc February 27, 2018 11:10
@chenglou
Copy link
Member Author

Before:
screenshot 2018-02-27 03 10 14

After:
screenshot 2018-02-27 03 08 59

@chenglou
Copy link
Member Author

chenglou commented Feb 27, 2018 •
edited
Loading

@cristianoc doesn't work if it's like this:

let a: int => option(string) = Some("hi");

This still gives:

  This has type:
    option('a)
  But somewhere wanted:
    int => option(string)

Does this mean we need to write our own equal comparator to compare between option('a) and option(string)? Is that even correct? (cc @jaredly)

Edit: we're gonna bail on this case.

@chenglou
Copy link
Member Author

^ Will fix this in another issue

@chenglou chenglou merged commit c5adfc6 into rescript-lang:master Feb 28, 2018
@chenglou chenglou deleted the heyhey branch February 28, 2018 00:15
chenglou added a commit to chenglou/rescript-compiler that referenced this pull request Feb 28, 2018
@chenglou
[Super errors] Tweak collect_missing_arguments equality check
56bff4c 
See for example rescript-lang#2548 (comment)

So `option('a)` is the same as `option('b)`, `array(string)` is the same
as `array(string)`, but `list('a)` isn't the same as `list(string)`.

Along with rescript-lang#2548, this fixes
reasonml-community/error-message-improvement#10
I believe
@chenglou chenglou mentioned this pull request Feb 28, 2018
Merged
chenglou added a commit that referenced this pull request Feb 28, 2018
@chenglou
[Super errors] Tweak collect_missing_arguments equality check (#2549)
12e10f3 
See for example #2548 (comment)

So `option('a)` is the same as `option('b)`, `array(string)` is the same
as `array(string)`, but `list('a)` isn't the same as `list(string)`.

Along with #2548, this fixes
reasonml-community/error-message-improvement#10
I believe
chenglou added a commit to chenglou/rescript-compiler that referenced this pull request Feb 28, 2018
@chenglou
[Super errors] Better missing param msg
eafdec1 
Continuation of rescript-lang#2548

Mini usability testing result out. Apparently the previous message was too confusing, so I'm adding back the fallback type clash msg.

Additionally, I'm now using `Ctype.matches`, which considers `option('a)` and `option(string)` as equal. This, along with the original code in rescript-lang#2548, fixes reasonml-community/error-message-improvement#14

But since this produces false positives "missing parameters" msg for such code below:

```reason
let a: int => float => option(int) = (a) => Some("");
```

Where `option(int)` matches `option('a)` inferred from `Some("")`, I've also tweaked the msg to indicate that we're _guessing_ that it _might_ be because of missing parameters (but that we're not sure). The fallback type clash msg gives us leeway to guess.

cc @cristianoc
@chenglou chenglou mentioned this pull request Feb 28, 2018
Merged
chenglou added a commit to chenglou/rescript-compiler that referenced this pull request Feb 28, 2018
@chenglou
[Super errors] Better missing param msg
9edc9b3 
Continuation of rescript-lang#2548

Mini usability testing result out. Apparently the previous message was too confusing, so I'm adding back the fallback type clash msg.

Additionally, I'm now using `Ctype.matches`, which considers `option('a)` and `option(string)` as equal. This, along with the original code in rescript-lang#2548, fixes reasonml-community/error-message-improvement#14

But since this produces false positives "missing parameters" msg for such code below:

```reason
let a: int => float => option(int) = (a) => Some("");
```

Where `option(int)` matches `option('a)` inferred from `Some("")`, I've also tweaked the msg to indicate that we're _guessing_ that it _might_ be because of missing parameters (but that we're not sure). The fallback type clash msg gives us leeway to guess.

cc @cristianoc
chenglou added a commit that referenced this pull request Feb 28, 2018
@chenglou
[Super errors] Better missing param msg (#2554)
08426fa 
* [Super errors] Better missing param msg

Continuation of #2548

Mini usability testing result out. Apparently the previous message was too confusing, so I'm adding back the fallback type clash msg.

Additionally, I'm now using `Ctype.matches`, which considers `option('a)` and `option(string)` as equal. This, along with the original code in #2548, fixes reasonml-community/error-message-improvement#14

But since this produces false positives "missing parameters" msg for such code below:

```reason
let a: int => float => option(int) = (a) => Some("");
```

Where `option(int)` matches `option('a)` inferred from `Some("")`, I've also tweaked the msg to indicate that we're _guessing_ that it _might_ be because of missing parameters (but that we're not sure). The fallback type clash msg gives us leeway to guess.

cc @cristianoc

* Clean up code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cristianoc cristianoc Awaiting requested review from cristianoc

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@chenglou