staticcheck: detect mismatching numbers of SQL bind parameters for certain scenarios

[fortytw2]

I've had cases like this ->

row := db.sql.QueryRowContext(ctx, `
	INSERT INTO source_creation_requests
	(user_id, input_url)
	VALUES
	($1, $2, $3)`, st, inputURL)

pop up many times in the past, where there are 3 bindVars in the query string, but only 2 get passed from Go.

I think there's a limited subset of this check that shouldn't have any false positives (i.e. a fixed length of sql args, a single static string as the query), would be really useful to be able to check this before runtime.

[dominikh]

Unfortunately, this would require us to implement a parser for SQL. And when I say "a parser", I really mean "lots of parsers" because every major RDBMS implements their own flavour of SQL. We can't rely on naive string matching, nor on a generic, simplistic parser, because neither would be able to accurately tell apart $1 the variable from $1 the sequence of characters in a string, name, or other context. This is especially true for systems that use ? for variables.

Implementing SQL parsers is out of scope for staticcheck.

[fortytw2]

I wonder if that sequence of characters is ever actually present in real world code?

I've never seen either potential case before in the wild, if that counts for anything.

[dominikh]

I'm sure there are cases where fields contain a value like foo$1 and someone has a LIKE query searching for that.

In general, staticcheck is built around the idea of minimal false positives, and only flagging things it knows are wrong, not things it doesn't know are right. Without actually understanding the query, implementing this check is a no-go.

[dominikh]

Reopening under the aggressive label, for a future set of checks specific to code review.