Skip to content

Commit

Permalink
Only store successfully parsed annotations in the arena
Browse files Browse the repository at this point in the history
  • Loading branch information
AlexWaygood committed Sep 2, 2024
1 parent 67f2ba8 commit c4d222e
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 19 deletions.
29 changes: 17 additions & 12 deletions crates/ruff_linter/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,7 @@ use ruff_python_ast::{
use ruff_python_ast::{helpers, str, visitor, PySourceType};
use ruff_python_codegen::{Generator, Stylist};
use ruff_python_index::Indexer;
use ruff_python_parser::typing::{
parse_type_annotation, AnnotationKind, AnnotationParseResult, ParsedAnnotation,
};
use ruff_python_parser::typing::{parse_type_annotation, AnnotationKind, ParsedAnnotation};
use ruff_python_parser::{Parsed, Tokens};
use ruff_python_semantic::all::{DunderAllDefinition, DunderAllFlags};
use ruff_python_semantic::analyze::{imports, typing};
Expand Down Expand Up @@ -235,7 +233,7 @@ impl<'a> Checker<'a> {
#[allow(clippy::too_many_arguments)]
pub(crate) fn new(
parsed: &'a Parsed<ModModule>,
parsed_annotations_arena: &'a typed_arena::Arena<AnnotationParseResult>,
parsed_annotations_arena: &'a typed_arena::Arena<ParsedAnnotation>,
settings: &'a LinterSettings,
noqa_line_for: &'a NoqaMapping,
noqa: flags::Noqa,
Expand Down Expand Up @@ -422,7 +420,7 @@ impl<'a> Checker<'a> {
pub(crate) fn parse_type_annotation(
&self,
annotation: &ast::ExprStringLiteral,
) -> &'a AnnotationParseResult {
) -> Option<&'a ParsedAnnotation> {
self.parsed_annotations_cache
.lookup_or_parse(annotation, self.locator.contents())
}
Expand Down Expand Up @@ -2195,7 +2193,7 @@ impl<'a> Checker<'a> {
let type_definitions = std::mem::take(&mut self.visit.string_type_definitions);
for (string_expr, snapshot) in type_definitions {
let annotation_parse_result = self.parse_type_annotation(string_expr);
if let Ok(parsed_annotation) = annotation_parse_result {
if let Some(parsed_annotation) = annotation_parse_result {
self.parsed_type_annotation = Some(parsed_annotation);

let annotation = string_expr.value.to_str();
Expand Down Expand Up @@ -2404,12 +2402,12 @@ impl<'a> Checker<'a> {
}

struct ParsedAnnotationsCache<'a> {
arena: &'a typed_arena::Arena<AnnotationParseResult>,
by_offset: RefCell<FxHashMap<TextSize, &'a AnnotationParseResult>>,
arena: &'a typed_arena::Arena<ParsedAnnotation>,
by_offset: RefCell<FxHashMap<TextSize, Option<&'a ParsedAnnotation>>>,
}

impl<'a> ParsedAnnotationsCache<'a> {
fn new(arena: &'a typed_arena::Arena<AnnotationParseResult>) -> Self {
fn new(arena: &'a typed_arena::Arena<ParsedAnnotation>) -> Self {
Self {
arena,
by_offset: RefCell::default(),
Expand All @@ -2420,11 +2418,18 @@ impl<'a> ParsedAnnotationsCache<'a> {
&self,
annotation: &ast::ExprStringLiteral,
source: &str,
) -> &'a AnnotationParseResult {
self.by_offset
) -> Option<&'a ParsedAnnotation> {
*self
.by_offset
.borrow_mut()
.entry(annotation.start())
.or_insert_with(|| self.arena.alloc(parse_type_annotation(annotation, source)))
.or_insert_with(|| {
if let Ok(annotation) = parse_type_annotation(annotation, source) {
Some(self.arena.alloc(annotation))
} else {
None
}
})
}

fn clear(&self) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@ fn check_dynamically_typed<F>(
{
if let Expr::StringLiteral(string_expr) = annotation {
// Quoted annotations
if let Ok(parsed_annotation) = checker.parse_type_annotation(string_expr) {
if let Some(parsed_annotation) = checker.parse_type_annotation(string_expr) {
if type_hint_resolves_to_any(
parsed_annotation.expression(),
checker,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ pub(crate) fn implicit_optional(checker: &mut Checker, parameters: &Parameters)

if let Expr::StringLiteral(string_expr) = annotation.as_ref() {
// Quoted annotation.
if let Ok(parsed_annotation) = checker.parse_type_annotation(string_expr) {
if let Some(parsed_annotation) = checker.parse_type_annotation(string_expr) {
let Some(expr) = type_hint_explicitly_allows_none(
parsed_annotation.expression(),
checker,
Expand Down
6 changes: 2 additions & 4 deletions crates/ruff_linter/src/rules/ruff/typing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,8 @@ impl<'a> TypingTarget<'a> {
Expr::StringLiteral(string_expr) => checker
.parse_type_annotation(string_expr)
.as_ref()
.map_or(None, |parsed_annotation| {
Some(TypingTarget::ForwardReference(
parsed_annotation.expression(),
))
.map(|parsed_annotation| {
TypingTarget::ForwardReference(parsed_annotation.expression())
}),
_ => semantic.resolve_qualified_name(expr).map_or(
// If we can't resolve the call path, it must be defined in the
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_python_parser/src/typing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use ruff_text_size::Ranged;

use crate::{parse_expression, parse_expression_range, ParseError, Parsed};

pub type AnnotationParseResult = Result<ParsedAnnotation, ParseError>;
type AnnotationParseResult = Result<ParsedAnnotation, ParseError>;

#[derive(Debug)]
pub struct ParsedAnnotation {
Expand Down

0 comments on commit c4d222e

Please sign in to comment.