-
Notifications
You must be signed in to change notification settings - Fork 40
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
Various fixes and improvements to module linting #95
Conversation
- do not require space before `functor` - do not match `functorx` as `functor x` For precedence reasons, `ocamlMPRestr2` must then be declared after `ocamlMPRestr3`.
- do not require space before contained `sig` - do not match `sigx` as `sig x` For precedence reasons, `ocamlMPRestr1` must then be declared after `ocamlMPRestr3`.
for precedence reasons, ocamlSig must be declared after ocamlMPRestr3
also, minor fix: do not require a character before `struct` when nested in parentheses; this fixes the following example: open F( struct end )
(but what is that for?)
The `module` keyword is now treated uniformly. Previously, it was matched differently in signatures (matchgroup `ocamlModSpec`) than elsewhere (matchgroup `ocamlModule`). Both cases have almost identical syntax: (* in signatures: *) module M (X1 : T1) … (Xn: Tn) : MODULE TYPE (* in structures: *) module M (X1 : T1) … (Xn: Tn) [: MODULE TYPE] = MODULE DEF The case distinction was not taking profit of the small difference in syntax, and it was creating complexity and mismatch between both cases. Each case had shortcomings. In signatures, functor parameters like this where not highlighted corrrectly: (* in signatures: *) module M (X1 : T1) : … Also, the only case where the `functor` keyword was highlighted correctly was in a `module` type annotation *in a signature*. It was not highlighted correctly in `module` type annotations *in structures*, nor in `module` definitions, nor in `module type` definitions. (* in signatures: *) module type T = functor (X1 : T1) -> … module M : functor (X1 : T1) -> … (* the only case that was working *) (* in structures: *) module type T = functor (X1 : T1) -> … module M : functor (X1 : T1) -> … = … module M = functor (X1 : T1) -> … These bugs are now fixed. The `ocamlModule` matchgroup subsumes the features of both former cases (`module` in signatures / in structures). The `functor` keyword is no more "contained", so that it now matches in all of the 5 situations above.
(those I could make sense of)
Thanks! I tested this against some files by diffing the output of module F : Bar = Foo module Test : Make(Int).S = struct end ( Besides that, this is a clear improvement, and I hope we can get this merged.
It should be fine as long as the highlighting doesn't fail catastrophically (as in #91). After all, it looks like a strict improvement with this patch (at least until Simon discovers some new alien syntax :-)) |
I'll merge this, as it's clearly better than before. |
Great, thank you! |
This PR consists of incremental fixes and simplifications and is best reviewed per commit, with help from the (long) commit messages.
Although some of the bugs fixed were made obvious by the new type linter, they existed since the beginning of the Git history, years ago. The most notable bugfixes are:
module type of
(was not working in signatures);(X: T)
(was not working in signatures);functor
construct (was not working except in one situation).Test suite for
module type of
(comments indicate behavior before this PR):Test suite for functor parameters:
Test suite for
functor
:This PR closes
:
onmodule
)Note: the module linting stuff has more bugs that I did not care fixing, because I am not in mood for rewriting it from scratch. I cannot figure out the purpose of several bits, neither if some bits are astute hacks, obsolete stuff or just mistakes. For instance, issue #6 remains unsolved; I saw which part of the code causes it, it seems to be done on purpose?