Skip to content

Commit ceafd0e

Browse files
iahsmeta-codesync[bot]
authored andcommitted
Ban unstructured annotations
Summary: Deprecated in favor of structured annotations. Reviewed By: vitaut Differential Revision: D95829948 fbshipit-source-id: 8654644098b1c1dc3eaf9cb2363c27cec4736601
1 parent 7d7a07a commit ceafd0e

8 files changed

Lines changed: 142 additions & 107 deletions

File tree

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -942,6 +942,10 @@ std::string parse_args(
942942
sparams.forbid_unstructured_annotations = true;
943943
continue;
944944
}
945+
if (validator == "allow_unstructured_annotations") {
946+
sparams.forbid_unstructured_annotations = false;
947+
continue;
948+
}
945949
if (validator == "warn_on_redundant_custom_default_values") {
946950
sparams.redundant_custom_default_values =
947951
sema_params::validation_level::warn;

third-party/thrift/src/thrift/compiler/sema/sema_context.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ struct sema_params {
134134
// for a field, but that value is equal to the intrinsic default value.
135135
validation_level redundant_custom_default_values = validation_level::none;
136136

137-
bool forbid_unstructured_annotations = false;
137+
bool forbid_unstructured_annotations = true;
138138

139139
// Action to take when @cpp.EnableCustomTypeOrdering is present on a
140140
// (structured) type that does not need it.

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

Lines changed: 118 additions & 96 deletions
Large diffs are not rendered by default.

third-party/thrift/src/thrift/compiler/test/fixture_utils.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,8 @@ def _parse_fixture_cmd(
216216
fixture_dir_abspath,
217217
"-I",
218218
repo_root_dir_abspath,
219+
"--extra-validation",
220+
"allow_unstructured_annotations",
219221
]
220222
if _should_build_included_files_recursively(generator_spec):
221223
base_args.append("-r")

third-party/thrift/src/thrift/compiler/test/json_compiler_test.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,13 @@ def check_run_thrift(annotate, *args):
4343
gen_str = "json"
4444
if annotate:
4545
gen_str = "json:annotate"
46-
argsx = [thrift, "--gen", gen_str] + list(args)
46+
argsx = [
47+
thrift,
48+
"--gen",
49+
gen_str,
50+
"--extra-validation",
51+
"allow_unstructured_annotations",
52+
] + list(args)
4753
pipe = subprocess.PIPE
4854
return subprocess.check_call(argsx, stdout=pipe, stderr=pipe)
4955

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,17 +27,17 @@ TEST(StandardValidatorTest, BadPriority) {
2727
void foo() (priority = "bad2");
2828
} (priority = "bad1")
2929
# expected-error@-3: Bad priority 'bad1'. Choose one of ["HIGH_IMPORTANT", "HIGH", "IMPORTANT", "NORMAL", "BEST_EFFORT"].
30-
# expected-warning@-4: The annotation priority is deprecated. Please use @thrift.Priority instead.
30+
# expected-error@-4: The annotation priority has been removed. Please use @thrift.Priority instead.
3131
# expected-error@-4: Bad priority 'bad2'. Choose one of ["HIGH_IMPORTANT", "HIGH", "IMPORTANT", "NORMAL", "BEST_EFFORT"].
32-
# expected-warning@-5: The annotation priority is deprecated. Please use @thrift.Priority instead.
32+
# expected-error@-5: The annotation priority has been removed. Please use @thrift.Priority instead.
3333
3434
interaction Interaction {
3535
void foo() (priority = "bad4");
3636
} (priority = "bad3")
3737
# expected-error@-3: Bad priority 'bad3'. Choose one of ["HIGH_IMPORTANT", "HIGH", "IMPORTANT", "NORMAL", "BEST_EFFORT"].
38-
# expected-warning@-4: The annotation priority is deprecated. Please use @thrift.Priority instead.
38+
# expected-error@-4: The annotation priority has been removed. Please use @thrift.Priority instead.
3939
# expected-error@-4: Bad priority 'bad4'. Choose one of ["HIGH_IMPORTANT", "HIGH", "IMPORTANT", "NORMAL", "BEST_EFFORT"].
40-
# expected-warning@-5: The annotation priority is deprecated. Please use @thrift.Priority instead.
40+
# expected-error@-5: The annotation priority has been removed. Please use @thrift.Priority instead.
4141
)");
4242
}
4343

@@ -66,7 +66,7 @@ TEST(StandardValidatorTest, UnionErrors) {
6666
# expected-warning@-1: The 'required' qualifier is deprecated and ignored by most language implementations. Leave the field unqualified instead: `req` (in `Union`).
6767
2: optional i64 opt; # expected-error: Unions cannot contain qualified fields. Remove `optional` qualifier from field `opt`.
6868
3: Struct mixin (cpp.mixin); # expected-error: Union `Union` cannot contain mixin field `mixin`.
69-
# expected-warning@-1: The annotation cpp.mixin is deprecated. Please use @thrift.Mixin instead.
69+
# expected-error@-1: The annotation cpp.mixin has been removed. Please use @thrift.Mixin instead.
7070
4: i64 non;
7171
}
7272
)");
@@ -95,7 +95,7 @@ TEST(StandardValidatorTest, ValidateExceptionMessage) {
9595
}
9696
9797
exception MyExceptionWithDuplicatedDeprecatedExceptionMessage { # expected-error: Duplicate message annotation.
98-
# expected-warning@-1: The annotation message is deprecated. Please use @thrift.ExceptionMessage instead.
98+
# expected-error@-1: The annotation message has been removed. Please use @thrift.ExceptionMessage instead.
9999
@thrift.ExceptionMessage
100100
1: string valid_message;
101101
2: string invalid_message;

third-party/thrift/src/thrift/test/SchemaTest.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ TEST_F(SchemaTest, linked) {
108108
TEST_F(SchemaTest, static_schema) {
109109
auto static_schema = schema::detail::mergeSchemas(
110110
facebook::thrift::test::schema::schema_constants::
111-
_fbthrift_schema_b303fc86e3a41852_includes());
111+
_fbthrift_schema_8b1d51c766420544_includes());
112112
const type::Program* static_program = nullptr;
113113
for (const auto& program : *static_schema.programs()) {
114114
if (program.path() == "thrift/test/schema.thrift") {
@@ -131,7 +131,7 @@ TEST_F(SchemaTest, static_schema) {
131131

132132
TEST_F(SchemaTest, merged_schema_add_after_access) {
133133
auto data = facebook::thrift::test::schema::schema_constants::
134-
_fbthrift_schema_b303fc86e3a41852();
134+
_fbthrift_schema_8b1d51c766420544();
135135

136136
BaseSchemaRegistry base;
137137
SchemaRegistry registry(base);

third-party/thrift/src/thrift/test/schema.thrift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,4 +99,5 @@ struct Annot {
9999
}
100100

101101
@Annot{val = 42}
102-
struct Annotated {} (annot_with_val = 2023, annot_without_val)
102+
@thrift.DeprecatedUnvalidatedAnnotations{items = {"annot_with_val": "2023", "annot_without_val": "1"}}
103+
struct Annotated {}

0 commit comments

Comments
 (0)