Skip to content

Commit 0133741

Browse files
Brooooooklynclaude
andcommitted
refactor: improve host directive name extraction and deduplicate ctor deps
- Handle ReadProp (namespace-qualified, e.g. i1.SomeDirective) and External expression variants in extract_directive_name_from_expr - Replace silent "unknown" fallback with descriptive panic for unhandled variants - Extract shared ctor deps formatting logic into generate_ctor_deps_type helper, reducing ~80 lines of duplication Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent c6baba3 commit 0133741

File tree

1 file changed

+76
-79
lines changed
  • crates/oxc_angular_compiler/src

1 file changed

+76
-79
lines changed

crates/oxc_angular_compiler/src/dts.rs

Lines changed: 76 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -438,53 +438,63 @@ fn type_with_parameters(class_name: &str, count: u32) -> String {
438438
}
439439
}
440440

441+
/// Shared helper: given an iterator of per-dependency info tuples, produce the
442+
/// constructor deps type string (`never` or a tuple type like
443+
/// `[null, {attribute: "title", optional: true}, null]`).
444+
///
445+
/// Each tuple element is `(attribute_entry, optional, host, self_, skip_self)`.
446+
/// `attribute_entry` is the pre-formatted `attribute: …` string fragment when
447+
/// the dependency has an attribute flag (e.g. `attribute: "title"` or
448+
/// `attribute: string`), or `None` otherwise.
449+
fn generate_ctor_deps_type(
450+
deps: impl Iterator<Item = (Option<String>, bool, bool, bool, bool)>,
451+
) -> String {
452+
let dep_types: Vec<Option<String>> = deps
453+
.map(|(attribute_entry, optional, host, self_, skip_self)| {
454+
let mut entries: Vec<String> = Vec::new();
455+
if let Some(attr) = attribute_entry {
456+
entries.push(attr);
457+
}
458+
if optional {
459+
entries.push("optional: true".to_string());
460+
}
461+
if host {
462+
entries.push("host: true".to_string());
463+
}
464+
if self_ {
465+
entries.push("self: true".to_string());
466+
}
467+
if skip_self {
468+
entries.push("skipSelf: true".to_string());
469+
}
470+
if entries.is_empty() { None } else { Some(format!("{{ {} }}", entries.join(", "))) }
471+
})
472+
.collect();
473+
474+
let has_types = dep_types.iter().any(|t| t.is_some());
475+
if !has_types {
476+
"never".to_string()
477+
} else {
478+
let entries: Vec<String> =
479+
dep_types.into_iter().map(|t| t.unwrap_or_else(|| "null".to_string())).collect();
480+
format!("[{}]", entries.join(", "))
481+
}
482+
}
483+
441484
/// Generate the constructor deps type parameter for `ɵɵFactoryDeclaration`.
442485
///
443486
/// Returns `never` if no dependency has any special flags (attribute, optional, host, self, skipSelf).
444487
/// Otherwise returns a tuple type like `[null, {attribute: "title", optional: true}, null]`.
445488
fn generate_ctor_deps_type_from_component_deps(deps: Option<&[R3DependencyMetadata]>) -> String {
446489
match deps {
447490
None => "never".to_string(),
448-
Some(deps) => {
449-
let dep_types: Vec<Option<String>> = deps
450-
.iter()
451-
.map(|d| {
452-
let mut entries: Vec<String> = Vec::new();
453-
if let Some(name) = &d.attribute_name {
454-
entries
455-
.push(format!("attribute: \"{}\"", escape_dts_string(name.as_str())));
456-
}
457-
if d.optional {
458-
entries.push("optional: true".to_string());
459-
}
460-
if d.host {
461-
entries.push("host: true".to_string());
462-
}
463-
if d.self_ {
464-
entries.push("self: true".to_string());
465-
}
466-
if d.skip_self {
467-
entries.push("skipSelf: true".to_string());
468-
}
469-
if entries.is_empty() {
470-
None
471-
} else {
472-
Some(format!("{{ {} }}", entries.join(", ")))
473-
}
474-
})
475-
.collect();
476-
477-
let has_types = dep_types.iter().any(|t| t.is_some());
478-
if !has_types {
479-
"never".to_string()
480-
} else {
481-
let entries: Vec<String> = dep_types
482-
.into_iter()
483-
.map(|t| t.unwrap_or_else(|| "null".to_string()))
484-
.collect();
485-
format!("[{}]", entries.join(", "))
486-
}
487-
}
491+
Some(deps) => generate_ctor_deps_type(deps.iter().map(|d| {
492+
let attribute_entry = d
493+
.attribute_name
494+
.as_ref()
495+
.map(|name| format!("attribute: \"{}\"", escape_dts_string(name.as_str())));
496+
(attribute_entry, d.optional, d.host, d.self_, d.skip_self)
497+
})),
488498
}
489499
}
490500

