```tsql Bad theme={null} CREATE PROCEDURE MyProc AS DECLARE @return_status int = 0; WHILE @return_status = 0 BEGIN EXEC @return_status = something; CONTINUE; -- Noncompliant END; RETURN; -- Noncompliant GO ``` ```tsql Fix theme={null} CREATE PROCEDURE MyProc AS DECLARE @return_status int = 0; WHILE @return_status = 0 BEGIN EXEC @return_status = something; END; GO ```

\`NOCOUNT is by default deactivated (OFF) at server level. It means by default, the server will send to the client the number of rows affected by the SQL query executed which is, in most cases, useless because no one will read this information.

Deactivating this feature will save some network traffic and improve the execution performance of stored procedures and triggers that’s why it is recommended to define SET NOCOUNT ON at the beginning of the definition of PROCEDUREs and TRIGGERs, before any query is processed.

This rule raises an issue when NOCOUNT is not set or is set to OFF between the beginning of the PROCEDURE (or TRIGGER) definition and the first statement that is not a SET, IF or DECLARE\`.

```tsql Bad theme={null} CREATE PROCEDURE dbo.MyProc AS BEGIN DECLARE @var INT; SET NOCOUNT OFF; -- Noncompliant; deactivate NOCOUNT SELECT COUNT(*) FROM MY_TABLE END; ``` ```tsql Fix theme={null} CREATE PROCEDURE dbo.MyProc AS BEGIN -- Noncompliant; SET NOCOUNT is not specified so behaviour of the procedure execution is based on server configuration (OFF by default) SELECT COUNT(*) FROM MY_TABLE END; ```

\`COALESCE and IIF (which evaluate to CASE expressions under the covers), as well as CASE input expressions should not be used with subqueries because the subquery will be evaluated once for each option in the expression, and each evaluation could return different results depending on the isolation level. To ensure consistent results, use the SNAPSHOT ISOLATION isolation level. To ensure consistent results and better performance, move the subquery out of the expression.

Note it is also an option to replace COALESCE with ISNULL\`.

```tsql Bad theme={null} ... COALESCE((SELECT a FROM b WHERE c) , 1) -- Noncompliant ... ``` ```tsql Fix theme={null} ... CASE WHEN (SELECT COUNT(*) FROM A) > 0 THEN (SELECT COUNT(*) FROM A) + 42 ... ELSE otherExpression END ... ```

The complexity of an expression is defined by the number of AND and OR operators it contains.

A single expression’s complexity should not become too high to keep the code readable.

```tsql Bad theme={null} IF ((@a = 1 AND @b > 2) OR (@c <> 3 AND @d <= 4)) AND @e IS NULL ... ``` ```tsql Fix theme={null} ```

String data types (\`char, varchar, nchar, nvarchar) default to a size of 1 if no size is specified in the declaration. For char and nchar this is confusing at best, but it is most probably a mistake for varchar and nvarchar.

This rule raises an issue when no size is specified for varchar or nvarchar\`.

```tsql Bad theme={null} DECLARE @myStr varchar; -- Noncompliant ``` ```tsql Fix theme={null} DECLARE @myStr varchar(255); ```

SQL Server can be tuned at \`PROCEDURE and TRIGGER levels thanks to several SET statements that change the current session handling of specific information.

This rule raises an issue when expected configuration is not set or is set with an unexpected value between the beginning of the PROCEDURE (or TRIGGER) definition and the first statement that is not a SET, IF or DECLARE\`.

```tsql Bad theme={null} CREATE PROCEDURE dbo.MyProc AS BEGIN SET ARITHABORT OFF; -- Noncompliant; ARITHABORT is OFF SELECT COUNT(*) FROM MY_TABLE END; ``` ```tsql Fix theme={null} ALTER PROCEDURE dbo.MyProc AS BEGIN SELECT COUNT(*) FROM MY_TABLE SET ARITHABORT ON; -- Noncompliant; ARITHABORT is not set at the beginning of the procedure definition [...] END; ```

A \`WHILE loop with at most one iteration is equivalent to the use of an IF statement to conditionally execute one piece of code. No developer expects to find such usage of a loop statement. If the initial intention of the author was really to conditionally execute one piece of code, an IF statement should be used in place.

At worst that was not the initial intention of the author and so the body of the loop should be fixed to use the nested RETURN, BREAK or THROW\` statements in a more appropriate way.

```tsql Bad theme={null} WHILE @cond -- noncompliant, loop only executes once BEGIN EXEC something; BREAK; END; ... WHILE @cond1 -- noncompliant, loop only executes once BEGIN IF @cond2 BEGIN EXEC something; BREAK; END ELSE BEGIN RETURN @value; END; END; ``` ```tsql Fix theme={null} IF @cond BEGIN EXEC something; BREAK; END; ... WHILE @cond BEGIN IF @cond2 BEGIN EXEC something; END ELSE BEGIN RETURN @value; END; END; ```

Theoretically, CASE, COALESCE, and IIF evaluate conditions sequentially, stopping with the first one that evaluates to TRUE. In reality, these expressions evaluate aggregate functions and non-native service calls such as CONTAIN and FREETEXT first, and then pass the results into the expression. That means that if you’re relying on short-circuit behavior to avoid runtime errors with these arguments, you will not get it. You can work around the issue by wrapping such calls in a subselect.

```tsql Bad theme={null} SELECT CASE WHEN @i = 0 THEN 1 ELSE MIN(1/@i) -- Noncompliant END; ``` ```tsql Fix theme={null} SELECT CASE WHEN @i = 0 THEN 1 ELSE (SELECT MIN(1/@i)) END; ```

Deprecated system tables and views are those that have been retained temporarily for backward compatibility, but which will eventually be removed from the language. In effect, deprecation announces a grace period to allow the smooth transition from the old features to the new ones.

This rule raises an issue when system tables or views are used. Catalog tables and views should be used instead.

```tsql Bad theme={null} SELECT name FROM syscolumns -- Noncompliant ``` ```tsql Fix theme={null} SELECT name FROM sys.columns ```

A CASE and a chain of IF/ELSE IF statements is evaluated from top to bottom. At most, only one branch will be executed: the first one with a condition that evaluates to true.

Therefore, duplicating a condition automatically leads to dead code. Usually, this is due to a copy/paste error. At best, it’s simply dead code and at worst, it’s a bug that is likely to induce further bugs as the code is maintained, and obviously it could lead to unexpected behavior.

```tsql Bad theme={null} IF @x = 1 PRINT 'A' ELSE IF @x = 2 PRINT 'B' ELSE IF @x = 1 -- Noncompliant PRINT 'C' SELECT CASE col1 WHEN 1 THEN 'A' WHEN 2 THEN 'B' WHEN 1 -- Noncompliant THEN 'C' ELSE 'D' END FROM table1 ``` ```tsql Fix theme={null} IF @x = 1 PRINT 'A' ELSE IF @x = 2 PRINT 'B' ELSE IF @x = 3 PRINT 'C' SELECT CASE col1 WHEN 1 THEN 'A' WHEN 2 THEN 'B' WHEN 3 THEN 'C' ELSE 'D' END FROM table1 ```

Jump statements (BREAK, CONTINUE, RETURN, GOTO, and THROW), move control flow out of the current code block. So any statements that come after a jump are dead code.

```tsql Bad theme={null} CREATE PROCEDURE AS BEGIN ... RETURN -- Noncompliant, remove following statements PRINT 'End' END ``` ```tsql Fix theme={null} CREATE PROCEDURE AS BEGIN ... RETURN END ```

Using \`TOP in a SELECT without ordering the results from which the "top" results are chosen will return a seemingly random set of rows, and is surely a mistake.

The same random behavior also occurs when using TOP in a DELETE, INSERT, UPDATE and MERGE\`.

```tsql Bad theme={null} SELECT TOP 10 -- Noncompliant selects 10 random rows fname, lname, city FROM people WHERE city IS NOT NULL; DELETE TOP (10) -- Noncompliant deletes 10 random rows FROM PurchaseOrder WHERE DueDate < '20020701'; ``` ```tsql Fix theme={null} SELECT TOP 10 fname, lname, city FROM people WHERE city IS NOT NULL ORDER BY birthdate; DELETE FROM PurchaseOrder WHERE OrderID IN ( SELECT TOP 10 OrderID FROM PurchaseOrder WHERE DueDate < '20020701' ORDER BY DueDate ASC ); ```

This rule applies whenever an \`IF statement is followed by one or more ELSE IF statements; the final ELSE IF should be followed by an ELSE statement.

The requirement for a final ELSE statement is defensive programming.

The ELSE statement should either take appropriate action or contain a suitable comment as to why no action is taken. This is consistent with the requirement to have a final ELSE clause in a CASE\` statement.

```tsql Bad theme={null} IF @x = 1 PRINT 'A' ELSE IF @x = 2 PRINT 'B' ELSE IF @x = 3 PRINT 'C' -- Noncompliant; final ELSE is missing ``` ```tsql Fix theme={null} IF @x = 1 PRINT 'A' ELSE IF @x = 2 PRINT 'B' ELSE IF @x = 3 PRINT 'C' ELSE PRINT 'Z' ```

When you add a new constraint to a table, (\`ALTER TABLE ... ADD CONSTRAINT ...), WITH CHECK is assumed by default, and existing data are automatically validated.

But when you disable/enable an existing constraint, WITH NOCHECK is assumed by default, and existing data are no longer trusted. In this case you will face an integrity issue that prevents some rows from being updated, and a performance issue because the query optimizer cannot trust this constraint anymore.

Of course, WITH CHECK is obviously preferred, but if NOCHECK behavior is desired, it should not be selected by omission, but specified explicitly because WITH NOCHECK has such a significant impact. By making NOCHECK explicit, the developer documents that this behavior has been selected on purpose.

Note: You can list the existing constraints that are in an untrusted state using:

SELECT \* FROM sys.foreign\_keys WHERE is\_not\_trusted = 1;

SELECT \* FROM sys.check\_constraints WHERE is\_not\_trusted = 1;\`

```tsql Bad theme={null} -- Create a trusted constraint ALTER TABLE users ADD CONSTRAINT max_age CHECK (age < 200) ; -- Disable the constraint ALTER TABLE users NOCHECK CONSTRAINT max_age; -- Enable the constraint ALTER TABLE users CHECK CONSTRAINT max_age; -- Noncompliant, 'WITH NOCHECK' is the default mode, but is it really intentional? ``` ```tsql Fix theme={null} -- Create a trusted constraint ALTER TABLE users ADD CONSTRAINT max_age CHECK (age < 200) ; -- Disable the constraint ALTER TABLE users NOCHECK CONSTRAINT max_age; -- Enable the constraint ALTER TABLE users WITH CHECK CHECK CONSTRAINT max_age; -- OR ALTER TABLE users WITH NOCHECK CHECK CONSTRAINT max_age; ```

The repetition of a prefix operator ( +, -, \~, or NOT) is usually a typo. The second operator invalidates the first one:

```tsql Bad theme={null} DECLARE @v1 INTEGER = 1 DECLARE @v2 INTEGER = - - -@v1 -- Noncompliant: equivalent to "-@v1" DECLARE @v3 INTEGER = ~~~@v1 -- Noncompliant: equivalent to "~@v1" DECLARE @v4 INTEGER = ++@v1 -- Noncompliant: equivalent to "@v1" IF NOT NOT @v1 <> @v2 -- Noncompliant BEGIN PRINT @msg END ``` ```tsql Fix theme={null} ```

Nested control flow statements IF...ELSE, WHILE and TRY...CATCH are often key ingredients in creating what’s known as "Spaghetti code". This code smell can make your program difficult to understand and maintain.

When numerous control structures are placed inside one another, the code becomes a tangled, complex web. This significantly reduces the code’s readability and maintainability, and it also complicates the testing process.

```tsql Bad theme={null} IF @flag1 = 1 -- Compliant - depth = 1 BEGIN IF @flag2 = 2 -- Compliant - depth = 2 BEGIN WHILE @var1 > 0 -- Compliant - depth = 3 BEGIN IF @flag3 = 3 -- Compliant - depth = 4, not exceeding the limit BEGIN IF @flag4 = 4 -- Noncompliant - depth = 5 BEGIN IF @flag5 = 5 -- Depth = 6, exceeding the limit, but issues are only reported on depth = 5 BREAK END END SET @var1 = @var1 - 1 END END END ``` ```tsql Fix theme={null} ```

A CATCH clause that only rethrows the caught exception has the same effect as omitting the CATCH altogether and letting it bubble up automatically.

```tsql Bad theme={null} BEGIN TRY SELECT 1/0; END TRY BEGIN CATCH -- Noncompliant THROW; END CATCH; ``` ```tsql Fix theme={null} SELECT 1/0; ```

While not technically incorrect, the omission of \`BEGIN...END can be misleading and may lead to the introduction of errors during maintenance.

In the following example, the two statements seem to be attached to the IF statement, but only the first one is, and somethingElse\` will always be executed:

```tsql Bad theme={null} IF @flag = 1 -- Noncompliant EXEC something; EXEC somethingElse; ``` ```tsql Fix theme={null} IF @flag = 1 BEGIN EXEC something; EXEC somethingElse; END; ```

In Transact-SQL, the semicolon statement terminator is in most cases optional. Therefore many developers don’t use semicolons. However, in some situations missing semicolons may yield insidious errors.

Semicolons are required by the ANSI standard, and Microsoft recommends the consistent usage of semicolons and might make semicolons mandatory in a future version of SQL Server. Also, semicolons make the code more portable.

```tsql Bad theme={null} BEGIN TRY BEGIN TRAN; SELECT 1/0 AS AnException; COMMIT; END TRY BEGIN CATCH SELECT ERROR_MESSAGE() -- Noncompliant; no exception will be thrown THROW END CATCH ``` ```tsql Fix theme={null} BEGIN TRY BEGIN TRAN; SELECT 1/0 AS AnException; COMMIT; END TRY BEGIN CATCH SELECT ERROR_MESSAGE(); THROW; END CATCH ```

Disabling "ANSI\_WARNINGS" and/or "ARITHABORT" in a procedure may silence errors, decrease performance, or block index creation.

From the documentation (ARITHABORT, ANSI\_WARNINGS), disabling these options could result in (among others):

\`SET ANSI\_WARNINGS OFF

CREATE, UPDATE, INSERT, and DELETE statements on tables with indexes on computed columns or indexed views will fail
No warning issued if null values appear in aggregate functions, such as SUM, AVG, MAX, MIN, STDEV, STDEVP, VAR, VARP, or COUNT
The divide-by-zero and arithmetic overflow errors cause null values to be returned, no roll-back

SET ARITHABORT OFF

It can negatively impact query optimization, leading to performance issues
An arithmetic, overflow, divide-by-zero, or domain error, during INSERT, UPDATE, or DELETE statement will cause SQL Server to insert or update a NULL value

This rule raises an issue when "ANSI\_WARNINGS" and/or "ARITHABORT" are set to OFF\` in a procedure.

```tsql Bad theme={null} CREATE PROCEDURE myProc AS BEGIN SET ANSI_WARNINGS OFF; -- Noncompliant SET ARITHABORT OFF; -- Noncompliant -- ... END ``` ```tsql Fix theme={null} CREATE PROCEDURE myProc AS BEGIN -- OK - Default value is ON END CREATE PROCEDURE myProc AS BEGIN SET ANSI_WARNINGS, ARITHABORT ON; END ```

Changing the configuration of database options ANSI\_NULLS, ANSI\_PADDING and CONCAT\_NULL\_YIELDS\_NULL is deprecated. The future versions of SQL Server will only support the ON value, and the SET statement for those options to OFF will eventually generate an error.

```tsql Bad theme={null} SET ANSI_NULLS OFF -- Noncompliant SELECT column1 FROM table1 WHERE id = NULL SET ANSI_PADDING OFF -- Noncompliant SET CONCAT_NULL_YIELDS_NULL ON -- Noncompliant SET ANSI_PADDING ON -- "ON" is ignored ``` ```tsql Fix theme={null} SELECT column1 FROM table1 WHERE id IS NULL ```

Declaring multiple variable on one line is difficult to read.

```tsql Bad theme={null} DECLARE @aaa AS INTEGER = 5, @bbb AS INTEGER = 42, @ccc AS CHAR(3) = 'foo' -- Noncompliant ``` ```tsql Fix theme={null} DECLARE @aaa AS INTEGER = 5 DECLARE @bbb AS INTEGER = 42 DECLARE @ccc AS CHAR(3) = 'foo' ```

Under the covers, Simple \`CASE expressions are evaluated as searched CASE expressions. That is,

CASE @foo
        WHEN 1 THEN 'a'
        WHEN 2 THEN 'b'

is actually evaluated as

CASE
        WHEN @foo = 1 THEN 'a'
        WHEN @foo = 2 THEN 'b'

In most situations the difference is inconsequential, but when the input expression isn’t fixed, for instance if RAND() is involved, it is likely to yield unexpected results. For that reason, it is better to evaluate the input expression once, assign it to a variable, and use the variable as the CASE's input expression.

This rule raises an issue when any of the following is used in a CASE input expression: RAND, NEWID, CRYPT\_GEN\_RANDOM\`.

```tsql Bad theme={null} CASE CONVERT(SMALLINT, RAND()*@foo) -- Noncompliant WHEN 1 THEN 'a' WHEN 2 THEN 'b' ``` ```tsql Fix theme={null} DECLARE @bar SMALLINT = CONVERT(SMALLINT, RAND()*@foo) CASE @bar WHEN 1 THEN 'a' WHEN 2 THEN 'b' ```

Functions or procedures with a long parameter list are difficult to use, as one must figure out the role of each parameter.

```tsql Bad theme={null} CREATE PROCEDURE dbo.SetCoordinates @x1 INT, @y1 INT, @z1 INT, @x2 INT, @y2 INT, @z2 INT AS -- ... ``` ```tsql Fix theme={null} CREATE PROCEDURE dbo.SetOrigin ( @x INT, @y INT, @z INT ) AS SELECT @x GO CREATE PROCEDURE dbo.SetSize ( @width INT, @height INT, @depth INT ) AS SELECT @width GO ```

As soon as a WHEN clause contains too much logic this highly decreases the readability of the overall expression. In such case, the content of the WHEN clause may be extracted into a dedicated function.

```tsql Bad theme={null} SELECT CASE column1 WHEN 1 THEN CASE column2 WHEN 'a' THEN -- Noncompliant, 7 lines till ELSE 'x' ELSE 'y' END ELSE 42 END FROM table1; ``` ```tsql Fix theme={null} SELECT CASE WHEN column1 = 1 AND column2 = 'a' THEN 'x' WHEN column1 = 1 THEN 'y' ELSE 42 END FROM table1; ```

Referencing a column by specifying the schema or the database is deprecated. It is retained temporarily for backward compatibility, but it will eventually be removed from the language. You should only use one part (column\_name) or two part (table\_name.column\_name) references.

```tsql Bad theme={null} SELECT dbo.table1.col1, -- Noncompliant, three-part column reference MY_DB.dbo.table1.col2 -- Noncompliant, four-part column reference FROM MY_DB.dbo.table1; SELECT dbo.table1.name, -- Noncompliant dbo.table2.name -- Noncompliant FROM dbo.table1 JOIN dbo.table2 ON dbo.table2.id = dbo.table1.id; -- Noncompliant ``` ```tsql Fix theme={null} SELECT col1, col2 FROM MY_DB.dbo.table1; SELECT table1.name, table2.name FROM dbo.table1 JOIN dbo.table2 ON table2.id = table1.id; ```

A FETCH statement fails when the number of variables does not match the number of columns selected in the CURSOR definition.

```tsql Bad theme={null} DECLARE c1 cursor FOR SELECT FirstName, LastName FROM customer; OPEN c1; FETCH NEXT FROM c1 INTO @Name; -- Noncompliant ``` ```tsql Fix theme={null} DECLARE c1 cursor FOR SELECT FirstName, LastName FROM customer; OPEN c1; FETCH NEXT FROM c1 INTO @FirstName, @LastName; ```

Having all branches of a CASE, IF or IIF chain with the same implementation indicates a problem.

In the following code:

```tsql Bad theme={null} IF @x < 25 -- Noncompliant PRINT 'A' ELSE IF @x < 10 PRINT 'A' ELSE PRINT 'A' SELECT CASE col1 -- Noncompliant WHEN 1 THEN 'A' WHEN 2 THEN 'A' ELSE 'A' END, IIF(col1 < 25, 'A', 'A') -- Noncompliant FROM table1 ``` ```tsql Fix theme={null} IF @x < 25 -- no issue, this could have been done on purpose to make the code more readable PRINT 'A' ELSE IF @x > 10 PRINT 'A' ```

@@IDENTITY returns the last identity column value created on a connection, regardless of the scope. That means it could return the last identity value you produced, or it could return a value generated by a user defined function or trigger, possibly one fired because of your insert. In order to access the last identity value created in your scope, use SCOPE\_IDENTITY() instead.

```tsql Bad theme={null} INSERT ... SET @id = @@IDENTITY -- Noncompliant ``` ```tsql Fix theme={null} INSERT ... SET @id = SCOPE_IDENTITY() ```

In a Zen-like manner, "NULL" is never equal to anything, even itself. Therefore comparisons using equality operators will always return \`False, even when the value actually IS NULL.

For that reason, comparison operators should never be used to make comparisons with NULL; IS NULL and IS NOT NULL\` should be used instead.

```tsql Bad theme={null} UPDATE books SET title = 'unknown' WHERE title = NULL -- Noncompliant ``` ```tsql Fix theme={null} UPDATE books SET title = 'unknown' WHERE title IS NULL ```

The requirement for a final ELSE clause is defensive programming. The clause should either take appropriate action, or contain a suitable comment as to why no action is taken.

```tsql Bad theme={null} SELECT CASE category WHEN 'A' THEN 21 WHEN 'B' THEN 33 END shipping_cost FROM product ``` ```tsql Fix theme={null} SELECT CASE category WHEN 'A' THEN 21 WHEN 'B' THEN 33 ELSE 42 END shipping_cost FROM product ```

When the line immediately after a conditional has neither an enclosing BEGIN…END block nor indentation, the intent of the code is unclear and perhaps not what is executed. Additionally, such code is confusing to maintainers.

```tsql Bad theme={null} IF @condition -- Noncompliant EXEC doTheThing; EXEC doTheOtherThing; -- Was the intent to call this function unconditionally? ``` ```tsql Fix theme={null} IF @condition -- Noncompliant -- EXEC doTheThing; EXEC doTheOtherThing; -- Was the intent to call this function conditionally? ```

There are valid cases for passing a variable multiple times into the same function or procedure call, but usually doing so is a mistake, and something else was intended for one of the arguments.

```tsql Bad theme={null} SET @result = dbo.MyAdd(@val1, @val1) -- Noncompliant ``` ```tsql Fix theme={null} SET @result = dbo.MyAdd(@val1, @val2) ```

It is needlessly complex to invert the result of a boolean comparison. The opposite comparison should be made instead.

```tsql Bad theme={null} IF NOT (@a = 2) -- Noncompliant BEGIN ... END IF NOT (@b < 10) -- Noncompliant BEGIN ... END ``` ```tsql Fix theme={null} IF @a <> 2 BEGIN ... END IF @b >= 10 BEGIN ... END ```

Putting multiple statements on a single line lowers the code readability and makes debugging the code more complex.

Unresolved directive in \ - include::\{noncompliant}\[]

Write one statement per line to improve readability.

Unresolved directive in \ - include::\{compliant}\[]

```tsql Bad theme={null} IF @x > 0 SET @x = 0; IF @y > 0 SET @y = 0; -- Noncompliant ``` ```tsql Fix theme={null} IF @x > 0 SET @x = 0; IF @y > 0 SET @y = 0; ```

When a SQL query is joining n tables (with n>=2), it is expected to have join conditions defined to determine on which columns these n tables should be joined. At minimum, for n joined tables, the SQL query should contain (n-1) join conditions involving all the joined table to avoid a full cartesian product between the rows of the n tables.

Not doing so will imply that too many rows will be returned. If this is not the case and unless this has been done on purpose, the SQL query should be reviewed and missing conditions should be added or useless tables should be removed from the SQL query.

This rule is raising no issue when the SQL query is involving CROSS JOIN, NATURAL JOIN statements.

```tsql Bad theme={null} select c.id, c.name, o.id, o.item_id, o.item_quantity from ORDERS o INNER JOIN CUSTOMERS c // Noncompliant; no JOIN condition at all select f.name, d.title, dlines.* from FOLDER f, DOCUMENTS d, DOC_LINES dlines // Noncompliant; missing at least one condition related to DOC_LINES in the WHERE clause WHERE f.id = d.folder_id ``` ```tsql Fix theme={null} select c.id, c.name, o.id, o.item_id, o.item_quantity from ORDERS o INNER JOIN CUSTOMERS c WHERE o.customer_id = c.id // Compliant select f.name, d.title, dlines.* from FOLDER f, DOCUMENTS d, DOC_LINES dlines WHERE f.id = d.folder_id AND d.id = dlines.document_id // Compliant ```

