diff --git a/editors/vscode/src/extension.ts b/editors/vscode/src/extension.ts index f9e57bfd9..159ce797d 100644 --- a/editors/vscode/src/extension.ts +++ b/editors/vscode/src/extension.ts @@ -83,6 +83,23 @@ export async function activate(context: ExtensionContext) { synchronize: { fileEvents: workspace.createFileSystemWatcher("**/.clientrc"), }, + middleware: { + // Space triggers exist only for `import ` module completion. + // This guard is intentionally stricter than the server-side + // detection (exact single-space forms only): it merely avoids + // request round-trips, while the server independently answers + // space triggers outside import contexts with an empty list. + provideCompletionItem: async (document, position, context, token, next) => { + if (context.triggerCharacter === " ") { + const line = document.lineAt(position.line).text.slice(0, position.character); + const trimmed = line.trimStart(); + if (trimmed !== "import " && trimmed !== "export import ") { + return []; + } + } + return next(document, position, context, token); + }, + }, }; client = new LanguageClient("clice", "clice", serverOptions, clientOptions); diff --git a/editors/vscode/src/test/e2e.test.ts b/editors/vscode/src/test/e2e.test.ts index 84273de8a..4380a4afd 100644 --- a/editors/vscode/src/test/e2e.test.ts +++ b/editors/vscode/src/test/e2e.test.ts @@ -182,4 +182,29 @@ suite("clice E2E", function () { ); assert.ok(completions.items.length > 0, "completion returned no items"); }); + + test("space trigger gated outside imports", async function () { + this.timeout(60 * 1000); + assert.ok(document, "main file was not opened (earlier test failed)"); + + // The middleware swallows space-triggered requests on non-import + // lines; the cursor sits inside ordinary code here. VS Code still + // contributes word-based suggestions (kind Text), so assert that + // nothing beyond those — i.e. no server-provided item — shows up. + const completions = await vscode.commands.executeCommand( + "vscode.executeCompletionItemProvider", + document.uri, + position.translate(0, 1), + " ", + ); + const serverItems = completions.items.filter( + (item) => item.kind !== undefined && item.kind !== vscode.CompletionItemKind.Text, + ); + const labels = serverItems.slice(0, 10).map((item) => item.label); + assert.strictEqual( + serverItems.length, + 0, + `space trigger outside an import line must yield no server items, got: ${JSON.stringify(labels)}`, + ); + }); }); diff --git a/src/server/service/feature_router.cpp b/src/server/service/feature_router.cpp index 51083746d..024850ec1 100644 --- a/src/server/service/feature_router.cpp +++ b/src/server/service/feature_router.cpp @@ -235,6 +235,7 @@ FeatureRouter::RawResult FeatureRouter::code_action(std::shared_ptr ses FeatureRouter::RawResult FeatureRouter::completion(std::shared_ptr session, const protocol::Position& position, + llvm::StringRef trigger_character, std::optional token) { auto pause = indexer.scoped_pause(); @@ -252,8 +253,21 @@ FeatureRouter::RawResult FeatureRouter::completion(std::shared_ptr sess auto map = session->line_map(); auto offset = map.to_offset(position); + + PreambleCompletionContext pctx; + if(offset) { + pctx = detect_completion_context(session->text, *offset); + } + + // Space is advertised as a trigger character only so that `import ` + // opens module suggestions. Clients without request-side gating + // (nvim, zed) forward every space keystroke; answer everything else + // with an empty list before any include scanning or completion build. + if(trigger_character == " " && pctx.kind != CompletionContext::Import) { + co_return serde_raw{"[]"}; + } + if(offset) { - auto pctx = detect_completion_context(session->text, *offset); if(pctx.kind == CompletionContext::IncludeQuoted || pctx.kind == CompletionContext::IncludeAngled) { std::string directory; diff --git a/src/server/service/feature_router.h b/src/server/service/feature_router.h index 334e86004..3546643ba 100644 --- a/src/server/service/feature_router.h +++ b/src/server/service/feature_router.h @@ -100,8 +100,10 @@ class FeatureRouter { /// Code completion. Serves preamble contexts (include/import) locally from /// the include graph and module map; delegates ordinary code completion to /// a stateless worker. Pauses background indexing for the request's span. + /// Space-triggered requests are only answered for import contexts. RawResult completion(std::shared_ptr session, const protocol::Position& position, + llvm::StringRef trigger_character = {}, std::optional token = {}); /// Signature help, forwarded to a stateless build. Pauses background diff --git a/src/server/transport/lsp_client.cpp b/src/server/transport/lsp_client.cpp index 0ea37dda6..0c643c98c 100644 --- a/src/server/transport/lsp_client.cpp +++ b/src/server/transport/lsp_client.cpp @@ -120,7 +120,7 @@ void LSPClient::register_lifecycle() { caps.hover_provider = true; caps.completion_provider = protocol::CompletionOptions{ - .trigger_characters = StringVec{".", "<", ">", ":", "\"", "/", "*"}, + .trigger_characters = StringVec{".", "<", ">", ":", "\"", "/", "*", " "}, }; caps.signature_help_provider = protocol::SignatureHelpOptions{ .trigger_characters = StringVec{"(", ")", "{", "}", "<", ">", ","}, @@ -443,8 +443,13 @@ void LSPClient::register_language_features() { resolve_uri(params.text_document_position_params.text_document.uri); if(!session) co_return kota::outcome_error(document_not_open()); + llvm::StringRef trigger; + if(params.context && params.context->trigger_character) { + trigger = *params.context->trigger_character; + } co_return co_await srv.features.completion(session, params.text_document_position_params.position, + trigger, ctx.cancellation); }); diff --git a/src/syntax/completion.cpp b/src/syntax/completion.cpp index 5d68f69f7..0fe7e5a9d 100644 --- a/src/syntax/completion.cpp +++ b/src/syntax/completion.cpp @@ -10,6 +10,8 @@ namespace clice { PreambleCompletionContext detect_completion_context(llvm::StringRef text, std::uint32_t offset) { + // TODO: cache newline offsets from incremental text updates to avoid + // the linear rfind/find scans on every completion trigger. auto line_start = text.rfind('\n', offset > 0 ? offset - 1 : 0); line_start = (line_start == llvm::StringRef::npos) ? 0 : line_start + 1; @@ -30,6 +32,9 @@ PreambleCompletionContext detect_completion_context(llvm::StringRef text, std::u return {}; } + // FIXME: the import detection is purely textual and can false-positive + // on a type named `import` (context-sensitive keyword). Use the + // module-name scanner from the syntax module for precise disambiguation. auto import_check = trimmed; if(import_check.consume_front("export") && !import_check.empty() && !std::isalnum(import_check[0])) { @@ -55,6 +60,10 @@ std::vector complete_module_import(const llvm::DenseMap& modules, llvm::StringRef prefix) { std::vector results; + // FIXME: exclude the current file's own module name from results + // (self-import is never valid). Needs the requesting path_id passed in. + // TODO: `modules` is only refreshed on file save; unsaved new module + // files won't appear in completions until written to disk. for(auto& [path_id, module_name]: modules) { if(llvm::StringRef(module_name).starts_with(prefix)) { results.push_back(module_name); diff --git a/tests/integration/features/test_completion.py b/tests/integration/features/test_completion.py index 183e90360..2ba3fe17b 100644 --- a/tests/integration/features/test_completion.py +++ b/tests/integration/features/test_completion.py @@ -124,6 +124,50 @@ async def test_import_completion_basic(client, workspace): assert "A" in labels, f"Expected 'A' in completion labels, got: {labels}" +@pytest.mark.workspace("modules/chained_modules") +async def test_space_trigger_serves_import(client, workspace): + """Space-triggered completion on an import line lists modules.""" + await client.open_and_wait(workspace / "mod_a.cppm") + + uri_b, _ = client.open(workspace / "mod_b.cppm") + did_change(client, uri_b, 1, "import ") + + result = await client.completion_at(uri_b, 0, 7, trigger_character=" ") + + assert result is not None + items = result.items if hasattr(result, "items") else result + labels = [item.label for item in items] + assert "A" in labels, f"Expected 'A' in completion labels, got: {labels}" + + +@pytest.mark.workspace("modules/chained_modules") +async def test_space_trigger_gated_elsewhere(client, workspace): + """Space-triggered completion outside import lines returns no items.""" + uri_b, _ = client.open(workspace / "mod_b.cppm") + did_change(client, uri_b, 1, "int main() { return 0; }") + + # Cursor right after "return " — a space trigger here must be answered + # with an empty list instead of a full completion build. + result = await client.completion_at(uri_b, 0, 20, trigger_character=" ") + + items = result.items if hasattr(result, "items") else result + assert not items, f"Expected no items for gated space trigger, got: {items}" + + +@pytest.mark.workspace("include_completion") +async def test_space_trigger_gated_include(client, workspace): + """Space-triggered completion in an include context returns no items.""" + uri, _ = await client.open_and_wait(workspace / "main.cpp") + did_change(client, uri, 1, "#include ") + + # The space gate must run before include scanning: no directory + # enumeration and no candidates for a trailing-space trigger. + result = await client.completion_at(uri, 0, 18, trigger_character=" ") + + items = result.items if hasattr(result, "items") else result + assert not items, f"Expected no items for gated space trigger, got: {items}" + + @pytest.mark.workspace("modules/chained_modules") async def test_import_completion_with_prefix(client, workspace): """Import completion with prefix should filter to matching modules.""" diff --git a/tests/integration/features/test_server.py b/tests/integration/features/test_server.py index 6b28c5028..1d49ceb2f 100644 --- a/tests/integration/features/test_server.py +++ b/tests/integration/features/test_server.py @@ -40,6 +40,7 @@ def capability_enabled(capability: object) -> bool: caps = client.init_result.capabilities assert caps.hover_provider is True assert caps.completion_provider is not None + assert " " in caps.completion_provider.trigger_characters assert capability_enabled(caps.definition_provider) assert capability_enabled(caps.document_symbol_provider) assert capability_enabled(caps.folding_range_provider) diff --git a/tests/tools/client.py b/tests/tools/client.py index fda469e83..e7f9abc13 100644 --- a/tests/tools/client.py +++ b/tests/tools/client.py @@ -12,7 +12,9 @@ ClientCapabilities, CodeActionContext, CodeActionParams, + CompletionContext, CompletionParams, + CompletionTriggerKind, DeclarationParams, DefinitionParams, Diagnostic, @@ -298,14 +300,27 @@ async def references_at( ) async def completion_at( - self, uri: str, line: int, character: int, *, timeout: float = 30.0 + self, + uri: str, + line: int, + character: int, + *, + trigger_character: str | None = None, + timeout: float = 30.0, ): """Send completion request at given position.""" + context = None + if trigger_character is not None: + context = CompletionContext( + trigger_kind=CompletionTriggerKind.TriggerCharacter, + trigger_character=trigger_character, + ) return await asyncio.wait_for( self.text_document_completion_async( CompletionParams( text_document=TextDocumentIdentifier(uri=uri), position=Position(line=line, character=character), + context=context, ) ), timeout=timeout, diff --git a/tests/unit/syntax/completion_tests.cpp b/tests/unit/syntax/completion_tests.cpp index aed64c2ed..28e8c870f 100644 --- a/tests/unit/syntax/completion_tests.cpp +++ b/tests/unit/syntax/completion_tests.cpp @@ -77,6 +77,50 @@ TEST_CASE(HashOnly) { EXPECT_EQ(ctx.kind, CompletionContext::None); } +TEST_CASE(ImportDottedPrefix) { + auto ctx = detect_completion_context("import std.io", 13); + EXPECT_EQ(ctx.kind, CompletionContext::Import); + EXPECT_EQ(ctx.prefix, "std.io"); +} + +TEST_CASE(ImportPartitionPrefix) { + auto ctx = detect_completion_context("import :core", 12); + EXPECT_EQ(ctx.kind, CompletionContext::Import); + EXPECT_EQ(ctx.prefix, ":core"); +} + +TEST_CASE(ImportPartitionEmpty) { + auto ctx = detect_completion_context("import :", 8); + EXPECT_EQ(ctx.kind, CompletionContext::Import); + EXPECT_EQ(ctx.prefix, ":"); +} + +TEST_CASE(ImportWithLeadingSpaces) { + auto ctx = detect_completion_context(" import std", 12); + EXPECT_EQ(ctx.kind, CompletionContext::Import); + EXPECT_EQ(ctx.prefix, "std"); +} + +TEST_CASE(ExportImportEmpty) { + auto ctx = detect_completion_context("export import ", 14); + EXPECT_EQ(ctx.kind, CompletionContext::Import); + EXPECT_EQ(ctx.prefix, ""); +} + +TEST_CASE(ImportAfterNewline) { + std::string text = "module foo;\nimport "; + auto ctx = detect_completion_context(text, text.size()); + EXPECT_EQ(ctx.kind, CompletionContext::Import); + EXPECT_EQ(ctx.prefix, ""); +} + +TEST_CASE(ImportCursorMidLine) { + // The prefix is truncated at the cursor; trailing text is ignored. + auto ctx = detect_completion_context("import std.io", 10); + EXPECT_EQ(ctx.kind, CompletionContext::Import); + EXPECT_EQ(ctx.prefix, "std"); +} + }; // TEST_SUITE(DetectCompletionContext) TEST_SUITE(CompleteModuleImport) { @@ -119,6 +163,43 @@ TEST_CASE(EmptyModules) { EXPECT_TRUE(results.empty()); } +TEST_CASE(DottedPrefix) { + llvm::DenseMap modules; + modules[1] = "std"; + modules[2] = "std.io"; + modules[3] = "std.core"; + modules[4] = "boost.asio"; + + auto results = complete_module_import(modules, "std."); + EXPECT_EQ(results.size(), 2u); + for(auto& name: results) { + EXPECT_TRUE(name.starts_with("std.")); + } +} + +TEST_CASE(PartitionPrefix) { + llvm::DenseMap modules; + modules[1] = "foo"; + modules[2] = "foo:core"; + modules[3] = "foo:utils"; + modules[4] = "bar:impl"; + + auto results = complete_module_import(modules, "foo:"); + EXPECT_EQ(results.size(), 2u); + for(auto& name: results) { + EXPECT_TRUE(name.starts_with("foo:")); + } +} + +TEST_CASE(PrefixIsFullName) { + llvm::DenseMap modules; + modules[1] = "std"; + modules[2] = "std.io"; + + auto results = complete_module_import(modules, "std"); + EXPECT_EQ(results.size(), 2u); +} + }; // TEST_SUITE(CompleteModuleImport) } // namespace