Better error message when specializing generic abstract type with unit

[smoothdeveloper]

Attempt to fix #35

I'll need some guidance in terms of where to add tests and of course if there is anything wrong with the matching I'm doing / assumptions I'm taking.

I'm concerned the matching might yield false positive if there are cases where using unit as a generic type argument would be valid, but this is difficult for me to reason about as of now.

Also looking for better ideas for the error message.

[forki]

regarding tests: tests\fsharpqa\Source\Warnings\env.lst is now a dumping ground for tests that check error messages. I think it would be ok to add it there

[forki]

most similar pattern matches call striptypEqns - maybe @dsyme can tell if it's needed here

[smoothdeveloper]

thanks @forki, I'll pay a look and try to add few tests supposed to trigger that new message.

[smoothdeveloper]

Mmmh reading details @dsyme left in the ancient conversation and my proposed fix is not really handling things at the level he was intending the changes to happen but later down the road, although it looks simpler (if that doesn't raise false positives), maybe it doesn't follow the normal conventions for deciding of errors, but there are few other conditionals to decide which error message to raise where I've added the logic.

To add a better error message, the logical place to put it would be here: https://github.com/fsharp/fsharp/blob/6ac6318858da25fe4f1d4728901c5824e9f3bf3a/src/fsharp/typrelns.fs#L1191

"IsExactMatch" would need to be duplicated and adjusted to a "IsExactMatchUpToInstantiatedUnitReturnType". If an override exists satisfying that predicate, then a better error message can then be given, much as for code already present to give a better error message when there is an override satisfying IsPartialMatch or IsNameMatch.

I'm trying to see if other cases would trigger the initial / new error messages, this compiles fine with and without the change:

type Foo<'t> =
  abstract member Bar : 't -> int

type Bar() =
  interface Foo<unit> with
    member x.Bar _ = 1

I'll look if fsharpqa also has tests checking no error / warning shows up to add that case.

If anyone knows of additional cases I should account for, that would be great.

[enricosada]

it's not an error (this sintax is supported), but can you pls write that as xml like (supported too)?

//<Expects status="success"></Expects>

i am trying to convert the suite to nunit, and to simplify test spec i'll need to convert that as xml like later (easier to parse, validate)

[smoothdeveloper]

@KevinRansom or @dsyme could we get a review on proposed fix and some direction for the error message?

[KevinRansom]

@dsyme
This looks good to me, but I may be missing something, could you take a look please.

Kevin

[KevinRansom]

Thank you for taking care of this

Kevin