Nested code - blocks of code inside blocks of code - is eventually necessary, but increases complexity. This is why keeping the code as flat as possible, by avoiding unnecessary nesting, is considered a good practice.

Merging if statements when possible will decrease the nesting of the code and improve its readability.

```tsql Bad theme={null} IF something IF something_else -- Noncompliant -- ... ``` ```tsql Fix theme={null} IF something AND something_else -- Compliant -- ... ```

Each source file should start with a header stating file ownership and the license which must be used to distribute the application.

This rule must be fed with the header text that is expected at the beginning of every file.

```tsql Bad theme={null} -- -- SonarQube, open source software quality management tool. -- Copyright (C) 2008-2018 SonarSource -- mailto:contact AT sonarsource DOT com -- -- SonarQube is free software; you can redistribute it and/or -- modify it under the terms of the GNU Lesser General Public -- License as published by the Free Software Foundation; either -- version 3 of the License, or (at your option) any later version. -- -- SonarQube is distributed in the hope that it will be useful, -- but WITHOUT ANY WARRANTY; without even the implied warranty of -- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU -- Lesser General Public License for more details. -- -- You should have received a copy of the GNU Lesser General Public License -- along with this program; if not, write to the Free Software Foundation, -- Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. -- ``` ```tsql Fix theme={null} ```

