RPG Linter Shows Wrong Error For "endsr" In Global and Local Scope if Error Suppressed on "begsr"

[BrianGodsend]

If the RPG Lint option NoGlobalSubroutines is enabled (set to true), when a subroutine is defined in the global scope, the linter flags both the begsr and endsr line with the error "Subroutines should not be defined in the global scope.".

The @rpglint-skip-rules linter directive can be used to suppress this error. However, when the @rpglint-skip-rules comment is placed before only the begsr, the error message shown on the corresponding endsr is incorrectly changed to "Statement unexpected. Likely missing the equivalent `DCL..`/`BEG..`".

NOTE: The error "Subroutines should not be defined in procedures." error is flagged only on the begsr of subroutines defined in a subprocedure (assuming the NoLocalSubroutines setting is true). However, adding the linter override @rpglint-skip-rules before the begsr causes the same erroneous error to be flagged on the (local) endsr.

For example:

**FREE
*inLR = *ON;
return;

begsr errorOnGlobalBegSrAndEndSr;
  *in01 = not *in01;
endsr;

// @rpglint-skip-rules
begsr incorrectErrorOnGlobalEndSr;
  *in01 = not *in01;
endsr;

// @rpglint-skip-rules
begsr noGlobalErrorsReported;
  *in01 = not *in01;
// @rpglint-skip-rules
endsr;

dcl-proc test;
  dcl-pi *N;
  end-pi;

  begsr onlyBegSrIsFlaggedAsAnError;
    *in01 = not *in01;
  endsr;

  // @rpglint-skip-rules
  begsr incorrectErrorOnLocalEndSr;
    *in01 = not *in01;
  endsr;

  // @rpglint-skip-rules
  begsr noLocalErrorsReported;
    *in01 = not *in01;
  // @rpglint-skip-rules
  endsr;
end-proc;

I have the following observations:

1. As with the local subroutine error message, the global subroutine error messages should only flag the begsr line as being in error.

2. Suppressing the error on the begsr should not change the message on the endsr.

3. The erroneous error message on the endsr uses the word equivalent, but, (I think) the correct word should be either corresponding or matching.

4. There is no ending period on the erroneous error message on the endsr. That is, the error message "Statement unexpected. Likely missing the equivalent `DCL..`/`BEG..`" should be something like "Statement unexpected. Likely missing the matching `DCL..`/`BEG..`."

5. The subroutine *INZSR can only exist in the global scope. The *INZSR subroutine has the unique ability to allow the soft-initialization of variables that can then later be "reset" to the value they had at the end of *INZSR. While this may be a weak excuse to allow the *INZSR subroutine, I do come across this situation regularly in legacy code.

In considering the potential need to allow *INZSR, perhaps a linter configuration option, AllowedGlobalSubroutines, could be added to provide a list of allowed global subroutine names. The option would allow for a list of case-insensitive subroutine names that could be used in the global scope without triggering the error. If this option is added, a similar option, AllowedLocalSubroutines, should also be added to provide the list of local subroutine names that are allowed as well. The AllowedLocalSubroutines option might also support a special value (e.g., *ALLOWEDGLOBALSUBROUTINES or *GLOBAL) which would add all of the allowed global subroutine names to the list of allowed local subroutine names (basically, to allow the user to maintain 1 list of names).

---

For reference, I am running the latest version of VS Code on an up-to-date Windows 11 Pro 64-bit system. The Code for IBM i version is v2.0.2 and the RPGLE extension is v0.20.2.

---

I realize that I have now posted a few bug reports without ever properly saying THANK YOU. So, thank you for the work you are doing and for providing this wonderful, quality tool to the public.

Not to start a side-debate, let me just plainly state the obvious, SEU is becoming more and more painful to use. Being able to standardize on an IDE for most of my development needs is only made possible with your fantastic extension(s) to VS Code.

Again, thank you.

[worksofliam]

