Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions editors/vscode/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
25 changes: 25 additions & 0 deletions editors/vscode/src/test/e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.CompletionList>(
"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)}`,
);
});
});
16 changes: 15 additions & 1 deletion src/server/service/feature_router.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ FeatureRouter::RawResult FeatureRouter::code_action(std::shared_ptr<Session> ses

FeatureRouter::RawResult FeatureRouter::completion(std::shared_ptr<Session> session,
const protocol::Position& position,
llvm::StringRef trigger_character,
std::optional<kota::cancellation_token> token) {
auto pause = indexer.scoped_pause();

Expand All @@ -252,8 +253,21 @@ FeatureRouter::RawResult FeatureRouter::completion(std::shared_ptr<Session> 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;
Expand Down
2 changes: 2 additions & 0 deletions src/server/service/feature_router.h
Original file line number Diff line number Diff line change
Expand Up @@ -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> session,
const protocol::Position& position,
llvm::StringRef trigger_character = {},
std::optional<kota::cancellation_token> token = {});

/// Signature help, forwarded to a stateless build. Pauses background
Expand Down
7 changes: 6 additions & 1 deletion src/server/transport/lsp_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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{"(", ")", "{", "}", "<", ">", ","},
Expand Down Expand Up @@ -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);
});

Expand Down
9 changes: 9 additions & 0 deletions src/syntax/completion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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])) {
Expand All @@ -55,6 +60,10 @@ std::vector<std::string>
complete_module_import(const llvm::DenseMap<std::uint32_t, std::string>& modules,
llvm::StringRef prefix) {
std::vector<std::string> 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);
Expand Down
44 changes: 44 additions & 0 deletions tests/integration/features/test_completion.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 <vector> ")

# 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."""
Expand Down
1 change: 1 addition & 0 deletions tests/integration/features/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
17 changes: 16 additions & 1 deletion tests/tools/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@
ClientCapabilities,
CodeActionContext,
CodeActionParams,
CompletionContext,
CompletionParams,
CompletionTriggerKind,
DeclarationParams,
DefinitionParams,
Diagnostic,
Expand Down Expand Up @@ -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,
Expand Down
81 changes: 81 additions & 0 deletions tests/unit/syntax/completion_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -119,6 +163,43 @@ TEST_CASE(EmptyModules) {
EXPECT_TRUE(results.empty());
}

TEST_CASE(DottedPrefix) {
llvm::DenseMap<std::uint32_t, std::string> 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<std::uint32_t, std::string> 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<std::uint32_t, std::string> modules;
modules[1] = "std";
modules[2] = "std.io";

auto results = complete_module_import(modules, "std");
EXPECT_EQ(results.size(), 2u);
}

}; // TEST_SUITE(CompleteModuleImport)

} // namespace
Expand Down