The use of parentheses, even those not required to enforce a desired order of operations, can clarify the intent behind a piece of code. However, redundant pairs of parentheses could be misleading and should be removed.

```tsql Bad theme={null} DECLARE @x INT = (@y / 2 + 1); -- Compliant even if the parentheses are ignored IF (@x > 0) AND ((@x+@y > 0)) -- Noncompliant BEGIN -- ... END ``` ```tsql Fix theme={null} DECLARE @x INT = (@y / 2 + 1); IF (@x > 0) AND (@x+@y > 0) BEGIN -- ... END ```

UPDATE and DELETE statements should contain WHERE clauses to keep the modification of records under control. Otherwise unexpected data loss could result.

```tsql Bad theme={null} DELETE FROM countries UPDATE employee SET status = 'retired' FROM table1 AS employee ``` ```tsql Fix theme={null} TRUNCATE TABLE countries DELETE FROM countries WHERE CODE = @country_code UPDATE employee SET status = 'retired' FROM table1 AS employee WHERE age > @maxAge ```

A typical code smell known as unused function parameters refers to parameters declared in a function but not used anywhere within the function’s body. While this might seem harmless at first glance, it can lead to confusion and potential errors in your code. Disregarding the values passed to such parameters, the function’s behavior will be the same, but the programmer’s intention won’t be clearly expressed anymore. Therefore, removing function parameters that are not being utilized is considered best practice.

```tsql Bad theme={null} CREATE PROCEDURE SalesByCustomer @CustomerName nvarchar(50) -- Noncompliant AS SELECT c.customer_name, sum(ctr.amount) AS TotalAmount FROM customers c, contracts ctr WHERE c.customer_id = ctr.customer_id GROUP BY c.customer_name ORDER BY c.customer_name ``` ```tsql Fix theme={null} CREATE PROCEDURE SalesByCustomer @CustomerName nvarchar(50) AS SELECT c.customer_name, sum(ctr.amount) AS TotalAmount FROM customers c, contracts ctr WHERE c.customer_id = ctr.customer_id AND c.customer_name = @CustomerName GROUP BY c.customer_name ORDER BY c.customer_name ```

There is no reason to re-assign a variable to itself. Either this statement is redundant and should be removed, or the re-assignment is a mistake and some other value or variable was intended for the assignment instead.

```tsql Bad theme={null} DECLARE @a INT; DECLARE @b INT = 2; SET @a = @a; -- Noncompliant ``` ```tsql Fix theme={null} DECLARE @a INT; DECLARE @b INT = 2; SET @a = @b; ```

Executing code dynamically is security-sensitive. It has led in the past to the following vulnerabilities:

Some APIs enable the execution of dynamic code by providing it as strings at runtime. These APIs might be useful in some very specific meta-programming use-cases. However most of the time their use is frowned upon because they also increase the risk of maliciously Injected Code. Such attacks can either run on the server or in the client (example: XSS attack) and have a huge impact on an application’s security.

