mirror of https://github.com/astral-sh/ruff
[ty] Suppress keyword argument completions unless we're in the "arguments"
Otherwise, given a case like this: ``` (lambda foo: (<CURSOR> + 1))(2) ``` we'll offer _argument_ completions for `foo` at the cursor position. While we do actually want to offer completions for `foo` in this context, it is currently difficult to do so. But we definitely don't want to offer completions for `foo` as an argument to a function here. Which is what we were doing. We also add an end-to-end test here to verify that the actual label we offer in completion suggestions includes the `=` suffix. Closes https://github.com/astral-sh/ruff/pull/21970
This commit is contained in:
parent
cc4d1190ae
commit
8877431ee7
|
|
@ -504,6 +504,18 @@ fn detect_function_arg_completions<'db>(
|
||||||
parsed: &ParsedModuleRef,
|
parsed: &ParsedModuleRef,
|
||||||
offset: TextSize,
|
offset: TextSize,
|
||||||
) -> Option<Vec<Completion<'db>>> {
|
) -> Option<Vec<Completion<'db>>> {
|
||||||
|
// But be careful: this isn't as simple as just finding a call
|
||||||
|
// expression. We also have to make sure we are in the "arguments"
|
||||||
|
// portion of the call. Otherwise we risk incorrectly returning
|
||||||
|
// something for `(<CURSOR>)(arg1, arg2)`-style expressions.
|
||||||
|
if !covering_node(parsed.syntax().into(), TextRange::empty(offset))
|
||||||
|
.ancestors()
|
||||||
|
.take_while(|node| !node.is_statement())
|
||||||
|
.any(|node| node.is_arguments())
|
||||||
|
{
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
|
||||||
let sig_help = signature_help(db, file, offset)?;
|
let sig_help = signature_help(db, file, offset)?;
|
||||||
let set_function_args = detect_set_function_args(parsed, offset);
|
let set_function_args = detect_set_function_args(parsed, offset);
|
||||||
|
|
||||||
|
|
@ -2558,7 +2570,7 @@ def frob(): ...
|
||||||
|
|
||||||
assert_snapshot!(
|
assert_snapshot!(
|
||||||
builder.skip_keywords().skip_builtins().build().snapshot(),
|
builder.skip_keywords().skip_builtins().build().snapshot(),
|
||||||
@"foo=",
|
@"<No completions found after filtering out completions>",
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -2572,7 +2584,7 @@ def frob(): ...
|
||||||
|
|
||||||
assert_snapshot!(
|
assert_snapshot!(
|
||||||
builder.skip_keywords().skip_builtins().build().snapshot(),
|
builder.skip_keywords().skip_builtins().build().snapshot(),
|
||||||
@"foo=",
|
@"<No completions found after filtering out completions>",
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -124,6 +124,11 @@ fn get_call_expr(
|
||||||
})?;
|
})?;
|
||||||
|
|
||||||
// Find the covering node at the given position that is a function call.
|
// Find the covering node at the given position that is a function call.
|
||||||
|
// Note that we are okay with the range being anywhere within a call
|
||||||
|
// expression, even if it's not in the arguments portion of the call
|
||||||
|
// expression. This is because, e.g., a user can request signature
|
||||||
|
// information at a call site, and this should ideally work anywhere
|
||||||
|
// within the call site, even at the function name.
|
||||||
let call = covering_node(root_node, token.range())
|
let call = covering_node(root_node, token.range())
|
||||||
.find_first(|node| {
|
.find_first(|node| {
|
||||||
if !node.is_expr_call() {
|
if !node.is_expr_call() {
|
||||||
|
|
|
||||||
|
|
@ -283,3 +283,62 @@ TypedDi<CURSOR>
|
||||||
|
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Tests that completions for function arguments will
|
||||||
|
/// show a `=` suffix.
|
||||||
|
#[test]
|
||||||
|
fn function_parameter_shows_equals_suffix() -> Result<()> {
|
||||||
|
let workspace_root = SystemPath::new("src");
|
||||||
|
let foo = SystemPath::new("src/foo.py");
|
||||||
|
let foo_content = "\
|
||||||
|
import re
|
||||||
|
re.match('', '', fla<CURSOR>
|
||||||
|
";
|
||||||
|
|
||||||
|
let mut server = TestServerBuilder::new()?
|
||||||
|
.with_initialization_options(ClientOptions::default().with_auto_import(false))
|
||||||
|
.with_workspace(workspace_root, None)?
|
||||||
|
.with_file(foo, foo_content)?
|
||||||
|
.build()
|
||||||
|
.wait_until_workspaces_are_initialized();
|
||||||
|
|
||||||
|
server.open_text_document(foo, foo_content, 1);
|
||||||
|
let _ = server.await_notification::<PublishDiagnostics>();
|
||||||
|
|
||||||
|
let completions = server.completion_request(&server.file_uri(foo), Position::new(1, 20));
|
||||||
|
|
||||||
|
insta::assert_json_snapshot!(completions, @r#"
|
||||||
|
[
|
||||||
|
{
|
||||||
|
"label": "flags=",
|
||||||
|
"kind": 6,
|
||||||
|
"detail": "int",
|
||||||
|
"sortText": "0",
|
||||||
|
"insertText": "flags="
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"label": "FloatingPointError",
|
||||||
|
"kind": 7,
|
||||||
|
"detail": "<class 'FloatingPointError'>",
|
||||||
|
"documentation": "Floating-point operation failed.\n",
|
||||||
|
"sortText": "1"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"label": "PythonFinalizationError",
|
||||||
|
"kind": 7,
|
||||||
|
"detail": "<class 'PythonFinalizationError'>",
|
||||||
|
"documentation": "Operation blocked during Python finalization.\n",
|
||||||
|
"sortText": "2"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"label": "float",
|
||||||
|
"kind": 7,
|
||||||
|
"detail": "<class 'float'>",
|
||||||
|
"documentation": "Convert a string or number to a floating-point number, if possible.\n",
|
||||||
|
"sortText": "3"
|
||||||
|
}
|
||||||
|
]
|
||||||
|
"#);
|
||||||
|
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -36,6 +36,7 @@ mod notebook;
|
||||||
mod publish_diagnostics;
|
mod publish_diagnostics;
|
||||||
mod pull_diagnostics;
|
mod pull_diagnostics;
|
||||||
mod rename;
|
mod rename;
|
||||||
|
mod signature_help;
|
||||||
|
|
||||||
use std::collections::{BTreeMap, HashMap, VecDeque};
|
use std::collections::{BTreeMap, HashMap, VecDeque};
|
||||||
use std::num::NonZeroUsize;
|
use std::num::NonZeroUsize;
|
||||||
|
|
@ -54,7 +55,8 @@ use lsp_types::notification::{
|
||||||
};
|
};
|
||||||
use lsp_types::request::{
|
use lsp_types::request::{
|
||||||
Completion, DocumentDiagnosticRequest, HoverRequest, Initialize, InlayHintRequest,
|
Completion, DocumentDiagnosticRequest, HoverRequest, Initialize, InlayHintRequest,
|
||||||
PrepareRenameRequest, Request, Shutdown, WorkspaceConfiguration, WorkspaceDiagnosticRequest,
|
PrepareRenameRequest, Request, Shutdown, SignatureHelpRequest, WorkspaceConfiguration,
|
||||||
|
WorkspaceDiagnosticRequest,
|
||||||
};
|
};
|
||||||
use lsp_types::{
|
use lsp_types::{
|
||||||
ClientCapabilities, CompletionItem, CompletionParams, CompletionResponse,
|
ClientCapabilities, CompletionItem, CompletionParams, CompletionResponse,
|
||||||
|
|
@ -64,11 +66,11 @@ use lsp_types::{
|
||||||
DocumentDiagnosticParams, DocumentDiagnosticReportResult, FileEvent, Hover, HoverParams,
|
DocumentDiagnosticParams, DocumentDiagnosticReportResult, FileEvent, Hover, HoverParams,
|
||||||
InitializeParams, InitializeResult, InitializedParams, InlayHint, InlayHintClientCapabilities,
|
InitializeParams, InitializeResult, InitializedParams, InlayHint, InlayHintClientCapabilities,
|
||||||
InlayHintParams, NumberOrString, PartialResultParams, Position, PreviousResultId,
|
InlayHintParams, NumberOrString, PartialResultParams, Position, PreviousResultId,
|
||||||
PublishDiagnosticsClientCapabilities, Range, TextDocumentClientCapabilities,
|
PublishDiagnosticsClientCapabilities, Range, SignatureHelp, SignatureHelpParams,
|
||||||
TextDocumentContentChangeEvent, TextDocumentIdentifier, TextDocumentItem,
|
SignatureHelpTriggerKind, TextDocumentClientCapabilities, TextDocumentContentChangeEvent,
|
||||||
TextDocumentPositionParams, Url, VersionedTextDocumentIdentifier, WorkDoneProgressParams,
|
TextDocumentIdentifier, TextDocumentItem, TextDocumentPositionParams, Url,
|
||||||
WorkspaceClientCapabilities, WorkspaceDiagnosticParams, WorkspaceDiagnosticReportResult,
|
VersionedTextDocumentIdentifier, WorkDoneProgressParams, WorkspaceClientCapabilities,
|
||||||
WorkspaceEdit, WorkspaceFolder,
|
WorkspaceDiagnosticParams, WorkspaceDiagnosticReportResult, WorkspaceEdit, WorkspaceFolder,
|
||||||
};
|
};
|
||||||
use ruff_db::system::{OsSystem, SystemPath, SystemPathBuf, TestSystem};
|
use ruff_db::system::{OsSystem, SystemPath, SystemPathBuf, TestSystem};
|
||||||
use rustc_hash::FxHashMap;
|
use rustc_hash::FxHashMap;
|
||||||
|
|
@ -940,6 +942,28 @@ impl TestServer {
|
||||||
None => vec![],
|
None => vec![],
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Sends a `textDocument/signatureHelp` request for the document at the given URL and position.
|
||||||
|
pub(crate) fn signature_help_request(
|
||||||
|
&mut self,
|
||||||
|
uri: &Url,
|
||||||
|
position: Position,
|
||||||
|
) -> Option<SignatureHelp> {
|
||||||
|
let signature_help_id = self.send_request::<SignatureHelpRequest>(SignatureHelpParams {
|
||||||
|
text_document_position_params: TextDocumentPositionParams {
|
||||||
|
text_document: TextDocumentIdentifier { uri: uri.clone() },
|
||||||
|
position,
|
||||||
|
},
|
||||||
|
work_done_progress_params: lsp_types::WorkDoneProgressParams::default(),
|
||||||
|
context: Some(lsp_types::SignatureHelpContext {
|
||||||
|
trigger_kind: SignatureHelpTriggerKind::INVOKED,
|
||||||
|
trigger_character: None,
|
||||||
|
is_retrigger: false,
|
||||||
|
active_signature_help: None,
|
||||||
|
}),
|
||||||
|
});
|
||||||
|
self.await_response::<SignatureHelpRequest>(&signature_help_id)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
impl fmt::Debug for TestServer {
|
impl fmt::Debug for TestServer {
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,37 @@
|
||||||
|
use anyhow::Result;
|
||||||
|
use lsp_types::{Position, notification::PublishDiagnostics};
|
||||||
|
use ruff_db::system::SystemPath;
|
||||||
|
use ty_server::ClientOptions;
|
||||||
|
|
||||||
|
use crate::TestServerBuilder;
|
||||||
|
|
||||||
|
/// Tests that we get signature help even when the cursor
|
||||||
|
/// is on the function name.
|
||||||
|
///
|
||||||
|
/// This is a regression test to ensure we don't accidentally
|
||||||
|
/// cause this case to stop working.
|
||||||
|
#[test]
|
||||||
|
fn works_in_function_name() -> Result<()> {
|
||||||
|
let workspace_root = SystemPath::new("src");
|
||||||
|
let foo = SystemPath::new("src/foo.py");
|
||||||
|
let foo_content = "\
|
||||||
|
import re
|
||||||
|
re.match('', '')
|
||||||
|
";
|
||||||
|
|
||||||
|
let mut server = TestServerBuilder::new()?
|
||||||
|
.with_initialization_options(ClientOptions::default())
|
||||||
|
.with_workspace(workspace_root, None)?
|
||||||
|
.with_file(foo, foo_content)?
|
||||||
|
.build()
|
||||||
|
.wait_until_workspaces_are_initialized();
|
||||||
|
|
||||||
|
server.open_text_document(foo, foo_content, 1);
|
||||||
|
let _ = server.await_notification::<PublishDiagnostics>();
|
||||||
|
|
||||||
|
let signature_help = server.signature_help_request(&server.file_uri(foo), Position::new(1, 6));
|
||||||
|
|
||||||
|
insta::assert_json_snapshot!(signature_help);
|
||||||
|
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,39 @@
|
||||||
|
---
|
||||||
|
source: crates/ty_server/tests/e2e/signature_help.rs
|
||||||
|
expression: signature_help
|
||||||
|
---
|
||||||
|
{
|
||||||
|
"signatures": [
|
||||||
|
{
|
||||||
|
"label": "(pattern: str | Pattern[str], string: str, flags: int = Literal[0]) -> Match[str] | None",
|
||||||
|
"documentation": "Try to apply the pattern at the start of the string, returning/na Match object, or None if no match was found.\n",
|
||||||
|
"parameters": [
|
||||||
|
{
|
||||||
|
"label": "pattern: str | Pattern[str]"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"label": "string: str"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"label": "flags: int = Literal[0]"
|
||||||
|
}
|
||||||
|
]
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"label": "(pattern: bytes | Pattern[bytes], string: Buffer, flags: int = Literal[0]) -> Match[bytes] | None",
|
||||||
|
"parameters": [
|
||||||
|
{
|
||||||
|
"label": "pattern: bytes | Pattern[bytes]"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"label": "string: Buffer"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"label": "flags: int = Literal[0]"
|
||||||
|
}
|
||||||
|
]
|
||||||
|
}
|
||||||
|
],
|
||||||
|
"activeSignature": 0,
|
||||||
|
"activeParameter": 0
|
||||||
|
}
|
||||||
Loading…
Reference in New Issue