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
cc23af944f
commit
0f373603eb
|
|
@ -515,6 +515,18 @@ fn detect_function_arg_completions<'db>(
|
|||
parsed: &ParsedModuleRef,
|
||||
offset: TextSize,
|
||||
) -> 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 set_function_args = detect_set_function_args(parsed, offset);
|
||||
|
||||
|
|
@ -2594,7 +2606,7 @@ def frob(): ...
|
|||
|
||||
assert_snapshot!(
|
||||
builder.skip_keywords().skip_builtins().build().snapshot(),
|
||||
@"foo=",
|
||||
@"<No completions found after filtering out completions>",
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -2608,7 +2620,7 @@ def frob(): ...
|
|||
|
||||
assert_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.
|
||||
// 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())
|
||||
.find_first(|node| {
|
||||
if !node.is_expr_call() {
|
||||
|
|
|
|||
|
|
@ -283,3 +283,62 @@ TypedDi<CURSOR>
|
|||
|
||||
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 pull_diagnostics;
|
||||
mod rename;
|
||||
mod signature_help;
|
||||
|
||||
use std::collections::{BTreeMap, HashMap, VecDeque};
|
||||
use std::num::NonZeroUsize;
|
||||
|
|
@ -54,7 +55,8 @@ use lsp_types::notification::{
|
|||
};
|
||||
use lsp_types::request::{
|
||||
Completion, DocumentDiagnosticRequest, HoverRequest, Initialize, InlayHintRequest,
|
||||
PrepareRenameRequest, Request, Shutdown, WorkspaceConfiguration, WorkspaceDiagnosticRequest,
|
||||
PrepareRenameRequest, Request, Shutdown, SignatureHelpRequest, WorkspaceConfiguration,
|
||||
WorkspaceDiagnosticRequest,
|
||||
};
|
||||
use lsp_types::{
|
||||
ClientCapabilities, CompletionItem, CompletionParams, CompletionResponse,
|
||||
|
|
@ -64,11 +66,11 @@ use lsp_types::{
|
|||
DocumentDiagnosticParams, DocumentDiagnosticReportResult, FileEvent, Hover, HoverParams,
|
||||
InitializeParams, InitializeResult, InitializedParams, InlayHint, InlayHintClientCapabilities,
|
||||
InlayHintParams, NumberOrString, PartialResultParams, Position, PreviousResultId,
|
||||
PublishDiagnosticsClientCapabilities, Range, TextDocumentClientCapabilities,
|
||||
TextDocumentContentChangeEvent, TextDocumentIdentifier, TextDocumentItem,
|
||||
TextDocumentPositionParams, Url, VersionedTextDocumentIdentifier, WorkDoneProgressParams,
|
||||
WorkspaceClientCapabilities, WorkspaceDiagnosticParams, WorkspaceDiagnosticReportResult,
|
||||
WorkspaceEdit, WorkspaceFolder,
|
||||
PublishDiagnosticsClientCapabilities, Range, SignatureHelp, SignatureHelpParams,
|
||||
SignatureHelpTriggerKind, TextDocumentClientCapabilities, TextDocumentContentChangeEvent,
|
||||
TextDocumentIdentifier, TextDocumentItem, TextDocumentPositionParams, Url,
|
||||
VersionedTextDocumentIdentifier, WorkDoneProgressParams, WorkspaceClientCapabilities,
|
||||
WorkspaceDiagnosticParams, WorkspaceDiagnosticReportResult, WorkspaceEdit, WorkspaceFolder,
|
||||
};
|
||||
use ruff_db::system::{OsSystem, SystemPath, SystemPathBuf, TestSystem};
|
||||
use rustc_hash::FxHashMap;
|
||||
|
|
@ -940,6 +942,28 @@ impl TestServer {
|
|||
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 {
|
||||
|
|
|
|||
|
|
@ -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