This rule marks for review each occurrence of such dynamic code execution. This rule does not detect code injections. It only highlights the use of APIs which should be used sparingly and very carefully.

```tsql Bad theme={null} CREATE PROCEDURE USER_BY_EMAIL(@email VARCHAR(255)) AS BEGIN EXEC sp_executesql 'USE AuthDB; SELECT id FROM user WHERE email = @user_email;', '@user_email VARCHAR(255)', @user_email = @email; END ``` ```tsql Fix theme={null} ```

If a label is declared but not used in the program, it can be considered as dead code and should therefore be removed.

This will improve maintainability as developers will not wonder what this label is used for.

```tsql Bad theme={null} label: -- Noncompliant PRINT 'hello world'; GO ``` ```tsql Fix theme={null} PRINT 'hello world'; GO ```

Nested ternaries are hard to read and can make the order of operations complex to understand.

Unresolved directive in \ - include::\{noncompliant}\[]

Instead, use another line to express the nested operation in a separate statement.

Unresolved directive in \ - include::\{compliant}\[]

```tsql Bad theme={null} SELECT IIF(@status = 'RUNNING', 'Running', IIF(@hasError, 'Failed', 'Succeeded')) -- Noncompliant ``` ```tsql Fix theme={null} IF @status = 'RUNNING' SELECT 'Running'; ELSE SELECT IIF(@hasError, 'Failed', 'Succeeded'); ```

