Skip to content

Commit 2566e57

Browse files
authored
fix(analyzer): Handle multiple docblocks before a statements (#818)
1 parent eeb4090 commit 2566e57

File tree

4 files changed

+44
-47
lines changed

4 files changed

+44
-47
lines changed

crates/analyzer/src/context/mod.rs

Lines changed: 32 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use mago_codex::metadata::CodebaseMetadata;
55
use mago_codex::ttype::resolution::TypeResolutionContext;
66
use mago_collector::Collector;
77
use mago_database::file::File;
8-
use mago_docblock::document::Document;
8+
use mago_docblock::document::Element;
99
use mago_names::ResolvedNames;
1010
use mago_names::scope::NamespaceScope;
1111
use mago_reporting::Annotation;
@@ -137,46 +137,44 @@ impl<'ctx, 'arena> Context<'ctx, 'arena> {
137137
}
138138
}
139139

140-
pub fn get_docblock(&self) -> Option<&'arena Trivia<'arena>> {
141-
comments::docblock::get_docblock_before_position(
142-
self.source_file,
143-
self.comments,
144-
self.statement_span.start.offset,
145-
)
146-
}
147-
148-
pub fn get_parsed_docblock(&mut self) -> Option<Document<'arena>> {
149-
let trivia = self.get_docblock()?;
150-
151-
match mago_docblock::parse_trivia(self.arena, trivia) {
152-
Ok(document) => Some(document),
153-
Err(error) => {
154-
let error_span = error.span();
140+
pub fn get_parsed_docblocks(&mut self) -> Vec<Element<'arena>> {
141+
let mut elements = vec![];
142+
let mut start = self.statement_span.start.offset;
143+
while let Some(trivia) =
144+
comments::docblock::get_docblock_before_position(self.source_file, self.comments, start)
145+
{
146+
match mago_docblock::parse_trivia(self.arena, trivia) {
147+
Ok(document) => elements.extend(document.elements),
148+
Err(error) => {
149+
let error_span = error.span();
150+
151+
let mut issue = Issue::error(error.to_string())
152+
.with_annotation(
153+
Annotation::primary(error_span)
154+
.with_message("This part of the docblock has a syntax error"),
155+
)
156+
.with_note(error.note());
157+
158+
if trivia.span != error_span {
159+
issue = issue.with_annotation(
160+
Annotation::secondary(trivia.span).with_message("The error is within this docblock"),
161+
);
162+
}
155163

156-
let mut issue = Issue::error(error.to_string())
157-
.with_annotation(
158-
Annotation::primary(error_span).with_message("This part of the docblock has a syntax error"),
159-
)
160-
.with_note(error.note());
161-
162-
if trivia.span != error_span {
163164
issue = issue.with_annotation(
164-
Annotation::secondary(trivia.span).with_message("The error is within this docblock"),
165+
Annotation::secondary(self.statement_span)
166+
.with_message("This docblock is associated with the following statement"),
165167
);
166-
}
167168

168-
issue = issue.with_annotation(
169-
Annotation::secondary(self.statement_span)
170-
.with_message("This docblock is associated with the following statement"),
171-
);
169+
issue = issue.with_help(error.help());
172170

173-
issue = issue.with_help(error.help());
174-
175-
self.collector.report_with_code(IssueCode::InvalidDocblock, issue);
176-
177-
None
171+
self.collector.report_with_code(IssueCode::InvalidDocblock, issue);
172+
}
178173
}
174+
start = trivia.span.start.offset;
179175
}
176+
177+
elements
180178
}
181179

182180
pub fn record<T>(&mut self, callback: impl FnOnce(&mut Context<'ctx, 'arena>) -> T) -> (T, IssueCollection) {

crates/analyzer/src/utils/docblock.rs

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,10 @@ pub fn populate_docblock_variables_excluding<'ctx>(
7575
}
7676
}
7777

78-
/// Retrieves all `@var`, `@psalm-var`, and `@phpstan-var` tags from the docblock of the
78+
/// Retrieves all `@var`, `@psalm-var`, and `@phpstan-var` tags from the docblocks preceding the
7979
/// current statement in the context, parsing their variable types.
8080
///
81-
/// This function scans the docblock associated with the current statement in the context,
81+
/// This function scans the docblocks associated with the current statement in the context,
8282
/// extracting all variable type declarations. It returns a vector of tuples, each containing:
8383
///
8484
/// - An optional variable name (if specified in the tag)
@@ -104,11 +104,7 @@ pub fn get_docblock_variables<'ctx>(
104104
artifacts: &mut AnalysisArtifacts,
105105
allow_tracing: bool,
106106
) -> Vec<(Option<mago_atom::Atom>, TUnion, Span)> {
107-
let Some(elements) = context.get_parsed_docblock().map(|document| document.elements) else {
108-
return vec![];
109-
};
110-
111-
elements
107+
context.get_parsed_docblocks()
112108
.into_iter()
113109
// Filter out non-tag elements
114110
.filter_map(|element| match element {

crates/analyzer/tests/cases/docblock_var_on_non_assignment.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,16 @@
1818
}
1919

2020
// Also test with multiple @var annotations
21-
/** @var array<string, object> $map */
21+
/** @var array<int|string, object> $map */
2222
$map = ['key' => new DateTime()];
2323

24+
function want_string(string $x): string {
25+
return $x;
26+
}
27+
2428
foreach ($map as $key => $value) {
2529
/** @var string $key */
2630
/** @var DateTime $value */
2731
$result = $value->format('Y-m-d');
32+
want_string($key);
2833
}

crates/analyzer/tests/cases/issue_801.php

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,9 @@ public function __construct()
2222
}
2323
}
2424

25-
/**
26-
* @var Config $config
27-
* @var Session $session
28-
*/
25+
/** @var Config $config */
26+
/** @var Session $session */
2927
if (!$session->isLoggedIn()) {
3028
}
3129

32-
echo $config->site_name;
30+
echo $config->site_name;

0 commit comments

Comments
 (0)