Skip to content

Commit 2c902c7

Browse files
authored
[ty] fix bug in string annotations and clean up diagnostics (#22913)
This multiplication was always using only using the larger sub-ast size value, which didn't do anything unsound but made sub-string annotations become exhausted on files 32x smaller than expected (16k nodes instead of 512k nodes). Also I decided to do a cleanup pass on the diagnostics to make them more precise and helpful.
1 parent 45acbd0 commit 2c902c7

3 files changed

Lines changed: 33 additions & 19 deletions

File tree

crates/ruff_db/src/parsed.rs

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -60,15 +60,21 @@ pub fn parsed_string_annotation(
6060
indexed::ensure_indexed(&expr, string.node_index().load()).map_err(|err| {
6161
let message = match err {
6262
NodeIndexError::NoParent => {
63-
"Internal Error: string annotation's parent had no NodeIndex".to_owned()
63+
"internal error: string annotation's parent had no NodeIndex".to_owned()
6464
}
65-
NodeIndexError::OutOfIndices => {
66-
"File too long, ran out of encoding space for string annotations".to_owned()
65+
NodeIndexError::TooNested => "too many levels of nested string annotations; remove the redundant nested quotes".to_owned(),
66+
NodeIndexError::OverflowedIndices => {
67+
"file too long for string annotations; either break up the file or don't use string annotations".to_owned()
6768
}
68-
NodeIndexError::OutOfSubIndices => {
69-
"Substring annotation is too complex, ran out of encoding space".to_owned()
69+
NodeIndexError::OverflowedSubIndices => {
70+
"file too long for nested string annotations; remove the redundant nested quotes".to_owned()
71+
}
72+
NodeIndexError::ExhaustedSubIndices => {
73+
"string annotation is too long; consider introducing type aliases to simplify".to_owned()
74+
}
75+
NodeIndexError::ExhaustedSubSubIndices => {
76+
"nested string annotation is too long; remove the redundant nested quotes".to_owned()
7077
}
71-
NodeIndexError::TooNested => "Too many levels of nested string annotations".to_owned(),
7278
};
7379

7480
ParseError {
@@ -222,7 +228,12 @@ mod indexed {
222228
AnyNodeRef::from(parsed.syntax()).visit_source_order(&mut visitor);
223229

224230
if visitor.overflowed {
225-
return Err(NodeIndexError::OutOfSubIndices);
231+
let level = sub_ast_level(parent_index);
232+
if level == 0 {
233+
return Err(NodeIndexError::ExhaustedSubIndices);
234+
} else {
235+
return Err(NodeIndexError::ExhaustedSubSubIndices);
236+
}
226237
}
227238

228239
Ok(())

crates/ruff_python_ast/src/node_index.rs

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,14 @@ impl NodeIndex {
7878
}
7979
}
8080

81+
#[derive(Debug, Copy, Clone)]
8182
pub enum NodeIndexError {
82-
TooNested,
83-
OutOfSubIndices,
8483
NoParent,
85-
OutOfIndices,
84+
TooNested,
85+
ExhaustedSubIndices,
86+
ExhaustedSubSubIndices,
87+
OverflowedIndices,
88+
OverflowedSubIndices,
8689
}
8790

8891
const MAX_LEVEL: u32 = 2;
@@ -94,7 +97,7 @@ const SUB_SUB_NODES: u32 = 8;
9497
pub const MAX_REAL_INDEX: u32 = (1 << LEVEL_SHIFT) - 1;
9598

9699
/// sub-AST level is stored in the top two bits
97-
fn sub_ast_level(index: u32) -> u32 {
100+
pub fn sub_ast_level(index: u32) -> u32 {
98101
(index & LEVEL_MASK) >> LEVEL_SHIFT
99102
}
100103

@@ -106,10 +109,10 @@ pub fn sub_indices(index: u32) -> Result<(u32, u32), NodeIndexError> {
106109
}
107110
let next_level = (level + 1) << LEVEL_SHIFT;
108111
let without_level = index & !LEVEL_MASK;
109-
let nodes_in_level = if level == 0 {
110-
SUB_NODES
112+
let (nodes_in_level, error_kind) = if level == 0 {
113+
(SUB_NODES, NodeIndexError::OverflowedIndices)
111114
} else if level == 1 {
112-
SUB_SUB_NODES
115+
(SUB_SUB_NODES, NodeIndexError::OverflowedSubIndices)
113116
} else {
114117
unreachable!(
115118
"Someone made a mistake updating the encoding of node indices: {index:08X} had level {level}"
@@ -119,10 +122,10 @@ pub fn sub_indices(index: u32) -> Result<(u32, u32), NodeIndexError> {
119122
// If this overflows the file has hundreds of thousands of lines of code,
120123
// but that *can* happen (we just can't support string annotations that deep)
121124
let sub_index_without_level = without_level
122-
.checked_mul(SUB_NODES)
123-
.ok_or(NodeIndexError::OutOfIndices)?;
125+
.checked_mul(nodes_in_level)
126+
.ok_or(error_kind)?;
124127
if sub_index_without_level > MAX_REAL_INDEX {
125-
return Err(NodeIndexError::OutOfIndices);
128+
return Err(error_kind);
126129
}
127130

128131
let first_index = sub_index_without_level | next_level;

crates/ty_python_semantic/resources/mdtest/annotations/string.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,12 +128,12 @@ reveal_type(Aliases.not_forward) # revealed: str
128128
```py
129129
a: "int" = 1
130130
b: "'int'" = 1
131-
# error: [invalid-syntax-in-forward-annotation] "Too many levels of nested string annotations"
131+
# error: [invalid-syntax-in-forward-annotation] "too many levels of nested string annotations; remove the redundant nested quotes"
132132
c: """'"int"'""" = 1
133133
d: "Foo"
134134
# error: [invalid-assignment] "Object of type `Literal[1]` is not assignable to `Foo`"
135135
e: "Foo" = 1
136-
# error: [invalid-syntax-in-forward-annotation] "too complex"
136+
# error: [invalid-syntax-in-forward-annotation] "nested string annotation is too long; remove the redundant nested quotes"
137137
f: "'str | int | bool | Foo | Bar'" = 1
138138

139139
class Foo: ...

0 commit comments

Comments
 (0)