Duplicated string literals make the process of refactoring complex and error-prone, as any change would need to be propagated on all occurrences.

```tsql Bad theme={null} IF @x='Yes' SELECT ... FROM ... WHERE field='Yes' ... ... IF @x='Yes' ... ``` ```tsql Fix theme={null} DECLARE @Yes VARCHAR(3) = 'Yes' IF @x=@Yes SELECT ... FROM ... WHERE field=@Yes ... ... IF @x=@Yes ... ```

SELECT \* should be avoided because it releases control of the returned columns and could therefore lead to errors and potentially to performance issues.

```tsql Bad theme={null} SELECT * -- Noncompliant FROM persons WHERE city = 'NEW YORK' ``` ```tsql Fix theme={null} SELECT firstname, lastname FROM persons WHERE city = 'NEW YORK' ```

Marking a parameter for output means that callers will expect its value to be updated with a result from the execution of the procedure. Failing to update the parameter before the procedure returns is surely an error.

```tsql Bad theme={null} CREATE PROCEDURE greet @Name varchar(20), @Greeting varchar(25) OUTPUT -- Noncompliant AS DECLARE @Message VARCHAR(45) SET @Message = N'Hello ' + RTRIM(@Name); PRINT @Message GO ``` ```tsql Fix theme={null} CREATE PROCEDURE greet @Name varchar(20), @Greeting varchar(25) OUTPUT AS SELECT @Greeting = N'Hello ' + RTRIM(@Name); GO ```

