fix(types): рекурсивная функция получает свой же тип, а не теряет его - #4440
Merged
Conversation
`computeReturnTypes` не помечал считаемый метод как считающийся, поэтому вызов функцией самой себя выглядел обычным вызовом и шёл в индекс за значением, которого там ещё нет. Возвращалась пустота, и весь вклад рекурсивной ветки терялся: поле, заполняемое таким вызовом, пропадало из типа целиком. Теперь метод кладётся в стек расчёта до разбора собственного тела, а рекурсивное ребро отдаёт ссылку на метод (LazyTypeSet) вместо содержимого: ссылка равна себе по ключу, поэтому подстановка не углубляет тип. Тело пересчитывается со своим же приближением, пока набор имён полей растёт — двух проходов хватает, третий предусмотрен страховкой. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017cu7S3zYn7n6GMsdYf5v1q
Contributor
📝 WalkthroughWalkthroughThe inferencer now supports recursive return types through lazy self-referential approximations and bounded refinement. A Spring-integrated test verifies recursive nested fields. ChangesRecursive return-type inference
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Test
participant ExpressionTypeInferencer
participant RecursiveMethod
participant FlowAnalysis
Test->>ExpressionTypeInferencer: query recursive function expression type
ExpressionTypeInferencer->>RecursiveMethod: analyze method body
RecursiveMethod->>ExpressionTypeInferencer: call recursive method
ExpressionTypeInferencer->>FlowAnalysis: refine recursive return type
FlowAnalysis-->>ExpressionTypeInferencer: return stabilized fields
ExpressionTypeInferencer-->>Test: return nested recursive type
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/types/inferencer/ExpressionTypeInferencer.java`:
- Around line 884-925: Track the currently refined method across
refinedByOwnValue and the MethodReturnTypeIndexer lookup path, so an indirect
cycle such as A → B → A detects the repeated active method before
computeIfAbsent re-entry and returns recursiveKnot. Preserve existing refinement
and dependency propagation behavior, and add an integration test covering the
indirect recursion scenario.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: fcc55e82-4416-4703-b14f-bcc07b9a4d46
⛔ Files ignored due to path filters (1)
src/test/resources/types/RecursiveReturn.bslis excluded by!src/test/resources/**
📒 Files selected for processing (2)
src/main/java/com/github/_1c_syntax/bsl/languageserver/types/inferencer/ExpressionTypeInferencer.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/RecursiveReturnTypeTest.java
Comment on lines
+884
to
+925
| if (ctx.cycleCut && !types.isEmpty() && !ctx.inProgress.containsKey(method)) { | ||
| types = refinedByOwnValue(method, types, ctx); | ||
| } | ||
| return new MethodReturnTypeIndexer.ComputedReturnTypes(types, Set.copyOf(ctx.consulted), ctx.sawMissing); | ||
| } catch (StackOverflowError | RuntimeException e) { | ||
| return new MethodReturnTypeIndexer.ComputedReturnTypes(TypeSet.EMPTY, Set.of(), false); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Значение рекурсивной функции, пересчитанное по телу с её же первым приближением. | ||
| * <p> | ||
| * В первом проходе рекурсивное ребро отвечать нечем: значения метода ещё нет, и поле, | ||
| * заполняемое вызовом самого себя, теряется целиком. Второй проход идёт с приближением | ||
| * на руках, поэтому ребро отдаёт узел ({@link #recursiveKnot}) и поле остаётся — со | ||
| * ссылкой на метод вместо вложенного значения. Больше двух проходов не нужно: узел | ||
| * равен себе по ключу, поэтому третий дал бы то же самое. | ||
| * | ||
| * @param method рекурсивная функция. | ||
| * @param approximation значение, посчитанное первым проходом. | ||
| * @param ctx контекст расчёта; в него переносится всё, что увидел второй проход. | ||
| * @return уточнённое значение; приближение, если второй проход не дал ничего. | ||
| */ | ||
| private TypeSet refinedByOwnValue(MethodSymbol method, TypeSet approximation, InferenceContext ctx) { | ||
| var current = approximation; | ||
| for (var pass = 0; pass < MAX_REFINING_PASSES; pass++) { | ||
| var refining = new InferenceContext(ctx.documentContext); | ||
| refining.depth = ctx.depth + 1; | ||
| refining.refining = true; | ||
| refining.visited.add(method); | ||
| refining.inProgress.put(method, current); | ||
| var refined = returnTypeFromBodyInference.of(method, expression -> inferInternal(expression, refining)); | ||
| ctx.consulted.addAll(refining.consulted); | ||
| ctx.dependencies.addAll(refining.dependencies); | ||
| ctx.sawMissing = ctx.sawMissing || refining.sawMissing; | ||
| if (refined.isEmpty() || refined.getAllFieldNames().equals(current.getAllFieldNames())) { | ||
| return refined.isEmpty() ? current : refined; | ||
| } | ||
| current = refined; | ||
| } | ||
| return current; | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
indexer="$(fd -a '^MethodReturnTypeIndexer\.java$' src/main/java | head -n 1)"
test_file="$(fd -a '^RecursiveReturnTypeTest\.java$' src/test/java | head -n 1)"
ast-grep outline "$indexer" --items all
rg -n -C 8 'computeIfAbsent|computeReturnTypes|inferredReturnTypes' "$indexer"
sed -n '1,220p' "$test_file"
rg -n -C 6 'recursiveKnot|cycleCut|inProgress|refinedByOwnValue' \
src/main/java/com/github/_1c_syntax/bsl/languageserver/types/inferencer/ExpressionTypeInferencer.javaRepository: 1c-syntax/bsl-language-server
Length of output: 17103
🏁 Script executed:
#!/bin/bash
set -euo pipefail
indexer="$(fd -a '^MethodReturnTypeIndexer\.java$' src/main/java | head -n 1)"
inferencer="$(fd -a '^ExpressionTypeInferencer\.java$' src/main/java | head -n 1)"
printf '%s\n' '--- indexer computeIfAbsent and storage ---'
sed -n '118,170p' "$indexer"
sed -n '300,370p' "$indexer"
printf '%s\n' '--- method inference and body inference ---'
sed -n '740,895p' "$inferencer"
printf '%s\n' '--- all indirect-recursion fixtures and tests ---'
rg -n -i -C 5 'indirect|mutual|взаим|цик[лк]|recursive|рекурс' src/test src/main \
-g '*.java' -g '*.bsl' -g '*.md' || true
printf '%s\n' '--- callers of computeIfAbsent and returnTypesOfBody ---'
rg -n -C 5 'computeIfAbsent\(|returnTypesOfBody\(' src/main/java src/test -g '*.java'Repository: 1c-syntax/bsl-language-server
Length of output: 50386
🏁 Script executed:
#!/bin/bash
set -euo pipefail
indexer="$(fd -a '^MethodReturnTypeIndexer\.java$' src/main/java | head -n 1)"
inferencer="$(fd -a '^ExpressionTypeInferencer\.java$' src/main/java | head -n 1)"
printf '%s\n' '--- exact indexer methods ---'
rg -n 'computeIfAbsent|store\(|ComputedReturnTypes|indexed' "$indexer" | head -n 80
sed -n '120,160p' "$indexer"
sed -n '300,350p' "$indexer"
printf '%s\n' '--- exact inferencer call path ---'
rg -n 'methodReturnType|returnTypesOfBody|refinedByOwnValue|visited\.add|visited\.remove|inProgress\.put|inProgress\.remove|cycleCut' "$inferencer"
sed -n '760,895p' "$inferencer"
printf '%s\n' '--- focused indirect-recursion matches ---'
rg -n -i 'indirect|mutual|взаимн|взаим|A.*B|Б.*А|рекурсив' src/test -g '*.java' -g '*.bsl' | head -n 120 || trueRepository: 1c-syntax/bsl-language-server
Length of output: 31805
🏁 Script executed:
#!/bin/bash
set -euo pipefail
inferencer="$(fd -a '^ExpressionTypeInferencer\.java$' src/main/java | head -n 1)"
printf '%s\n' '--- recursion limits and relevant context fields ---'
rg -n -C 3 'MAX_DEPTH|MAX_REFINING_PASSES|class InferenceContext|inProgress|cycleCut|refining' "$inferencer" | head -n 180
printf '%s\n' '--- deterministic call-graph verifier for A -> B -> A ---'
python3 - <<'PY'
indexed = set()
events = []
active = set()
def method_return_type(method, visited):
if method in visited:
events.append(f"cycle cut at {method}")
return
visited.add(method)
events.append(f"enter methodReturnType({method})")
if method == "A":
compute_if_absent("A", visited)
elif method == "B":
compute_if_absent("B", visited)
visited.remove(method)
events.append(f"leave methodReturnType({method})")
def compute_if_absent(method, visited):
if method in indexed:
events.append(f"skip indexed {method}")
return
if method in active:
events.append(f"REENTER before store {method}")
events.append(f"computeIfAbsent({method})")
active.add(method)
# The method body calls the other method.
other = "B" if method == "A" else "A"
method_return_type(other, visited)
active.remove(method)
indexed.add(method)
events.append(f"store {method}")
# First pass computes A. Its body calls B, whose body closes the cycle at A.
# The changed implementation then refines B with only B in inProgress.
method_return_type("A", {"A"})
events.append("refine B: visited={B}, inProgress={B}")
method_return_type("B", {"B"})
events.append("refinement would call A with A absent from visited/inProgress")
for event in events:
print(event)
assert "REENTER before store A" in events, \
"The simplified path did not reproduce A re-entry"
print("RESULT: A is re-entered before its first computeIfAbsent call stores a result.")
PYRepository: 1c-syntax/bsl-language-server
Length of output: 6714
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
indexed = set()
active_computations = set()
events = []
def method_return_type(method, visited):
if method in visited:
events.append(f"cycle cut at {method}")
return
visited.add(method)
events.append(f"enter methodReturnType({method})")
compute_if_absent(method, visited)
visited.remove(method)
events.append(f"leave methodReturnType({method})")
def compute_if_absent(method, visited):
if method in indexed:
events.append(f"skip indexed {method}")
return
if method in active_computations:
events.append(f"REENTER before store {method}")
events.append(f"computeIfAbsent({method})")
active_computations.add(method)
other = "B" if method == "A" else "A"
method_return_type(other, visited)
active_computations.remove(method)
indexed.add(method)
events.append(f"store {method}")
# Initial A computation. A calls B, and B calls A, which cuts the cycle.
events.append("initial compute A")
compute_if_absent("A", {"A"})
# returnTypesOfBody(B) sees cycleCut and refines B with only B protected.
events.append("refine B: visited={B}, inProgress={B}")
method_return_type("B", {"B"})
for event in events:
print(event)
assert "REENTER before store A" in events, \
"The modeled refinement did not re-enter A before its first store"
print("RESULT: A is re-entered before its first computeIfAbsent call stores a result.")
PYRepository: 1c-syntax/bsl-language-server
Length of output: 514
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
indexed = set()
active_computations = set()
events = []
def method_return_type(method, visited):
if method in visited:
events.append(f"cycle cut at {method}")
return
visited.add(method)
events.append(f"enter methodReturnType({method})")
compute_if_absent(method, visited)
visited.remove(method)
events.append(f"leave methodReturnType({method})")
def compute_body(method, visited):
other = "B" if method == "A" else "A"
events.append(f"body({method}) calls {other}")
method_return_type(other, visited)
def compute_if_absent(method, visited):
if method in indexed:
events.append(f"skip indexed {method}")
return
if method in active_computations:
events.append(f"REENTER before store {method}")
events.append(f"computeIfAbsent({method})")
active_computations.add(method)
compute_body(method, visited)
active_computations.remove(method)
indexed.add(method)
events.append(f"store {method}")
# Initial A computation. A calls B, and B calls A, which cuts the cycle.
events.append("initial compute A")
compute_if_absent("A", {"A"})
# returnTypesOfBody(B) sees cycleCut and refines B directly.
# The refinement context protects B only; A is absent from both visited and inProgress.
events.append("refine B: visited={B}, inProgress={B}")
compute_body("B", {"B"})
for event in events:
print(event)
assert "REENTER before store A" in events, \
"The modeled refinement did not re-enter A before its first store"
print("RESULT: A is re-entered before its first computeIfAbsent call stores a result.")
PYRepository: 1c-syntax/bsl-language-server
Length of output: 614
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
indexed = set()
active_computations = set()
events = []
def method_return_type(method, visited):
if method in visited:
events.append(f"cycle cut at {method}")
return
visited.add(method)
events.append(f"enter methodReturnType({method})")
compute_if_absent(method, visited)
visited.remove(method)
events.append(f"leave methodReturnType({method})")
def compute_body(method, visited):
other = "B" if method == "A" else "A"
events.append(f"body({method}) calls {other}")
method_return_type(other, visited)
def compute_if_absent(method, visited):
if method in indexed:
events.append(f"skip indexed {method}")
return
if method in active_computations:
events.append(f"REENTER before store {method}")
events.append(f"computeIfAbsent({method})")
active_computations.add(method)
compute_body(method, visited)
# This is the changed returnTypesOfBody condition:
# B observed the cycle, and its refinement context protects B only.
if method == "B":
events.append("refine B: visited={B}, inProgress={B}")
compute_body("B", {"B"})
active_computations.remove(method)
indexed.add(method)
events.append(f"store {method}")
events.append("initial compute A")
compute_if_absent("A", {"A"})
for event in events:
print(event)
assert "REENTER before store A" in events, \
"The modeled refinement did not re-enter A while A was still computing"
print("RESULT: A is re-entered before its first computeIfAbsent call stores a result.")
PYRepository: 1c-syntax/bsl-language-server
Length of output: 303
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
indexed = set()
active_stack = []
events = []
class ReentryObserved(Exception):
pass
def method_return_type(method, visited):
if method in visited:
events.append(f"cycle cut at {method}")
return
visited.add(method)
events.append(f"enter methodReturnType({method})")
compute_if_absent(method, visited)
visited.remove(method)
events.append(f"leave methodReturnType({method})")
def compute_body(method, visited):
other = "B" if method == "A" else "A"
events.append(f"body({method}) calls {other}")
method_return_type(other, visited)
def compute_if_absent(method, visited):
if method in indexed:
events.append(f"skip indexed {method}")
return
if method in active_stack:
events.append(f"REENTER before store {method}")
raise ReentryObserved(method)
events.append(f"computeIfAbsent({method})")
active_stack.append(method)
try:
compute_body(method, visited)
if method == "B":
events.append("refine B: visited={B}, inProgress={B}")
compute_body("B", {"B"})
finally:
active_stack.pop()
indexed.add(method)
events.append(f"store {method}")
try:
events.append("initial compute A")
compute_if_absent("A", {"A"})
except ReentryObserved:
events.append("stopped after first re-entry")
for event in events:
print(event)
assert "REENTER before store A" in events, \
"The modeled refinement did not re-enter A while A was still computing"
print("RESULT: A is re-entered before its first computeIfAbsent call stores a result.")
PYRepository: 1c-syntax/bsl-language-server
Length of output: 507
Track the active cycle method during refinement.
In an A → B → A cycle, refining B protects only B. A is re-entered before computeIfAbsent stores its first result because the indexer tracks indexed methods only. Track the active cycle method and return recursiveKnot for the repeated method. Add an indirect-recursion integration test covering ExpressionTypeInferencer.java:884-915 and MethodReturnTypeIndexer.java:138-142.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/types/inferencer/ExpressionTypeInferencer.java`
around lines 884 - 925, Track the currently refined method across
refinedByOwnValue and the MethodReturnTypeIndexer lookup path, so an indirect
cycle such as A → B → A detects the repeated active method before
computeIfAbsent re-entry and returns recursiveKnot. Preserve existing refinement
and dependency propagation behavior, and add an integration test covering the
indirect recursion scenario.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes recursive return-type inference so self-referential fields retain their types.
Changes:
- Tracks the currently inferred method to detect recursion.
- Adds lazy recursive type knots and refinement passes.
- Adds a recursive structure inference test and fixture.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
ExpressionTypeInferencer.java |
Implements recursive return-type refinement. |
RecursiveReturnTypeTest.java |
Tests nested recursive type preservation. |
RecursiveReturn.bsl |
Provides the recursive BSL fixture. |
Suppressed comments (1)
src/main/java/com/github/_1c_syntax/bsl/languageserver/types/inferencer/ExpressionTypeInferencer.java:835
- During a refining pass this resolver still reads
symbolTypeIndex, which does not contain the current approximation untilstoreruns. Thus the recursive node exposes field names but not their types to its own body. For example, with a base branch returning{A: Number}, thenR = F(); R.Insert("B", R.A); return R,R.Aresolves to empty during refinement andBis never inferred. The element resolver above has the same problem. Resolve lazy contents from the activectx.inProgressapproximation while refining, then fall back to the finalized index after the pass.
() -> symbolTypeIndex.getReturnTypes(method).getLocalFields(ref)
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| */ | ||
| private TypeSet recursiveKnot(MethodSymbol method, InferenceContext ctx) { | ||
| var approximation = ctx.inProgress.get(method); | ||
| var known = approximation == null ? symbolTypeIndex.getReturnTypes(method) : approximation; |
|
Contributor
Member
Author
|
Замечание подтвердилось — воспроизвёл тестом на фикстуре из двух функций, вызывающих друг друга: поле, заполненное вызовом соседа, теряется целиком. Причём теряется у той функции, которую обход дерева символов встретил первой: поменяйте функции местами в модуле — потеряется другое поле. Попробовал самое дешёвое лечение — считать обрыв цикла незавершённым расчётом, чтобы значение доводил до неподвижной точки уже существующий проход после наполнения области. Замеры на ssl_3_1 (волны прохода, DEBUG индексатора):
Последние два метода не сходятся не из-за цикла: они считаются то в значение, то в пустоту, потому что записи их зависимостей стираются при перечитывании документа — это #4429. На ветке, где та стирка вылечена, проход сходится штатно: изменений 8228 → 629 → 48 → 0 за четыре волны, и результат при этом побайтово тот же, что без этой правки. Вывод: в этот PR не беру. На текущем корпусе выигрыша нет (расхождение подписи 327 строк — внутри собственного разброса анализа 273–363), стоимость +13 % времени, сходимость зависит от чужой правки, а путь редактора она не лечит вовсе — связи внутри документа не хранятся, поэтому в юнит-тесте с одним файлом взаимная рекурсия остаётся красной. Возьму её вместе с работой по #4429 либо отдельной правкой модели (ленивый источник полей у типа), где итерации не нужны вовсе. |
This was referenced Aug 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Проблема
Функция, вызывающая саму себя, теряла весь вклад рекурсивной ветки. Живой пример из БСП —
УправлениеДоступомСлужебный.УпрощенноеУсловиеОграничения, обход дерева условий:Тип такой функции — решение уравнения
T = Структура{Аргумент: T}. Вместо этого полеАргументпропадало из типа целиком.Причина не в защите от циклов, а раньше:
computeReturnTypesне помечал считаемый метод как считающийся — не клал его в стек расчёта. Из-за этого вызов функцией самой себя не распознавался как рекурсия: он шёл обычным путём вMethodReturnTypeIndexerза значением метода, которого там ещё нет (текущий расчёт его как раз и считает), и получал пустоту. По логу видно, что вызов при этом резолвится штатно — есть и ссылка, и символ, — а стек расчёта пуст.Что сделано
LazyTypeSet) вместо содержимого. Механизм тот же, что уже работает для объявленных самоссылокУзел: Массив из см. Узел: ссылка равна себе по ключу, поэтому подстановка не углубляет тип, а разыменование выражения под курсором форсит по одному уровню.Замеры
Пакетный анализ ssl_3_1, репортер SARIF,
mode: onlyсUnknownMember+EventHandlerInvalidSignature.Разница в пределах собственной недетерминированности анализа (#4429): у develop между двумя прогонами расходится 273 строки подписи. Деградации по времени нет.
Отдельно — на сборке с наработками по #4429, где каждая функция считается отдельной единицей расчёта и рекурсивное ребро отдаёт накопленное значение. Там подстановка содержимого вкладывала тип на уровень глубже с каждой волной, и это давало разрастание в гигабайты:
Тесты
RecursiveReturnTypeTestна фикстуреtypes/RecursiveReturn.bsl: функция, кладущая свой же результат в поле структуры, получает типT = Структура{Имя, Вложенный: T}. До правки полеВложенныйв типе отсутствовало.Связанные задачи
Найдено при работе над #4429. Заменяет закрытый #4439: тот ограничивал глубину слияния наборов, то есть боролся со следствием; с этой правкой ограничитель не срабатывает ни разу, и прогон без него проходит штатно.
Чего здесь нет
Взаимная рекурсия (
АвызываетБ,БвызываетА) не лечится — поле теряется у той функции, которую обход дерева символов встретил первой. Разбор и замеры — в треде ниже; коротко: дешёвое лечение через существующий проход доразрешения на develop не сходится, потому что упирается в стирку записей при перечитывании документа (#4429), а путь редактора им не лечится вовсе. Возьму отдельно.