From f7528bd325e20205facad414cf8462e922bbffa8 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Wed, 10 Dec 2025 17:47:41 +0100 Subject: [PATCH] [ty] Checking files without extension (#21867) --- crates/ty/tests/cli/main.rs | 47 +++++++++++++ crates/ty_project/src/walk.rs | 15 ++-- crates/ty_server/src/session.rs | 17 ++++- crates/ty_server/src/session/index.rs | 15 ++-- crates/ty_server/tests/e2e/main.rs | 1 - .../tests/e2e/publish_diagnostics.rs | 68 +++++++++++++++++- ..._language_of_file_without_extension-2.snap | 21 ++++++ ...ng_language_of_file_without_extension.snap | 70 +++++++++++++++++++ ...without_extension_but_python_language.snap | 70 +++++++++++++++++++ 9 files changed, 312 insertions(+), 12 deletions(-) create mode 100644 crates/ty_server/tests/e2e/snapshots/e2e__publish_diagnostics__changing_language_of_file_without_extension-2.snap create mode 100644 crates/ty_server/tests/e2e/snapshots/e2e__publish_diagnostics__changing_language_of_file_without_extension.snap create mode 100644 crates/ty_server/tests/e2e/snapshots/e2e__publish_diagnostics__on_did_open_file_without_extension_but_python_language.snap diff --git a/crates/ty/tests/cli/main.rs b/crates/ty/tests/cli/main.rs index a352ba355a..546d3ac260 100644 --- a/crates/ty/tests/cli/main.rs +++ b/crates/ty/tests/cli/main.rs @@ -580,6 +580,53 @@ fn check_non_existing_path() -> anyhow::Result<()> { Ok(()) } +#[test] +fn check_file_without_extension() -> anyhow::Result<()> { + let case = CliTest::with_file("main", "a = b")?; + + assert_cmd_snapshot!( + case.command().arg("main"), + @r" + success: false + exit_code: 1 + ----- stdout ----- + error[unresolved-reference]: Name `b` used when not defined + --> main:1:5 + | + 1 | a = b + | ^ + | + info: rule `unresolved-reference` is enabled by default + + Found 1 diagnostic + + ----- stderr ----- + " + ); + + Ok(()) +} + +#[test] +fn check_file_without_extension_in_subfolder() -> anyhow::Result<()> { + let case = CliTest::with_file("src/main", "a = b")?; + + assert_cmd_snapshot!( + case.command().arg("src"), + @r" + success: true + exit_code: 0 + ----- stdout ----- + All checks passed! + + ----- stderr ----- + WARN No python files found under the given path(s) + " + ); + + Ok(()) +} + #[test] fn concise_diagnostics() -> anyhow::Result<()> { let case = CliTest::with_file( diff --git a/crates/ty_project/src/walk.rs b/crates/ty_project/src/walk.rs index 8c9958c416..16a73dd65f 100644 --- a/crates/ty_project/src/walk.rs +++ b/crates/ty_project/src/walk.rs @@ -202,11 +202,16 @@ impl<'a> ProjectFilesWalker<'a> { } } else { // Ignore any non python files to avoid creating too many entries in `Files`. - if entry - .path() - .extension() - .and_then(PySourceType::try_from_extension) - .is_none() + // Unless the file is explicitly passed, we then always assume it's a python file. + let source_type = entry.path().extension().and_then(PySourceType::try_from_extension).or_else(|| { + if entry.depth() == 0 { + Some(PySourceType::Python) + } else { + db.system().source_type(entry.path()) + } + }); + + if source_type.is_none() { return WalkState::Continue; } diff --git a/crates/ty_server/src/session.rs b/crates/ty_server/src/session.rs index 233b0e6aa3..4ebfe7ead0 100644 --- a/crates/ty_server/src/session.rs +++ b/crates/ty_server/src/session.rs @@ -20,6 +20,7 @@ use lsp_types::{ use ruff_db::Db; use ruff_db::files::{File, system_path_to_file}; use ruff_db::system::{System, SystemPath, SystemPathBuf}; +use ruff_python_ast::PySourceType; use ty_combine::Combine; use ty_project::metadata::Options; use ty_project::watch::{ChangeEvent, CreatedKind}; @@ -1522,7 +1523,8 @@ impl DocumentHandle { pub(crate) fn close(&self, session: &mut Session) -> crate::Result { let is_cell = self.is_cell(); let path = self.notebook_or_file_path(); - session.index_mut().close_document(&self.key())?; + + let removed_document = session.index_mut().close_document(&self.key())?; // Close the text or notebook file in the database but skip this // step for cells because closing a cell doesn't close its notebook. @@ -1535,6 +1537,19 @@ impl DocumentHandle { AnySystemPath::System(system_path) => { if let Some(file) = db.files().try_system(db, system_path) { db.project().close_file(db, file); + + // In case we preferred the language given by the Client + // over the one detected by the file extension, remove the file + // from the project to handle cases where a user changes the language + // of a file (which results in a didClose and didOpen for the same path but with different languages). + if removed_document.language_id().is_some() + && system_path + .extension() + .and_then(PySourceType::try_from_extension) + .is_none() + { + db.project().remove_file(db, file); + } } else { // This can only fail when the path is a directory or it doesn't exists but the // file should exists for this handler in this branch. This is because every diff --git a/crates/ty_server/src/session/index.rs b/crates/ty_server/src/session/index.rs index 6a34fb4ea2..95237212cf 100644 --- a/crates/ty_server/src/session/index.rs +++ b/crates/ty_server/src/session/index.rs @@ -1,7 +1,7 @@ use rustc_hash::FxHashMap; use std::sync::Arc; -use crate::document::DocumentKey; +use crate::document::{DocumentKey, LanguageId}; use crate::session::DocumentHandle; use crate::{ PositionEncoding, TextDocument, @@ -187,12 +187,12 @@ impl Index { handle } - pub(super) fn close_document(&mut self, key: &DocumentKey) -> Result<(), DocumentError> { - let Some(_) = self.documents.remove(key) else { + pub(super) fn close_document(&mut self, key: &DocumentKey) -> Result { + let Some(document) = self.documents.remove(key) else { return Err(DocumentError::NotFound(key.clone())); }; - Ok(()) + Ok(document) } pub(super) fn document_mut( @@ -229,6 +229,13 @@ impl Document { } } + pub(crate) fn language_id(&self) -> Option { + match self { + Self::Text(document) => document.language_id(), + Self::Notebook(_) => None, + } + } + pub(crate) fn as_notebook_mut(&mut self) -> Option<&mut NotebookDocument> { Some(match self { Self::Notebook(notebook) => Arc::make_mut(notebook), diff --git a/crates/ty_server/tests/e2e/main.rs b/crates/ty_server/tests/e2e/main.rs index 5ef434cef6..579e730129 100644 --- a/crates/ty_server/tests/e2e/main.rs +++ b/crates/ty_server/tests/e2e/main.rs @@ -787,7 +787,6 @@ impl TestServer { } /// Send a `textDocument/didClose` notification - #[expect(dead_code)] pub(crate) fn close_text_document(&mut self, path: impl AsRef) { let params = DidCloseTextDocumentParams { text_document: TextDocumentIdentifier { diff --git a/crates/ty_server/tests/e2e/publish_diagnostics.rs b/crates/ty_server/tests/e2e/publish_diagnostics.rs index 64580bc88c..bbe7094325 100644 --- a/crates/ty_server/tests/e2e/publish_diagnostics.rs +++ b/crates/ty_server/tests/e2e/publish_diagnostics.rs @@ -1,7 +1,10 @@ use std::time::Duration; use anyhow::Result; -use lsp_types::{FileChangeType, FileEvent, notification::PublishDiagnostics}; +use lsp_types::{ + DidOpenTextDocumentParams, FileChangeType, FileEvent, TextDocumentItem, + notification::{DidOpenTextDocument, PublishDiagnostics}, +}; use ruff_db::system::SystemPath; use crate::TestServerBuilder; @@ -160,3 +163,66 @@ def foo() -> str: Ok(()) } + +#[test] +fn on_did_open_file_without_extension_but_python_language() -> Result<()> { + let foo = SystemPath::new("src/foo"); + let foo_content = "\ +def foo() -> str: + return 42 +"; + + let mut server = TestServerBuilder::new()? + .with_workspace(SystemPath::new("src"), 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 diagnostics = server.await_notification::(); + + insta::assert_debug_snapshot!(diagnostics); + + Ok(()) +} + +#[test] +fn changing_language_of_file_without_extension() -> Result<()> { + let foo = SystemPath::new("src/foo"); + let foo_content = "\ +def foo() -> str: + return 42 +"; + + let mut server = TestServerBuilder::new()? + .with_workspace(SystemPath::new("src"), 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 diagnostics = server.await_notification::(); + + insta::assert_debug_snapshot!(diagnostics); + + server.close_text_document(foo); + + let params = DidOpenTextDocumentParams { + text_document: TextDocumentItem { + uri: server.file_uri(foo), + language_id: "text".to_string(), + version: 1, + text: foo_content.to_string(), + }, + }; + server.send_notification::(params); + let _close_diagnostics = server.await_notification::(); + + let diagnostics = server.await_notification::(); + + insta::assert_debug_snapshot!(diagnostics); + + Ok(()) +} diff --git a/crates/ty_server/tests/e2e/snapshots/e2e__publish_diagnostics__changing_language_of_file_without_extension-2.snap b/crates/ty_server/tests/e2e/snapshots/e2e__publish_diagnostics__changing_language_of_file_without_extension-2.snap new file mode 100644 index 0000000000..91a4a10b81 --- /dev/null +++ b/crates/ty_server/tests/e2e/snapshots/e2e__publish_diagnostics__changing_language_of_file_without_extension-2.snap @@ -0,0 +1,21 @@ +--- +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: "/src/foo", + query: None, + fragment: None, + }, + diagnostics: [], + version: Some( + 1, + ), +} diff --git a/crates/ty_server/tests/e2e/snapshots/e2e__publish_diagnostics__changing_language_of_file_without_extension.snap b/crates/ty_server/tests/e2e/snapshots/e2e__publish_diagnostics__changing_language_of_file_without_extension.snap new file mode 100644 index 0000000000..d6a9af02de --- /dev/null +++ b/crates/ty_server/tests/e2e/snapshots/e2e__publish_diagnostics__changing_language_of_file_without_extension.snap @@ -0,0 +1,70 @@ +--- +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: "/src/foo", + query: None, + fragment: None, + }, + diagnostics: [ + Diagnostic { + range: Range { + start: Position { + line: 1, + character: 11, + }, + end: Position { + line: 1, + character: 13, + }, + }, + severity: Some( + Error, + ), + code: Some( + String( + "invalid-return-type", + ), + ), + code_description: Some( + CodeDescription { + href: Url { + scheme: "https", + cannot_be_a_base: false, + username: "", + password: None, + host: Some( + Domain( + "ty.dev", + ), + ), + port: None, + path: "/rules", + query: None, + fragment: Some( + "invalid-return-type", + ), + }, + }, + ), + source: Some( + "ty", + ), + message: "Return type does not match returned value: expected `str`, found `Literal[42]`", + related_information: None, + tags: None, + data: None, + }, + ], + version: Some( + 1, + ), +} diff --git a/crates/ty_server/tests/e2e/snapshots/e2e__publish_diagnostics__on_did_open_file_without_extension_but_python_language.snap b/crates/ty_server/tests/e2e/snapshots/e2e__publish_diagnostics__on_did_open_file_without_extension_but_python_language.snap new file mode 100644 index 0000000000..d6a9af02de --- /dev/null +++ b/crates/ty_server/tests/e2e/snapshots/e2e__publish_diagnostics__on_did_open_file_without_extension_but_python_language.snap @@ -0,0 +1,70 @@ +--- +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: "/src/foo", + query: None, + fragment: None, + }, + diagnostics: [ + Diagnostic { + range: Range { + start: Position { + line: 1, + character: 11, + }, + end: Position { + line: 1, + character: 13, + }, + }, + severity: Some( + Error, + ), + code: Some( + String( + "invalid-return-type", + ), + ), + code_description: Some( + CodeDescription { + href: Url { + scheme: "https", + cannot_be_a_base: false, + username: "", + password: None, + host: Some( + Domain( + "ty.dev", + ), + ), + port: None, + path: "/rules", + query: None, + fragment: Some( + "invalid-return-type", + ), + }, + }, + ), + source: Some( + "ty", + ), + message: "Return type does not match returned value: expected `str`, found `Literal[42]`", + related_information: None, + tags: None, + data: None, + }, + ], + version: Some( + 1, + ), +}