Skip to content

add shouldNotTypecheckWith#11

Open
soenkehahn wants to merge 1 commit intoCRogers:masterfrom
soenkehahn:shouldNotTypecheckWith
Open

add shouldNotTypecheckWith#11
soenkehahn wants to merge 1 commit intoCRogers:masterfrom
soenkehahn:shouldNotTypecheckWith

Conversation

@soenkehahn
Copy link
Contributor

@CRogers: Awesome library. What do you think about this addition?

@soenkehahn
Copy link
Contributor Author

The failure on ghc-8 seems to be caused by ghc including the substring as contextual information in the error message. And then of course it'll be in there. Not sure how to address this.

@isovector
Copy link

This is exactly the function I've been looking for. Any chance of getting this merged, @CRogers @soenkehahn ?

@CRogers
Copy link
Owner

CRogers commented Jun 14, 2019

If you can get this to pass CI and compile on a recent GHC version, for sure. I just don't have time to do it myself at the moment.

@isovector
Copy link

Sounds great. I'll get started.

@soenkehahn
Copy link
Contributor Author

Glad someone is interested in this. :)

However, IIRC, back then when I experimented with this branch I had problems with tests that should have failed but passed. The issue was roughly this:

  • You write a test-case that includes shouldNotTypecheckWith "some error message".
  • Your test case has a (unintentional) type error.
  • The error message contains -- as part of the expression that fails the typecheck -- the string literal "some error message".
  • The test passes, because shouldNotTypecheckWith detects that there was a type error containing the string some error message.

I thought that it was pretty bad that your tests can accidentally pass, because you have an unintentional type error in your testing code. Not sure whether there is a good solution to this.

@isovector
Copy link

@soenkehahn I've gotten around this in #15 by just stripping off the context. IMO the primary usecase for this function is to ensure that custom type errors say what you'd like them to, so the context is probably a safe thing to ignore.

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