diff --git a/.gitignore b/.gitignore index ad7de8ce76cffb..71b64ee74b5f76 100644 --- a/.gitignore +++ b/.gitignore @@ -29,6 +29,10 @@ tracing.folded tracing-flamechart.svg tracing-flamegraph.svg +# insta +.rs.pending-snap + + ### # Rust.gitignore ### diff --git a/Cargo.lock b/Cargo.lock index 79580e37b0c3f3..efb58028dd4cbe 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2332,6 +2332,7 @@ dependencies = [ "red_knot_server", "regex", "ruff_db", + "ruff_python_trivia", "salsa", "tempfile", "toml", diff --git a/crates/red_knot/Cargo.toml b/crates/red_knot/Cargo.toml index 5bc5c89c3e2355..805bf194838d52 100644 --- a/crates/red_knot/Cargo.toml +++ b/crates/red_knot/Cargo.toml @@ -33,6 +33,7 @@ tracing-tree = { workspace = true } [dev-dependencies] ruff_db = { workspace = true, features = ["testing"] } +ruff_python_trivia = { workspace = true } insta = { workspace = true, features = ["filters"] } insta-cmd = { workspace = true } diff --git a/crates/red_knot/tests/cli.rs b/crates/red_knot/tests/cli.rs index 7bea404922406c..91cff2ecb32a80 100644 --- a/crates/red_knot/tests/cli.rs +++ b/crates/red_knot/tests/cli.rs @@ -1,6 +1,7 @@ use anyhow::Context; +use insta::Settings; use insta_cmd::{assert_cmd_snapshot, get_cargo_bin}; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::process::Command; use tempfile::TempDir; @@ -8,46 +9,43 @@ use tempfile::TempDir; /// project's configuration. #[test] fn config_override() -> anyhow::Result<()> { - let tempdir = TempDir::new()?; - - std::fs::write( - tempdir.path().join("pyproject.toml"), - r#" -[tool.knot.environment] -python-version = "3.11" -"#, - ) - .context("Failed to write settings")?; - - std::fs::write( - tempdir.path().join("test.py"), - r#" -import sys - -# Access `sys.last_exc` that was only added in Python 3.12 -print(sys.last_exc) -"#, - ) - .context("Failed to write test.py")?; - - insta::with_settings!({filters => vec![(&*tempdir_filter(tempdir.path()), "/")]}, { - assert_cmd_snapshot!(knot().arg("--project").arg(tempdir.path()), @r" - success: false - exit_code: 1 - ----- stdout ----- - error[lint:unresolved-attribute] /test.py:5:7 Type `` has no attribute `last_exc` - - ----- stderr ----- + let case = TestCase::with_files([ + ( + "pyproject.toml", + r#" + [tool.knot.environment] + python-version = "3.11" + "#, + ), + ( + "test.py", + r#" + import sys + + # Access `sys.last_exc` that was only added in Python 3.12 + print(sys.last_exc) + "#, + ), + ])?; + + case.insta_settings().bind(|| { + assert_cmd_snapshot!(case.command(), @r" + success: false + exit_code: 1 + ----- stdout ----- + error[lint:unresolved-attribute] /test.py:5:7 Type `` has no attribute `last_exc` + + ----- stderr ----- "); - }); - assert_cmd_snapshot!(knot().arg("--project").arg(tempdir.path()).arg("--python-version").arg("3.12"), @r" - success: true - exit_code: 0 - ----- stdout ----- + assert_cmd_snapshot!(case.command().arg("--python-version").arg("3.12"), @r" + success: true + exit_code: 0 + ----- stdout ----- - ----- stderr ----- - "); + ----- stderr ----- + "); + }); Ok(()) } @@ -69,69 +67,48 @@ print(sys.last_exc) /// And the command is run in the `child` directory. #[test] fn cli_arguments_are_relative_to_the_current_directory() -> anyhow::Result<()> { - let tempdir = TempDir::new()?; - - let project_dir = tempdir.path().canonicalize()?; - - let libs = project_dir.join("libs"); - std::fs::create_dir_all(&libs).context("Failed to create `libs` directory")?; - - let child = project_dir.join("child"); - std::fs::create_dir(&child).context("Failed to create `child` directory")?; - - std::fs::write( - tempdir.path().join("pyproject.toml"), - r#" -[tool.knot.environment] -python-version = "3.11" -"#, - ) - .context("Failed to write `pyproject.toml`")?; - - std::fs::write( - libs.join("utils.py"), - r#" -def add(a: int, b: int) -> int: - a + b -"#, - ) - .context("Failed to write `utils.py`")?; - - std::fs::write( - child.join("test.py"), - r#" -from utils import add - -stat = add(10, 15) -"#, - ) - .context("Failed to write `child/test.py`")?; - - let project_filter = tempdir_filter(&project_dir); - let filters = vec![ - (&*project_filter, "/"), - (r#"\\(\w\w|\s|\.|")"#, "/$1"), - ]; - - // Make sure that the CLI fails when the `libs` directory is not in the search path. - insta::with_settings!({filters => filters}, { - assert_cmd_snapshot!(knot().current_dir(&child), @r#" - success: false - exit_code: 1 - ----- stdout ----- - error[lint:unresolved-import] /child/test.py:2:1 Cannot resolve import `utils` - - ----- stderr ----- + let case = TestCase::with_files([ + ( + "pyproject.toml", + r#" + [tool.knot.environment] + python-version = "3.11" + "#, + ), + ( + "libs/utils.py", + r#" + def add(a: int, b: int) -> int: + a + b + "#, + ), + ( + "child/test.py", + r#" + from utils import add + + stat = add(10, 15) + "#, + ), + ])?; + + case.insta_settings().bind(|| { + // Make sure that the CLI fails when the `libs` directory is not in the search path. + assert_cmd_snapshot!(case.command().current_dir(case.project_dir().join("child")), @r#" + success: false + exit_code: 1 + ----- stdout ----- + error[lint:unresolved-import] /child/test.py:2:1 Cannot resolve import `utils` + + ----- stderr ----- "#); - }); - insta::with_settings!({filters => vec![(&*tempdir_filter(&project_dir), "/")]}, { - assert_cmd_snapshot!(knot().current_dir(child).arg("--extra-search-path").arg("../libs"), @r" - success: true - exit_code: 0 - ----- stdout ----- + assert_cmd_snapshot!(case.command().current_dir(case.project_dir().join("child")).arg("--extra-search-path").arg("../libs"), @r" + success: true + exit_code: 0 + ----- stdout ----- - ----- stderr ----- + ----- stderr ----- "); }); @@ -153,52 +130,39 @@ stat = add(10, 15) /// ``` #[test] fn paths_in_configuration_files_are_relative_to_the_project_root() -> anyhow::Result<()> { - let tempdir = TempDir::new()?; - - let project_dir = tempdir.path(); - - let libs = project_dir.join("libs"); - std::fs::create_dir_all(&libs).context("Failed to create `libs` directory")?; - - let child = project_dir.join("child"); - std::fs::create_dir(&child).context("Failed to create `child` directory")?; - - std::fs::write( - tempdir.path().join("pyproject.toml"), - r#" -[tool.knot.environment] -python-version = "3.11" -extra-paths = ["libs"] -"#, - ) - .context("Failed to write `pyproject.toml`")?; - - std::fs::write( - libs.join("utils.py"), - r#" -def add(a: int, b: int) -> int: - a + b -"#, - ) - .context("Failed to write `utils.py`")?; - - std::fs::write( - child.join("test.py"), - r#" -from utils import add - -stat = add(10, 15) -"#, - ) - .context("Failed to write `child/test.py`")?; - - insta::with_settings!({filters => vec![(&*tempdir_filter(tempdir.path()), "/")]}, { - assert_cmd_snapshot!(knot().current_dir(child), @r" - success: true - exit_code: 0 - ----- stdout ----- - - ----- stderr ----- + let case = TestCase::with_files([ + ( + "pyproject.toml", + r#" + [tool.knot.environment] + python-version = "3.11" + extra-paths = ["libs"] + "#, + ), + ( + "libs/utils.py", + r#" + def add(a: int, b: int) -> int: + a + b + "#, + ), + ( + "child/test.py", + r#" + from utils import add + + stat = add(10, 15) + "#, + ), + ])?; + + case.insta_settings().bind(|| { + assert_cmd_snapshot!(case.command().current_dir(case.project_dir().join("child")), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- "); }); @@ -208,96 +172,154 @@ stat = add(10, 15) /// The rule severity can be changed in the configuration file #[test] fn rule_severity() -> anyhow::Result<()> { - let tempdir = TempDir::new()?; - - let project_dir = tempdir.path().canonicalize()?; - - std::fs::write( - project_dir.join("test.py"), + let case = TestCase::with_file( + "test.py", r#" -y = 4 / 0 - -for a in range(0, y): - x = a - -print(x) # possibly-unresolved-reference -"#, - ) - .context("Failed to write `test.py`")?; - - // Assert that there's a possibly unresolved reference diagnostic - // and that division-by-zero has a severity of error by default. - insta::with_settings!({filters => vec![(&*tempdir_filter(&project_dir), "/")]}, { - assert_cmd_snapshot!(knot().current_dir(&project_dir), @r" - success: false - exit_code: 1 - ----- stdout ----- - error[lint:division-by-zero] /test.py:2:5 Cannot divide object of type `Literal[4]` by zero - warning[lint:possibly-unresolved-reference] /test.py:7:7 Name `x` used when possibly not defined - - ----- stderr ----- + y = 4 / 0 + + for a in range(0, y): + x = a + + print(x) # possibly-unresolved-reference + "#, + )?; + + case.insta_settings().bind(|| { + // Assert that there's a possibly unresolved reference diagnostic + // and that division-by-zero has a severity of error by default. + assert_cmd_snapshot!(case.command(), @r" + success: false + exit_code: 1 + ----- stdout ----- + error[lint:division-by-zero] /test.py:2:5 Cannot divide object of type `Literal[4]` by zero + warning[lint:possibly-unresolved-reference] /test.py:7:7 Name `x` used when possibly not defined + + ----- stderr ----- "); - }); - std::fs::write( - project_dir.join("pyproject.toml"), - r#" -[tool.knot.rules] -division-by-zero = "warn" # demote to warn -possibly-unresolved-reference = "ignore" -"#, - ) - .context("Failed to write `pyproject.toml`")?; - - insta::with_settings!({filters => vec![(&*tempdir_filter(&project_dir), "/")]}, { - assert_cmd_snapshot!(knot().current_dir(project_dir), @r" - success: false - exit_code: 1 - ----- stdout ----- - warning[lint:division-by-zero] /test.py:2:5 Cannot divide object of type `Literal[4]` by zero - - ----- stderr ----- + case.write_file("pyproject.toml", r#" + [tool.knot.rules] + division-by-zero = "warn" # demote to warn + possibly-unresolved-reference = "ignore" + "#)?; + + assert_cmd_snapshot!(case.command(), @r" + success: false + exit_code: 1 + ----- stdout ----- + warning[lint:division-by-zero] /test.py:2:5 Cannot divide object of type `Literal[4]` by zero + + ----- stderr ----- "); - }); - Ok(()) + Ok(()) + }) } /// Red Knot warns about unknown rules #[test] fn unknown_rules() -> anyhow::Result<()> { - let tempdir = TempDir::new()?; - - let project_dir = tempdir.path().canonicalize()?; - - std::fs::write( - project_dir.join("pyproject.toml"), - r#" -[tool.knot.rules] -division-by-zer = "warn" # incorrect rule name -"#, - ) - .context("Failed to write `pyproject.toml`")?; - - std::fs::write(project_dir.join("test.py"), r#"print(10)"#) - .context("Failed to write `test.py`")?; - - insta::with_settings!({filters => vec![(&*tempdir_filter(&project_dir), "/")]}, { - assert_cmd_snapshot!(knot().current_dir(project_dir), @r" - success: false - exit_code: 1 - ----- stdout ----- - warning[unknown-rule] /pyproject.toml:3:1 Unknown lint rule `division-by-zer` - - ----- stderr ----- + let case = TestCase::with_files([ + ( + "pyproject.toml", + r#" + [tool.knot.rules] + division-by-zer = "warn" # incorrect rule name + "#, + ), + ("test.py", "print(10)"), + ])?; + + case.insta_settings().bind(|| { + assert_cmd_snapshot!(case.command(), @r" + success: false + exit_code: 1 + ----- stdout ----- + warning[unknown-rule] /pyproject.toml:3:1 Unknown lint rule `division-by-zer` + + ----- stderr ----- "); }); Ok(()) } -fn knot() -> Command { - Command::new(get_cargo_bin("red_knot")) +struct TestCase { + _temp_dir: TempDir, + project_dir: PathBuf, +} + +impl TestCase { + fn new() -> anyhow::Result { + let temp_dir = TempDir::new()?; + + // Canonicalize the tempdir path because macos uses symlinks for tempdirs + // and that doesn't play well with our snapshot filtering. + let project_dir = temp_dir + .path() + .canonicalize() + .context("Failed to canonicalize project path")?; + + Ok(Self { + project_dir, + _temp_dir: temp_dir, + }) + } + + fn with_files<'a>(files: impl IntoIterator) -> anyhow::Result { + let case = Self::new()?; + case.write_files(files)?; + Ok(case) + } + + fn with_file(path: impl AsRef, content: &str) -> anyhow::Result { + let case = Self::new()?; + case.write_file(path, content)?; + Ok(case) + } + + fn write_files<'a>( + &self, + files: impl IntoIterator, + ) -> anyhow::Result<()> { + for (path, content) in files { + self.write_file(path, content)?; + } + + Ok(()) + } + + fn write_file(&self, path: impl AsRef, content: &str) -> anyhow::Result<()> { + let path = path.as_ref(); + let path = self.project_dir.join(path); + + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent) + .with_context(|| format!("Failed to create directory `{}`", parent.display()))?; + } + std::fs::write(&path, &*ruff_python_trivia::textwrap::dedent(content)) + .with_context(|| format!("Failed to write file `{path}`", path = path.display()))?; + + Ok(()) + } + + fn project_dir(&self) -> &Path { + &self.project_dir + } + + // Returns the insta filters to escape paths in snapshots + fn insta_settings(&self) -> Settings { + let mut settings = insta::Settings::clone_current(); + settings.add_filter(&tempdir_filter(&self.project_dir), "/"); + settings.add_filter(r#"\\(\w\w|\s|\.|")"#, "/$1"); + settings + } + + fn command(&self) -> Command { + let mut command = Command::new(get_cargo_bin("red_knot")); + command.current_dir(&self.project_dir); + command + } } fn tempdir_filter(path: &Path) -> String {