Place cursor after parens for parameterless completions#3969
Merged
Conversation
When a method/function/constructor with no parameters is completed, insert ready-made "()" and leave the cursor after the parentheses instead of between them, and skip the triggerParameterHints command (there is nothing to hint). Methods with parameters keep the existing snippet "($0)" behavior. A synthetic "return-type-only" signature flag distinguishes platform methods whose parameters are simply undocumented (params unknown -> cursor stays between parens) from genuinely parameterless members.
Contributor
📝 WalkthroughWalkthroughCompletion insertion now distinguishes parameterless callables (insert ChangesParameter-aware completion insertion and improved type support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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
There was a problem hiding this comment.
Pull request overview
This PR updates completion insertion behavior so genuinely parameterless callables insert closed parentheses and leave the cursor after them, while callables with parameters retain snippet/parameter-hint behavior.
Changes:
- Adds
syntheticReturnTypeOnlytoSignatureDescriptorto distinguish unknown parameters from true zero-parameter signatures. - Updates completion item insertion for methods, local methods, and constructors based on parameter availability.
- Adds tests for parameterless completion behavior and signature descriptor synthetic flags.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionProvider.java |
Applies new callable insertion behavior based on parameter/signature metadata. |
src/main/java/com/github/_1c_syntax/bsl/languageserver/types/model/SignatureDescriptor.java |
Adds synthetic return-type-only signature flag and factory. |
src/main/java/com/github/_1c_syntax/bsl/languageserver/types/model/MemberDescriptor.java |
Preserves synthetic signature metadata during specialization. |
src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/BuiltinPlatformTypesProvider.java |
Marks synthesized return-type-only platform signatures as synthetic. |
src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/BslContextPlatformTypesProvider.java |
Marks synthesized context platform signatures as synthetic. |
src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionProviderTest.java |
Covers parameterless and parameterized completion insertion behavior. |
src/test/java/com/github/_1c_syntax/bsl/languageserver/types/model/SignatureDescriptorTest.java |
Covers synthetic signature factory and default constructor behavior. |
| var ctors = typeService.getConstructors(ref, fileType); | ||
| if (!ctors.isEmpty()) { | ||
| var first = ctors.get(0); | ||
| ctorHasParameters = !first.parameters().isEmpty(); |
Member
Author
There was a problem hiding this comment.
Исправлено в 54dc8a5: при нескольких перегрузках конструктора (ctors.size() > 1) курсор остаётся между скобок — больше не полагаемся на первую перегрузку. Логика вынесена в buildPlatformClassCompletionItem, добавлены регрессионные тесты (единственный беспараметровый ctor → (), несколько перегрузок → ($0)).
![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/providers/CompletionProvider.java`:
- Around line 321-325: The constructor-parameter detection is too optimistic:
instead of relying on the first constructor only, iterate the constructors
returned by typeService.getConstructors(ref, fileType) and determine
ctorHasParameters conservatively — set ctorHasParameters = true if there are
multiple overloads (ctors.size() > 1), or if any constructor signature reports
non-empty parameters, or if any signature indicates unknown/synthetic info (e.g.
syntheticReturnTypeOnly); only mark ctorHasParameters = false when there is
exactly one constructor and its parameters list is empty and it is not
synthetic/unknown. Update the logic around typeService.getConstructors(...) and
the variable ctorHasParameters to implement this conservative check.
🪄 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: cfbb76bf-6b75-421b-a9a4-ee7112e60967
📒 Files selected for processing (7)
src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/model/MemberDescriptor.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/model/SignatureDescriptor.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/BslContextPlatformTypesProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/BuiltinPlatformTypesProvider.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionProviderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/model/SignatureDescriptorTest.java
Contributor
Test Results 2 946 files + 36 2 946 suites +36 1h 8m 8s ⏱️ - 10m 20s Results for commit 54dc8a5. ± Comparison against base commit c3ae226. This pull request removes 9 and adds 82 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Платформенный JSON-fallback нёс только имена методов без сигнатур — отсюда synthetic-заглушки «только returnType», а отсутствие сигнатур в JSON трактовалось как «параметры неизвестны». Отсутствие сигнатуры в источнике — это пробел данных, а не намеренный сигнал. Перенесены из синтакс-помощника платформы (bsl-context, 8.3.26.1521) реальные данные для 38 курируемых типов (примитивы, универсальные коллекции, СКД, шаблонные типы конфигурации): сигнатуры с двуязычными именами параметров, типами параметров (массив `types`), union типов возврата (`returnTypes`), описаниями и конструкторами. BuiltinPlatformTypesProvider теперь читает `types`/`returnTypes` (с фолбэком на одиночные `type`/`returnType`) и берёт тип возврата метода из JSON явно, не выводя из сигнатур. Поскольку данные заполнены, обходной механизм SignatureDescriptor .syntheticReturnTypeOnly из PR удалён: completion опирается строго на данные (ровно одна сигнатура без параметров → беспараметровый метод), синтез заглушек в обоих провайдерах убран. В SignatureSelection универсальный тип Произвольный/Arbitrary распознаётся как вершина решётки и не штрафует выбор перегрузки по типам аргументов. Co-Authored-By: Claude Opus 4.8 <[email protected]>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionProvider.java (1)
321-328:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCompute constructor parameter presence across all overloads.
This still derives
ctorHasParametersfromctors.get(0)only. If a type has bothCtor()andCtor(arg)overloads and the zero-arg one is first, completion will insertType()and skip parameter hints even though other valid constructors need arguments. Treat constructors conservatively here as well: only classify as parameterless when there is exactly one constructor and its parameter list is empty.Suggested fix
var ctors = typeService.getConstructors(ref, fileType); if (!ctors.isEmpty()) { var first = ctors.get(0); - ctorHasParameters = !first.parameters().isEmpty(); - var paramList = first.parameters().stream() - .map(p -> p.displayName(scriptVariant)) - .collect(java.util.stream.Collectors.joining(", ")); - item.setDetail("(" + paramList + ")"); + if (ctors.size() == 1) { + ctorHasParameters = !first.parameters().isEmpty(); + var paramList = first.parameters().stream() + .map(p -> p.displayName(scriptVariant)) + .collect(java.util.stream.Collectors.joining(", ")); + item.setDetail("(" + paramList + ")"); + } else { + ctorHasParameters = true; + item.setDetail(formatSignaturesCount(ctors.size(), scriptVariant)); + } }🤖 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/providers/CompletionProvider.java` around lines 321 - 328, The code currently sets ctorHasParameters based solely on the first constructor; change this to mark a type as parameterless only when there is exactly one constructor and that constructor has no parameters: use the result of typeService.getConstructors(ref, fileType) (ctors) and set ctorHasParameters = !(ctors.size() == 1 && first.parameters().isEmpty()). Keep using the same first constructor for building the paramList and item.setDetail("(" + paramList + ")"), but ensure the boolean is computed conservatively as described.
🤖 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/registry/BuiltinPlatformTypesProvider.java`:
- Around line 199-204: The code collapses a member-level union into a single
TypeRef before calling readSignatures, losing the full union for
SignatureDescriptor; instead pass the full TypeSet (returnTypes) as the fallback
to readSignatures (i.e. replace the single returnType argument with the TypeSet
or TypeSet.of(...) as appropriate). Update the analogous constructor-site call
(where rawCtors are processed) the same way so constructors receive a TypeSet
fallback rather than a single TypeRef; reference
BuiltinPlatformTypesProvider.readSignatures, the local variables
returnTypes/returnType, and the constructors/ctors processing around the other
occurrence.
---
Duplicate comments:
In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionProvider.java`:
- Around line 321-328: The code currently sets ctorHasParameters based solely on
the first constructor; change this to mark a type as parameterless only when
there is exactly one constructor and that constructor has no parameters: use the
result of typeService.getConstructors(ref, fileType) (ctors) and set
ctorHasParameters = !(ctors.size() == 1 && first.parameters().isEmpty()). Keep
using the same first constructor for building the paramList and
item.setDetail("(" + paramList + ")"), but ensure the boolean is computed
conservatively as described.
🪄 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: b974c5c3-eb20-4518-8a8b-4c2bfa43cd86
📒 Files selected for processing (8)
src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/model/MemberDescriptor.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/BslContextPlatformTypesProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/BuiltinPlatformTypesProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/util/SignatureSelection.javasrc/main/resources/com/github/_1c_syntax/bsl/languageserver/types/registry/builtin-platform-types.jsonsrc/test/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionProviderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/BuiltinPlatformTypesProviderTest.java
💤 Files with no reviewable changes (1)
- src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/BslContextPlatformTypesProvider.java
- CompletionProvider: при нескольких перегрузках конструктора курсор остаётся между скобок (ctors.size() > 1) — не считаем тип беспараметровым по первой перегрузке (замечание Copilot/CodeRabbit про Новый HTTPЗапрос). Построение item вынесено в buildPlatformClassCompletionItem (S134 — вложенность). - BuiltinPlatformTypesProvider: в readSignatures fallback теперь передаётся полный union типов возврата метода (TypeSet), а не первый тип — сигнатуры не теряют варианты (замечание CodeRabbit). - SignatureSelection.isAny: строковый литерал слева в сравнении (S1132). - MemberDescriptor.sourceSymbol и PlatformMetadata.accessMode помечены @nullable — оба легитимно nullable (S4449). - Тесты: текст-блоки вместо конкатенации (S6126); регрессии на конструкторы (единственный беспараметровый → `()`, несколько перегрузок → `($0)`). Co-Authored-By: Claude Opus 4.8 <[email protected]>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/BuiltinPlatformTypesProvider.java (1)
211-219:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPreserve unions for non-method members too.
readTypeSet(...)now captures member-level unions, but both property branches still rebuild the descriptor fromreturnType, so every non-method member drops all variants after the first. That makes property type info less accurate than method type info.💡 Proposed fix
if (kind == MemberKind.METHOD) { // returnTypes берём из JSON явно (в т.ч. union), а не выводим из сигнатур — // чтобы тип возврата сохранялся и у методов без описанных параметров. descriptor = new MemberDescriptor(name, MemberKind.METHOD, description, returnTypes, signatures, null, false, PlatformMetadata.EMPTY); } else if (generic) { - descriptor = MemberDescriptor.genericProperty(name, returnType, description); + descriptor = new MemberDescriptor(name, MemberKind.PROPERTY, description, returnTypes, + List.of(), null, true, PlatformMetadata.EMPTY); } else { - descriptor = MemberDescriptor.property(name, returnType, description); + descriptor = MemberDescriptor.property(name, returnTypes, description); }🤖 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/registry/BuiltinPlatformTypesProvider.java` around lines 211 - 219, The non-method branches rebuild MemberDescriptor using returnType which discards member-level unions captured in returnTypes; update the else-if and else branches in BuiltinPlatformTypesProvider so that when kind != MemberKind.METHOD you construct the descriptor using the member-level returnTypes (analogous to the METHOD branch) instead of the single returnType—ensure MemberDescriptor.genericProperty and MemberDescriptor.property calls (or the constructor) receive returnTypes or an equivalent that preserves unions, keeping the generic flag and description handling the same.
🤖 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.
Outside diff comments:
In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/BuiltinPlatformTypesProvider.java`:
- Around line 211-219: The non-method branches rebuild MemberDescriptor using
returnType which discards member-level unions captured in returnTypes; update
the else-if and else branches in BuiltinPlatformTypesProvider so that when kind
!= MemberKind.METHOD you construct the descriptor using the member-level
returnTypes (analogous to the METHOD branch) instead of the single
returnType—ensure MemberDescriptor.genericProperty and MemberDescriptor.property
calls (or the constructor) receive returnTypes or an equivalent that preserves
unions, keeping the generic flag and description handling the same.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8e749c84-54b5-4335-9d46-7c4bf19bf4c1
📒 Files selected for processing (6)
src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/model/MemberDescriptor.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/model/PlatformMetadata.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/BuiltinPlatformTypesProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/util/SignatureSelection.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionProviderTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionProvider.java
|
This was referenced Jun 12, 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.
When a method/function/constructor with no parameters is completed,
insert ready-made "()" and leave the cursor after the parentheses
instead of between them, and skip the triggerParameterHints command
(there is nothing to hint).
Methods with parameters keep the existing snippet "($0)" behavior.
A synthetic "return-type-only" signature flag distinguishes platform
methods whose parameters are simply undocumented (params unknown ->
cursor stays between parens) from genuinely parameterless members.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores