Add checker for sqlint (https://github.com/purcell/sqlint)

[purcell]

Hi! This PR adds a new syntastic checker definition for the sqlint linter (https://github.com/purcell/sqlint). The checker definition is based on a couple of existing syntastic checkers: please let me know if anything needs fixing up. Cheers!

[lcd047]

Nice project, and reasonably clean implementation of the checker. However, I don't think the licence you chose, GPLv3, is compatible to the license of syntastic, WTFPL. shrug

[purcell]

@lcd047 We've changed the sqlint checker license to MIT, but in any case that's the license for the checker, not for the syntastic plugin code, so I don't think license compatibility is a concern.

[purcell]

Oh, hang on - I see what you mean. Let me fix that.

[purcell]

Updated - sorry for the confusion. :-)

[lcd047]

I was referring to the license of the syntastic plugin code for which you sent the pull request. The license of sqlint itself is not really relevant, as far as syntastic is concerned.

[purcell]

@lcd047 Yep, gotcha - that's fixed now.

[lcd047]

Updated - sorry for the confusion. :-)

Ok, thank you. Can you please post a few code snippets that produce multiline messages, preferably both errors and warnings?

[purcell]

Sure:

SELECT 1;
SELECT 1 FROM ';
;

Which produces

sql-syntax-error.sql:2:15:ERROR unterminated quoted string at or near "';
  ;
  "

We don't have WARNING examples yet, because we need to work with the upstream project which does the heavy lifting for the parsing, but the format will be identical apart from the ERROR/WARNING indicator, which we add. The trailing lines of multiline error messages are each prefixed with 2 spaces in order to unambiguously separate them from the filename:line:col... lines.

[lcd047]

Thank you, I think I have everything I need.

On a side note however, including pieces of the source in the error messages without sanitising them first seems like a really bad idea, see f.i. #1435 for the kind of problem this can cause for syntastic.

[purcell]

Hmm, I thought that using the double-space prefix for all continuation lines would be sufficient sanitisation to make the output unproblematic, since presumably the filename matches are anchored to the line start positions.

[purcell]

The behaviour in #1435 appeared to be due to a raw newline in the error string, but in our error strings those newlines would still result in indented lines.

[lcd047]

Yes. Now think of a terminal control code instead of a newline.

Unrelated: the offset reported seems to be off by one. F.i. SELECT * ROM foo; would put the error on the space before ROM, and ELECT * FROM foo; would trigger an error at the end of the preceding line. And if ELECT is on the first line, that's a crash. :)

[lcd047]

Ok, merged with this patch on top: 1e475a7

[purcell]

Yes. Now think of a terminal control code instead of a newline.

That may be handled by the underlying parsing library, which is the C PostgreSQL query parser, but I'll investigate and make sure that gets handled.

Unrelated: the offset reported seems to be off by one. F.i. SELECT * ROM foo; would put the error on the space before ROM, and ELECT * FROM foo; would trigger an error at the end of the preceding line.

Pretty sure it's correct as-is. The line and column numbers and one-based, like most linters. I definitely see the correct positions in vim with syntastic and this checker definition.

[lcd047]

I'll investigate and make sure that gets handled.

It's probably a good idea to either kill or replace all characters < 0x20 except tab, just to be safe.

The line and column numbers and one-based, like most linters. I definitely see the correct positions in vim with syntastic and this checker definition.

$ echo 'SELECT * ROM foo' >test.sql
$ sqlint test.sql 
test.sql:1:9:ERROR syntax error at or near "ROM"
$ echo 123456789; cat test.sql
123456789
SELECT * ROM foo
$ sqlint --version
0.0.3

Syntastic correctly highlights the space before ROM. My point is, maybe your checker should print 1:10 instead of 1:9.

[purcell]

Ah, there's a 0.0.4 already. That offset was one thing that got fixed. Sorry for not guessing faster at the likely problem.

[purcell]

Version 0.0.5 (just released) fixes the control character loophole by printing them safely as escapes, e.g. \0027.

[lcd047]

Nuch better, thank you. :) Ok, highlighting should be nicer now too: d353962

[purcell]

Great!

[purcell]

Needless to say, thanks for your help and patience. I hesitate to admit it, but I'm an Emacs guy these days, but I wanted to make sure vim users could benefit from sqlint, and I knew enough to know that syntastic was the way to go. :-)

[Telling]

Just installed sqlint and the updated syntastic. I opened an SQL file, and got the error seen below:

If i run sqlint by hand, i get:

~ » sqlint function-metadata.sql
function-metadata.sql:1:1:ERROR syntax error at or near ""

I don't expect this to be the correct behavior?

[lcd047]

@Telling Believe it or not, sqlint has changed since this morning. Ok, this should be fixed in 76ec53f.

[purcell]

@Telling Believe it or not, sqlint has changed since this morning. Ok, this should be fixed in 76ec53f.

Sorry, yes, we noticed that there was no indication from the exit code whether problems were found, as is customary, so we fixed that.