Zero tolerance for out-of-scope references!

I received this question last week:

Hi Steven,
Here is a question that I have been pondering for sometime and haven't found a definitive answer to.  Perhaps you can shed some light.

When writing a procedure in PL/SQL, I pass it all the variables that it needs to do it's job.  Of course, it has access to any variables that are defined at the global level - but I rarely if ever use these (perhaps a few constants here and there). 

However, if I have a procedure that has a couple nested sub-procedures - should I follow the same pattern as, above?  It seems a bit silly to pass parameters to a proc and then re-pass those same parameters to a nested sub-proc when that nested sub-proc already has the variables in scope. 

So should the rule of thumb be: 

For procedures, pass all parameters in or use local variables as needed. Use global variables only in extraordinary circumstances. 

or 

For nested sub-procedures, don't pass variables in or out.  Use what is already in scope in the outer procedure. Only pass parameters to nested sub-procedures for special situations.  

Thanks in advance, Alan

And here is my response:

In an ideal world (looking around my basement office, can't find it), the answer is simple and clear:

NEVER code out-of-scope references in your subprograms. Use the parameter list to pass in and out any variables that are declared outside that subprogram.

There, all done. Nice and neat.

Well, except for reality. The reality is that it can be time consuming to extract all out-of-scope references in that new subprogram and convert them into parameters.

So check to see if your editor can help you do that!

But if you do take the time to carefully refactor your code when you created nested subprograms (and you should do both: created nested subprograms and carefully refactor your code), the results will be appreciated by all

The likelihood of you putting a bug into your code as you write drops. And there will be an even steeper decline in the chance of someone coming along later and inadvertently introducing a bug.

Example: instead of this...

CREATE OR REPLACE PROCEDURE my_proc AUTHID DEFINER
IS
   TYPE numbers_t IS TABLE OF NUMBER;

   l_numbers   numbers_t;

   PROCEDURE double_up
   IS
   BEGIN
      FOR indx IN l_numbers.FIRST .. l_numbers.LAST
      LOOP
         l_numbers (indx) := 2 * l_numbers (indx);
      END LOOP;
   END;
BEGIN
   l_numbers := numbers_t (1,2,3,4,5);

   double_up;
END;
/

do this...

CREATE OR REPLACE PROCEDURE my_proc AUTHID DEFINER
IS
   TYPE numbers_t IS TABLE OF NUMBER;

   l_numbers   numbers_t;

   PROCEDURE double_up (numbers_io IN OUT numbers_t)
   IS
   BEGIN
      FOR indx IN numbers_io.FIRST .. numbers_io.LAST
      LOOP
         numbers_io (indx) := 2 * numbers_io (indx);
      END LOOP;
   END;
BEGIN
   l_numbers := numbers_t (1,2,3,4,5);

   double_up (l_numbers);
END;
/

[4 September addition to post:  As you will see in the comments, Joao suggested using PL/Scope to help us identify out of scope references. I agreed. I even agreed as I was writing and publishing the original post. But I was lazy or busy or something. So no PL/Scope solution was included. Then I got less busy or less lazy or something. See what you think of the following.]

So we all agree - ? - that out of scope references should be avoided. It would be awfully nice if we could automatically detect all such references. If only, if only, PL/SQL gave us a way to do such analysis....

It does! It's called PL/Scope and it is a fantastic and still poorly understand (by me) and under-utilized (by everyone) code analysis tool built into PL/SQL. Read all about it here.

So suppose I execute the following statements (first, I enable collection of identifier data for PL/Scope, then I compile a package). Out of scope references marked in orange.

ALTER SESSION SET plscope_settings='identifiers:all'
/

CREATE OR REPLACE PACKAGE plscope_demo AUTHID DEFINER
IS
   g_global   NUMBER;

   PROCEDURE my_procedure (
      param1_in   IN INTEGER,
      param2      IN employees.last_name%TYPE);
END plscope_demo;
/

CREATE OR REPLACE PACKAGE BODY plscope_demo
IS
   g_private_global   NUMBER;

   PROCEDURE my_procedure (
      param1_in   IN INTEGER,
      param2      IN employees.last_name%TYPE)
   IS
      c_no_such       CONSTANT NUMBER := 100;
      l_local_variable         NUMBER;
      another_local_variable   DATE;
      
      FUNCTION return_local RETURN NUMBER
      IS
      BEGIN
         RETURN l_local_variable;
      END;
   BEGIN
      IF param1_in > l_local_variable
      THEN
         DBMS_OUTPUT.put_line (param2);
         g_global := 100;
      ELSE
         DBMS_OUTPUT.put_line (c_no_such);
      END IF;
      
      IF c_no_such IS NOT NULL
      THEN 
         g_private_global := 1;
      END IF;
   END my_procedure;
END plscope_demo;
/

I can then run the following query to identify variables, constants and exceptions that are referenced out of scope:

WITH declared_in
     AS (SELECT decl.NAME variable_name,
                ctxt.TYPE || '-' || ctxt.NAME declared_in
           FROM USER_IDENTIFIERS decl, USER_IDENTIFIERS ctxt
          WHERE     decl.USAGE_CONTEXT_ID = ctxt.USAGE_ID
                AND decl.TYPE IN ('CONSTANT',
                                  'VARIABLE',
                                  'EXCEPTION')
                AND decl.USAGE = 'DECLARATION'
                AND decl.OBJECT_NAME = ctxt.OBJECT_NAME
                AND decl.OBJECT_TYPE = ctxt.OBJECT_TYPE),
     vars_used_in
     AS (SELECT decl.NAME variable_name,
                ctxt.TYPE || '-' || ctxt.NAME referenced_in
           FROM USER_IDENTIFIERS decl, USER_IDENTIFIERS ctxt
          WHERE     decl.USAGE_CONTEXT_ID = ctxt.USAGE_ID
                AND decl.TYPE IN ('VARIABLE',
                                  'EXCEPTION')
                AND decl.USAGE IN ('REFERENCE', 'ASSIGNMENT')
                AND decl.OBJECT_NAME = ctxt.OBJECT_NAME
                AND decl.OBJECT_TYPE = ctxt.OBJECT_TYPE),
     constants_used_in 
     AS (SELECT decl.NAME variable_name,
                ctxt.TYPE || '-' || ctxt.NAME referenced_in
           FROM USER_IDENTIFIERS decl, USER_IDENTIFIERS ctxt
          WHERE     decl.USAGE_CONTEXT_ID = ctxt.USAGE_ID
                AND decl.TYPE = 'CONSTANT'
                AND decl.USAGE = 'REFERENCE'
                AND decl.OBJECT_NAME = ctxt.OBJECT_NAME
                AND decl.OBJECT_TYPE = ctxt.OBJECT_TYPE)
(SELECT * FROM vars_used_in UNION SELECT * FROM constants_used_in) 
MINUS
SELECT * FROM declared_in
/

Here's my output:

Now there is certainly more to be done here: add line numbers to make it easy to locate the out of scope ref, and more. 

But I feel, I feel, I feel a sense of laziness or is it busy-ness or something coming over me now, so perhaps someone else can take my starting script and finish it up.

Pretty please?