This is a great report - thanks! You are explicitly opening a bug here, as you are uncovering the truth that ``//@skip` rules are not to be fooled with. They are bad in general for any language that has them.

As for items 1-2: This is tricky. The reason we highlight them both is not necessarily to show errors, but because the auto fix feature will change it from BEGSR/ENDSR to Dcl-Proc and End-Proc. We can't do this without showing error messages. The next thing is that it'd be good if we didn't show errors on the ENDSR if we 'skip' the BEGSR, but sadly that's not the nature of how the skip works.

Items 3-4: I will look into doing a commit to fix my spelling and grammar 😄

Item 5~: Yes, we should really be allowing *INZSR (and possibly *PSSR) even if the rule is enabled. Thought we did this already but we actually don't. If you provide a working example of that, I will get on with it.

---

Thanks for your kind words. I just fixed your issues (239 and 240) and they should be coming out this week when we do a release of vscode-rpgle. Thank you for your support.

[worksofliam]

@BrianGodsend Since we are trying to collect lots of feedback and testimonials in one place, if you feel comfy, would you mind adding a reply to this thread on our forum? Thanks! https://github.com/orgs/halcyon-tech/discussions/350

[BrianGodsend]

Thank you for taking such quick action on this issues.

I was unaware (my complete ignorance) of the need of quick-fix to have both the begsr and endsr flagged so that the fix could be applied. I can see the restriction on not allowing a quick-fix to alter lines of code that are not flagged in error as being reasonable. Honestly, I would not want any lines of code changed without me first knowing that there was a problem with that line.

This also explains why the behavior of the global and local scoped subroutines error flagging works differently. In the local scope, I imagine there is no quick-fix, so, there is no need to flag the endsr in the local scope (one cannot create a subprocedure inside of a subprocedure, so, there is no quick-fix to be taken).

I think we are in complete agreement on the use of the "// @Skip" rules. While I like that they are there (really I do), I personally will not use them. To me, it seems like an admission of something fundamentally wrong with my code or perhaps a situation that needs to be considered by the linter. Regardless, I would rather see the error instead of hiding it. This is not said lightly, I have a hard time "seeing" errors and not wanting to fix them (I can be a bit OCD, for example, I cannot stand seeing a "full" recycle bin on my desktop. As a result, I have to change the "full" recycle bin icon to match the "empty" recycle bin icon, otherwise, I find that empty the recycle bin as soon as it has anything in it, effectively nullifying its purpose. I know, sick comes to my mind too).

[BrianGodsend]

I am not sure what example you are looking for. Following is a compile-able RPGLE source with the different conditions I could think up annotated. I took the liberty of assuming the RPG linter would be expanded to support a list of subroutine names to be allowed in the global and/or local scope. Let me know if you where looking for something else or if there is a better way I could write the example.

Note: On thinking about this more, because the RPG linter options will typically apply to all source in a given project (or even on a given IBM i), perhaps having a special value to include the allowed global scope routines is not all that useful, nor desirable.

**FREE
///
// Assuming the RPGLINT configuration with the following settings
//  and added support of new properties `AllowedGlobalSubroutines`
//  and `AllowedLocalSubroutines`:
//  {
//    "NoGlobalSubroutines": true,
//    "AllowedGlobalSubroutines": ["*Inzsr", "*pssr", "allowedInGlobal", "allowedInBoth"],
//    "NoLocalSubroutines": true,
//    "AllowedLocalSubroutines": ["*pssr", "allowedInLocal", "allowedInBoth"],
//  }
///
ctl-opt dftactgrp(*NO);

// INFSR() subroutines must be declared in global scope.
dcl-f QSOURCE usage(*OUTPUT) infsr(myInfSr) extdesc('QRPGLESRC');

dcl-s comment_t varchar(132) template;
dcl-s comment like(comment_t);

*inLR = *ON;
return;

// Should flag error on both begsr and endsr.
begsr globalTest1;
  comment = 'Global scoped subroutines flagged correctly on begsr and endsr.';
