mirror of https://github.com/astral-sh/ruff
[ty] Improve diagnostic range for `non-subscriptable` diagnostics (#21461)
## Summary Currently our diagnostic only covers the range of the thing being subscripted: <img width="1702" height="312" alt="image" src="https://github.com/user-attachments/assets/7e630431-e846-46ca-93c1-139f11aaba11" /> But it should probably cover the _whole_ subscript expression (arguably the more "incorrect" bit is the `["foo"]` part of this expression, not the `x` part of this expression!) ## Test Plan Added a snapshot Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com>
This commit is contained in:
parent
c5d654bce8
commit
d63b4b0383
|
|
@ -153,7 +153,7 @@ fn both_warnings_and_errors() -> anyhow::Result<()> {
|
||||||
"#,
|
"#,
|
||||||
)?;
|
)?;
|
||||||
|
|
||||||
assert_cmd_snapshot!(case.command().arg("--warn").arg("unresolved-reference"), @r###"
|
assert_cmd_snapshot!(case.command().arg("--warn").arg("unresolved-reference"), @r"
|
||||||
success: false
|
success: false
|
||||||
exit_code: 1
|
exit_code: 1
|
||||||
----- stdout -----
|
----- stdout -----
|
||||||
|
|
@ -171,14 +171,14 @@ fn both_warnings_and_errors() -> anyhow::Result<()> {
|
||||||
|
|
|
|
||||||
2 | print(x) # [unresolved-reference]
|
2 | print(x) # [unresolved-reference]
|
||||||
3 | print(4[1]) # [non-subscriptable]
|
3 | print(4[1]) # [non-subscriptable]
|
||||||
| ^
|
| ^^^^
|
||||||
|
|
|
|
||||||
info: rule `non-subscriptable` is enabled by default
|
info: rule `non-subscriptable` is enabled by default
|
||||||
|
|
||||||
Found 2 diagnostics
|
Found 2 diagnostics
|
||||||
|
|
||||||
----- stderr -----
|
----- stderr -----
|
||||||
"###);
|
");
|
||||||
|
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
@ -193,7 +193,7 @@ fn both_warnings_and_errors_and_error_on_warning_is_true() -> anyhow::Result<()>
|
||||||
"###,
|
"###,
|
||||||
)?;
|
)?;
|
||||||
|
|
||||||
assert_cmd_snapshot!(case.command().arg("--warn").arg("unresolved-reference").arg("--error-on-warning"), @r###"
|
assert_cmd_snapshot!(case.command().arg("--warn").arg("unresolved-reference").arg("--error-on-warning"), @r"
|
||||||
success: false
|
success: false
|
||||||
exit_code: 1
|
exit_code: 1
|
||||||
----- stdout -----
|
----- stdout -----
|
||||||
|
|
@ -211,14 +211,14 @@ fn both_warnings_and_errors_and_error_on_warning_is_true() -> anyhow::Result<()>
|
||||||
|
|
|
|
||||||
2 | print(x) # [unresolved-reference]
|
2 | print(x) # [unresolved-reference]
|
||||||
3 | print(4[1]) # [non-subscriptable]
|
3 | print(4[1]) # [non-subscriptable]
|
||||||
| ^
|
| ^^^^
|
||||||
|
|
|
|
||||||
info: rule `non-subscriptable` is enabled by default
|
info: rule `non-subscriptable` is enabled by default
|
||||||
|
|
||||||
Found 2 diagnostics
|
Found 2 diagnostics
|
||||||
|
|
||||||
----- stderr -----
|
----- stderr -----
|
||||||
"###);
|
");
|
||||||
|
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
@ -233,7 +233,7 @@ fn exit_zero_is_true() -> anyhow::Result<()> {
|
||||||
"#,
|
"#,
|
||||||
)?;
|
)?;
|
||||||
|
|
||||||
assert_cmd_snapshot!(case.command().arg("--exit-zero").arg("--warn").arg("unresolved-reference"), @r###"
|
assert_cmd_snapshot!(case.command().arg("--exit-zero").arg("--warn").arg("unresolved-reference"), @r"
|
||||||
success: true
|
success: true
|
||||||
exit_code: 0
|
exit_code: 0
|
||||||
----- stdout -----
|
----- stdout -----
|
||||||
|
|
@ -251,14 +251,14 @@ fn exit_zero_is_true() -> anyhow::Result<()> {
|
||||||
|
|
|
|
||||||
2 | print(x) # [unresolved-reference]
|
2 | print(x) # [unresolved-reference]
|
||||||
3 | print(4[1]) # [non-subscriptable]
|
3 | print(4[1]) # [non-subscriptable]
|
||||||
| ^
|
| ^^^^
|
||||||
|
|
|
|
||||||
info: rule `non-subscriptable` is enabled by default
|
info: rule `non-subscriptable` is enabled by default
|
||||||
|
|
||||||
Found 2 diagnostics
|
Found 2 diagnostics
|
||||||
|
|
||||||
----- stderr -----
|
----- stderr -----
|
||||||
"###);
|
");
|
||||||
|
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -617,7 +617,7 @@ fn gitlab_diagnostics() -> anyhow::Result<()> {
|
||||||
let _s = settings.bind_to_scope();
|
let _s = settings.bind_to_scope();
|
||||||
|
|
||||||
assert_cmd_snapshot!(case.command().arg("--output-format=gitlab").arg("--warn").arg("unresolved-reference")
|
assert_cmd_snapshot!(case.command().arg("--output-format=gitlab").arg("--warn").arg("unresolved-reference")
|
||||||
.env("CI_PROJECT_DIR", case.project_dir), @r###"
|
.env("CI_PROJECT_DIR", case.project_dir), @r#"
|
||||||
success: false
|
success: false
|
||||||
exit_code: 1
|
exit_code: 1
|
||||||
----- stdout -----
|
----- stdout -----
|
||||||
|
|
@ -655,14 +655,14 @@ fn gitlab_diagnostics() -> anyhow::Result<()> {
|
||||||
},
|
},
|
||||||
"end": {
|
"end": {
|
||||||
"line": 3,
|
"line": 3,
|
||||||
"column": 8
|
"column": 11
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
]
|
]
|
||||||
----- stderr -----
|
----- stderr -----
|
||||||
"###);
|
"#);
|
||||||
|
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
@ -677,15 +677,15 @@ fn github_diagnostics() -> anyhow::Result<()> {
|
||||||
"#,
|
"#,
|
||||||
)?;
|
)?;
|
||||||
|
|
||||||
assert_cmd_snapshot!(case.command().arg("--output-format=github").arg("--warn").arg("unresolved-reference"), @r###"
|
assert_cmd_snapshot!(case.command().arg("--output-format=github").arg("--warn").arg("unresolved-reference"), @r"
|
||||||
success: false
|
success: false
|
||||||
exit_code: 1
|
exit_code: 1
|
||||||
----- stdout -----
|
----- stdout -----
|
||||||
::warning title=ty (unresolved-reference),file=<temp_dir>/test.py,line=2,col=7,endLine=2,endColumn=8::test.py:2:7: unresolved-reference: Name `x` used when not defined
|
::warning title=ty (unresolved-reference),file=<temp_dir>/test.py,line=2,col=7,endLine=2,endColumn=8::test.py:2:7: unresolved-reference: Name `x` used when not defined
|
||||||
::error title=ty (non-subscriptable),file=<temp_dir>/test.py,line=3,col=7,endLine=3,endColumn=8::test.py:3:7: non-subscriptable: Cannot subscript object of type `Literal[4]` with no `__getitem__` method
|
::error title=ty (non-subscriptable),file=<temp_dir>/test.py,line=3,col=7,endLine=3,endColumn=11::test.py:3:7: non-subscriptable: Cannot subscript object of type `Literal[4]` with no `__getitem__` method
|
||||||
|
|
||||||
----- stderr -----
|
----- stderr -----
|
||||||
"###);
|
");
|
||||||
|
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,33 @@
|
||||||
|
---
|
||||||
|
source: crates/ty_test/src/lib.rs
|
||||||
|
expression: snapshot
|
||||||
|
---
|
||||||
|
---
|
||||||
|
mdtest name: instance.md - Instance subscript - `__getitem__` unbound
|
||||||
|
mdtest path: crates/ty_python_semantic/resources/mdtest/subscript/instance.md
|
||||||
|
---
|
||||||
|
|
||||||
|
# Python source files
|
||||||
|
|
||||||
|
## mdtest_snippet.py
|
||||||
|
|
||||||
|
```
|
||||||
|
1 | class NotSubscriptable: ...
|
||||||
|
2 |
|
||||||
|
3 | a = NotSubscriptable()[0] # error: [non-subscriptable]
|
||||||
|
```
|
||||||
|
|
||||||
|
# Diagnostics
|
||||||
|
|
||||||
|
```
|
||||||
|
error[non-subscriptable]: Cannot subscript object of type `NotSubscriptable` with no `__getitem__` method
|
||||||
|
--> src/mdtest_snippet.py:3:5
|
||||||
|
|
|
||||||
|
1 | class NotSubscriptable: ...
|
||||||
|
2 |
|
||||||
|
3 | a = NotSubscriptable()[0] # error: [non-subscriptable]
|
||||||
|
| ^^^^^^^^^^^^^^^^^^^^^
|
||||||
|
|
|
||||||
|
info: rule `non-subscriptable` is enabled by default
|
||||||
|
|
||||||
|
```
|
||||||
|
|
@ -2,10 +2,12 @@
|
||||||
|
|
||||||
## `__getitem__` unbound
|
## `__getitem__` unbound
|
||||||
|
|
||||||
|
<!-- snapshot-diagnostics -->
|
||||||
|
|
||||||
```py
|
```py
|
||||||
class NotSubscriptable: ...
|
class NotSubscriptable: ...
|
||||||
|
|
||||||
a = NotSubscriptable()[0] # error: "Cannot subscript object of type `NotSubscriptable` with no `__getitem__` method"
|
a = NotSubscriptable()[0] # error: [non-subscriptable]
|
||||||
```
|
```
|
||||||
|
|
||||||
## `__getitem__` not callable
|
## `__getitem__` not callable
|
||||||
|
|
|
||||||
|
|
@ -2034,7 +2034,7 @@ pub(super) fn report_index_out_of_bounds(
|
||||||
/// Emit a diagnostic declaring that a type does not support subscripting.
|
/// Emit a diagnostic declaring that a type does not support subscripting.
|
||||||
pub(super) fn report_non_subscriptable(
|
pub(super) fn report_non_subscriptable(
|
||||||
context: &InferContext,
|
context: &InferContext,
|
||||||
node: AnyNodeRef,
|
node: &ast::ExprSubscript,
|
||||||
non_subscriptable_ty: Type,
|
non_subscriptable_ty: Type,
|
||||||
method: &str,
|
method: &str,
|
||||||
) {
|
) {
|
||||||
|
|
|
||||||
|
|
@ -11369,11 +11369,11 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
||||||
.as_class_literal()
|
.as_class_literal()
|
||||||
.is_some_and(|class| class.iter_mro(db, None).contains(&ClassBase::Generic))
|
.is_some_and(|class| class.iter_mro(db, None).contains(&ClassBase::Generic))
|
||||||
{
|
{
|
||||||
report_non_subscriptable(context, value_node.into(), value_ty, "__class_getitem__");
|
report_non_subscriptable(context, subscript, value_ty, "__class_getitem__");
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
if expr_context != ExprContext::Store {
|
if expr_context != ExprContext::Store {
|
||||||
report_non_subscriptable(context, value_node.into(), value_ty, "__getitem__");
|
report_non_subscriptable(context, subscript, value_ty, "__getitem__");
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue