Skip to content

Commit b1c47dc

Browse files
iahsmeta-codesync[bot]
authored andcommitted
Pass URI through placeholder typedefs
Summary: Fixes a bug where annotations applied to the package statement, which frequently appears before include statements, are silently dropped because placeholder typedefs do not update their URIs when resolved Reviewed By: aristidisp Differential Revision: D89898540 fbshipit-source-id: c2dc08ffc350288975bbac9ff17278d52643170c
1 parent 20a9ef1 commit b1c47dc

2 files changed

Lines changed: 52 additions & 1 deletion

File tree

third-party/thrift/src/thrift/compiler/ast/t_typedef.cc

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,10 @@ bool t_placeholder_typedef::resolve() {
7575
// type_ref instead.
7676
set_name(aliased_type_ref_->name());
7777
set_program(aliased_type_ref_->program());
78-
// Successfully resolved, and updated name, program.
78+
// Copy the URI from the resolved type so that URI-based lookups work
79+
// correctly (e.g., find_structured_annotation_or_null).
80+
set_uri(aliased_type_ref_->uri(), aliased_type_ref_->explicit_uri());
81+
// Successfully resolved, and updated name, program, uri.
7982
return true;
8083
}
8184

third-party/thrift/src/thrift/compiler/test/parser_test.cc

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,3 +286,51 @@ TEST(ParserTest, unresolved_include_circular_references) {
286286
EXPECT_TRUE(diag.has_value());
287287
EXPECT_EQ("Circular typedef: Foo --> Foo", diag->message());
288288
}
289+
290+
// Test that when a structured annotation is used BEFORE its include
291+
// (on the program itself), the URI lookup still works correctly.
292+
// This is the key bug case where a placeholder typedef is created.
293+
TEST(ParserTest, structured_annotation_before_include_uri) {
294+
auto source_mgr = source_manager();
295+
source_mgr.add_virtual_file("annotation.thrift", R"(
296+
package "facebook.com/thrift/test/annotation"
297+
struct MyAnnotation {
298+
1: string value;
299+
}
300+
)");
301+
302+
// The program annotation @annotation.MyAnnotation appears BEFORE the include.
303+
// This creates a placeholder typedef that must be resolved correctly.
304+
source_mgr.add_virtual_file("test.thrift", R"(
305+
@annotation.MyAnnotation{value="test"}
306+
package "facebook.com/thrift/test"
307+
308+
include "annotation.thrift"
309+
310+
struct S {
311+
1: i32 field;
312+
}
313+
)");
314+
315+
auto diag = std::optional<diagnostic>();
316+
auto diags = diagnostics_engine(
317+
source_mgr, [&diag](const diagnostic& d) { diag = d; });
318+
319+
auto programs = parse_ast(source_mgr, diags, "test.thrift", {});
320+
EXPECT_FALSE(diags.has_errors()) << (diag ? diag->message() : "no diag");
321+
ASSERT_NE(programs, nullptr);
322+
323+
// The program itself should have the structured annotation
324+
auto& root = *programs->root_program();
325+
ASSERT_EQ(root.structured_annotations().size(), 1);
326+
327+
// The annotation type should have the correct URI from the resolved type
328+
const auto& annotation = root.structured_annotations()[0];
329+
EXPECT_EQ(
330+
annotation.type()->uri(),
331+
"facebook.com/thrift/test/annotation/MyAnnotation");
332+
333+
// has_structured_annotation should also work with URI lookup
334+
EXPECT_TRUE(root.has_structured_annotation(
335+
"facebook.com/thrift/test/annotation/MyAnnotation"));
336+
}

0 commit comments

Comments
 (0)