Code Analysys: Wrong rule 6405

I have the following piece of code in a program:

v_file_hdr is being used on line 11 and 12.

Why is it that Toad thinks it is not being referenced inside the program?

Because you are only using it in the file_hdr subprocedure.
If you reference it in the main parse_file procedure (where right now you only have null;), does the message go away?

hm, looks like it is a bug though. If you call the file_hdr proc in the main proc, Toad still gives the same message.

If it appears in the body (where null is), it goes away.
But if I called the sub procedure it remains.
I think is a bug, since it is used in the sub procedure. It is within the scope of the main program.

Probably can be fixed in the Xpath expression but I would have to learn how to.

And it also happens in you declare the variable before a cursor and use the variable in the cursor. All withing the definition section.
Again I think it is a bug.

Yes, I agree. I'll log it.

Thanks. Any chances of posting the xml_path code here?

I don't have a solution yet, but we can post that when there is a solution.

Thank you. have a good day.

Oh BTW, will this bug skew the results of the Analysis metrics?
Specifically the Maintainability Index.

It's a bug due to the nested procedure.

This code (re-typed from your screenshot and simplified) produces the bug:

CREATE OR REPLACE PROCEDURE pf
IS
   v_file_hdr    BOOLEAN := FALSE;
   v_file_hdr2   BOOLEAN := FALSE;

   PROCEDURE fh
   IS
   BEGIN
      IF (NOT v_file_hdr)
      THEN
         v_file_hdr := TRUE;
      ELSE
         NULL;
      END IF;
   END;
BEGIN
   NULL;
END;

This code works fine:

CREATE OR REPLACE PROCEDURE pf_simplified
IS
   v_file_hdr    BOOLEAN := FALSE;
   v_file_hdr2   BOOLEAN := FALSE;
BEGIN
   IF (NOT v_file_hdr)
   THEN
      v_file_hdr := TRUE;
   ELSE
      NULL;
   END IF;
END;

Time to look what's going wrong ... (logged as QP-3737)

Thanks
Andre

How about this XPath expression?

for $vd in //BLOCK/STMT/DECL_SET/(VAR_DECL, CONST_DECL)
return $vd[let $vd_id      := $vd/QNAME/IDENTIFIER,
               $vd_dbvalue := $vd_id/@dbvalue,
               $vd_offset  := $vd_id/@offset,
               $usages     := ($vd/parent::*/parent::*/following-sibling::*//QNAME[IDENTIFIER/@dbvalue = $vd_dbvalue]
                                                         [not((parent::VAR_DECL, parent::CONST_DECL))],
                               $vd/parent::*/(PROCEDURE_BODY, FUNCTION_BODY, CURSOR_DECL)/STMT//QNAME[IDENTIFIER/@dbvalue = $vd_dbvalue]
                                                         [not((parent::VAR_DECL, parent::CONST_DECL))])                           
           return
           (
                 if(not($usages))
                 then   true()
                 else   (: variable with the same name is used, but it's declared deeper :)
                        $usages[ancestor::STMT/BLOCK/STMT/DECL_SET/(VAR_DECL, CONST_DECL)/QNAME/IDENTIFIER[@dbvalue  = $vd_dbvalue]
                                                                                     [@offset > $vd_offset]]
           )
          ]/QNAME
1 Like

Nothing changed

Can you please paste your pl/sql code in text format? That would exclude typos when testing it.
Thanks,
Andre

Not sure why it didn't work. But while awaiting the code I reworked the expression by turning the logic upside down: rather than looking for usages for each definition, which is bloody difficult in the context of possible nested variable declarations with a same name, this new expression just lists all declarations found over the whole place, and then finds all usages, and each found usage will remove its declaration (nearest one going outwards) from the initial list. So we are left with the unused declarations.

//BLOCK/STMT/DECL_SET/(VAR_DECL, CONST_DECL)   (: initially all decls :)
except                                         (: remove decls which are being used :)
(for $vd in //BLOCK/STMT/DECL_SET/(VAR_DECL, CONST_DECL)
 return let $vd_id      := $vd/QNAME/IDENTIFIER,
            $vd_dbvalue := $vd_id/@dbvalue
        return (: usages :)
                   $vd/../(../following-sibling::*, (PROCEDURE_BODY, FUNCTION_BODY, CURSOR_DECL)/STMT)
                           //QNAME[IDENTIFIER/@dbvalue = $vd_dbvalue]
                                  [not((parent::VAR_DECL, parent::CONST_DECL))] /
               (: their nearest declaration :)
                   (ancestor::BLOCK[STMT/DECL_SET/(VAR_DECL, CONST_DECL)/QNAME/IDENTIFIER/@dbvalue = $vd_dbvalue] /
                           STMT/DECL_SET/(VAR_DECL, CONST_DECL)[QNAME/IDENTIFIER/@dbvalue = $vd_dbvalue])[1]
)

This expression is certainly not final, not fully tested, but it returned correct results on a series of examples with a variety of unused and also nested variables and procedures. However it will sure not work correctly with qualified variables (records, objects etc.). Not yet.

I assume you'll be creating a user defined rule (numbered as of 7000), it's the safest bet as the Quest supplied rules should not be touched, and will be overwritten on each Toad update.

Thanks for trying it out!
Andre

Sorry I was tied up yesterday.

I am not sure this works either.
This is the code:

set serveroutput on
declare
  l_file blob;
  l_dir_listing as_sftp.tp_dir_listing;
  l_not6405 boolean;
  procedure retbol is
  begin
    l_not6405 := true;
  end;
begin
  as_sftp.open_connection( i_host => 'sftp.host.com',i_port =>22 );
  as_sftp.login( i_user => 'user', i_password => 'demo' );
  as_sftp.set_log_level( 3 );
  if as_sftp.is_file_exists ('/tmp/OGI.txt') then
    as_sftp.delete_file ('/tmp/OGI.txt');
  end if;
  as_sftp.put_file( i_path => '/tmp/OGI.txt', i_directory =>'TX1242', i_filename => 'OGIt.txt' );
  as_sftp.close_connection;
end;

Running the Analysis with the stock rule says that the following three variables fall withing rule 6405:

  •   l_file blob;
    
  •   l_dir_listing as_sftp.tp_dir_listing;
    
  •   l_not6405 boolean;
    

The first two surely do. Not the last one. As it is being used in the private procedure.

Here is what I did to test your new expression.

  1. Created rule 7000 with the same info from current 6405. I did this to establish that Analysis will show both. After the Analyzing the code, rule 7000 did not show as a problem. Just 6405.
  2. Then I copied your expression into rule 7000. Same result as above
  3. Lastly I exported the rules, changed 6405 to use your expression. Imported the rules back in.
    Analysis kept showing variable l_not6405 as an unused variable.

Not sure I got it. What I did: I left rule 6405 (stock) unchanged, and created rule 7000 (my latest expression). Then I selected "All rules". With your code I'm getting hits on the three lines of variable declarations: 6405+7000 on first and second, and only 6405 on third. Hence 7000 works fine.
Or what am I missing?
Your Toad version is 13.3 or later?

Ah, I did not select all rules.
By selecting all rules, I can see now that 7000 is not noted on l_not6405 but is is noted on the other two unused variables.
So your expression seems to work. Al least in this small program.
Will this make it into the next version?
Thanks

Glad to hear that.
Next version: sure, but first I'll do extensive testing and casual optimizing for performance.
In the meantime you could replace the 6405 stock code by the new code, why not.

Thanks for your help,
Andre

I tried it, via import/export, but I must be doing something wrong. It seems it goes back to stock.
But I will check again when I have more time.
Thanks for your help.

1 Like