Skip to content

Commit 9555ddb

Browse files
aristidispmeta-codesync[bot]
authored andcommitted
Remove duplicate namespace directives codemod
Summary: Duplicate `namespace` directives for the same language in Thrift IDL files are being deprecated. This codemod removes duplicate namespace directives, keeping only the first one for each language. As a safety measure, if the duplicate namespace line has any trailing content after the namespace node's source range (before end-of-line), the codemod skips the file and notifies the user (e.g. comments like `namespace cpp2 Foo // keep this`). Files created/modified: - `remove_duplicate_namespaces.cc`: Codemod binary using `program_.all_namespace_nodes()` to find duplicates. - `remove_duplicate_namespaces_test.py`: Python unittest with 5 test cases (no duplicates, basic removal, multiple languages, trailing content safety check, trailing semicolon). - `BUCK`: Added `cpp_binary` and `python_unittest` targets. Reviewed By: Mizuchi Differential Revision: D94761544 fbshipit-source-id: e3bdb92aa8af40935e3017829848f8ce031a4f75
1 parent a1a273b commit 9555ddb

File tree

3 files changed

+304
-0
lines changed

3 files changed

+304
-0
lines changed

third-party/thrift/src/thrift/compiler/ast/t_program.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,9 @@ class t_program : public t_named {
264264
* directives with the same language, only the first one is returned by
265265
* `namespaces()`.
266266
*
267+
* The returned nodes are guaranteed to be in source order (i.e. the order
268+
* in which the `namespace` directives appear in the IDL file).
269+
*
267270
* As of February 2026, work is ongoing to forbid duplicate namespaces from
268271
* Thrift IDL, so eventually the distinction above should become irrelevant.
269272
*/
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
#include <set>
18+
#include <string>
19+
#include <fmt/core.h>
20+
#include <thrift/compiler/ast/t_program.h>
21+
#include <thrift/compiler/ast/t_program_bundle.h>
22+
#include <thrift/compiler/codemod/codemod.h>
23+
#include <thrift/compiler/codemod/file_manager.h>
24+
25+
using apache::thrift::compiler::source_manager;
26+
using apache::thrift::compiler::t_program_bundle;
27+
28+
namespace apache::thrift::compiler {
29+
namespace {
30+
31+
class RemoveDuplicateNamespaces final {
32+
public:
33+
RemoveDuplicateNamespaces(source_manager& src_manager, t_program& program)
34+
: source_manager_(src_manager),
35+
program_(program),
36+
file_manager_(source_manager_, program) {}
37+
38+
void run() {
39+
const std::string_view old_content = file_manager_.old_content();
40+
std::set<std::string> seen_languages;
41+
bool has_duplicates = false;
42+
43+
for (const t_namespace* ns : program_.all_namespace_nodes()) {
44+
const std::string& language = ns->language();
45+
46+
if (seen_languages.insert(language).second) {
47+
// First occurrence of this language — keep it.
48+
continue;
49+
}
50+
51+
// Duplicate found. Compute the line range to remove.
52+
const size_t begin_offset =
53+
file_manager_.to_offset(ns->src_range().begin);
54+
const size_t src_end_offset =
55+
file_manager_.to_offset(ns->src_range().end);
56+
const size_t newline_offset = old_content.find('\n', src_end_offset);
57+
58+
// Check for any trailing content after the namespace node
59+
// (e.g. inline comments) before the end of the line.
60+
if (newline_offset != std::string_view::npos &&
61+
newline_offset != src_end_offset) {
62+
// Compute a 1-based line number for the warning.
63+
size_t line_number = 1;
64+
for (size_t i = 0; i < begin_offset && i < old_content.size(); ++i) {
65+
if (old_content[i] == '\n') {
66+
++line_number;
67+
}
68+
}
69+
fmt::print(
70+
stderr,
71+
"{}:{}: warning: skipping file due to trailing content on "
72+
"duplicate namespace '{}'\n",
73+
program_.path(),
74+
line_number,
75+
language);
76+
return;
77+
}
78+
79+
// Remove the entire line (including the trailing newline).
80+
const size_t end_offset = newline_offset == std::string_view::npos
81+
? old_content.size()
82+
: newline_offset + 1;
83+
file_manager_.add({begin_offset, end_offset, ""});
84+
has_duplicates = true;
85+
}
86+
87+
if (has_duplicates) {
88+
file_manager_.apply_replacements();
89+
}
90+
}
91+
92+
private:
93+
source_manager& source_manager_;
94+
t_program& program_;
95+
codemod::file_manager file_manager_;
96+
};
97+
98+
} // namespace
99+
} // namespace apache::thrift::compiler
100+
101+
int main(int argc, char** argv) {
102+
return apache::thrift::compiler::run_codemod(
103+
argc, argv, [](source_manager& sm, t_program_bundle& pb) {
104+
apache::thrift::compiler::RemoveDuplicateNamespaces(
105+
sm, *pb.root_program())
106+
.run();
107+
});
108+
}
Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
# Copyright (c) Meta Platforms, Inc. and affiliates.
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
# pyre-unsafe
16+
17+
import os
18+
import shutil
19+
import subprocess
20+
import tempfile
21+
import textwrap
22+
import unittest
23+
24+
import pkg_resources
25+
from xplat.thrift.compiler.codemod.test_utils import read_file, run_binary, write_file
26+
27+
28+
class RemoveDuplicateNamespacesTest(unittest.TestCase):
29+
def setUp(self):
30+
tmp = tempfile.mkdtemp()
31+
self.addCleanup(shutil.rmtree, tmp, True)
32+
self.tmp = tmp
33+
self.addCleanup(os.chdir, os.getcwd())
34+
os.chdir(self.tmp)
35+
self.maxDiff = None
36+
37+
def test_no_duplicates(self):
38+
write_file(
39+
"foo.thrift",
40+
textwrap.dedent(
41+
"""\
42+
namespace cpp2 foo
43+
namespace python bar
44+
45+
struct S {
46+
1: i32 x;
47+
}
48+
"""
49+
),
50+
)
51+
52+
binary = pkg_resources.resource_filename(__name__, "codemod")
53+
run_binary(binary, "foo.thrift")
54+
55+
self.assertEqual(
56+
read_file("foo.thrift"),
57+
textwrap.dedent(
58+
"""\
59+
namespace cpp2 foo
60+
namespace python bar
61+
62+
struct S {
63+
1: i32 x;
64+
}
65+
"""
66+
),
67+
)
68+
69+
def test_basic_duplicate_removal(self):
70+
write_file(
71+
"foo.thrift",
72+
textwrap.dedent(
73+
"""\
74+
namespace cpp2 first
75+
namespace python bar
76+
namespace cpp2 second
77+
78+
struct S {
79+
1: i32 x;
80+
}
81+
"""
82+
),
83+
)
84+
85+
binary = pkg_resources.resource_filename(__name__, "codemod")
86+
run_binary(binary, "foo.thrift")
87+
88+
self.assertEqual(
89+
read_file("foo.thrift"),
90+
textwrap.dedent(
91+
"""\
92+
namespace cpp2 first
93+
namespace python bar
94+
95+
struct S {
96+
1: i32 x;
97+
}
98+
"""
99+
),
100+
)
101+
102+
def test_multiple_languages_with_duplicates(self):
103+
write_file(
104+
"foo.thrift",
105+
textwrap.dedent(
106+
"""\
107+
namespace cpp2 a
108+
namespace python b
109+
namespace cpp2 c
110+
namespace python d
111+
112+
struct S {
113+
1: i32 x;
114+
}
115+
"""
116+
),
117+
)
118+
119+
binary = pkg_resources.resource_filename(__name__, "codemod")
120+
run_binary(binary, "foo.thrift")
121+
122+
self.assertEqual(
123+
read_file("foo.thrift"),
124+
textwrap.dedent(
125+
"""\
126+
namespace cpp2 a
127+
namespace python b
128+
129+
struct S {
130+
1: i32 x;
131+
}
132+
"""
133+
),
134+
)
135+
136+
def test_trailing_content_on_duplicate_skips_file(self):
137+
original = textwrap.dedent(
138+
"""\
139+
namespace cpp2 first
140+
namespace cpp2 second // keep this
141+
142+
struct S {
143+
1: i32 x;
144+
}
145+
"""
146+
)
147+
write_file("foo.thrift", original)
148+
149+
binary = pkg_resources.resource_filename(__name__, "codemod")
150+
result = subprocess.run(
151+
[binary, "foo.thrift"],
152+
capture_output=True,
153+
text=True,
154+
)
155+
156+
# File should be unchanged.
157+
self.assertEqual(read_file("foo.thrift"), original)
158+
# Warning should be emitted on stderr.
159+
self.assertIn("warning", result.stderr)
160+
self.assertIn("trailing content", result.stderr)
161+
162+
def test_duplicate_with_trailing_semicolon(self):
163+
write_file(
164+
"foo.thrift",
165+
textwrap.dedent(
166+
"""\
167+
namespace cpp2 first;
168+
namespace python bar
169+
namespace cpp2 second;
170+
171+
struct S {
172+
1: i32 x;
173+
}
174+
"""
175+
),
176+
)
177+
178+
binary = pkg_resources.resource_filename(__name__, "codemod")
179+
run_binary(binary, "foo.thrift")
180+
181+
self.assertEqual(
182+
read_file("foo.thrift"),
183+
textwrap.dedent(
184+
"""\
185+
namespace cpp2 first;
186+
namespace python bar
187+
188+
struct S {
189+
1: i32 x;
190+
}
191+
"""
192+
),
193+
)

0 commit comments

Comments
 (0)