Code analysis rule 5903

Toad for Oracle & DevOps Toolkit
1

Hello all,

I am trying to use code analysis rule 5903.

Below I would identify as literals. Is there a different rule to use to identify them or would i need a custom rule?

AND RL.ROLE_ID = ORM.ROLE_ID;

    P_TRAN_STATUS := 'SEARCH EXECUTED SUCCESSFULLY';
    P_TRAN_DESC   := 'SEARCH EXECUTED SUCCESSFULLY FOR REQ: ' || P_REQ_ID;
EXCEPTION
    WHEN NO_DATA_FOUND
THEN
            P_TRAS_STATUS := 'SEARCH FAILED';
            P_TRAN_DESC   := 'DID NOT FIND REQ: ' || P_REQ_ID;

Thanks

Dave

2

Hi Dave,

Currently the 5903 rule has been implemented for queries only, as the rule’s title suggests. (“All queries (dynamic and fixed) must leverage bind variables for performance, security, and maintainability purposes.”)

The current XPath expression …

//SELECT_ITEM//LITERAL,
//(WHERE, HAVING)//LITERAL
[not(parent::EXPR[count(LITERAL) = 2 and LITERAL[1]/@value = LITERAL[2]/@value and @cat = ("eq", "gt", "lt", "ne")])]
(::: exclude comparisons between equal literal values, like 1 = 1, 'a' < 'a' ::)

… will work for literals inside select items and WHERE and HAVING clauses only. The tail of the expression excludes literals found in expressions such as WHERE 1 = 1, a widespread practice.

If you’d like to target all literals (in a custom rule 7000+) then you could catch them all using an expression as simple as

//LITERAL

but that’s quite rude. It may be better to compose it from more specific searches such as the RHS in assignments:

//ASSIGN/RHS/LITERAL

and grab literals from all default clauses:

//DEFAULT/LITERAL

which combined would give you a single rule:

//ASSIGN/RHS/LITERAL, //DEFAULT/LITERAL

Let me know what else you’re thinking of which could be added to this rule, I’ll be glad to help.

Thanks,
Andre

3

Cool thanks, so I was trying to create my own rule but the line numbers are not coming back.

Matches Case-Insensitively: The matches(@dbvalue, '^[CGLV]_', 'i') function uses the 'i' flag to ensure c_, C_, v_, and V_ are all correctly ignored.

Identify Three Specific Areas:

  1. General Assignments: Catches strings assigned to variables that aren't constants.

  2. Specific RHS Literals: Finds hard-coded values assigned to variables, provided they don't have your specific "safe" prefixes (C, G, L, V).

  3. Default Values: Catches hard-coded values in variable declarations (e.g., v_date DATE := '01-JAN-2026') unless the variable name starts with your chosen prefixes.

    //ASSIGN_STMT[not(ancestor::CONST_DECL)]//LITERAL_STRING
    |
    //ASSIGN[not(LHS//IDENTIFIER[matches(@dbvalue, ‘ [CGLV]_', 'i')])] /RHS/LITERAL*
    |
    //DEFAULT[not(ancestor::VARIABLE_DECLARATION/IDENTIFIER[matches(@dbvalue, '‘*[CGLV]_’, 'i')])] /LITERAL

I assume below is what’s being flagged but without a line number I am not sure.

image
image1342×193 17.2 KB

4

Items 1 and 2 seem overlapping, 1 catches them all, 2 is selective. For 1 did you mean variables that are constants, instead?

Well, for the fun, let’s assume that for now. Then the following seems to work for me:

//ASSIGN[RHS/LITERAL]/LHS/QNAME/IDENTIFIER[ancestor::BLOCK[1]/STMT/DECL_SET/CONST_DECL/QNAME/IDENTIFIER/@dbvalue = @dbvalue]
,
//ASSIGN[not(LHS//IDENTIFIER[matches(@dbvalue, '^CGLV')])]/RHS/LITERAL
,
//VAR_DECL/DEFAULT[not(../QNAME/IDENTIFIER[matches(@dbvalue, '^CGLV')])]/LITERAL

Please don’t use | to combine results, use comma instead.

The first expression (item 1) looks up the constant declaration in the current block only ( BLOCK[1] ).

You can also rewrite expression 1 to obtain the following:

declare function ConstantDecl()
{
  ./(
      let $name := ./IDENTIFIER/@dbvalue
      return ./ancestor::BLOCK[1][STMT/DECL_SET/CONST_DECL/QNAME/IDENTIFIER/@dbvalue = $name]
    )
}

//ASSIGN[RHS/LITERAL]/LHS/QNAME[ConstantDecl()]
,
//ASSIGN[not(LHS//IDENTIFIER[matches(@dbvalue, '^CGLV')])]/RHS/LITERAL
,
//VAR_DECL/DEFAULT[not(../QNAME/IDENTIFIER[matches(@dbvalue, '^CGLV')])]/LITERAL

which decomposes things, hence should be a bit more clear. That will also ease future code expansion.

Andre