endsr;

// Should *not* flag error on begsr/endsr as the error has been suppressed.
// @rpglint-skip-rules
begsr globalTest2;
  comment = '@rule on begsr causes incorrect error on endsr line in global scope.';
endsr;

// Should *not* flag error on begsr/endsr as the error has been suppressed.
// ?? Should the skip rule be flagged as extraneous on the endsr ??
// @rpglint-skip-rules
begsr globalTest3;
  comment = '@rule on begsr and endsr suppress errors correctly in global scope.';
// @rpglint-skip-rules
endsr;

// Target of INFSR must be global, no entry in `AllowedGlobalScope` required to suppress.
begsr myInfSr;
  comment = 'Subroutines for INFSR() must be in global scope.';
endsr;

// *PSSR can be global or local; should require entry in appropriate "Allowed...Subroutines" to suppress.
begsr *PSSR;
  comment = '*PSSR subroutines can be created in global scope.';
endsr;

// *INZSR is only allowed in global; however, an entry should be required in "AllowedGlobalSubroutines" to suppress.
begsr *INZSR;
  comment = '*INZSR subroutines can be created only in global scope.';
endsr;

// Because the routine is in "AllowedGlobalSubroutines", no error should be flagged.
begsr allowedInGlobal;
  comment = 'Subroutine `allowedInGlobal` is allowed in global scope.';
endsr;

// Because the routine is in "AllowedGlobalSubroutines", no error should be flagged.
begsr allowedInBoth;
  comment = 'Subroutine `allowedInBoth` is either global or local scope.';
endsr;

// No entry in "AllowedGlobalSubroutines", so an error should be flagged.
begsr allowedInLocal;
  comment = 'ERROR: Subroutine `allowedInLocal` is not allowed in global scope.';
endsr;

dcl-proc test;
  dcl-pi *N;
  end-pi;

  dcl-s comment like(comment_t);

  return;

  // Should flag error on both begsr and endsr.
  begsr localTest1;
    comment = 'Local scoped subroutines flagged correctly on begsr only.';
  endsr;

  // Should *not* flag error on begsr/endsr as the error has been suppressed.
  // @rpglint-skip-rules
  begsr localTest2;
    comment = '@rule on begsr causes incorrect error on endsr line in local scope.';
  endsr;

  // Should *not* flag error on begsr/endsr as the error has been suppressed.
  // ?? Should the skip rule be flagged as extraneous on the endsr ??
  // @rpglint-skip-rules
  begsr localTest3;
    comment = '@rule on begsr and endsr suppress errors correctly in local scope.';
  // @rpglint-skip-rules
  endsr;

  // *PSSR can be global or local; should require entry in appropriate "Allowed...Subroutines" to suppress.
  begsr *PSSR;
    comment = '*PSSR subroutines can be created in local scope.';
  endsr;

  // No entry in "AllowedGlobalSubroutines", so an error should be flagged.
  begsr allowedInGlobal;
    comment = 'ERROR: Subroutine `allowedInGlobal` is not allowed in local scope.';
  endsr;

  // Because the routine is in "AllowedLocalSubroutines", no error should be flagged.
  begsr allowedInBoth;
    comment = 'Subroutine `allowedInBoth` is either global or local scope.';
  endsr;

  // Because the routine is in "AllowedLocalSubroutines", no error should be flagged.
  begsr allowedInLocal;
    comment = 'Subroutine `allowedInLocal` is allowed in local scope.';
  endsr;
end-proc;

[worksofliam]

Still a problem?

[BrianGodsend]

Yes. One must add the // @rpglint-skip-rules directive before each BEGSR and ENDSR. If the directive is added only before the BEGSR, the "Statement unexpected" error is always shown on the ENDSR.

NOTE: If the // @rpglint-skip-rules is placed only before the BEGSR, the "Statement unexpected" error is ALWAYS shown ... even if the NoGlobalSubroutines / NoLocalSubroutines is set to false.