syntastic icon indicating copy to clipboard operation
syntastic copied to clipboard

Added Purescript language and Pulp syntax checker

Open seanhess opened this issue 10 years ago • 17 comments

I've tested with as many different kinds of errors I can generate, and it appears to be useful. Please let me know if there's anything you'd like me to change.

seanhess avatar Aug 31 '15 21:08 seanhess

This has been tested by other community members now too.

seanhess avatar Aug 31 '15 22:08 seanhess

This is great! :+1:

I believe it does need some work, though:

  • Using pulp build --main .. will not work in all cases. When editing a file in the test directory of a pulp project, pulp build will only compile the files in src and ignore the test directory. What about non-pulp projects?
  • A standard type mismatch error gets displayed as Cannot unify typePrim.Intwith typePrim.String (missing whitespace)
  • There are a few errors which are not recognized / rendered properly: e.g.: Orphan instances errors
  • This is really more a problem of syntastic, but a full purescript build (via pulp) can be quite long (1s and more). Since pulp is run synchronously and blocks vim, this makes it almost unusable as a syntax checker.

sharkdp avatar Sep 01 '15 07:09 sharkdp

Aside from the issues pointed out by @sharkdp, errorformat is wrong. Without looking at what this is supposed to match, the following fixes the syntax errors:

    let errorformat =
        \ '%E%\s%#psc: %m,' .
        \ '%C%\s%#at "%f" (line %l\, column %c)%.%#,' .
        \ '%E%\s%#"%f" (line %l\, column %c)%.%#,' .
        \ '%E%.%#Error at %f line %l\, column %c - %.%#,'.
        \ '%-G%\%# ERROR: Subcommand%.%#,' .
        \ '%\%# ERROR: %m,' .
        \ '%-G%\s%#See http%.%#,' .
        \ '%C%\s%#%m'

I'd suggest the following framework for testing errorformats:

let &errorformat = '...'
lgetexpr [
    \ 'Error found:',
    \ 'Error at /home/lcd047/tmp/purs/src/Main.purs line 5, column 4 - line 5, column 4:',
    \ '  Unable to parse module:',
    \ '    unexpected ]',
    \ '    expecting binder, \| or =',
    \ 'See https://github.com/purescript/purescript/wiki/Error-Code-ErrorParsingModule for more information, or to contribute content related to this error.'
    \ ]
echomsg string(getloclist(0))
lopen

lcd047 avatar Sep 01 '15 10:09 lcd047

@sharkdp: I'm new to Purescript, pulp build is how I learned how to do it. The syntax checker is called pulp on purpose. If you don't have pulp installed it won't do anything. Would it be faster to use psc directly? What would the correct thing to type on the command-line to compile a single file and have all of its dependencies available?

A standard type mismatch error gets displayed as Cannot unify typePrim.Intwith typePrim.String (missing whitespace)

There's no way to display multiple lines in syntastic errors that I am aware of. @lcd047 is that correct? Is there another way you'd like to see that error on one line? It seems readable to me.

How can I generate an orphan instance?

seanhess avatar Sep 01 '15 14:09 seanhess

@lcd047 That's super useful thanks! It's such a pain to test the errorformat on a real project.

You are getting syntax errors in the current errorformat? How can I see them? I definitely generated some earlier while developing it, but I don't see any now and it appears to work. I'm happy to fix them but it would be difficult without being able to reproduce them.

seanhess avatar Sep 01 '15 14:09 seanhess

@lcd047 I'm not seeing any errors with my errorformat even using your testing method. Can you help me reproduce the problem? Thanks!

seanhess avatar Sep 01 '15 15:09 seanhess

There's no way to display multiple lines in syntastic errors that I am aware of. @lcd047 is that correct?

It depends on what you mean by that. Loclist windows don't display multiline text fields as multiple lines. That's a limitation of Vim, not syntastic. But you can join messages spread across multiple lines, and format them to look reasonable.

You are getting syntax errors in the current errorformat?

Like I said, I haven't run that in Vim. But some of the constructs you're using don't work the way you seem to expect.

Anyway, looking at the list of possible errors, I don't think you stand any chance to cover them all with a reasonable effort. Not with errorformat anyway. shrug

lcd047 avatar Sep 01 '15 15:09 lcd047

I'm new to Purescript, pulp build is how I learned how to do it. The syntax checker is called pulp on purpose. If you don't have pulp installed it won't do anything.

