SQL Lint / Code Analysis Tools

Transact-SQL
1

There has been some chatter in other threads about keywords that are "optional" or "not enforced" in the T-SQL syntax (and the absence of a STRICT mode).

I've got a few - I hate the fact that CONVERT(varchar, @MyGUID) defaults to a length which is too short, and I'd like to enforce "AS" in Select clause Alias assignments to avoid the accident of

SELECT Col1    -- I forgot the comma here
       Col2    -- which makes this an Alias Name,
               --    not a Column to include

I went off wandering the Internet with Mr Google and found an article
Ideas for SET STRICT_CHECKS ON by Erland Sommarskog, Its a massive article and a great piece of work.

My conclusion was that I wanted some, but not all, of the checks Erland listed and it reminded me that many decades ago I used to rely heavily on Lint, when programming in C, to find potential problems in my code.

So I am on the lookout for a version of Lint for SQL - do you use one? or know of one?

I found something that looks interesting (but I expect there are others, maybe better ones?) SQL Enlight by Ubitsoft Ltd which has code formatting / refactoring tools, but also Lint-style tools to "to detect and correct defects in managed code". There is an online cut&paste trial box, I stuck the contents of the SProc I was working on into it and got:

Message (Line & Column No.s removed for brevity)
1 SA0002 : Variable @strParameters declared but never used.
2 SA0004 : Variable @intXXXNo is re-assigned at line 152 without its previous value being used.
3 SA0011 : SELECT * in stored procedures, views and table-valued functions.
4 SA0029 : The input parameter @intSilent never used.
5 SA0035 : TODO,HACK or UNDONE phrase found in a comment.
6 SA0054 : Parameter @XXX_ID modification prior to use in a query, may negatively affect performance.

So I'm not quite the Rock Star coder that I thought I was and some dumb-ass machine is smarter than me - that always smarts!

#1 is definitely a bug, I'm grateful for that alone being spotted.
#5 is handy as I do indeed have a TODO comment in the code, and they tend to get left there forever whereas I ought to have occasional days where I "Fix a load of ToDos".
I think the rest are fine, (the SELECT * is in an Error Catch where I want to get all columns regardless of what shape the table might become, AND I want to keep the error-handling code as simple as possible)

List of the their Analysis Rules available is here: http://www.ubitsoft.com/products/sqlenlight-for-ssms/analysis-rules.php

2

I don't know of any code checker like the Resharper for Visual Studio or the SQL Enlight (which I was enlightened of only when I read Kristen's post). But, I have lost count of how many times I have been bitten by this Co1 Col2 issue!! I have literally pulled my hair trying to figure those out.

A strict mode would really really be useful for me, almost as much as my other pet peeve, the "String or binary data would be truncated" message which Microsoft refuses to fix.

1 Like
3

Resharper looks amazing ... pity I don't write anything much in the way of APP code ... (doesn't analyse SQL unless I missed something on their website?)

A solution?? is to prefix all columns (in SELECT list) with an alias. I don't do that though (as our column names are unique within the database, so only ambiguous if we self-reference a table)

4

That seems like too much work if you meant SELECT Col1 AS Col1, etc. After suffering through this a few times, the first thing I look for these days after writing the code is whether I have all the required commas. Nonetheless, availability of a STRICT option would have been nice.

5

Sorry, no I was meaning adding an alias to the Table and prefixing each column with that alias:

SELECT A.Col1,
       B.Col1,
       ...
FROM TableA AS A
     JOIN TableB AS B

This will, then, raise an error

SELECT A.Col1    -- Comma missing
       B.Col1,
       ...
FROM TableA AS A
     JOIN TableB AS B

I read a blog article the other day which experimented with whether this improved performance, or not. Can't remember the answer - Doh! - but there appeared to be a significant difference between the two

6

If you're using Database Projects in Visual Studio (2013 or higher, maybe 2012) there are additional Code Analysis rules you can apply to your project. I've found them to be very handy and am incorporating them into our continuous integration at my current work. SQL Enlight may cover all of them too, I know it has more than VS (the NOLOCK rule is gonna get heavy use) :smile:

There's also the canonical John Carmack article:

It's not about SQL but coding in general. Very eye-opening. I'm striving to get our dev team to start using it, as our current codebase, with all Microsoft rules turned on, has 250K warnings over 1.6M LOC. And a lot of them can be fixed with find/replace in a few minutes.

7

I've been using the online SQL Enlight code analysis tool (i.e. Cut & Paste) on each SProc I've needed to work on over the last couple of day (which has enabled me to persuade myself that I should be spending my time refactoring code for fun, rather than working on anything on the project's critical path - where's the fun in that?!!)

SQL Enlight has not turned up anything critical yet, which I suppose is to be expected, or at the very least a relief!! as this is all fully tested production code!! - but it has turned up enough things that it would earn its keep. Main ones are:

"Variable declared but not used". No idea if there is a minute performance penalty for SQL instantiating the variable, but I'd prefer to have the DECLARE commented out a change int he code makes the variable actually needed - that will make anyone using it think about whether anything else needs to happen when they find that the DECLARE is deliberately commented out.

"Variable changed before its assigned value used". This happens mostly for OUTPUT parameters which are then initialised (such as Error Number were I don't care what the caller gave ME :smile: ), but there have been a few that I went and checked to make sure that the behaviour was OK.

I also used Poor SQL to compress one Sproc because the online limit for SQL Enlight is 25K : :hushed: (yeah, we have some Sprocs bigger than that ... not many though)

"during that time [when he was fixing bugs reported by the /analyze] there was an epic multi-programmer, multi-day bug hunt that wound up being traced to something that /analyze had flagged, but I hadn't fixed yet"

That alone ought to be enough for anyone who has spent ages, or worse the whole team has spent ages, searching for a bug, which Defensive Programming would have caught ought, to be persuaded as to its worth.

John Carmack mentioned that he used PC-Lint, back-in-the-day, to analyse his C code, but the mountain of notifications was off-putting - lots of false positives I expect. We used PC-Lint around that time and attended to every single warning (even if it was to add a pre-processor directive to ignore that warning on that line/block of code). I considered it worth its weight in gold back then ... so for anyone starting out on this "today" I can well imagine that there will be weeks/months of work to get the code into fit state to then run cleanly with the analyser. Assuming a few critical bugs are found during that time the "cost" will be worth it and, going forwards, the saving in finding-early on new code will be significant.

No way of, then, estimating the cost saving from NOT having a "Black Swan Event" ... that might be difficult for budgetary approval, going forwards. My advice? Set a Base Line NOW and a generous bonus for improving on whatever it has been historically, and then use Analysers to improve the base line dramatically :smile:

If you've not read them you may well find books like Code Complete interesting - in terms of setting a code development strategy that is "Defensive", as well as how you educate a team to the importance and so on.

Increasing I am asked to do consultancy work for teams that only seem to produce thrown-together code, which is a big shame. I'm now working with Clients (who are buying/considering SQL APPs) to introduce "Fix/Change this before we will consider buying your APP" - i.e. Client will specify rules for vendors as part of that Quality Control issue. Bugs in Vendor code cost the user hugely - I have a client with 10 attorneys who's billing rate is $Huge sitting around for 10 minutes at a time waiting for the Admin to create a folder that the Document Management System is supposed to do, in real time, as soon as the Matter Record is created in the database ... some small percentage of the time it doesn't happen (my money is on the NOLOCKs that they add to their code with a Pepper-shaker :frowning: )

I resent my client paying me to give a leg-up to their application software suppliers ... but its the Client's money and they see it as a net saving.

One of the things I worry about is (total lack of) code coverage in our error handlers. A Static Code Analyser that spotted an error in an error-handler would prevent the possibility that the error handler crashes - converting a "should not happen but its logged/reported for attention" into an "Application crashed with no message" report from a user, which are extremely expensive to fix, more so for situations where we have no remote access to maintain the system.