fix(lsp): выставлять workspace-контекст в didClose#4283
Merged
Conversation
… потоков без него ServerContextDocumentClosedEvent (и другие события с source = ServerContext) терялись, если метод-мутатор вызван с потока без установленного WorkspaceContextHolder: рассылка падала ScopeNotActiveException при резолве workspace-scoped @EventListener-бинов (callStatementByReceiverIndex и др.). Так происходит в textDocument/didClose — он приходит с потока LSP4J. EventPublisherAspect теперь выставляет workspace-контекст из workspace источника события на время рассылки, если на потоке он не установлен. Имя workspace берётся через новый WorkspaceContextHolder.nameForUri() (зарегистрированное или производное от URI; логика перенесена из ServerContextProvider). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019dRpi5rmAfg8gXvT9ZG8wQ
…versioning
Экспериментальный синтаксис $$"…" требует Kotlin 2.2+ (Gradle 9.6) и не
парсится более старыми версиями Gradle. Обычное экранирование \$ даёт те же
литеральные плейсхолдеры ${…} для git-versioning и работает в любой версии.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019dRpi5rmAfg8gXvT9ZG8wQ
Contributor
📝 WalkthroughWalkthroughDocument closure now runs with the workspace context active so close-event listeners can resolve the correct workspace. A regression test verifies this behavior, and Git version templates use updated formatting syntax while preserving their existing variables. ChangesWorkspace event routing
Git version templates
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Contributor
![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 `@build.gradle.kts`:
- Around line 48-51: The develop branch version template is incompatible with
Sentry’s expected develop-version format. Update the develop branch
configuration to emit the develop- prefixed distance format, preserving the
dirty suffix, and keep SentryScopeConfigurer.resolveEnvironment unchanged unless
the project’s intended contract explicitly requires the existing SNAPSHOT
format.
🪄 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
Run ID: d031da77-581f-431d-923c-cf428f123669
📒 Files selected for processing (5)
build.gradle.ktssrc/main/java/com/github/_1c_syntax/bsl/languageserver/aop/EventPublisherAspect.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/context/ServerContextProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/WorkspaceContextHolder.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/index/CallStatementByReceiverIndexTest.java
…спекте didClose приходит с потока диспетчеризации LSP4J без установленного WorkspaceContextHolder — единственная входная точка без обёртки. Ставим контекст явно, как в didOpen и остальных обработчиках, вместо восполнения контекста на уровне EventPublisherAspect: изменения аспекта, WorkspaceContextHolder и ServerContextProvider откачены, регрессионный тест переведён на закрытие через BSLTextDocumentService.didClose. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019dRpi5rmAfg8gXvT9ZG8wQ
nixel2007
changed the title
Имя класса повторяет реализуемый LSP4J-интерфейс TextDocumentService; стереотип @component — общепроектная конвенция (@service в проекте не используется), переименование невозможно. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019dRpi5rmAfg8gXvT9ZG8wQ
Тест проверяет поведение BSLTextDocumentService.didClose (доставку ServerContextDocumentClosedEvent workspace-scoped подписчикам), поэтому его место — рядом с didOpen-аналогом в BSLTextDocumentServiceTest, а не в тестовом классе индекса. CallStatementByReceiverIndexTest возвращён к исходному виду без зависимости от сервиса. Также убрано подавление @SuppressWarnings(java:S5673): в проекте включено правило S1309, запрещающее подавления; S5673 по @component обрабатывается на стороне quality profile. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019dRpi5rmAfg8gXvT9ZG8wQ
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/java/com/github/_1c_syntax/bsl/languageserver/lsp/BSLTextDocumentService.java (1)
824-833: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider wrapping diagnostic publishing in the workspace context.
For consistency with
didOpen(wherevalidate()runs inside the workspace context) and to future-proof againstScopeNotActiveExceptionifpublishEmptyDiagnosticListever interacts with workspace-scoped beans, consider moving the diagnostic clearing logic inside theWorkspaceContextHolder.runlambda block.♻️ Proposed refactor
- WorkspaceContextHolder.run(serverContext.getWorkspaceUri(), - () -> serverContext.closeDocument(documentContext)); - - if (!clientSupportsPullDiagnostics) { - diagnosticProvider.publishEmptyDiagnosticList(documentContext); - } + WorkspaceContextHolder.run(serverContext.getWorkspaceUri(), () -> { + serverContext.closeDocument(documentContext); + if (!clientSupportsPullDiagnostics) { + diagnosticProvider.publishEmptyDiagnosticList(documentContext); + } + });🤖 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/lsp/BSLTextDocumentService.java` around lines 824 - 833, Move the clientSupportsPullDiagnostics check and diagnosticProvider.publishEmptyDiagnosticList(documentContext) call inside the WorkspaceContextHolder.run lambda in the document-close flow, keeping both document closing and diagnostic clearing under the same workspace context.
🤖 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.
Nitpick comments:
In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/lsp/BSLTextDocumentService.java`:
- Around line 824-833: Move the clientSupportsPullDiagnostics check and
diagnosticProvider.publishEmptyDiagnosticList(documentContext) call inside the
WorkspaceContextHolder.run lambda in the document-close flow, keeping both
document closing and diagnostic clearing under the same workspace context.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6beaeaed-e8e2-4595-8586-fe1062bd4f47
📒 Files selected for processing (2)
src/main/java/com/github/_1c_syntax/bsl/languageserver/lsp/BSLTextDocumentService.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/lsp/BSLTextDocumentServiceTest.java
|
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
2 participants
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.
Описание
Исправляет предупреждение при
textDocument/didClose:didCloseприходит с потока диспетчеризации LSP4J (pool-2-thread-1), на котором не установленWorkspaceContextHolder. КогдаServerContext.closeDocument()черезEventPublisherAspectпубликуетServerContextDocumentClosedEvent, workspace-scoped@EventListener-бины (например,CallStatementByReceiverIndex) не могут зарезолвиться — рассылка падает, и все подписчики молча теряют событие: per-URI индексы не чистятся при закрытии документа и удерживают AST-узлы закрытого документа.Инвентаризация показала, что идиома «входная точка сама выставляет workspace-контекст» соблюдается во всех ~15 местах проекта (didOpen, файловые события, MCP, CLI, вотчеры, воркеры executor'ов) —
didCloseбыл единственным пропуском.Изменения:
BSLTextDocumentService.didClose— вызовserverContext.closeDocument(...)обёрнут вWorkspaceContextHolder.run(...), симметричноdidOpen.CallStatementByReceiverIndexTest: закрывает документ черезtextDocumentService.didCloseс потока без workspace-контекста и проверяет, что индекс реально сброшен. Без фикса тест воспроизводит ровно то самое предупреждение из лога и падает.build.gradle.kts— экспериментальная multi-dollar interpolation ($$"…") в блокеgitVersioningзаменена на стандартное экранирование"\${…}": шаблоны для плагина идентичны, но скрипт больше не требует Kotlin 2.2 и собирается более старыми Gradle.Связанные задачи
Closes —
Чеклист
Общие
gradlew precommit)Для диагностик
Дополнительно
Рассматривался и альтернативный вариант — восполнять отсутствующий контекст централизованно в
EventPublisherAspect(поworkspaceUriизServerContext-источника события); он был реализован, проверен и затем откачен в пользу идиоматичной call-site-обёртки (см. историю коммитов ветки).В среде разработки был недоступен дистрибутив Gradle 9.6.1 (загрузка заблокирована сетевой политикой), поэтому
gradlew precommitне запускался; тесты гонялись на Gradle 8.14.3: fail-first (тест падает без фикса, воспроизводя предупреждение) → с фиксом зелёныеCallStatementByReceiverIndexTestиBSLTextDocumentServiceTest.🤖 Generated with Claude Code
https://claude.ai/code/session_019dRpi5rmAfg8gXvT9ZG8wQ
Summary by CodeRabbit
Bug Fixes
Tests
Chores