@@ -500,45 +510,14 @@ fn generate_ctor_deps_type_from_factory_deps(
500510
) -> String {
501511
match deps {
502512
None => "never".to_string(),
503-
Some(deps) => {
504-
let dep_types: Vec<Option<String>> = deps
505-
.iter()
506-
.map(|d| {
507-
let mut entries: Vec<String> = Vec::new();
508-
if d.attribute_name_type.is_some() {
509-
entries.push("attribute: string".to_string());
510-
}
511-
if d.optional {
512-
entries.push("optional: true".to_string());
513-
}
514-
if d.host {
515-
entries.push("host: true".to_string());
516-
}
517-
if d.self_ {
518-
entries.push("self: true".to_string());
519-
}
520-
if d.skip_self {
521-
entries.push("skipSelf: true".to_string());
522-
}
523-
if entries.is_empty() {
524-
None
525-
} else {
526-
Some(format!("{{ {} }}", entries.join(", ")))
527-
}
528-
})
529-
.collect();
530-
531-
let has_types = dep_types.iter().any(|t| t.is_some());
532-
if !has_types {
533-
"never".to_string()
513+
Some(deps) => generate_ctor_deps_type(deps.iter().map(|d| {
514+
let attribute_entry = if d.attribute_name_type.is_some() {
515+
Some("attribute: string".to_string())
534516
} else {
535-
let entries: Vec<String> = dep_types
536-
.into_iter()
537-
.map(|t| t.unwrap_or_else(|| "null".to_string()))
538-
.collect();
539-
format!("[{}]", entries.join(", "))
540-
}
541-
}
517+
None
518+
};
519+
(attribute_entry, d.optional, d.host, d.self_, d.skip_self)
520+
})),
542521
}
543522
}
544523

@@ -715,12 +694,30 @@ fn generate_host_directives_type_from_directive(
715694
}
716695

717696
/// Extract a directive name from an `OutputExpression`.
697+
///
698+
/// Handles:
699+
/// - `ReadVar`: simple variable name (e.g. `SomeDirective`)
700+
/// - `ReadProp`: namespace-qualified reference (e.g. `i1.SomeDirective`)
701+
/// - `External`: external module reference, using the export name
718702
fn extract_directive_name_from_expr(expr: &crate::output::ast::OutputExpression) -> String {
719703
match expr {
720704
crate::output::ast::OutputExpression::ReadVar(read_var) => {
721705
read_var.name.as_str().to_string()
722706
}
723-
_ => "unknown".to_string(),
707+
crate::output::ast::OutputExpression::ReadProp(read_prop) => {
708+
let receiver = extract_directive_name_from_expr(&read_prop.receiver);
709+
format!("{}.{}", receiver, read_prop.name.as_str())
710+
}
711+
crate::output::ast::OutputExpression::External(external) => match &external.value.name {
712+
Some(name) => name.as_str().to_string(),
713+
None => panic!("ExternalExpr in host directive has no export name"),
714+
},
715+
other => {
716+
panic!(
717+
"Unexpected OutputExpression variant in host directive type: {:?}",
718+
std::mem::discriminant(other)
719+
)
720+
}
724721
}
725722
}
726723

0 commit comments

Comments
 (0)