Skip to content

Commit

Permalink
ruff_db: make diagnostic rendering prettier
Browse files Browse the repository at this point in the history 
This change does a simple swap of the existing renderer for one that
uses our vendored copy of `annotate-snippets`. We don't change anything
about the diagnostic data model, but this alone already makes
diagnostics look a lot nicer!
  • Loading branch information
@BurntSushi
BurntSushi committed Jan 31, 2025
1 parent b0b8b06 commit 1080155
Show file tree
Hide file tree
Showing 7 changed files with 392 additions and 113 deletions.
3 changes: 3 additions & 0 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

257 changes: 186 additions & 71 deletions crates/red_knot/tests/cli.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,21 @@ fn config_override() -> anyhow::Result<()> {
),
])?;

assert_cmd_snapshot!(case.command(), @r"
success: false
exit_code: 1
----- stdout -----
error[lint:unresolved-attribute] <temp_dir>/test.py:5:7 Type `<module 'sys'>` has no attribute `last_exc`
assert_cmd_snapshot!(case.command(), @r###"
success: false
exit_code: 1
----- stdout -----
error: lint:unresolved-attribute
--> <temp_dir>/test.py:5:7
|
4 | # Access `sys.last_exc` that was only added in Python 3.12
5 | print(sys.last_exc)
| ^^^^^^^^^^^^ Type `<module 'sys'>` has no attribute `last_exc`
|
----- stderr -----
");
----- stderr -----
"###);

assert_cmd_snapshot!(case.command().arg("--python-version").arg("3.12"), @r"
success: true
Expand Down Expand Up @@ -91,14 +98,22 @@ fn cli_arguments_are_relative_to_the_current_directory() -> anyhow::Result<()> {
])?;

// 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] <temp_dir>/child/test.py:2:1 Cannot resolve import `utils`
assert_cmd_snapshot!(case.command().current_dir(case.project_dir().join("child")), @r###"
success: false
exit_code: 1
----- stdout -----
error: lint:unresolved-import
--> <temp_dir>/child/test.py:2:1
|
2 | from utils import add
| ^^^^^^^^^^^^^^^^^^^^^ Cannot resolve import `utils`
3 |
4 | stat = add(10, 15)
|
----- stderr -----
"#);
----- stderr -----
"###);

assert_cmd_snapshot!(case.command().current_dir(case.project_dir().join("child")).arg("--extra-search-path").arg("../libs"), @r"
success: true
Expand Down Expand Up @@ -180,15 +195,31 @@ fn configuration_rule_severity() -> anyhow::Result<()> {

// 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] <temp_dir>/test.py:2:5 Cannot divide object of type `Literal[4]` by zero
warning[lint:possibly-unresolved-reference] <temp_dir>/test.py:7:7 Name `x` used when possibly not defined
assert_cmd_snapshot!(case.command(), @r###"
success: false
exit_code: 1
----- stdout -----
error: lint:division-by-zero
--> <temp_dir>/test.py:2:5
|
2 | y = 4 / 0
| ^^^^^ Cannot divide object of type `Literal[4]` by zero
3 |
4 | for a in range(0, y):
|
warning: lint:possibly-unresolved-reference
--> <temp_dir>/test.py:7:7
|
5 | x = a
6 |
7 | print(x) # possibly-unresolved-reference
| - Name `x` used when possibly not defined
|
----- stderr -----
");
----- stderr -----
"###);

case.write_file(
"pyproject.toml",
Expand All @@ -199,14 +230,22 @@ fn configuration_rule_severity() -> anyhow::Result<()> {
"#,
)?;

assert_cmd_snapshot!(case.command(), @r"
success: false
exit_code: 1
----- stdout -----
warning[lint:division-by-zero] <temp_dir>/test.py:2:5 Cannot divide object of type `Literal[4]` by zero
assert_cmd_snapshot!(case.command(), @r###"
success: false
exit_code: 1
----- stdout -----
warning: lint:division-by-zero
--> <temp_dir>/test.py:2:5
|
2 | y = 4 / 0
| ----- Cannot divide object of type `Literal[4]` by zero
3 |
4 | for a in range(0, y):
|
----- stderr -----
");
----- stderr -----
"###);

Ok(())
}
Expand All @@ -230,16 +269,42 @@ fn cli_rule_severity() -> anyhow::Result<()> {

// 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:unresolved-import] <temp_dir>/test.py:2:8 Cannot resolve import `does_not_exit`
error[lint:division-by-zero] <temp_dir>/test.py:4:5 Cannot divide object of type `Literal[4]` by zero
warning[lint:possibly-unresolved-reference] <temp_dir>/test.py:9:7 Name `x` used when possibly not defined
assert_cmd_snapshot!(case.command(), @r###"
success: false
exit_code: 1
----- stdout -----
error: lint:unresolved-import
--> <temp_dir>/test.py:2:8
|
2 | import does_not_exit
| ^^^^^^^^^^^^^ Cannot resolve import `does_not_exit`
3 |
4 | y = 4 / 0
|
error: lint:division-by-zero
--> <temp_dir>/test.py:4:5
|
2 | import does_not_exit
3 |
4 | y = 4 / 0
| ^^^^^ Cannot divide object of type `Literal[4]` by zero
5 |
6 | for a in range(0, y):
|
warning: lint:possibly-unresolved-reference
--> <temp_dir>/test.py:9:7
|
7 | x = a
8 |
9 | print(x) # possibly-unresolved-reference
| - Name `x` used when possibly not defined
|
----- stderr -----
");
----- stderr -----
"###);

assert_cmd_snapshot!(
case
Expand All @@ -250,15 +315,33 @@ fn cli_rule_severity() -> anyhow::Result<()> {
.arg("division-by-zero")
.arg("--warn")
.arg("unresolved-import"),
@r"
@r###"
success: false
exit_code: 1
----- stdout -----
warning[lint:unresolved-import] <temp_dir>/test.py:2:8 Cannot resolve import `does_not_exit`
warning[lint:division-by-zero] <temp_dir>/test.py:4:5 Cannot divide object of type `Literal[4]` by zero
warning: lint:unresolved-import
--> <temp_dir>/test.py:2:8
|
2 | import does_not_exit
| ------------- Cannot resolve import `does_not_exit`
3 |
4 | y = 4 / 0
|
warning: lint:division-by-zero
--> <temp_dir>/test.py:4:5
|
2 | import does_not_exit
3 |
4 | y = 4 / 0
| ----- Cannot divide object of type `Literal[4]` by zero
5 |
6 | for a in range(0, y):
|
----- stderr -----
"
"###
);

Ok(())
Expand All @@ -282,15 +365,31 @@ fn cli_rule_severity_precedence() -> anyhow::Result<()> {

// 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] <temp_dir>/test.py:2:5 Cannot divide object of type `Literal[4]` by zero
warning[lint:possibly-unresolved-reference] <temp_dir>/test.py:7:7 Name `x` used when possibly not defined
assert_cmd_snapshot!(case.command(), @r###"
success: false
exit_code: 1
----- stdout -----
error: lint:division-by-zero
--> <temp_dir>/test.py:2:5
|
2 | y = 4 / 0
| ^^^^^ Cannot divide object of type `Literal[4]` by zero
3 |
4 | for a in range(0, y):
|
warning: lint:possibly-unresolved-reference
--> <temp_dir>/test.py:7:7
|
5 | x = a
6 |
7 | print(x) # possibly-unresolved-reference
| - Name `x` used when possibly not defined
|
----- stderr -----
");
----- stderr -----
"###);

assert_cmd_snapshot!(
case
Expand All @@ -302,14 +401,22 @@ fn cli_rule_severity_precedence() -> anyhow::Result<()> {
// Override the error severity with warning
.arg("--ignore")
.arg("possibly-unresolved-reference"),
@r"
success: false
exit_code: 1
----- stdout -----
warning[lint:division-by-zero] <temp_dir>/test.py:2:5 Cannot divide object of type `Literal[4]` by zero
@r###"
success: false
exit_code: 1
----- stdout -----
warning: lint:division-by-zero
--> <temp_dir>/test.py:2:5
|
2 | y = 4 / 0
| ----- Cannot divide object of type `Literal[4]` by zero
3 |
4 | for a in range(0, y):
|
----- stderr -----
"
----- stderr -----
"###
);

Ok(())
Expand All @@ -329,14 +436,21 @@ fn configuration_unknown_rules() -> anyhow::Result<()> {
("test.py", "print(10)"),
])?;

assert_cmd_snapshot!(case.command(), @r"
success: false
exit_code: 1
----- stdout -----
warning[unknown-rule] <temp_dir>/pyproject.toml:3:1 Unknown lint rule `division-by-zer`
assert_cmd_snapshot!(case.command(), @r###"
success: false
exit_code: 1
----- stdout -----
warning: unknown-rule
--> <temp_dir>/pyproject.toml:3:1
|
2 | [tool.knot.rules]
3 | division-by-zer = "warn" # incorrect rule name
| --------------- Unknown lint rule `division-by-zer`
|
----- stderr -----
");
----- stderr -----
"###);

Ok(())
}
Expand All @@ -346,14 +460,15 @@ fn configuration_unknown_rules() -> anyhow::Result<()> {
fn cli_unknown_rules() -> anyhow::Result<()> {
let case = TestCase::with_file("test.py", "print(10)")?;

assert_cmd_snapshot!(case.command().arg("--ignore").arg("division-by-zer"), @r"
success: false
exit_code: 1
----- stdout -----
warning[unknown-rule] Unknown lint rule `division-by-zer`
assert_cmd_snapshot!(case.command().arg("--ignore").arg("division-by-zer"), @r###"
success: false
exit_code: 1
----- stdout -----
warning: unknown-rule: Unknown lint rule `division-by-zer`
----- stderr -----
");
----- stderr -----
"###);

Ok(())
}
Expand Down
11 changes: 10 additions & 1 deletion crates/red_knot_wasm/tests/api.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,15 @@ fn check() {

assert_eq!(
result,
vec!["error[lint:unresolved-import] /test.py:1:8 Cannot resolve import `random22`"]
vec![
"\
error: lint:unresolved-import
--> /test.py:1:8
|
1 | import random22
| ^^^^^^^^ Cannot resolve import `random22`
|
",
],
);
}
1 change: 1 addition & 0 deletions crates/ruff_benchmark/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ url = { workspace = true }
ureq = { workspace = true }

[dev-dependencies]
colored = { workspace = true }
ruff_db = { workspace = true }
ruff_linter = { workspace = true }
ruff_python_ast = { workspace = true }
Expand Down
Loading

0 comments on commit 1080155

Please sign in to comment.