Skip to content

Commit e50bb4e

Browse files
committed
Fix interface-extends-interface constants
1 parent e5d9ffd commit e50bb4e

3 files changed

Lines changed: 122 additions & 36 deletions

File tree

docs/todo/bugs.md

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -77,38 +77,6 @@ completion.
7777

7878
---
7979

80-
#### B12. Interface-extends-interface constants (and other members) not merged
81-
82-
| | |
83-
|---|---|
84-
| **Impact** | Low-Medium |
85-
| **Effort** | Low-Medium |
86-
87-
When an interface extends multiple parent interfaces (e.g.
88-
`interface CarbonInterface extends DateTimeInterface, JsonSerializable, UnitValue`),
89-
`resolve_class_with_inheritance` only walks the `parent_class` field (the
90-
first extended interface). The remaining parent interfaces stored in the
91-
`interfaces` list are not traversed for member merging.
92-
93-
Then `resolve_class_fully_inner` calls `resolve_class_with_inheritance` on
94-
each interface collected from the class, but that inner call has the same
95-
limitation — it does not recurse into the interface's own `interfaces` list.
96-
97-
**Reproducer:** `Illuminate\Support\Carbon::JANUARY` — the `JANUARY` constant
98-
lives on `Carbon\Constants\UnitValue`, which `CarbonInterface` extends (6th
99-
in the extends list). PHPantom reports "Member 'JANUARY' not found on class
100-
'Illuminate\Support\Carbon'".
101-
102-
**Fix:** In `resolve_class_with_inheritance`, when processing an interface
103-
(or always), also merge members from all entries in `self.interfaces`, not
104-
just `parent_class`. Alternatively, make `resolve_class_fully_inner`
105-
recursively collect parent interfaces when resolving each interface.
106-
107-
Affects 4 diagnostics in shared (Carbon month constants) and likely more in
108-
other projects that use interfaces with multi-extends chains.
109-
110-
---
111-
11280
#### B13. Variable type resolved from reassignment target inside RHS expression
11381

11482
| | |

src/virtual_members/mod.rs

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -738,14 +738,45 @@ fn resolve_class_fully_inner(
738738
}
739739
}
740740

