codefori / vscode-rpgle

RPGLE language tools for VS Code
MIT License
39 stars 20 forks source link

Global Scope Subroutines with a Return Cannot Be Converted to a Subprocedure #247

Open BrianGodsend opened 1 year ago

BrianGodsend commented 1 year ago

When formatting RPGLE source in **FREE format, global scope subroutines are converted to subprocedures. This changes the behavior of a return from a global scope subroutine. When defined in a global scope subroutine, a return will immediately end execution of the program and return to the caller of the program. When the subroutine is converted to a subprocedure, the behavior of the return will change to simply return control to the calling routine of the subprocedure within the current program.

Also note, this change in behavior is done silently, no errors are caused by the change; after all, a return from a subprocedure is quite normal. As such, the program will compile just fine, but, the behavior of the program will be unpredictable (at best).

While it can be argued that code should never exit/drop out of a program from an arbitrary point, I am working with a package that uses an exit subroutine as a common exit point that will perform various clean-up tasks and often set return values. While in many cases, the conversion of the exit subroutine requires only a small amount of rework (and improvement), there are several instances where this simple change would require a complete redesign of the program.

My expected outcome would be:

1) A subroutine that contains a return can never be automatically changed to a subprocedure. 2) Additionally, any subroutine that calls a subroutine that contains a return can also never be converted to a subprocedure. This would have to be walked all the way up the chain. That is, if from the main line we execute subrA, which then executes subrB, which executes subrC, which executes SubrD that contains a return, none of these subroutines could be converted. 3) If all subroutines are not converted to subprocedures, the code would need to be rearranged, moving all of the subroutines up before the first subprocedure definition. 4) For any subroutine converted to a subprocedure, the corresponding exsr should be changed (drop the exsr and based on the RequiresParameter setting have parentheses added).

I realize the above is a huge ask; perhaps not even possible (I am not sure you can just start re-arranging sections of code). With that in mind, a simpler work-around would be to add an additional setting that could turn off the auto-conversion of the subroutines to subprocedures and add a quick-fix that would allow manual conversion one subroutine at a time (or all selected subroutine ... not sure what to do about the corresponding exsr statements). My thinking here is that I do want to move away from global scope subroutines. So, having the global subroutines flagged as an error (really a warning) is a good thing. However, given the code breaking nature of converting a subroutine that contains a return, this action cannot be automated.


For example, assume a fixed form RPGLE program of:

      * The `exitPgm` subroutine should *not* be convert
      *
     h dftactgrp(*NO)
      *****************************************************************
      *
     d globalField     s             10a
      *****************************************************************
     c/FREE
       exsr exitPgm;

        // These lines of code _should_ never be executed.
       *inH1 = *ON;
       *inLR = *ON;
       return;
       //**************************************************************

       begsr exitPgm;
         *inLR = *ON;
         return;
       endsr;
       //**************************************************************

      * Dummy procedure
     p dummyProc       b
     d                 pi
     d  parm1                        10a   const
      *----------------------------------------------------------------
      *
     d field1          s             10a
      *----------------------------------------------------------------
     c/FREE
       if parm1 = field1;
         *in01 = not *in01;
       endif;

       return;
     p                 e
      *****************************************************************

After converting the above code to **FREE form and then using the Format Document feature, the exitPgm subroutine will be erroneously converted to a subprocedure; thus changing the behavior of the program, allowing the previously unreachable section of code to be executed. The resulting code from the conversion and formatting is as follows:

**FREE
// This is an example program.

ctl-opt dftactgrp(*NO);
// ****************************************************************

dcl-s globalField  Char(10);
// ****************************************************************
exitPgm();

// These lines of code _should_ never be executed.
// HOWEVER, with the change of exitPgm from a subroutine to a subprocedure, these lines will now be executed.
*inH1 = *ON;
*inLR = *ON;
return;
// **************************************************************

dcl-proc exitPgm;
  *inLR = *ON;
  // ** THE BEHAVIOR OF THE RETURN HAS CHANGED WITH THE CHANGE FROM BEGSR TO DCl-PROC.
  return;
end-proc;
// **************************************************************

// Dummy procedure
dcl-proc dummyProc;
  dcl-pi *N;
    parm1          Char(10)   const;
    // ----------------------------------------------------------------

  end-pi;
  dcl-s field1       Char(10);
  // ----------------------------------------------------------------
  if parm1 = field1;
    *in01 = not *in01;
  endif;

  return;
end-proc;
// ****************************************************************

In short, the auto-conversion of subroutines to subprocedures should be enabled/disabled via a settings while still allowing the global subroutines to be flagged as an error and manually running a quick-fix on a selected section of code.