Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configurable illegal constructs #12

Open
rspeele opened this issue Aug 11, 2017 · 0 comments
Open

Configurable illegal constructs #12

rspeele opened this issue Aug 11, 2017 · 0 comments

Comments

@rspeele
Copy link
Collaborator

rspeele commented Aug 11, 2017

Motivation

There are some constructs that are legal SQL but are almost certainly mistakes if found in a static, handwritten query.

The primary examples that come to mind are:

NULL comparison is always false.

select * from Foo where Bar = null
-- `expr = null` is always false, should probably be `Bar IS null`

UPDATE/DELETE without WHERE clause is usually a mistake

update User set Email = @newEmail
-- uhhh... you probably forgot to put `where Id = @userId`

Idea

RZSQL should have some "linters" that implement a visitor for SQL statements and expressions (like ASTMapping.fs but without any output). There should be a separate linter class for each type of bogus construct we can think of.

In rzsql.json there could be an option like so, to determine which linters will run on the SQL statements in the project.

    "constructs": {
         "allowed": ["MissingWhereClause"],
         "banned": ["NullComparison", "UnsafeInjectRawSQL"]
    }  

This would let you override the linters enabled/disabled. Having both a whitelist and blacklist would let us pick a reasonable set of default linters that wouldn't be overly strict or overly lenient.

Linters would be named after the construct they throw an error upon recognizing. This way the constructs configuration setting reads like a list of what kinds of SQL are allowed and banned in your project. The goal here is to avoid the double-negative confusion that would result from something like the below, where we are effectively saying "we DON'T want the linter that checks for NOT having a where clause, because we DO want those statements in our SQL code".

    "linters": {
         "blacklist": ["NoMissingWhereClause"],
         "whitelist": ["NullComparisonAlwaysFalse", "NoUnsafeInjectRawSQL"]
    }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant