Skip to content

Commit baf7909

Browse files
committed
fix(vdev): match grouped dropped-event calls syntactically
Require the canonical struct receiver and cover unrelated comments, types, strings, and method calls with regression tests. Environment: Datadog workspace
1 parent acaa189 commit baf7909

1 file changed

Lines changed: 75 additions & 15 deletions

File tree

‎vdev/src/commands/check/events.rs‎

Lines changed: 75 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -381,9 +381,14 @@ static RE_EMIT_DROPPED: LazyLock<Regex> = LazyLock::new(|| {
381381
Regex::new(r"(?:emit|register)!\([ \t\r\n]*ComponentEventsDropped(?:[^A-Za-z0-9_]|$)").unwrap()
382382
});
383383

384-
fn emits_component_events_dropped(source: &str) -> bool {
385-
RE_EMIT_DROPPED.is_match(source)
386-
|| (source.contains("ComponentEventsDropped") && source.contains(".emit_with_group("))
384+
fn is_grouped_drop_call(call: &syn::ExprMethodCall) -> bool {
385+
let mut receiver = call.receiver.as_ref();
386+
while let syn::Expr::Paren(paren) = receiver {
387+
receiver = &paren.expr;
388+
}
389+
call.method == "emit_with_group"
390+
&& matches!(receiver, syn::Expr::Struct(event)
391+
if event.path.segments.last().is_some_and(|segment| segment.ident == "ComponentEventsDropped"))
387392
}
388393

389394
/// `emit!(EventName)` / `register!(Path::EventName)` use-counting regex,
@@ -737,7 +742,7 @@ impl<'ast> Visit<'ast> for Scanner<'_> {
737742
"InternalEventHandle" => event.impl_event_handle = true,
738743
_ => {}
739744
}
740-
if emits_component_events_dropped(&raw_block) {
745+
if RE_EMIT_DROPPED.is_match(&raw_block) {
741746
event.emits_component_events_dropped = true;
742747
}
743748
self.impl_stack.push(ImplCtx { event_name });
@@ -752,6 +757,18 @@ impl<'ast> Visit<'ast> for Scanner<'_> {
752757
visit::visit_item_impl(self, node);
753758
}
754759

760+
fn visit_expr_method_call(&mut self, node: &'ast syn::ExprMethodCall) {
761+
if is_grouped_drop_call(node)
762+
&& let Some(ctx) = self.impl_stack.last()
763+
{
764+
self.events
765+
.entry(ctx.event_name.clone())
766+
.or_default()
767+
.emits_component_events_dropped = true;
768+
}
769+
visit::visit_expr_method_call(self, node);
770+
}
771+
755772
fn visit_macro(&mut self, node: &'ast syn::Macro) {
756773
let name = node
757774
.path
@@ -760,8 +777,8 @@ impl<'ast> Visit<'ast> for Scanner<'_> {
760777
.map(|s| s.ident.to_string())
761778
.unwrap_or_default();
762779

763-
// Use-counting, ComponentEventsDropped detection, and the
764-
// log-message format check all run via a separate raw-source pass
780+
// Use-counting, emit!/register! ComponentEventsDropped detection, and
781+
// the log-message format check run via a separate raw-source pass
765782
// because `syn` does not descend into the bodies of arbitrary
766783
// `tokio::select!` / `cfg_if!` / etc. macro invocations.
767784

@@ -884,7 +901,9 @@ impl Scanner<'_> {
884901
// Component-events-dropped emission.
885902
if expr.contains("emit ! (ComponentEventsDropped")
886903
|| expr.contains("register ! (ComponentEventsDropped")
887-
|| (expr.contains("ComponentEventsDropped") && expr.contains(". emit_with_group ("))
904+
|| syn::parse_str::<syn::Expr>(expr).is_ok_and(|expr| {
905+
matches!(expr, syn::Expr::MethodCall(call) if is_grouped_drop_call(&call))
906+
})
888907
{
889908
event.emits_component_events_dropped = true;
890909
}
@@ -1264,17 +1283,58 @@ mod tests {
12641283
assert_eq!(split_comma_args(input), vec![input.to_string()]);
12651284
}
12661285

1286+
fn scan_dropped_event(body: &str) -> Event {
1287+
let source = format!(
1288+
"struct TestEventsDropped; impl InternalEvent for TestEventsDropped {{ fn emit(self) {{ {body} }} }}"
1289+
);
1290+
let file = syn::parse_file(&source).expect("parse event");
1291+
let mut events = HashMap::new();
1292+
Scanner {
1293+
events: &mut events,
1294+
path_str: "src/internal_events/test.rs".into(),
1295+
in_internal_events_dir: true,
1296+
skip_dropped_for_file: false,
1297+
text: &source,
1298+
impl_stack: Vec::new(),
1299+
}
1300+
.visit_file(&file);
1301+
let mut event = events.remove("TestEventsDropped").expect("scanned event");
1302+
event.uses = 1;
1303+
event
1304+
}
1305+
12671306
#[test]
12681307
fn recognizes_component_events_dropped_emit_with_group() {
1269-
let source = r#"
1270-
ComponentEventsDropped::<INTENTIONAL> {
1271-
count: 1,
1272-
reason: "discarded",
1273-
}
1274-
.emit_with_group(group);
1275-
"#;
1308+
for body in [
1309+
r#"ComponentEventsDropped::<INTENTIONAL> { count: 1, reason: "discarded" }
1310+
.emit_with_group(group);"#,
1311+
r#"(vector_lib::internal_event::ComponentEventsDropped::<INTENTIONAL> {
1312+
count: 1, reason: "discarded",
1313+
}).emit_with_group(group);"#,
1314+
] {
1315+
let event = scan_dropped_event(body);
1316+
assert!(event.emits_component_events_dropped, "{body}");
1317+
assert!(run("TestEventsDropped", event).is_empty());
1318+
}
1319+
}
12761320

1277-
assert!(emits_component_events_dropped(source));
1321+
#[test]
1322+
fn unrelated_group_calls_do_not_bypass_dropped_event_validation() {
1323+
for body in [
1324+
"// ComponentEventsDropped\nother.emit_with_group(group);",
1325+
"let _: Option<ComponentEventsDropped> = None; other.emit_with_group(group);",
1326+
r#"let _ = "ComponentEventsDropped { count: 1 }.emit_with_group(group)";
1327+
other.emit_with_group(group);"#,
1328+
"let _ = ComponentEventsDropped { count: 1 }; other.emit_with_group(group);",
1329+
] {
1330+
let event = scan_dropped_event(body);
1331+
assert!(!event.emits_component_events_dropped, "{body}");
1332+
let reports = run("TestEventsDropped", event);
1333+
assert!(reports.iter().any(|report| report.contains("MUST log")));
1334+
assert!(reports.iter().any(|report| report.contains(&format!(
1335+
"This event MUST increment counter \"{METRIC_NAME_EVENTS_DROPPED}\"."
1336+
))));
1337+
}
12781338
}
12791339

12801340
#[test]

0 commit comments

Comments
 (0)