fix(cobol): globals drops WORKING-STORAGE header (#2805); args counts operands not clauses (#2804)

gitgalaxy/core/detector.py

@@ -2135,6 +2135,19 @@ def coding_analysis(
 
                     c = len(hit_indices)
 
+                    # #2804: COBOL's `args` rule captures a whole USING/RETURNING
+                    # operand list in one group, so a bare match count reads
+                    # CLAUSES, not declared parameters -- `USING A, B, C` scored 1,
+                    # the same as `USING A`. Count the operands the clauses actually
+                    # declare instead (a real linkage section passes several). The
+                    # per-clause `hit_indices`/`spatial_map` entries are left as-is
+                    # (they mark WHERE the clauses are, for line locations); only the
+                    # tallied magnitude switches from clause-count to operand-count.
+                    if seg_lang == "cobol" and rule_name == "args" and matches:
+                        c = sum(
+                            self._count_cobol_using_operands(m.group(1) if m.lastindex else m.group(0)) for m in matches
+                        )
+
                     t_elapsed = time.perf_counter() - t_rule_start
 
                     if regex_telemetry is not None:
@@ -2872,6 +2885,37 @@ def _count_assembly_register_args(self, matches: list[str]) -> int:
                 registers.add(m.group(4).lower() + "x")
         return len(registers)
 
+    # #2804: the phrase words a COBOL `USING`/`RETURNING` operand list can carry
+    # BETWEEN the clause keyword and each data-name -- `BY REFERENCE`/`BY CONTENT`/
+    # `BY VALUE` passing-mode prefixes. They are not operands and must not be counted
+    # as parameters. See `_count_cobol_using_operands`.
+    _COBOL_ARG_PHRASE_WORDS: ClassVar[frozenset[str]] = frozenset({"BY", "REFERENCE", "CONTENT", "VALUE"})
+
+    def _count_cobol_using_operands(self, captured: str) -> int:
+        """Count the DECLARED PARAMETERS in one COBOL `USING`/`RETURNING` operand
+        list, not the clause itself. COBOL's `args` rule captures the whole
+        comma-or-space-separated operand list of a clause in a single group
+        (`PROCEDURE DIVISION USING A, B, C` and `PROCEDURE DIVISION USING A` both
+        yield exactly one `finditer` match), so the file-level signal -- a bare
+        `len(finditer(...))` -- counted CLAUSES and read a 3-operand linkage the
+        same as a 1-operand one (#2804 Ground 2). Splits the captured list on
+        commas and whitespace and counts the data-name tokens, dropping the
+        `BY REFERENCE`/`BY CONTENT`/`BY VALUE` passing-mode phrase words
+        (`_COBOL_ARG_PHRASE_WORDS`) which sit between the clause keyword and each
+        operand but are not themselves parameters. `USING A, B, C` -> 3,
+        `USING BY REFERENCE A BY CONTENT B` -> 2, `USING ARGV-BLOCK` -> 1, an empty
+        capture -> 0. This is the same "one match, real operand count" shape as
+        `_count_agc_register_args`/`_count_assembly_register_args`, and generalises
+        to real Enterprise COBOL, whose linkage sections routinely pass several
+        operands and previously scored 1."""
+        if not captured:
+            return 0
+        count = 0
+        for token in re.split(r"[,\s]+", captured.strip()):
+            if token and token.upper() not in self._COBOL_ARG_PHRASE_WORDS:
+                count += 1
+        return count
+
     # galaxyscope:ignore sec_high_risk_execution
 
     # #1973/#2483: per-language line-continuation marker used to extend a Mode A

gitgalaxy/standards/language_standards/languages/cobol.py

@@ -54,8 +54,63 @@
         # with a negative lookahead excluding "RETURNING" from the
         # parameter-name alternative, so `finditer` now correctly yields
         # two separate matches (USING -> WS-A, RETURNING -> WS-B).
+        # BUG FIX #2804: the inter-operand separator was `\s*,?`, which
+        # consumes trailing space THEN an optional comma -- so after
+        # `WS-A,` the next operand iteration began on the space in
+        # `WS-A, WS-B` and its leading `[A-Z0-9_-]+` failed immediately,
+        # stopping the capture at the first operand. `USING WS-A, WS-B,
+        # WS-C` (comma-SPACE separated -- the dominant real formatting)
+        # captured only `WS-A,` and read as one parameter, silently
+        # dropping WS-B/WS-C (they carry no USING/RETURNING anchor of
+        # their own, so `finditer` never sees them either).
+        #
+        # The separator is `(?:[ \t\r\n]*,[ \t\r\n]*|[ \t]+)?`: an operand
+        # list may cross a newline ONLY when the operands are joined by a
+        # comma (COBOL's optional separator comma), and may otherwise be
+        # separated by inline horizontal whitespace on the same line. This
+        # is deliberate. A COBOL statement is terminated by a period, not a
+        # newline, and a period is frequently omitted until the end of a
+        # sentence -- so `CALL 'X' USING A , B , C` (no closing period)
+        # followed by a blank line and an `IF ...` statement has no lexical
+        # boundary a flat pattern can see. An earlier `[\s,]*` separator
+        # that crossed newlines unconditionally swept that trailing `IF
+        # WS-SEVERITY-N` straight into the operand list (real regression:
+        # carddemo CSUTLDPY.cpy read 5 operands for a 3-operand CALL).
+        # Requiring a comma to cross a line stops at the real list end for
+        # the common continued form (leading-comma continuation, which is
+        # exactly how carddemo wraps its USING lists) without over-reading
+        # into the next statement; a rarer newline-separated list with NO
+        # commas at all is conservatively counted from its first operand
+        # rather than risk swallowing the following verb.
+        #
+        # The same-line branch is `[ \t]{1,4}`, not `[ \t]+`, on purpose.
+        # Fixed-format COBOL carries an 8-char program identifier in
+        # columns 73-80 of every line (e.g. NIST CCVS `IC2014.2`), and a
+        # space-separated `USING DN1 DN2 DN3 DN4` with no terminating
+        # period runs a long whitespace gap straight to that column -- an
+        # unbounded `[ \t]+` bridged it and captured the sequence id as a
+        # bogus extra operand (real regression: IC2014.2.cbl read 45 for a
+        # 44-operand file). A real inter-operand gap on one physical line
+        # is one space (occasionally a few); capping the same-line run at 4
+        # blocks the ~30-space identification-area bridge while leaving
+        # normal spacing untouched. Wider single-line alignment (rare for a
+        # USING list -- multi-operand lists wrap with commas) is
+        # conservatively counted short, never over-read. Column-73 blanking
+        # is not done here because it is unsafe without fixed/free-format
+        # detection (free-format lines legitimately run past column 72).
+        #
+        # Both branches of the separator are disjoint from the operand's
+        # own `[A-Z0-9_-]+`, the operand is non-nullable, and the
+        # repetition stays `{0,20}`-bounded -- so this is still ReDoS-proof
+        # (no overlapping-quantifier ambiguity). The `(?!RETURNING\b)`
+        # boundary is unchanged, so `USING A RETURNING B` still yields two
+        # separate matches. The captured list is turned into a real
+        # parameter COUNT (operands, not clauses) by detector.py's
+        # `_count_cobol_using_operands`.
         "args": re.compile(
-            r"\b(?:USING|RETURNING)\s+((?:(?:BY\s+(?:REFERENCE|CONTENT|VALUE)\s+)?(?!RETURNING\b)[A-Z0-9_-]+\s*,?){0,20})",
+            r"\b(?:USING|RETURNING)\s+"
+            r"((?:(?:BY\s+(?:REFERENCE|CONTENT|VALUE)\s+)?(?!RETURNING\b)"
+            r"[A-Z0-9_-]+(?:[ \t\r\n]*,[ \t\r\n]*|[ \t]{1,4})?){0,20})",
             re.I,
         ),
         # 3. linear: Sequential I/O & Network Boundaries. Structural boundaries defining straight-line execution flow.
@@ -323,7 +378,21 @@
         # 17. closures: Closures / Anonymous Functions. (COBOL lacks native lambdas).
         "closures": None,
         # 18. globals: Global / Shared State. Global storage and external linkages.
-        "globals": re.compile(r"\b(WORKING-STORAGE\s+SECTION|COMMON|GLOBAL|EXTERNAL)\b", re.I),
+        # BUG FIX #2805: dropped `WORKING-STORAGE SECTION` from this rule. The
+        # item-level clauses (`GLOBAL`, `EXTERNAL`, `COMMON`) are what mark shared
+        # state -- `GLOBAL`/`EXTERNAL` publish one data item outside the program,
+        # `COMMON` a program to its siblings. `WORKING-STORAGE SECTION` is a REGION
+        # HEADER: it opens the area where a program declares its own state, and it
+        # is mandatory in every COBOL program that declares any. Counting it stacked
+        # a phantom `globals >= 1` on every real COBOL file with a working-storage
+        # section whether or not it shares anything -- an unconditional per-file +1
+        # (a program with a working-storage section and five `GLOBAL` items scored 6,
+        # one with a section and nothing global scored 1). It also inflated the
+        # `encapsulation_ratio` numerator (`1 - globals / (core_var_decl + globals)`)
+        # on that same file. Working-storage is program-private static storage, not
+        # global surface; `SECTION` still counts as a structural boundary via the
+        # `structural_boundaries` rule above, so no structural signal is lost.
+        "globals": re.compile(r"\b(COMMON|GLOBAL|EXTERNAL)\b", re.I),
         # 19. decorators: Decorators / Annotations. (COBOL uses compiler directives).
         "decorators": re.compile(
             r"^(?:[0-9a-zA-Z \t]{6}[ \-]?)?[ \t]*>>\s*(?:IF|ELSE|END-IF|DEFINE|CALL-CONVENTION)",

tests/extraction/languages/test_cobol_strict.py

@@ -79,7 +79,10 @@ def test_cobol_ghost_satellite_prevention():
     ("test", "ASSERT X = Y", "MOVE X TO Y."),
     ("concurrency", "EXEC CICS ENQ END-EXEC", "MOVE X TO Y."),
     ("ui_framework", "SCREEN SECTION.", "MOVE X TO Y."),
-    ("globals", "WORKING-STORAGE SECTION.", "MOVE X TO Y."),
+    # #2805: item-level GLOBAL/EXTERNAL/COMMON are the real shared-state surface;
+    # the WORKING-STORAGE SECTION region header (now the negative) is program-
+    # private static storage and no longer counts as globals.
+    ("globals", "01 WS-SHARED PIC 9 GLOBAL.", "WORKING-STORAGE SECTION."),
     ("decorators", "      >>DEFINE VAR", "MOVE X TO Y."),
     ("generics", "CLASS-ID. FOO USING BAR", "MOVE X TO Y."),
     ("scientific", "FUNCTION SQRT(X)", "MOVE X TO Y."),
@@ -559,3 +562,60 @@ def test_cobol_api_contract_2730():
     # Not declarations -- must not match.
     assert not api.search("           END-CALL."), "END-CALL scope terminator"
     assert not api.search("           CALL 'CEE3ABD'."), "CALL is a call site"
+
+
+# ==============================================================================
+# #2804: args counts declared PARAMETERS, not USING/RETURNING clauses.
+# ==============================================================================
+@pytest.mark.parametrize(
+    "clause,expected_operands",
+    [
+        # comma-SPACE separated -- the dominant real formatting. Before #2804
+        # the regex captured only `WS-A,` and the whole list read as 1 operand.
+        ("PROCEDURE DIVISION USING WS-A, WS-B, WS-C.", 3),
+        # space separated (no commas) -- already captured, still 3.
+        ("PROCEDURE DIVISION USING WS-A WS-B WS-C.", 3),
+        # comma, no space.
+        ("PROCEDURE DIVISION USING WS-A,WS-B,WS-C.", 3),
+        # leading-comma continuation across lines (how carddemo wraps its
+        # USING lists) -- crosses newlines because operands are comma-joined.
+        ("CALL 'SUB' USING WS-A\n    , WS-B\n    , WS-C", 3),
+        # over-capture guard (carddemo CSUTLDPY.cpy): a comma-continued list
+        # with NO closing period, followed by a blank line and an IF verb,
+        # must stop at the last operand and NOT swallow `IF WS-SEVERITY-N`.
+        ("USING WS-A\n    , WS-B\n    , WS-C\n\n    IF WS-SEVERITY-N = 0", 3),
+        # fixed-format col-73 guard (NIST CCVS IC2014.2.cbl): a space-
+        # separated list with no terminating period must not bridge the wide
+        # gap to the columns-73-80 program identifier (`IC2014`) and count it.
+        ("CALL ID1 USING DN1 DN2 DN3 DN4" + " " * 31 + "IC2014", 4),
+        # BY REFERENCE/CONTENT/VALUE are passing-mode phrases, not operands.
+        ("CALL 'SUB' USING BY REFERENCE WS-A BY CONTENT WS-B.", 2),
+        ("CALL 'SUB' USING BY VALUE WS-A.", 1),
+        # single operand.
+        ("PROCEDURE DIVISION USING ARGV-BLOCK.", 1),
+        # USING + RETURNING is two clauses -> two matches -> 1 param + 1 return.
+        ("PROCEDURE DIVISION USING WS-A, WS-B RETURNING WS-R.", 3),
+    ],
+)
+def test_cobol_args_counts_operands_not_clauses(clause, expected_operands):
+    """The file-level `args` signal must count the DECLARED PARAMETERS a
+    USING/RETURNING list contains, not the number of clauses. The regex
+    captures the whole operand list per clause (#2804 regex fix) and
+    `_count_cobol_using_operands` turns each capture into an operand count,
+    dropping the BY REFERENCE/CONTENT/VALUE passing-mode phrase words."""
+    from gitgalaxy.core.detector import StructuralExtractor
+
+    counter = StructuralExtractor.__new__(StructuralExtractor)
+    args_pattern = COBOL_RULES["args"]
+    total = sum(
+        counter._count_cobol_using_operands(m.group(1) if m.lastindex else m.group(0))
+        for m in args_pattern.finditer(clause)
+    )
+    assert total == expected_operands, f"{clause!r}: expected {expected_operands} operands, got {total}"
+
+
+def test_cobol_args_regex_is_redos_immune():
+    """The widened inter-operand separator (`[\\s,]*`, #2804) keeps the two
+    character classes disjoint and the repetition `{0,20}`-bounded, so no
+    overlapping-quantifier ambiguity is introduced."""
+    assert_redos_immune(COBOL_RULES["args"], "USING " + "WS-A, " * 40)