Fix MissingCommonModuleMethod false positive on chained common module accessor calls

[Copilot]

MissingCommonModuleMethod reported a spurious error when a variable was assigned the result of a method called on a common module reference. The reference indexer mapped the variable to the common module itself, so subsequent member calls were resolved against the module.

// Result of НастройкиВитрины(...) — not the module — but the variable was
// indexed as if it were the "Беру_сервер" common module.
НастройкиВитрины = ОбщегоНазначения.ОбщийМодуль("Беру_сервер").НастройкиВитрины(Витрина);

// FP: "Метод Свойство общего модуля Беру_сервер не существует"
... НастройкиВитрины.Свойство("IDКомпании") ...

Changes

• ModuleReference: an expression is now recognized as a common module reference only when the accessor call (.ОбщийМодуль("Имя")) is the terminal element of the chain.
  • hasMatchingModifierMethodCall / extractModuleNameFromMatchingModifier inspect only the last modifier (via new getMatchingTerminalModifier helper), so ОбщегоНазначения.ОбщийМодуль("X").Метод() no longer matches.
  • extractModuleNameFromGlobalMethodCall bails out when modifiers trail the local ОбщийМодуль("X") form.
• Tests: added ModuleReferenceTest unit coverage and ReferenceIndexFillerTest regression (with ReferenceIndexCommonModuleChainedResult.bsl fixture) verifying the chained result is not indexed as a module method call.

Direct accessor assignments (Перем = ОбщегоНазначения.ОбщийМодуль("X")) continue to resolve as before.

[nixel2007]

@claude pls review

[claude]

Claude finished @nixel2007's task in 2m 49s —— View job

---

Code Review

• Read changed files and gather context
• Review ModuleReference.java changes
• Review test changes and fixture
• Assess overall correctness and edge cases
• Post review findings

---

The fix is correct and well-targeted. Here's a detailed analysis:

How the fix works

The root cause was that hasMatchingModifierMethodCall and extractModuleNameFromMatchingModifier scanned all modifiers in a chain for the accessor call (e.g. .ОбщийМодуль("X")). This meant ОбщегоНазначения.ОбщийМодуль("X").Метод() matched even though .ОбщийМодуль("X") is not the terminal element.

The fix introduces getMatchingTerminalModifier which checks only modifiers.get(modifiers.size() - 1) — the terminal element. If the accessor call is not the final step in the chain, the expression is not treated as a module reference.

An additional guard in extractModuleNameFromGlobalMethodCall (line 266–268) handles the complementary local-call form: ОбщийМодуль("X").Метод() now correctly bails out when modifiers trail the global method call.

Correctness check

Expression	Expected	Result after fix
ОбщегоНазначения.ОбщийМодуль("X")	module ref	✅ last modifier is accessor
ОбщегоНазначения.ОбщийМодуль("X").Метод()	not module ref	✅ last modifier is .Метод()
ОбщийМодуль("X") (global form)	module ref	✅ no modifiers → guard passes
ОбщийМодуль("X").Метод() (global form)	not module ref	✅ new guard: modifiers non-empty → bail

Observations

1. Missing unit test for the global-form chained case

extractModuleNameFromGlobalMethodCall has a new guard for ОбщийМодуль("X").Метод(), but there's no dedicated test for this path in ModuleReferenceTest. The existing tests only cover the ОбщегоНазначения.ОбщийМодуль("X").Метод() (module-prefixed) form. Consider adding:

@Test
void testGlobalFormChainedCallIsNotCommonModuleExpression() {
    var code = """
      Процедура Тест()
        Значение = ОбщийМодуль("ПервыйОбщийМодуль").НастройкиВитрины(Витрина);
      КонецПроцедуры""";
    // ... assertThat isCommonModuleExpression → false
}

2. Pre-existing inconsistency in isCommonModuleExpressionMember (not regressed)

Случай 2a (lines 170–173) returns true if complexId.IDENTIFIER() matches a local method name, without checking for trailing modifiers. This is inconsistent with the terminal-only check now applied in Случай 2b. In practice this path only fires when someone uses a local accessor name as a variable identifier, so it's unlikely to matter — but it's worth noting. Fix this →

3. The Javadoc on getMatchingTerminalModifier is clear and well-placed — good documentation of the invariant.

4. Test fixture design

ReferenceIndexCommonModuleChainedResult.bsl calls НеУстаревшаяФункция() on line 8 (result assigned to Результат) and then calls НеУстаревшаяПроцедура() through it on line 9. The test asserts zero references to НеУстаревшаяПроцедура from the fixture file, which cleanly validates the fix.

Verdict

The fix is correct and the test coverage for the primary scenario is solid. The only minor gap is a missing test for the global-call chained form (ОбщийМодуль("X").Метод()). Everything else looks good.