The MD5 algorithm and its successor, SHA-1, are no longer considered secure, because it is too easy to create hash collisions with them. That is, it takes too little computational effort to come up with a different input that produces the same MD5 or SHA-1 hash, and using the new, same-hash value gives an attacker the same access as if he had the originally-hashed value. This applies as well to the other Message-Digest algorithms: MD2, MD4, MD6, HAVAL-128, HMAC-MD5, DSA (which uses SHA-1), RIPEMD, RIPEMD-128, RIPEMD-160, HMACRIPEMD160.

Consider using safer alternatives, such as SHA-256, SHA-512 or SHA-3.

```tsql Bad theme={null} SELECT HASHBYTES('SHA1', MyColumn) FROM dbo.MyTable; ``` ```tsql Fix theme={null} SELECT HASHBYTES('SHA2_256', MyColumn) FROM dbo.MyTable; ```

Shared coding conventions allow teams to collaborate efficiently. This rule checks that all function names match a provided regular expression.

```tsql Bad theme={null} CREATE PROCEDURE sp_PrintMagicNumber AS BEGIN PRINT 42 END GO CREATE FUNCTION MagicNumber() RETURNS INT AS BEGIN RETURN 42 END GO ``` ```tsql Fix theme={null} CREATE PROCEDURE usp_PrintMagicNumber AS BEGIN PRINT 42 END GO CREATE FUNCTION fn_MagicNumber() RETURNS INT AS BEGIN RETURN 42 END GO ```

Hardcoding IP addresses is security-sensitive. It has led in the past to the following vulnerabilities:

Today’s services have an ever-changing architecture due to their scaling and redundancy needs. It is a mistake to think that a service will always have the same IP address. When it does change, the hardcoded IP will have to be modified too. This will have an impact on the product development, delivery, and deployment:

The developers will have to do a rapid fix every time this happens, instead of having an operation team change a configuration file.
It misleads to use the same address in every environment (dev, sys, qa, prod).

Last but not least it has an effect on application security. Attackers might be able to decompile the code and thereby discover a potentially sensitive address. They can perform a Denial of Service attack on the service, try to get access to the system, or try to spoof the IP address to bypass security checks. Such attacks can always be possible, but in the case of a hardcoded IP address solving the issue will take more time, which will increase an attack’s impact.

