Fix/unprototyped procedure label hover

[geircodes]

fix(hover): a procedure label with no MAP prototype answers as the procedure

Branch fix/unprototyped-procedure-label-hover, based on origin/version-1.0.6 tip 6b760ce2.

What happened

Hovering the label of a standalone procedure that has no MAP prototype showed an unrelated symbol that happens to share its name, instead of the procedure. With an EQUATE of the same name reachable through the include chain, such as an ITEMIZE entry:

! shared.inc
  ITEMIZE
Worker    EQUATE
Other     EQUATE
  END

! module.clw
  MEMBER()
  INCLUDE('shared.inc'),ONCE
    MAP
        Helper(),LONG
    END

Helper        PROCEDURE()
  CODE
  RETURN 1

Worker        PROCEDURE        ! <- hover here
Count           LONG
  CODE
  Count = Helper()

hovering Worker on its own PROCEDURE line rendered the include's EQUATE:

Worker — EQUATE
🌍 Global constant
Worker EQUATE

The prototyped Helper on the line above was unaffected.

Root cause

ProcedureHoverResolver.resolveProcedureImplementation (HoverRouter step 5) builds its card only when it finds a MAP declaration, either in the current file or through the MEMBER parent:

const mapLocation = this.mapResolver.findMapDeclaration(procName, tokens, document, line);

if (!mapLocation) {
    const memberToken = TokenHelper.findMemberHeaderToken(tokens);
    if (memberToken?.referencedFile) {
        // ... search the MEMBER parent's MAP ...
        if (memberMapResult) {
            return this.formatter.formatProcedure(procName, memberMapResult.location, implLocation, document, position);
        }
    }
} else {
    return this.formatter.formatProcedure(procName, mapLocation, implLocation, document, position);
}

return null;

With no prototype anywhere it returned null, even though the line had already matched PROCEDURE_IMPLEMENTATION with the cursor on the name. Routing then continued through the later router steps and into HoverProvider's variable tiers, which match the bare name against anything reachable, and the include's EQUATE answered.

Passing null for the MAP declaration to HoverFormatter.formatProcedure would not have been enough on its own, because it returns null for a header-only card:

if (parts.length > 1) {
    return { contents: { kind: 'markdown', value: parts.join('\n') } };
}

return null;

With no prototype to preview, and the implementation link omitted because the cursor is already on the implementation, the header is the only part.

Fix

When no MAP declaration is found, the resolver now answers with the procedure itself and says that no prototype was found:

Worker (Procedure)

⚠️ No MAP prototype found

The note adds something the line itself doesn't show. formatProcedure is still tried first, so a doc comment on the procedure produces the normal card.

The fallback applies only when the hovered line carries a procedure token of subtype GlobalProcedure. A method declared in a CLASS or INTERFACE body matches the same PROCEDURE_IMPLEMENTATION regex, has no MAP entry either, and must keep reaching the method-declaration tier (step 7) as before. Without the guard, six existing local-CLASS method-declaration hover tests fail, which is what the guard protects.

Second commit: the label must be at column 0

A second, narrower gap sits in the same function. PROCEDURE_IMPLEMENTATION accepted leading whitespace on the label:

public static readonly PROCEDURE_IMPLEMENTATION = /^(\s*)(\w+)\s+(PROCEDURE|FUNCTION)/i;

A Clarion label must start at column 0 - confirmed elsewhere in this codebase against the real compiler, which desyncs entirely on an indented one. So an indented, non-compiling label still matched, and got the same full procedure card as a valid one on the line above it - MAP prototype preview and all, when one existed. The first commit's no-prototype fallback inherited the same gap by construction: an indented label with no MAP entry got the new procedure card too, just as wrongly.

Tightened the pattern to require the label at column 0 and updated the one call site's capture-group index (the leading-whitespace group is gone). An indented line no longer matches at all, so the resolver correctly has nothing to say about it.

For the no-MAP case with the label indented, the hover now falls through the router to a pre-existing, unrelated defect: a bare word can still resolve to a same-named EQUATE from an unrelated include regardless of what the hovered line actually is. That's not specific to a procedure declaration - it would equally mismatch any bare undeclared identifier - and is a broader change than this PR's scope, so it is called out here rather than folded into the diff.

Testing

New ProcedureHoverResolver.UnprototypedLabel.test.ts, written to real files on disk (a .clw that INCLUDEs an .inc holding the ITEMIZE):

1. Unprototyped label: the hover on Worker PROCEDURE is the procedure card with the missing-prototype note, and contains no EQUATE.
2. Prototyped label: Helper PROCEDURE() keeps its MAP declaration card, with the Helper(),LONG preview and no missing-prototype note.
3. Indented label, no MAP entry: the resolver's own answer no longer claims it's a procedure (no "(Procedure)" card, no missing-prototype note) - it does not assert the overall hover is empty, since the pre-existing EQUATE fallback above still answers it.
4. Indented label, has a MAP entry: no hover at all, matching an indented, non-compiling line.

Proved non-vacuous, both commits: with each fix stashed in turn and a clean rebuild, its own tests fail with the reported symptom, and pass again once restored. With both restored, the full server suite is 3381 passing, 0 failing, 3 pending, and the client suite is 307 passing.

