Skip to content

Fix/colon prefix dot access hover - #386

Open
geircodes wants to merge 2 commits into
msarson:version-1.0.1from
geircodes:fix/colon-prefix-dot-access-hover
Open

Fix/colon prefix dot access hover#386
geircodes wants to merge 2 commits into
msarson:version-1.0.1from
geircodes:fix/colon-prefix-dot-access-hover

Conversation

@geircodes

Copy link
Copy Markdown

Summary

Clarion allows a literal : inside an identifier — a common naming convention for globals (GLOB:Thing), separate from GROUP/QUEUE PRE(...) prefixing. Hovering a method reached via dot access on such a variable produced no hover at all; hovering a property the same way happened to still show something, which made this look property-safe / method-only at first. It isn't — both are broken the same way, one just has a lucky fallback masking it.

MyGlobals   CLASS
Setting       LONG
DoWork          PROCEDURE(LONG p1),LONG
            END

GLOB:Config &MyGlobals

  CODE
  GLOB:Config.Setting = 1        ! hover on Setting: happens to still show something
  GLOB:Config.DoWork(1)          ! hover on DoWork: nothing

Root cause

Two independent spots drop the colon-prefixed segment before the receiver's type ever gets resolved:

  1. TokenHelper.getWordRangeAtPosition — the forward scan (finding the word under the cursor) explicitly allows : (includeColons), but the backward scana few lines below it (walking left to include the prefix before a dot, e.g. for
    GLOB:Config.DoWork) only allows [A-Za-z0-9_]. For a colon-containing prefix, this produces the wrong compound hover "word" (Config.DoWork instead of GLOB:Config.DoWork).
  2. StructureFieldResolver.resolveFieldAccessstructureNameMatch = beforeDot.match(/(\w+)\s*$/) has the same gap: \w excludes :, so the receiver name extracted for variable-type resolution is truncated to Config, which doesn't
    match the actual declaration GLOB:Config. resolveVariableType then fails and the whole "look up this member in its class" path never runs.

Either bug alone is enough to break it. Why properties still "worked": a property access can incidentally be caught by a later, unrelated bare-name fallback further down the hover resolution ladder (a plain "does any symbol have this exact name
anywhere reachable" search) — which finds the property by coincidence, not because the receiver's type was correctly resolved. A method call has no equivalent fallback, so it just returns null. That's also why this could silently point at the wrong declaration if two unrelated classes happened to declare a same-named property — the accidental match isn't scoped to the receiver's real type at all.

Fixed both spots to allow : consistently, matching how the rest of the file already treats colon-containing identifiers.

Possibly related earlier work

Not a regression of either of these as far as I can tell — cited for anyone triaging future colon/prefix hover reports, since this is the third or so bug in this general area:

  • v0.7.1 (CHANGELOG-full.md) — "PREFIX and Structure Field Access
    Improvements" added dedicated support for LOC:MyVar / MyGroup.MyVar / bare
    access for GROUP PRE(...) fields, plus a companion "word extraction for dot
    notation" fix in this same getWordRangeAtPosition area. That work covers
    structure-PRE prefixes specifically; a colon that's simply part of a global's own
    name (no PRE(...) involved) falls through the same code but isn't a PRE-prefix
    case, so it wasn't covered.
  • [Unreleased] / 0.8.4 — "Fixed: Colon-handling in labels" (F12 now works on
    BRW1::View:Browse-style labels) is the same symptom family (colon dropped
    during identifier handling) but in the Go to Definition label-search path, not
    hover's dot-access chain.

Might be worth a "colon-in-identifiers" sweep across the remaining resolvers at some point, since this pattern (\w+-style regex, or a hand-rolled word-character check) recurs independently in at least three unrelated files now.

Test plan

  • npx tsc -b — clean compile
  • Added ColonPrefixedGlobalDotAccessHover.test.ts — synthetic CLASS + colon-prefixed global reference, one method-call case and one property-access case; both fail without the fix (confirmed by temporarily reverting it) and pass with it
  • Added ColonAndPreDotAccessInteraction.test.ts — adversarial check that widening both regexes to accept : doesn't regress the pre-existing GROUP PRE(...) dot-access case (Config.Nested.Setting, no : anywhere in that chain) when it coexists in the same file as the colon-prefixed global fix above. Also confirmed by reasoning over the code: multi-segment chains (2+ dots) never hit either changed regex at all — ChainedPropertyResolver already splits on . and passes the un-truncated segment straight into resolveVariableType, both before and after this PR.
  • Full test suite: 2344 passing, 0 failing, 4 pending (pre-existing, unrelated)
  • Manually verified via a direct HoverProvider.provideHover harness against a real-world .clw file with a colon-prefixed global class instance, confirmed the method-call hover now resolves (including correctly picking the right overload/class when another unrelated class declares a same-named method) and the property hover now goes through the real class-member path instead of the coincidental fallback

🤖 Generated with Claude Code

Clarion allows a literal ':' inside an identifier (e.g. GLOB:Thing), a naming
convention for globals that is unrelated to GROUP/QUEUE PRE(...) prefixing.
Hovering a member reached via dot access on such a variable (GLOB:Thing.Foo)
silently returned no hover for method calls, while property access happened to
still show something via an unrelated fallback.

Two independent spots dropped the colon-prefixed segment before resolving the
receiver's type:

- TokenHelper.getWordRangeAtPosition's backward scan (building the hover
  "word") accepted ':' when scanning forward for the word under the cursor,
  but not when scanning backward to include the prefix before a dot -
  producing a mismatched, truncated word for the dotted-access case.
- StructureFieldResolver.resolveFieldAccess's structureNameMatch regex
  (`\w+`) also excludes ':', independently truncating the receiver name
  before it's passed to variable-type resolution.

Either bug alone drops "GLOB:Thing" to "Thing", which then fails to match the
actual declaration, so the receiver's type never resolves and the member
lookup returns null. This is why it looked property-only: a property access
can still surface something via an unrelated bare-name fallback elsewhere in
the resolution ladder, but a method call has no such fallback and simply
produced no hover.

Added a regression test with a synthetic CLASS + colon-prefixed global
covering both a method call and a property access; confirmed both fail
without the fix and pass with it.
…d by the colon-prefix fix

Confirms widening the backward word-scan / structureNameMatch regex to accept
':' does not change resolution for GROUP PRE(...) prefixed field access via
dot notation (Config.Nested.Setting has no ':' in it at all, so the widened
regexes never apply), even when it coexists in the same file as the
colon-prefixed global dot-access chain the previous commit fixes.

While building this test, found and confirmed a separate, pre-existing,
unrelated bug: a procedure implementation's computed body range
(`finishesAt`) can over-reach past its own END into the very next
declaration when that declaration is followed later by another PROCEDURE.
Reproduced with a plain non-colon variable in the identical shape to confirm
it has nothing to do with colon handling - not fixed here, out of scope for
this PR, left as a note for whoever picks it up next.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant