Code action: add constraint #653
Conversation
|
|
| -- Suggests a constraint for an instance declaration with no existing constraints. | ||
| -- • No instance for (Eq a) arising from a use of ‘==’ | ||
| -- Possible fix: add (Eq a) to the context of the instance declaration | ||
| -- • In the expression: x == y | ||
| -- In an equation for ‘==’: (Wrap x) == (Wrap y) = x == y | ||
| -- In the instance declaration for ‘Eq (Wrap a)’ |
There was a problem hiding this comment.
Drive-by-comment: I think it would be nicer if this awesome comment is part of the haddock documentation, since hover will show it. Just my personal taste, though.
There was a problem hiding this comment.
I'm not sure what you mean here. I've added a documentation comment at line 395. The one you're referring to just explains what case the branch handles and an example of the missing constraint error from GHC (like it has been done for other code actions). Can you show me an example of what you mean?
| -- eq (Pair x y) (Pair x' y') = x == x' && y == y' | ||
| | Just c <- contents | ||
| , True <- _message =~ ("the type signature for:" :: String) | ||
| , Just constraint <- findMissingConstraint _message |
There was a problem hiding this comment.
findMissingConstraint _message can get evaluated up to 3 times, which seems a bit wasteful. It would be good to reorganize things so that it only needs to be called once.
There was a problem hiding this comment.
Good point! I'll try to address this.
There was a problem hiding this comment.
I've created an entrypoint for missing constraint suggestion in order to perform findMissingConstraint _message only once and then routing to the relevant implementation.
| -- | Suggests a constraint for an instance declaration for which a constraint is missing. | ||
| suggestInstanceConstraint :: Maybe T.Text -> Diagnostic -> [(T.Text, [TextEdit])] | ||
| suggestInstanceConstraint contents Diagnostic {..} | ||
| -- Suggests a constraint for an instance declaration with no existing constraints. | ||
| -- • No instance for (Eq a) arising from a use of ‘==’ | ||
| -- Possible fix: add (Eq a) to the context of the instance declaration | ||
| -- • In the expression: x == y | ||
| -- In an equation for ‘==’: (Wrap x) == (Wrap y) = x == y | ||
| -- In the instance declaration for ‘Eq (Wrap a)’ |
There was a problem hiding this comment.
| -- | Suggests a constraint for an instance declaration for which a constraint is missing. | |
| suggestInstanceConstraint :: Maybe T.Text -> Diagnostic -> [(T.Text, [TextEdit])] | |
| suggestInstanceConstraint contents Diagnostic {..} | |
| -- Suggests a constraint for an instance declaration with no existing constraints. | |
| -- • No instance for (Eq a) arising from a use of ‘==’ | |
| -- Possible fix: add (Eq a) to the context of the instance declaration | |
| -- • In the expression: x == y | |
| -- In an equation for ‘==’: (Wrap x) == (Wrap y) = x == y | |
| -- In the instance declaration for ‘Eq (Wrap a)’ | |
| -- | Suggests a constraint for an instance declaration for which a constraint is missing. | |
| -- | |
| -- Suggests a constraint for an instance declaration with no existing constraints. | |
| -- • No instance for (Eq a) arising from a use of ‘==’ | |
| -- Possible fix: add (Eq a) to the context of the instance declaration | |
| -- • In the expression: x == y | |
| -- In an equation for ‘==’: (Wrap x) == (Wrap y) = x == y | |
| -- In the instance declaration for ‘Eq (Wrap a)’ | |
| suggestInstanceConstraint :: Maybe T.Text -> Diagnostic -> [(T.Text, [TextEdit])] | |
| suggestInstanceConstraint contents Diagnostic {..} |
I mean this. This way, on hover on this function, you can see all comments.
But if you think that is a bad idea, that is fine, too.
There was a problem hiding this comment.
I see. The comment you moved refers only to the branch of the pattern matching, so it's relevant to an implementation detail. Besides, the first line would be kind of redundant, wouldn't it? There is also another comment similar to this one for the branch of the pattern match right below. If we moving both of them out of that context they would loose their meaning, since they strictly refer to the branches. What do you think?
There was a problem hiding this comment.
the first line would be kind of redundant, wouldn't it?
Yeah, but I dont think this is really bad.
There is also another comment similar to this one
Yeah, I would move that, too.
What do you think?
Your concern seems valid to me. Personally, I would like it to have it available as haddock comments. But, if you think it is too much of an implementation detail to actually show in the documentation, you are probably right. So, whatever feels right to you!
DenisFrezzato
changed the title
DenisFrezzato
force-pushed
the
code-action-add-constraint
branch
from
0dcc0cf
to
b7dc398
Compare
|
This PR needs rebasing on master since it has conflicted with #657. If you run into issues resolving the conflicts, I’m happy to help. |
Create entrypoint for missing constraint code action, in order to have a more efficient parsing by routing to the relevant implementation. Fix type signature name parsing. Minor refactor.
DenisFrezzato
force-pushed
the
code-action-add-constraint
branch
from
b7dc398
to
29dd059
Compare
* Add missing instance constraint * Add missing instance constraint with existing constraints * Add missing function constraint * Add missing function consraint with existing constraints * Add some comments * Improve type signature regex * Remove redundant bracket * Improve missing constraint searching. Create entrypoint for missing constraint code action, in order to have a more efficient parsing by routing to the relevant implementation. Fix type signature name parsing. Minor refactor. * Minor refactor
* Add missing instance constraint * Add missing instance constraint with existing constraints * Add missing function constraint * Add missing function consraint with existing constraints * Add some comments * Improve type signature regex * Remove redundant bracket * Improve missing constraint searching. Create entrypoint for missing constraint code action, in order to have a more efficient parsing by routing to the relevant implementation. Fix type signature name parsing. Minor refactor. * Minor refactor
* Add missing instance constraint * Add missing instance constraint with existing constraints * Add missing function constraint * Add missing function consraint with existing constraints * Add some comments * Improve type signature regex * Remove redundant bracket * Improve missing constraint searching. Create entrypoint for missing constraint code action, in order to have a more efficient parsing by routing to the relevant implementation. Fix type signature name parsing. Minor refactor. * Minor refactor
* Add missing instance constraint * Add missing instance constraint with existing constraints * Add missing function constraint * Add missing function consraint with existing constraints * Add some comments * Improve type signature regex * Remove redundant bracket * Improve missing constraint searching. Create entrypoint for missing constraint code action, in order to have a more efficient parsing by routing to the relevant implementation. Fix type signature name parsing. Minor refactor. * Minor refactor
* Add missing instance constraint * Add missing instance constraint with existing constraints * Add missing function constraint * Add missing function consraint with existing constraints * Add some comments * Improve type signature regex * Remove redundant bracket * Improve missing constraint searching. Create entrypoint for missing constraint code action, in order to have a more efficient parsing by routing to the relevant implementation. Fix type signature name parsing. Minor refactor. * Minor refactor
* Add missing instance constraint * Add missing instance constraint with existing constraints * Add missing function constraint * Add missing function consraint with existing constraints * Add some comments * Improve type signature regex * Remove redundant bracket * Improve missing constraint searching. Create entrypoint for missing constraint code action, in order to have a more efficient parsing by routing to the relevant implementation. Fix type signature name parsing. Minor refactor. * Minor refactor
* Add missing instance constraint * Add missing instance constraint with existing constraints * Add missing function constraint * Add missing function consraint with existing constraints * Add some comments * Improve type signature regex * Remove redundant bracket * Improve missing constraint searching. Create entrypoint for missing constraint code action, in order to have a more efficient parsing by routing to the relevant implementation. Fix type signature name parsing. Minor refactor. * Minor refactor
This PR adds code actions to generate suggestions for missing constraints, both for
instances declaration and type signatures. I've handled cases where no, one or more other constraints are already declared.I've also introduced operators like
>>>,&,<&>in order to compose from left to right (like the code flows from left to right) and to have code more easier to read and to reason about. I'm mentioning this because I didn't see them used in the code (at least not in the files I changed), so if for some reason they are not welcome, I'll change them.This closes https://github.com/digital-asset/ghcide/issues/345.