Skip to content

Commit 8d5f51d

Browse files
committed
fix(analyzer): warn when instantiating class-string<Interface> that may contain the interface itself
closes #627 Signed-off-by: azjezz <[email protected]>
1 parent 719a055 commit 8d5f51d

File tree

3 files changed

+49
-0
lines changed

3 files changed

+49
-0
lines changed

crates/analyzer/src/expression/instantiation.rs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,28 @@ fn analyze_class_instantiation<'ctx, 'arena>(
213213
return Ok(get_never());
214214
}
215215

216+
if classname.is_from_class_string() && (metadata.kind.is_interface() || metadata.kind.is_trait()) {
217+
let kind_name = if metadata.kind.is_interface() { "interface" } else { "trait" };
218+
219+
context.collector.report_with_code(
220+
IssueCode::UnsafeInstantiation,
221+
Issue::warning(format!(
222+
"Potentially unsafe instantiation: `class-string<{classname_str}>` may contain the {kind_name} `{classname_str}` itself, which cannot be instantiated.",
223+
))
224+
.with_annotation(
225+
Annotation::primary(class_expression_span)
226+
.with_message(format!(
227+
"This expression is `class-string<{classname_str}>` where `{classname_str}` is {article} {kind_name}",
228+
article = if metadata.kind.is_interface() { "an" } else { "a" }
229+
)),
230+
)
231+
.with_note(format!(
232+
"While `class-string<{classname_str}>` usually contains a concrete class implementing the {kind_name}, it could technically be `{classname_str}::class` itself.",
233+
))
234+
.with_help("Consider using a more specific type or adding runtime validation."),
235+
);
236+
}
237+
216238
let mut is_impossible = false;
217239
if metadata.flags.is_abstract() && !classname.can_extend_static() && !classname.is_from_class_string() {
218240
context.collector.report_with_code(
@@ -403,7 +425,11 @@ fn analyze_class_instantiation<'ctx, 'arena>(
403425
);
404426
}
405427

428+
let skip_constructor_warning =
429+
classname.is_from_class_string() && (metadata.kind.is_interface() || metadata.kind.is_trait());
430+
406431
if has_inconsistent_constructor
432+
&& !skip_constructor_warning
407433
&& (classname.is_static() || classname.is_from_class_string() || classname.is_object_instance())
408434
{
409435
let mut issue = if classname.is_static() {
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
interface MyInterface
6+
{
7+
}
8+
9+
final class MyClass implements MyInterface
10+
{
11+
}
12+
13+
/**
14+
* @param class-string<MyInterface> $classname
15+
*/
16+
function instantiate(string $classname): MyInterface
17+
{
18+
return new $classname(); // @mago-expect analysis:unsafe-instantiation
19+
}
20+
21+
$_ = instantiate(MyClass::class); // safe
22+
$_ = instantiate(MyInterface::class); // unsafe

crates/analyzer/tests/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,7 @@ test_case!(issue_621);
410410
test_case!(issue_622);
411411
test_case!(issue_623);
412412
test_case!(issue_626);
413+
test_case!(issue_627);
413414
test_case!(issue_633);
414415
test_case!(issue_637);
415416
test_case!(issue_639);

0 commit comments

Comments
 (0)