Sql Keyword Needs Whitespace

ID

csharp.sql_keyword_needs_whitespace

Severity

critical

Remediation Complexity

medium

Remediation Risk

low

Remediation Effort

medium

Resource

Code Smell

Language

CSharp

Tags

code_smell, sql, string

Description

Reports two concatenated string literals that fuse into a single glued token across an SQL keyword. "SELECT p.FirstName, p.LastName" + "FROM Person as p" produces …​LastNameFROM Person as p, which no database will parse.

Four conditions must hold together. Both operands are string literals joined by +, so their runtime text is known — an interpolated string or a variable makes the seam unreadable and is skipped. The characters on either side of the seam are both word characters, so the fragments really do fuse into one token; punctuation or a trailing space separates them and is fine. An SQL keyword sits at the seam. And the concatenation is a query at all, which is decided by a statement-opening keyword — SELECT, INSERT, UPDATE, DELETE, CREATE, ALTER, DROP, MERGE, EXEC, TRUNCATE or GRANT — appearing anywhere in it.

That last condition is what keeps prose out. Clause words such as AND, ORDER, SET and LEFT are ordinary English, so their presence says nothing: "Please review the order and" + "set the values on the left" is a message being built, and it has exactly the shape a missing space produces. A statement-opening word has no such reading. It is looked for across the whole concatenation rather than in the pair at the seam, because a long query is assembled fragment by fragment and its SELECT sits in the first one while the missing space can be at any later seam. A four-word minimum on the pair still separates a statement from an identifier assembled from "update" + "Set".

Rationale

The statement that reaches the database is not the statement written in the editor. Each fragment reads correctly on its own line, and the defect lives exactly at the seam between them, where the eye does not stop. Nothing happens at compile time — the concatenation is a valid string — so the first symptom is a syntax error raised by the driver at run time, quoting a statement that appears nowhere in the source.

That makes the failure both certain and late. The query is broken every single time it executes, yet it is discovered only when the code path runs; for a statement built on a branch that testing did not cover, the first execution can be in production. The fix is a single character, but only after someone has spent time matching a driver error against source that looks correct.

public class PersonRepository
{
    public string Broken()
    {
        return "SELECT p.FirstName, p.LastName" +
               "FROM Person as p";                    // FLAW - yields ...LastNameFROM
    }

    public string StillBroken()
    {
        return "SELECT * FROM" + "Person as p";        // FLAW - yields FROMPerson
    }

    public string Fine()
    {
        return "SELECT p.FirstName, p.LastName " +
               "FROM Person as p";                     // OK - the first fragment ends with a space
    }

    public string AlsoFine()
    {
        return "SELECT * FROM Person" +
               " WHERE p.Id = @id";                    // OK - the second fragment starts with a space
    }

    public string NotAQuery()
    {
        return "Please review the order and" +
               "set the values on the left";           // OK - prose, no statement keyword
    }
}

Remediation

Add the missing space to one of the fragments — conventionally at the end of each line, so that adding another line later cannot repeat the mistake. Better still, take the whitespace out of the question: a verbatim or raw string literal holds a multi-line statement without concatenation at all, and keeps the statement readable in the shape the database sees it. Whichever form is used, pass values as parameters rather than concatenating them in.

References