741-
for iface_name in &all_iface_names {
742-
if let Some(iface) = class_loader(iface_name) {
741+
// Use an index-based loop so that we can grow `all_iface_names`
742+
// while iterating — each resolved interface may itself extend
743+
// additional interfaces that need to be collected transitively.
744+
let mut iface_idx = 0;
745+
while iface_idx < all_iface_names.len() {
746+
let iface_name = all_iface_names[iface_idx].clone();
747+
iface_idx += 1;
748+
749+
if let Some(iface) = class_loader(&iface_name) {
750+
// Collect interfaces that this interface itself extends.
751+
// For example, CarbonInterface extends DateTimeInterface,
752+
// JsonSerializable, UnitValue — all of those need to be
753+
// resolved and their members merged transitively.
754+
for child_iface in &iface.interfaces {
755+
if !all_iface_names.contains(child_iface) {
756+
all_iface_names.push(child_iface.clone());
757+
}
758+
}
759+
760+
// Collect @extends / @implements generics from the
761+
// interface so that template substitutions flow through
762+
// transitive interface chains.
763+
for (name, args) in &iface.extends_generics {
764+
if !all_implements_generics.iter().any(|(n, _)| n == name) {
765+
all_implements_generics.push((name.clone(), args.clone()));
766+
}
767+
}
768+
for (name, args) in &iface.implements_generics {
769+
if !all_implements_generics.iter().any(|(n, _)| n == name) {
770+
all_implements_generics.push((name.clone(), args.clone()));
771+
}
772+
}
773+
743774
// Build a substitution map from `@implements` generics for
744775
// this interface. If the class (or a parent) declared
745776
// `@implements ThisInterface<Type1, Type2>`, map the
746777
// interface's template params to those concrete types.
747778
let iface_subs =
748-
build_implements_substitution_map(iface_name, &iface, &all_implements_generics);
779+
build_implements_substitution_map(&iface_name, &iface, &all_implements_generics);
749780

750781
// When we have substitutions to apply, we cannot use a
751782
// cached bare-interface resolution because the cached version

src/virtual_members/tests.rs

Lines changed: 88 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use super::*;
22
use crate::test_fixtures::{make_class, make_method, make_property};
3-
use crate::types::Visibility;
3+
use crate::types::{ClassLikeKind, ConstantInfo, Visibility};
44
use std::sync::Arc;
55

66
// ── VirtualMembers tests ────────────────────────────────────────────
@@ -1186,3 +1186,90 @@ fn evict_cast_class_chains_through_model_to_child() {
11861186
"cast eviction should chain: cast → model (via casts) → child (via parent)"
11871187
);
11881188
}
1189+
1190+
// ── B12: interface-extends-interface transitive member merging ───────
1191+
1192+
#[test]
1193+
fn resolve_class_fully_merges_transitive_interface_constants() {
1194+
// Simulates the Carbon scenario:
1195+
// interface UnitValue { const JANUARY = 1; }
1196+
// interface JsonSerializable { function jsonSerialize(): mixed; }
1197+
// interface DateTimeInterface { function format(string $f): string; }
1198+
// interface CarbonInterface extends DateTimeInterface, JsonSerializable, UnitValue {}
1199+
// class Carbon implements CarbonInterface {}
1200+
//
1201+
// Before the fix, only DateTimeInterface (the first extended
1202+
// interface, stored in `parent_class`) was merged. Members from
1203+
// JsonSerializable and UnitValue were lost.
1204+
1205+
let mut unit_value = make_class("Carbon\\Constants\\UnitValue");
1206+
unit_value.kind = ClassLikeKind::Interface;
1207+
unit_value.constants.push(ConstantInfo {
1208+
name: "JANUARY".to_string(),
1209+
name_offset: 0,
1210+
type_hint: Some("int".to_string()),
1211+
visibility: Visibility::Public,
1212+
deprecation_message: None,
1213+
deprecated_replacement: None,
1214+
see_refs: Vec::new(),
1215+
description: None,
1216+
is_enum_case: false,
1217+
enum_value: None,
1218+
value: Some("1".to_string()),
1219+
is_virtual: false,
1220+
});
1221+
1222+
let mut json_serializable = make_class("JsonSerializable");
1223+
json_serializable.kind = ClassLikeKind::Interface;
1224+
json_serializable
1225+
.methods
1226+
.push(make_method("jsonSerialize", Some("mixed")));
1227+
1228+
let mut datetime_iface = make_class("DateTimeInterface");
1229+
datetime_iface.kind = ClassLikeKind::Interface;
1230+
datetime_iface
1231+
.methods
1232+
.push(make_method("format", Some("string")));
1233+
1234+
let mut carbon_iface = make_class("CarbonInterface");
1235+
carbon_iface.kind = ClassLikeKind::Interface;
1236+
carbon_iface.parent_class = Some("DateTimeInterface".to_string());
1237+
carbon_iface.interfaces = vec![
1238+
"DateTimeInterface".to_string(),
1239+
"JsonSerializable".to_string(),
1240+
"Carbon\\Constants\\UnitValue".to_string(),
1241+
];
1242+
1243+
let mut carbon = make_class("Carbon");
1244+
carbon.interfaces = vec!["CarbonInterface".to_string()];
1245+
1246+
let class_loader = move |name: &str| -> Option<Arc<ClassInfo>> {
1247+
match name {
1248+
"CarbonInterface" => Some(Arc::new(carbon_iface.clone())),
1249+
"DateTimeInterface" => Some(Arc::new(datetime_iface.clone())),
1250+
"JsonSerializable" => Some(Arc::new(json_serializable.clone())),
1251+
"Carbon\\Constants\\UnitValue" => Some(Arc::new(unit_value.clone())),
1252+
_ => None,
1253+
}
1254+
};
1255+
1256+
let resolved = crate::virtual_members::resolve_class_fully(&carbon, &class_loader);
1257+
1258+
// DateTimeInterface::format — merged via CarbonInterface's parent_class chain
1259+
assert!(
1260+
resolved.methods.iter().any(|m| m.name == "format"),
1261+
"should have DateTimeInterface::format"
1262+
);
1263+
1264+
// JsonSerializable::jsonSerialize — merged via transitive interface collection
1265+
assert!(
1266+
resolved.methods.iter().any(|m| m.name == "jsonSerialize"),
1267+
"should have JsonSerializable::jsonSerialize (2nd extended interface)"
1268+
);
1269+
1270+
// UnitValue::JANUARY — merged via transitive interface collection
1271+
assert!(
1272+
resolved.constants.iter().any(|c| c.name == "JANUARY"),
1273+
"should have UnitValue::JANUARY constant (3rd extended interface)"
1274+
);
1275+
}

0 commit comments

Comments
 (0)