Open Bug 1549539 Opened 5 months ago Updated 5 months ago

Investigate adding a SQL linter for C++ and JS code to run against Phabricator

Categories

(Toolkit :: Storage, enhancement, P3)

enhancement

Tracking

()

People

(Reporter: asuth, Unassigned, NeedInfo)

Details

Inspired by real life events, I'm interested in seeing if it's possible to get a linter to sanity check SQL in reviews. Frequently the SQL is a string literal, albeit possibly spread over multiple lines using C/C++ adjacent string literal magic[1], so I think it's reasonably straightforward to extract the string. (At least when using clang and its plugin/AST matcher magic.) And I believe we now have a bunch of linting running against phabricator, so perhaps this can be added.

This obviously isn't the right component to track actual implementation, but since I'm also not ready to pursue this today, I'm putting this here in toolkit::storage and can move it later.

Looking into how things work, "mach lint" and "mach static-analysis" are magically run by daemons who then comment on the phabricator review. Since we already use eslint for JS and it's very extensible, that seems like the clear choice for JS. Since the string literal parsing is already a hassle, static-analysis sounds like the right check for C++.

needinfo-ing myself, but others should feel free to jump in!

1: Which was the source of another real life event error.

Flags: needinfo?(bugmail)
Type: defect → enhancement
Flags: needinfo?(bugmail)
Priority: -- → P3
Flags: needinfo?(bugmail)
You need to log in before you can comment on or make changes to this bug.