Doing nothing - if pulp is not available - is fine. In general though, using pulp as a syntax checker does feel wrong to me:

  • Pulp is a build tool and running pulp build has side effects (files will be written to the output folder). While this might be okay for most cases, a non-suspecting user might be surprised that vim suddenly builds his project if he saves a source file (or creates an output folder).
  • Pulp relies on a certain project structure (bower.json file in the project root)
  • As mentioned above, using pulp build --main does not work in the test directory of a project, because pulp simply builds the project from the src folder.

Would it be faster to use psc directly?

Probably not, since pulp is just a wrapper around psc. On the other hand, there might be a chance to speed up the build with something like psc --no-opts (disable optimizations, we are not interested in the generated code). I have no experience with this. Optimally, there would be something like --syntax-check-only for psc, without any side effects. I'm not sure if something like this exists (/cc @paf31).

What would the correct thing to type on the command-line to compile a single file and have all of its dependencies available?

It's actually not straightforward because we have to include all of the projects purescript and javascript files. This is where using pulp comes in handy.

Alternatively, I was thinking it would be really nice to have something like hlint for purescript. This could potentially be much faster than a full build of the project (?). I realize that I'm just piling up ideas here... that's not very helpful, sorry.

A standard type mismatch error gets displayed as Cannot unify typePrim.Intwith typePrim.String (missing whitespace)

There's no way to display multiple lines in syntastic errors that I am aware of. @lcd047 is that correct? Is there another way you'd like to see that error on one line? It seems readable to me.

If it's possible to concat these lines with some whitespace in between, that would be great.

How can I generate an orphan instance?

There is an example on the purescript wiki here. When using all of this code in a single file, your syntastic checker actually works nicely (correctly indicating the orphan instance error). So ignore my comment above, this is not due to the error formating.

However, when I split this code into separate modules A.purs, B.purs and C.purs, the checker incorrectly shows a syntax error in module A.purs on line 7 (there is no line 7 in A.purs). So it's probably a matter of parsing the output for syntax errors in the current file.

sharkdp avatar Sep 01 '15 19:09 sharkdp

There is no way to run the compiler without generating output files right now, at least not using the standard executables. An option could be added easily though and I'd happily take a PR to add a --check-only option or something like that. Would we want to run the typechecker or just check things like imports? Skipping type checking or separating it into its own build command would speed things up quite a lot, if that's possible.

Since the compiler accepts globs now, it might be easier to use psc directly, and then things like whether to include the test directory become simply a matter of changing a glob in the configuration.

paf31 avatar Sep 01 '15 19:09 paf31

@paf31 As far as I'm concerned building the project while checking is annoying, but not fatal. Other checkers do that, and people still use them. The real show stopper however is having to parse error messages from 100+ ad-hoc formats. It would be nice if psc would have an option to output errors in a machine-friendly format. I do understand that adding such a feature at this point might take some work, but without it it isn't reasonable to write a syntax checker for an editor. shrug

lcd047 avatar Sep 01 '15 20:09 lcd047

That's been requested before. I can prioritize it now that proper editor support is becoming a thing. Is there a best format I can use?

paf31 avatar Sep 01 '15 20:09 paf31

@paf31 Something like /path/to/file:6.3-6.7: Expression 33 does not have type Prim.String would be perfect. JSON would be fine too, and probably more flexible.

One more thing: column numbers should be counted as byte offsets from the beginning of line (1 tab = 1 character). Otherwise, the exact positions of the errors depend on tab size...

lcd047 avatar Sep 01 '15 20:09 lcd047

Do we want to try to push for something we can merge short-term, or wait until you make changes to psc @paf31? We're probably going to end up using Elm for our project so I would prefer to get this to a useful state, get it merged, and let you guys make PRs to it as you improve things.

seanhess avatar Sep 01 '15 21:09 seanhess

@seanhess The current code has various problems, and the changes kindly offered by @paf31 would make a big difference. If you need a checker right now, perhaps you can maintain it externally until the relevant features are added to psc?

lcd047 avatar Sep 01 '15 21:09 lcd047

@lcd047 It's already useful. While there are many diverse errors the vast majority fall within the same formats. I would rather clean up the things you mentioned (the errorformat issues) and get this merged while we wait so people can use it.

But I'm ok either way. I would like to do whatever is more helpful to the Purescript community.

seanhess avatar Sep 01 '15 22:09 seanhess

@seanhess Sadly, useful to you != problem-free for me. Missing errors is a great source of bug reports, and a legitimate one at that. shrug

lcd047 avatar Sep 02 '15 04:09 lcd047

I totally understand. Well I'll defer to you of course. If you want to wait that's fine. Thanks.

seanhess avatar Sep 02 '15 13:09 seanhess