mirror of https://github.com/astral-sh/ruff
[ty] Checking files without extension (#21867)
This commit is contained in:
parent
59b92b3522
commit
f7528bd325
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<bool> {
|
||||
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
|
||||
|
|
|
|||
|
|
@ -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<Document, DocumentError> {
|
||||
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<LanguageId> {
|
||||
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),
|
||||
|
|
|
|||
|
|
@ -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<SystemPath>) {
|
||||
let params = DidCloseTextDocumentParams {
|
||||
text_document: TextDocumentIdentifier {
|
||||
|
|
|
|||
|
|
@ -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::<PublishDiagnostics>();
|
||||
|
||||
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::<PublishDiagnostics>();
|
||||
|
||||
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::<DidOpenTextDocument>(params);
|
||||
let _close_diagnostics = server.await_notification::<PublishDiagnostics>();
|
||||
|
||||
let diagnostics = server.await_notification::<PublishDiagnostics>();
|
||||
|
||||
insta::assert_debug_snapshot!(diagnostics);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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: "<temp_dir>/src/foo",
|
||||
query: None,
|
||||
fragment: None,
|
||||
},
|
||||
diagnostics: [],
|
||||
version: Some(
|
||||
1,
|
||||
),
|
||||
}
|
||||
|
|
@ -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: "<temp_dir>/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,
|
||||
),
|
||||
}
|
||||
|
|
@ -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: "<temp_dir>/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,
|
||||
),
|
||||
}
|
||||
Loading…
Reference in New Issue