Skip to content
Draft
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
6 changes: 5 additions & 1 deletion crates/ruff_python_parser/src/parser/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@ pub(super) fn set_expr_ctx(expr: &mut Expr, new_ctx: ExprContext) {
set_expr_ctx(value, new_ctx);
}
Expr::UnaryOp(ast::ExprUnaryOp { operand, .. }) => {
set_expr_ctx(operand, new_ctx);
// `UnaryOp` is never a valid assignment or deletion target. When the parser
// recovers from bad input like `a, not = ...`, the operand isn't genuinely
// in target position, so mark it `Invalid` rather than propagating `new_ctx`
// and misleading consumers into treating it as a real target sub-expression.
set_expr_ctx(operand, ExprContext::Invalid);
}
Expr::List(ast::ExprList { elts, ctx, .. })
| Expr::Tuple(ast::ExprTuple { elts, ctx, .. }) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ Module(
node_index: NodeIndex(None),
range: 72..73,
id: Name("x"),
ctx: Store,
ctx: Invalid,
},
),
},
Expand Down Expand Up @@ -239,7 +239,7 @@ Module(
node_index: NodeIndex(None),
range: 92..93,
id: Name("x"),
ctx: Store,
ctx: Invalid,
},
),
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ Module(
node_index: NodeIndex(None),
range: 341..342,
id: Name("x"),
ctx: Store,
ctx: Invalid,
},
),
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ Module(
node_index: NodeIndex(None),
range: 138..139,
id: Name("x"),
ctx: Store,
ctx: Invalid,
},
),
},
Expand Down
102 changes: 101 additions & 1 deletion crates/ty_server/tests/e2e/publish_diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::time::Duration;

use anyhow::Result;
use lsp_types::{
DidOpenTextDocumentParams, FileChangeType, FileEvent, TextDocumentItem, Url,
DidOpenTextDocumentParams, FileChangeType, FileEvent, Position, Range, TextDocumentItem, Url,
notification::{DidOpenTextDocument, PublishDiagnostics},
};
use ruff_db::system::SystemPath;
Expand Down Expand Up @@ -214,6 +214,106 @@ def foo() -> str:
Ok(())
}

#[test]
fn on_did_change_invalid_tuple_assignment_target_does_not_panic() -> Result<()> {
let workspace_root = SystemPath::new("src");
let foo = SystemPath::new("src/foo.py");
let foo_content = "\
something, somethingelse = (1, 2)
";

let mut server = TestServerBuilder::new()?
.with_workspace(workspace_root, None)?
.with_file(foo, foo_content)?
.enable_pull_diagnostics(false)
.build()
.wait_until_workspaces_are_initialized();

server.open_text_document(foo, foo_content, 1);
let _ = server.await_notification::<PublishDiagnostics>();

server.change_text_document(
foo,
vec![lsp_types::TextDocumentContentChangeEvent {
range: Some(Range::new(Position::new(0, 11), Position::new(0, 24))),
range_length: None,
text: "not".to_string(),
}],
2,
);

let diagnostics = server.await_notification::<PublishDiagnostics>();

assert_eq!(diagnostics.version, Some(2));

insta::assert_debug_snapshot!(diagnostics);

Ok(())
}

#[test]
fn on_did_change_nested_invalid_tuple_assignment_target_does_not_panic() -> Result<()> {
let workspace_root = SystemPath::new("src");
let foo = SystemPath::new("src/foo.py");
let foo_content = "\
something, somethingelse = (1, 2)
";

let mut server = TestServerBuilder::new()?
.with_workspace(workspace_root, None)?
.with_file(foo, foo_content)?
.enable_pull_diagnostics(false)
.build()
.wait_until_workspaces_are_initialized();

server.open_text_document(foo, foo_content, 1);
let _ = server.await_notification::<PublishDiagnostics>();

server.change_text_document(
foo,
vec![lsp_types::TextDocumentContentChangeEvent {
range: Some(Range::new(Position::new(0, 11), Position::new(0, 24))),
range_length: None,
text: "not x".to_string(),
}],
2,
);

let diagnostics = server.await_notification::<PublishDiagnostics>();

assert_eq!(diagnostics.version, Some(2));

insta::assert_debug_snapshot!(diagnostics);

Ok(())
}

#[test]
fn completions_do_not_panic_on_invalid_not_target() -> Result<()> {
let workspace_root = SystemPath::new("src");
let foo = SystemPath::new("src/foo.py");
let foo_content = "\
something, not x = (1, 2)
something
";

let mut server = TestServerBuilder::new()?
.with_workspace(workspace_root, None)?
.with_file(foo, foo_content)?
.enable_pull_diagnostics(false)
.build()
.wait_until_workspaces_are_initialized();

server.open_text_document(foo, foo_content, 1);
let _ = server.await_notification::<PublishDiagnostics>();

// Ask for completions — forces `all_reachable_members` which calls
// `binding_type` on every live binding in scope.
let _ = server.completion_request(&server.file_uri(foo), Position::new(1, 9));

Ok(())
}

#[test]
fn on_did_change_diagnostics_off() -> Result<()> {
let workspace_root = SystemPath::new("src");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
---
source: crates/ty_server/tests/e2e/publish_diagnostics.rs
expression: diagnostics
---
PublishDiagnosticsParams {
uri: Url {
scheme: "file",
cannot_be_a_base: false,
username: "",
password: None,
host: None,
port: None,
path: "<temp_dir>/src/foo.py",
query: None,
fragment: None,
},
diagnostics: [
Diagnostic {
range: Range {
start: Position {
line: 0,
character: 11,
},
end: Position {
line: 0,
character: 14,
},
},
severity: Some(
Error,
),
code: Some(
String(
"invalid-syntax",
),
),
code_description: None,
source: Some(
"ty",
),
message: "Invalid assignment target",
related_information: None,
tags: None,
data: None,
},
Diagnostic {
range: Range {
start: Position {
line: 0,
character: 15,
},
end: Position {
line: 0,
character: 16,
},
},
severity: Some(
Error,
),
code: Some(
String(
"invalid-syntax",
),
),
code_description: None,
source: Some(
"ty",
),
message: "Expected an expression",
related_information: None,
tags: None,
data: None,
},
],
version: Some(
2,
),
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
---
source: crates/ty_server/tests/e2e/publish_diagnostics.rs
expression: diagnostics
---
PublishDiagnosticsParams {
uri: Url {
scheme: "file",
cannot_be_a_base: false,
username: "",
password: None,
host: None,
port: None,
path: "<temp_dir>/src/foo.py",
query: None,
fragment: None,
},
diagnostics: [
Diagnostic {
range: Range {
start: Position {
line: 0,
character: 11,
},
end: Position {
line: 0,
character: 16,
},
},
severity: Some(
Error,
),
code: Some(
String(
"invalid-syntax",
),
),
code_description: None,
source: Some(
"ty",
),
message: "Invalid assignment target",
related_information: None,
tags: None,
data: None,
},
],
version: Some(
2,
),
}
Loading