```tsql Bad theme={null} SET @IP = (SELECT ip_address FROM configuration); -- Compliant ``` ```tsql Fix theme={null} ```

Developers often use TODO tags to mark areas in the code where additional work or improvements are needed but are not implemented immediately. However, these TODO tags sometimes get overlooked or forgotten, leading to incomplete or unfinished code. This rule aims to identify and address unattended TODO tags to ensure a clean and maintainable codebase. This description explores why this is a problem and how it can be fixed to improve the overall code quality.

```tsql Bad theme={null} CREATE PROCEDURE doSomething AS BEGIN ... -- TODO something ... END GO ``` ```tsql Fix theme={null} ```

Tables without primary keys are largely unusable in a relational database because they cannot be joined to. A primary key should be specified at table creation to guarantee that all its records have primary key values.

```tsql Bad theme={null} CREATE TABLE employee ( employee_id INTEGER NOT NULL, first_name VARCHAR(42) NOT NULL, last_name VARCHAR(42) NOT NULL ); ``` ```tsql Fix theme={null} CREATE TABLE employee ( employee_id INTEGER NOT NULL PRIMARY KEY, first_name VARCHAR(42) NOT NULL, last_name VARCHAR(42) NOT NULL ); ```

Cryptographic hash algorithms such as MD2, MD4, MD5, MD6, HAVAL-128, HMAC-MD5, DSA (which uses SHA-1), RIPEMD, RIPEMD-128, RIPEMD-160, HMACRIPEMD160 and SHA-1 are no longer considered secure, because it is possible to have collisions (little computational effort is enough to find two or more different inputs that produce the same hash).

```tsql Bad theme={null} SELECT HASHBYTES('SHA2_512', MyColumn) FROM dbo.MyTable; ``` ```tsql Fix theme={null} ```

Having inconsistent indentation and omitting curly braces from a control structure, such as an if statement or for loop, is misleading and can induce bugs.

This rule raises an issue when the indentation of the lines after a control structure indicates an intent to include those lines in the block, but the omission of curly braces means the lines will be unconditionally executed once.

The following patterns are recognized:

Unresolved directive in \ - include::\{example}\[]

Note that this rule considers tab characters to be equivalent to 1 space. When mixing spaces and tabs, a code may look fine in one editor but be confusing in another configured differently.

```tsql Bad theme={null} IF (0=1) EXEC firstActionInBlock; EXEC secondAction; -- Noncompliant: secondAction is executed unconditionally EXEC thirdAction; ``` ```tsql Fix theme={null} IF (0=1) EXEC firstActionInBlock; EXEC secondAction; -- Noncompliant: secondAction is executed unconditionally ```

An unused local variable is a variable that has been declared but is not used anywhere in the block of code where it is defined. It is dead code, contributing to unnecessary complexity and leading to confusion when reading the code. Therefore, it should be removed from your code to maintain clarity and efficiency.

```tsql Bad theme={null} DECLARE @ClientId INT = 1, -- Noncompliant - @ClientId is unused @DeliveryId INT = 0; SELECT 'Date', 'Weight' FROM Claims WHERE Id = @DeliveryId; GO ``` ```tsql Fix theme={null} DECLARE @DeliveryId INT = 0; SELECT 'Date', 'Weight' FROM Claims WHERE Id = @DeliveryId; GO ```

A naming convention in software development is a set of guidelines for naming code elements like variables, functions, and classes. \{identifier\_capital\_plural} hold the meaning of the written code. Their names should be meaningful and follow a consistent and easily recognizable pattern. Adhering to a consistent naming convention helps to make the code more readable and understandable, which makes it easier to maintain and debug. It also ensures consistency in the code, especially when multiple developers are working on the same project.

This rule checks that \{identifier} names match a provided regular expression.

```tsql Bad theme={null} CREATE PROCEDURE proc1 @@var1 INT -- Noncompliant AS BEGIN DECLARE @var2@ INT; -- Noncompliant END ``` ```tsql Fix theme={null} CREATE PROCEDURE proc1 @var1 INT AS BEGIN DECLARE @var2 INT; END ```

Having two branches in an IF/ELSE IF chain with the same implementation is at best duplicate code, and at worst a coding error.

```tsql Bad theme={null} IF @SortOrder = 1 BEGIN SET @SortOrder = 0 SELECT LastName FROM Employees ORDER BY LastName END ELSE IF @SortOrder = 2 BEGIN SET @SortOrder = 0 SELECT LastName FROM Employees ORDER BY LastName -- Noncompliant END ELSE BEGIN SET @SortOrder = -1 SELECT LastName FROM Employees END GO ``` ```tsql Fix theme={null} IF (@SortOrder = 1 OR @SortOrder = 2) BEGIN SET @SortOrder = 0 SELECT LastName FROM Employees ORDER BY LastName END ELSE BEGIN SET @SortOrder = -1 SELECT LastName FROM Employees END GO ```