Scope

Two source files (ProcedureHoverResolver.ts, ClarionPatterns.ts), one new test file.

Open questions, left rather than folded into this fix:

1. The fallback header reads **Worker** (Procedure). When formatProcedure builds a card, it replaces that with a scope badge (📦 Module Procedure or 🌍 Global Procedure) worked out from the file's first statement. The fallback doesn't compute that badge - adding it means moving the badge logic into a helper both paths share, touching HoverFormatter, which every procedure hover goes through.
2. The pre-existing global-EQUATE fallback (noted above) answering for an indented, non-compiling label - a bare-word cross-file match with no check on what the hovered line itself is.

[msarson]

Thanks, this is the right idea, and the column-0 change is good. One regression before it can go in. The new fallback trusts findMapDeclaration plus the MEMBER-parent lookup, and neither follows an INCLUDE inside a MAP (the #313 pattern). A procedure prototyped that way used to get its correct card through the later tiers. Now it gets "⚠️ No MAP prototype found":

! protos.inc
Worker        PROCEDURE
! mod.clw
  MEMBER()
  MAP
    INCLUDE('protos.inc')
  END
Worker        PROCEDURE      ! hover: base = card linking protos.inc:1, PR = "No MAP prototype found"
  CODE

Suggest calling mapResolver.findDeclarationInMapIncludes(procName, document, tokens) before the fallback, as the call-site path does at ProcedureHoverResolver.ts:107-129 (the walk result is cached), and formatting the card from a hit. Only warn when the walk also comes back empty. Please add this case to the test file too. Everything else checks out: merged onto the current tip the suite is 3423 / 0, and your tests go red without the fix.

[geircodes]

Thanks, you're right. I reproduced it before changing anything: on the base your fixture hovers with a card linking protos.inc:1, and on the branch it gets "No MAP prototype found".

One thing came up when I applied your suggestion as written: findDeclarationInMapIncludes alone does not fix your fixture. The walk only accepts a prototype inside a MODULE block (findModuleScopedProcDeclLine), and Worker PROCEDURE in protos.inc is bare, so the walk comes back empty and the warning still shows. On the base the card came from the variable tier after the router returned nothing. That's the same tier that answered with the unrelated EQUATE in the original report, and the fallback now answers ahead of it in both cases.

What e78ba4b4 does

The fallback runs the #313 walk before warning and formats the card from a hit the same way the call-site path does. The warning only shows when the walk also comes back empty.

For the bare form, the walk takes a new opt-in flag, acceptBarePrototype. An included file's text becomes part of the MAP that includes it, so a bare Name PROCEDURE there is a local prototype. With the flag set, a GlobalProcedure or MapProcedure token of that name also counts; method subtypes stay excluded. Only the fallback passes it. The call-site hover and Go to Implementation keep the MODULE-only check, and the flag is part of the walk-result cache key, so their results and cache entries are unchanged. The index fast path keeps its MODULE-scoped check in both modes, so a bare hit only ever comes from the walk, which starts from INCLUDEs inside a MAP.

For your fixture the hover is now:

Worker (Procedure)
Worker PROCEDURE
protos.inc:1

That's the same card router step 3 already gives the MODULE-block form.

Tests

Two cases added to ProcedureHoverResolver.UnprototypedLabel.test.ts:

1. Bare prototype in a MAP-included file (your fixture): the card links protos.inc:1 and has no warning. It fails with "No MAP prototype found" with the commit stashed, and fails the same way with the walk called without the flag.
2. MODULE-block prototype in a MAP-included file: a regression guard. It passes with or without the commit, because router step 3 already runs the strict walk and answers before the fallback is reached.

Server 3383 passing, 0 failing, 3 pending on the branch; client 307 passing. Merged onto the current version-1.0.6 tip without conflicts: 3437 passing, 0 failing, 3 pending.

One thing I noticed while tracing this, which the PR doesn't change: the call-site hover (Worker()) uses the same MODULE-only walk, so a call to a procedure prototyped bare in a MAP-included file also misses at step 3. With your fixture plus a Worker() call, the router returns nothing and the card comes from the variable tier. Happy to look at that separately if you think it's worth it.

[msarson]

Thanks, good catch on the MODULE-only walk, and the reasoning for the bare prototype is right: the INCLUDE's text is part of the MAP.

Verified merged onto the current tip (which now has #639 and #642 to #645): server 3471 / 0, client 307; the MAP-INCLUDE fixture gets its card again and your new test goes red without the fix. On the real solution every procedure label in a 523-label sample hovers exactly as on the base, none with the warning, and the hover/F12 agreement sweep did not move.

One small note, not blocking: the walk also starts from the MEMBER parent's MAP, so with the flag set, a bare prototype INCLUDEd into the PROGRAM file's MAP also counts. That declares a procedure implemented in the PROGRAM module, so in that (rare) case the card would link another module's prototype instead of warning. Worth a comment in the code if you touch it again.

Your call-site observation (Worker() missing a bare prototype in a MAP-included file) is real; I've opened an issue for it, it's yours if you'd like it.