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

Ability to inspect when code doesn't type-check #84

Open
unode opened this issue Aug 24, 2017 · 9 comments
Open

Ability to inspect when code doesn't type-check #84

unode opened this issue Aug 24, 2017 · 9 comments
Labels
better-docs This issue reports expected behavior, but the documentation and/or error messaging could be better bug

Comments

@unode
Copy link

unode commented Aug 24, 2017

The ability to inspect types and info about any function or variable relies on code compiling successfully.
The plugin is smart enough to keep previous information about types but often it leads to misleading or incomplete information when code moves up/down in the file.

ghc includes -fdefer-type-errors and -fdefer-typed-holes (and possibly others) which allow for a successful compilation which fails type-checking.

Would it make sense to include these options by default while using Intero in order to ensure we always keep the ability to inspect code?

@decentral1se
Copy link
Contributor

decentral1se commented Aug 24, 2017

Hey @unode, a nice idea! After #72, it's possible to pass these options in with g:intero_ghci_options. As for setting them as default, I'm not sure. I don't think there are any drawbacks to these options or? Perhaps a little too auto-magical?

@unode
Copy link
Author

unode commented Aug 24, 2017

I don't know enough about those options either. I asked around for ways to have the code compile even when types don't match so one could still inspect and got those 2 recommendations from folks on #haskell@freenode.
The binary produced is clearly flawed if types don't match but I guess that's the point.

There's also -fno-code which was mentioned as a way to make ghc-mod faster. It skips compilation and only type-checks. I haven't tried it but a priori I'm not sure this one is always desireable so it might be useful but perhaps not as default.

Edit: @lwm g:intero_ghci_options is undocumented. Perhaps it could benefit from a brief mention in the docs?

@decentral1se
Copy link
Contributor

Ah yes, I've opened #85 to track that. Yes, -fno-code wouldn't be a good default. I thought a little about this again and I think with intero_ghci_options available, we should probably avoid forcing a default when it is easily configurable. Perhaps, down the line, some configuration options will be incompatible or conflict or whatever.

@decentral1se
Copy link
Contributor

I've opened up #88 now. Hopefully @rdnetto or @parsonsmatt will chime in here to resolve this one.

@rdnetto
Copy link
Collaborator

rdnetto commented Sep 4, 2017

IMO, by default we should match the default behaviour of stack build and upstream Intero, which is to fail compilation on type errors. There are a few reasons for this:

  • consistency with other tools in the ecosystem
  • ensuring that the type errors are rendered as errors instead of just warnings (which would make them harder to spot if your project already generates several warnings)
  • magically adding ghci options can cause things to break unexpectedly in certain edge cases, such as other ghc-options set in the cabal file, especially when working with FFI or template haskell
  • intero_ghci_options provides an acceptable workaround for opting-in to the desired behaviour

The real reason we want this in the first place is that we want to inspect code that doesn't compile at all, regardless of whether that's due to a type error, a syntax error, or something else entirely. One way to achieve this might be to keep the old version of a module around if the new version fails to load. I think that fixing that for all compilation errors (rather than a subset) would be a better solution, though it's going to require changes in Intero which is obviously more work.

@unode
Copy link
Author

unode commented Sep 4, 2017

@rdnetto aren't those two approaches somewhat conflicting? I don't see how it would be possible to have inspectable incomplete/broken code if you don't allow for partial/incomplete compilation and deviate from standard build options.
I think it should be safe to assume that in a development scenario you might be working with a "debug-enabled" build, rather than a "regular or release" build.

For instance:

  1. How would you handle code that has never successfully compiled or any new construct that wasn't present in previous compilation cycles?
  2. With regards to inspecting an "old version", how do you decide if the "old" is "outdated"? Getting misleading information is arguably worse than no information at all.

@rdnetto
Copy link
Collaborator

rdnetto commented Sep 7, 2017

I don't see how it would be possible to have inspectable incomplete/broken code if you don't allow for partial/incomplete compilation and deviate from standard build options.

You can get there if you inspect the latest working build, instead of the current state (which may be broken).

(Deviating from the standard build options in a way not defined by the end user is the part here that bothers me, given it will almost certainly break due to some combination of incompatible options)

How would you handle code that has never successfully compiled or any new construct that wasn't present in previous compilation cycles?

If the module/function has never compiled, there's not much we can do about it. Inspection needs to operate on a best-effort basis, given that code under development is going to be broken most of the time.

With regards to inspecting an "old version", how do you decide if the "old" is "outdated"? Getting misleading information is arguably worse than no information at all.

That's a more interesting question. One approach would be to always provide whatever information we have, and leave it up to the user to decide if it made sense or not. Another would be to check for things like differing type signatures, and invalidate the cached symbols based on that.

What approach we take to invalidation will probably depend on the granularity of the cache though, which makes it hard to figure out until we've actually spiked or implemented it.

@unode
Copy link
Author

unode commented Sep 7, 2017

You can get there if you inspect the latest working build, instead of the current state (which may be broken).

This "broken" state might be something that we can work with and avoid some of the other hurdles.

I'm now using the above mentioned options by default and it helps in some cases.
I still need to get more familiar with them and test it throroughly to assess what are the downsides.

@unode
Copy link
Author

unode commented Sep 24, 2017

I've been giving -fdefer-type-errors and -fdefer-typed-holes a more exhaustive try.
The use of these options does provide additional inspection capabilities but at the moment Intero is not prepared to handle the output produced failing to parse warnings.

This leads to useless messages being displayed in the neovim status bar:

intero: [-Wunused-matches] (w)

instead of

intero: error: • Couldn't match expected type ‘IO [Annotator]’ (E)

Regardless, the interactive session window provides all the information at the cost of manual scrolling when warnings/errors are too verbose.

On the positive side, since the build succeeds, even in cases when types are incorrect, inspection of code is possible to some degree.

@parsonsmatt parsonsmatt added better-docs This issue reports expected behavior, but the documentation and/or error messaging could be better bug labels Apr 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
better-docs This issue reports expected behavior, but the documentation and/or error messaging could be better bug
Projects
None yet
Development

No branches or pull requests

4 participants