Skip to content
This repository has been archived by the owner on Sep 20, 2023. It is now read-only.

Added Purescript language and Pulp syntax checker #1531

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

seanhess
Copy link

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
Copy link
Author

This has been tested by other community members now too.

@sharkdp
Copy link

sharkdp commented Sep 1, 2015

This is great! 👍

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.

@lcd047
Copy link
Collaborator

lcd047 commented Sep 1, 2015

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

@seanhess
Copy link
Author

seanhess commented Sep 1, 2015

@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
Copy link
Author

seanhess commented Sep 1, 2015

@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
Copy link
Author

seanhess commented Sep 1, 2015

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

@lcd047
Copy link
Collaborator

lcd047 commented Sep 1, 2015

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

@sharkdp
Copy link

sharkdp commented Sep 1, 2015

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.

@paf31
Copy link

paf31 commented Sep 1, 2015

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.

@lcd047
Copy link
Collaborator

lcd047 commented Sep 1, 2015

@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

@paf31
Copy link

paf31 commented Sep 1, 2015

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?

@lcd047
Copy link
Collaborator

lcd047 commented Sep 1, 2015

@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...

@seanhess
Copy link
Author

seanhess commented Sep 1, 2015

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.

@lcd047
Copy link
Collaborator

lcd047 commented Sep 1, 2015

@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?

@seanhess
Copy link
Author

seanhess commented Sep 1, 2015

@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.

@lcd047
Copy link
Collaborator

lcd047 commented Sep 2, 2015

@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

@seanhess
Copy link
Author

seanhess commented Sep 2, 2015

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants