refactor(types): срыв расчёта по потоку виден в журнале, а не подменяется типом - #4341
Conversation
…ется типом Обращение к переменной, которое расчёт по потоку не смог разместить в графе, уходило в объединение по всей области видимости. Подмена скрывала дефект расчёта: тип получался, значит и проблемы будто нет. Теперь такое место пишется LOGGER.error с документом, позицией обращения и позицией объявления переменной, а тип остаётся пустым. Туда же ушли перехваты срыва расчёта. Объединение по области видимости остаётся ответом там, где точки исполнения нет по существу — на объявлении `Перем`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MnWpDdNSEFohPZHG6voe6y
|
Warning Review limit reached
Next review available in: 49 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughExpressionTypeInferencer.java changes variable flow type inference so that missing flow results or exceptions during flow-based analysis are logged as errors and return an empty TypeSet, instead of falling back to declared variable types. A helper method formats one-based line:column positions for logging. ChangesFlow Inference Error Handling
Estimated code review effort: 2 (Simple) | ~10 minutes 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}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 717-720: Update both exception handlers in the type inference
flow, including the variable-based handler around the shown LOGGER.error call
and the position-based handler near the corresponding secondary location, to
include the failing query position in their diagnostic arguments: use
at(Ranges.create(use).getStart()) for the use-based path and at(position) for
the position-based path. Preserve the existing error handling and return
behavior.
- Around line 785-787: Update the public inferVariableAt Javadocs to document
that inference failures log an error and return TypeSet.EMPTY. Apply the same
contract update to the corresponding documentation in docs/ and docs/en/,
keeping both locales consistent.
🪄 Autofix (Beta)
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: 762c9d88-9992-47df-951d-f8d2ed6b9f53
📒 Files selected for processing (1)
src/main/java/com/github/_1c_syntax/bsl/languageserver/types/inferencer/ExpressionTypeInferencer.java
| LOGGER.error("Расчёт типа по потоку сорвался на переменной {} (объявлена {}): {}", | ||
| variable.getName(), at(variable.getSelectionRange().getStart()), owner.getUri(), e); | ||
| return TypeSet.EMPTY; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the new failure result in the public contract.
This handler now returns TypeSet.EMPTY and emits an error log. The public inferVariableAt Javadocs above this method do not describe this result or the logging side effect. Update those Javadocs and the corresponding documentation in docs/ and docs/en/.
As per coding guidelines, public Java APIs must document their contract, and behavior changes must update both documentation locales.
🤖 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 785 - 787, Update the public inferVariableAt Javadocs to document
that inference failures log an error and return TypeSet.EMPTY. Apply the same
contract update to the corresponding documentation in docs/ and docs/en/,
keeping both locales consistent.
Source: Coding guidelines
Оба перехвата писали переменную, её объявление и документ, но не место, где тип спросили, — без него по журналу не найти, на чём расчёт сорвался. Заодно в контракте обоих inferVariableAt описан пустой ответ при срыве.
|
1 participant
Что и почему
Обращение к переменной, которое расчёт по потоку не смог разместить в графе, уходило в объединение типов по всей области видимости. Подмена скрывала дефект: тип получался, значит и проблемы будто нет.
Теперь такое место пишется
LOGGER.error— с документом, позицией обращения и позицией объявления переменной, — а тип остаётся пустым. Туда же ушли перехваты срыва расчёта (StackOverflowError/RuntimeException), раньше молчавшие вdebug.Объединение по области видимости осталось ответом там, где точки исполнения нет по существу: на объявлении
Перемоператора не существует, и тип такой переменной — то, с чем она свою область покидает.Почему это безопасно сейчас
Замер на cpm (
analyze, счётчик в месте фоллбэка):developdevelop+ #4337Все 182 отказа на
develop— реквизиты управляемой формы, резолвившиеся переменными видаDYNAMIC. С типами форм (#4337) таких мест не остаётся.Проверки
Полный
./gradlew testлокально: 643 класса, падений по коду нет (четыре класса упали от исчерпанияinotifyна машине и проходят по отдельности).Summary by CodeRabbit