Skip to content

Commit ac8390f

Browse files
kpumukcodex
kpumuk
and
codex
authored
Enable multiline Ruby layout cops (#3717)
Client: rb Co-authored-by: OpenAI Codex (GPT-5.6) <codex@openai.com>
1 parent 9c7b492 commit ac8390f

42 files changed

Lines changed: 496 additions & 278 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎compiler/cpp/src/thrift/generate/t_rb_generator.cc‎

Lines changed: 80 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -718,10 +718,6 @@ void t_rb_generator::generate_field_defns(t_rb_ofstream& out, t_struct* tstruct)
718718
out.indent() << "FIELDS = {" << '\n';
719719
out.indent_up();
720720
for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) {
721-
if (f_iter != fields.begin()) {
722-
out << "," << '\n';
723-
}
724-
725721
// generate the field docstrings within the FIELDS constant. no real better place...
726722
generate_rdoc(out, *f_iter);
727723

@@ -732,9 +728,9 @@ void t_rb_generator::generate_field_defns(t_rb_ofstream& out, t_struct* tstruct)
732728
(*f_iter)->get_name(),
733729
(*f_iter)->get_value(),
734730
(*f_iter)->get_req() == t_field::T_OPTIONAL);
731+
out << "," << '\n';
735732
}
736733
out.indent_down();
737-
out << '\n';
738734
out.indent() << "}" << '\n' << '\n';
739735

740736
out.indent() << "def struct_fields; FIELDS; end" << '\n' << '\n';
@@ -750,50 +746,115 @@ void t_rb_generator::generate_field_data(t_rb_ofstream& out,
750746
t_const_value* field_value = nullptr,
751747
bool optional = false) {
752748
field_type = get_true_type(field_type);
749+
const bool multiline = field_value != nullptr
750+
&& (field_type->is_struct() || field_type->is_xception()
751+
|| field_type->is_map() || field_type->is_list()
752+
|| field_type->is_set());
753753

754754
// Begin this field's defn
755-
out << "{type: " << type_to_enum(field_type);
755+
out << "{";
756+
if (multiline) {
757+
out << '\n';
758+
out.indent_up();
759+
out.indent() << "type: " << type_to_enum(field_type) << "," << '\n';
760+
} else {
761+
out << "type: " << type_to_enum(field_type);
762+
}
756763

757764
if (!field_name.empty()) {
758-
out << ", name: \"" << field_name << "\"";
765+
if (multiline) {
766+
out.indent() << "name: \"" << field_name << "\"," << '\n';
767+
} else {
768+
out << ", name: \"" << field_name << "\"";
769+
}
759770
}
760771

761772
if (field_value != nullptr) {
762-
out << ", default: ";
763-
render_const_value(out, field_type, field_value);
773+
if (multiline) {
774+
out.indent() << "default: ";
775+
render_const_value(out, field_type, field_value) << "," << '\n';
776+
} else {
777+
out << ", default: ";
778+
render_const_value(out, field_type, field_value);
779+
}
764780
}
765781

766782
if (!field_type->is_base_type()) {
767783
if (field_type->is_struct() || field_type->is_xception()) {
768-
out << ", class: " << full_type_name((t_struct*)field_type);
784+
if (multiline) {
785+
out.indent() << "class: " << full_type_name((t_struct*)field_type) << "," << '\n';
786+
} else {
787+
out << ", class: " << full_type_name((t_struct*)field_type);
788+
}
769789
} else if (field_type->is_list()) {
770-
out << ", element: ";
790+
if (multiline) {
791+
out.indent() << "element: ";
792+
} else {
793+
out << ", element: ";
794+
}
771795
generate_field_data(out, ((t_list*)field_type)->get_elem_type());
796+
if (multiline) {
797+
out << "," << '\n';
798+
}
772799
} else if (field_type->is_map()) {
773-
out << ", key: ";
800+
if (multiline) {
801+
out.indent() << "key: ";
802+
} else {
803+
out << ", key: ";
804+
}
774805
generate_field_data(out, ((t_map*)field_type)->get_key_type());
775-
out << ", value: ";
806+
if (multiline) {
807+
out << "," << '\n';
808+
out.indent() << "value: ";
809+
} else {
810+
out << ", value: ";
811+
}
776812
generate_field_data(out, ((t_map*)field_type)->get_val_type());
813+
if (multiline) {
814+
out << "," << '\n';
815+
}
777816
} else if (field_type->is_set()) {
778-
out << ", element: ";
817+
if (multiline) {
818+
out.indent() << "element: ";
819+
} else {
820+
out << ", element: ";
821+
}
779822
generate_field_data(out, ((t_set*)field_type)->get_elem_type());
823+
if (multiline) {
824+
out << "," << '\n';
825+
}
780826
}
781-
} else {
782-
if (((t_base_type*)field_type)->is_binary()) {
827+
} else if (((t_base_type*)field_type)->is_binary()) {
828+
if (multiline) {
829+
out.indent() << "binary: true," << '\n';
830+
} else {
783831
out << ", binary: true";
784832
}
785833
}
786834

787835
if (optional) {
788-
out << ", optional: true";
836+
if (multiline) {
837+
out.indent() << "optional: true," << '\n';
838+
} else {
839+
out << ", optional: true";
840+
}
789841
}
790842

791843
if (field_type->is_enum()) {
792-
out << ", enum_class: " << full_type_name(field_type);
844+
if (multiline) {
845+
out.indent() << "enum_class: " << full_type_name(field_type) << "," << '\n';
846+
} else {
847+
out << ", enum_class: " << full_type_name(field_type);
848+
}
793849
}
794850

795851
// End of this field's defn
796-
out << "}";
852+
if (multiline) {
853+
out.indent_down();
854+
out.indent() << "}";
855+
} else {
856+
out << "}";
857+
}
797858
}
798859

799860
void t_rb_generator::begin_namespace(t_rb_ofstream& out, vector<std::string> modules) {

‎compiler/cpp/tests/rb/t_rb_generator_functional_tests.cc‎

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,10 +90,10 @@ TEST_CASE("t_rb_generator uses suffixed field id constants to avoid FIELDS colli
9090
const string generated_content = read_file("gen-rb/test_field_id_conflict_types.rb");
9191
REQUIRE(!generated_content.empty());
9292
REQUIRE(generated_content.find("FIELDS_FIELD_ID = 1") != string::npos);
93-
REQUIRE(generated_content.find("FIELDS_FIELD_ID => {type: ::Thrift::Types::STRING, name: \"fields\"}")
93+
REQUIRE(generated_content.find("FIELDS_FIELD_ID => {type: ::Thrift::Types::STRING, name: \"fields\"},")
9494
!= string::npos);
9595
REQUIRE(generated_content.find("FIELDS = 1") == string::npos);
96-
REQUIRE(generated_content.find("FIELDS => {type: ::Thrift::Types::STRING, name: \"fields\"}")
96+
REQUIRE(generated_content.find("FIELDS => {type: ::Thrift::Types::STRING, name: \"fields\"},")
9797
== string::npos);
9898

9999
std::remove(thrift_path.c_str());
@@ -127,3 +127,44 @@ TEST_CASE("t_rb_generator emits service arguments as a positional hash", "[funct
127127

128128
std::remove(thrift_path.c_str());
129129
}
130+
131+
TEST_CASE("t_rb_generator formats multiline Ruby literals, calls, and field metadata", "[functional]")
132+
{
133+
const string thrift_path = "test_multiline_layout.thrift";
134+
const string thrift_source =
135+
"struct Item {\n"
136+
" 1: string value\n"
137+
"}\n"
138+
"const Item ITEM = {\"value\": \"one\"}\n"
139+
"const set<i32> IDS = [1, 2]\n"
140+
"struct Defaults {\n"
141+
" 1: list<i32> values = [1, 2]\n"
142+
"}\n";
143+
144+
{
145+
std::ofstream thrift_file(thrift_path, std::ios::binary);
146+
REQUIRE(thrift_file.is_open());
147+
thrift_file << thrift_source;
148+
}
149+
150+
map<string, string> parsed_options;
151+
std::unique_ptr<t_program> program(new t_program(thrift_path, "test_multiline_layout"));
152+
parse_thrift_for_test(program.get());
153+
154+
std::unique_ptr<t_generator> gen(
155+
t_generator_registry::get_generator(program.get(), "rb", parsed_options, ""));
156+
REQUIRE(gen != nullptr);
157+
REQUIRE_NOTHROW(gen->generate_program());
158+
159+
const string constants = read_file("gen-rb/test_multiline_layout_constants.rb");
160+
REQUIRE(constants.find("ITEM = ::Item.new({\n %q\"value\" => %q\"one\",\n})")
161+
!= string::npos);
162+
REQUIRE(constants.find("IDS = Set.new([\n 1,\n 2,\n])") != string::npos);
163+
164+
const string types = read_file("gen-rb/test_multiline_layout_types.rb");
165+
REQUIRE(types.find("VALUES_FIELD_ID => {\n type: ::Thrift::Types::LIST,\n")
166+
!= string::npos);
167+
REQUIRE(types.find("default: [\n 1,\n 2,\n ],\n") != string::npos);
168+
169+
std::remove(thrift_path.c_str());
170+
}

‎lib/rb/.rubocop.yml‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,24 +9,94 @@ AllCops:
99
Exclude:
1010
- "**/vendor/**/*"
1111

12+
Layout/ArgumentAlignment:
13+
Enabled: true
14+
15+
Layout/ArrayAlignment:
16+
Enabled: true
17+
18+
Layout/BlockAlignment:
19+
Enabled: true
20+
EnforcedStyleAlignWith: start_of_block
21+
22+
Layout/CaseIndentation:
23+
Enabled: true
24+
EnforcedStyle: end
25+
26+
Layout/ClosingParenthesisIndentation:
27+
Enabled: true
28+
29+
Layout/CommentIndentation:
30+
Enabled: true
31+
1232
Layout/EmptyLines:
1333
Enabled: true
1434

1535
Layout/EmptyLinesAroundBlockBody:
1636
Enabled: true
1737

38+
Layout/ElseAlignment:
39+
Enabled: true
40+
41+
Layout/EndAlignment:
42+
Enabled: true
43+
EnforcedStyleAlignWith: variable
44+
1845
Layout/ExtraSpacing:
1946
Enabled: true
2047

48+
Layout/FirstArgumentIndentation:
49+
Enabled: true
50+
51+
Layout/FirstArrayElementIndentation:
52+
Enabled: true
53+
EnforcedStyle: consistent
54+
55+
Layout/FirstArrayElementLineBreak:
56+
Enabled: true
57+
58+
Layout/FirstHashElementIndentation:
59+
Enabled: true
60+
EnforcedStyle: consistent
61+
62+
Layout/FirstHashElementLineBreak:
63+
Enabled: true
64+
65+
Layout/FirstMethodArgumentLineBreak:
66+
Enabled: true
67+
AllowMultilineFinalElement: true
68+
69+
Layout/HashAlignment:
70+
Enabled: true
71+
EnforcedHashRocketStyle:
72+
- key
73+
- table
74+
2175
Layout/IndentationConsistency:
2276
Enabled: true
2377

78+
Layout/IndentationWidth:
79+
Enabled: true
80+
Width: 2
81+
2482
Layout/LeadingCommentSpace:
2583
Enabled: true
2684

2785
Layout/LeadingEmptyLines:
2886
Enabled: true
2987

88+
Layout/MultilineArrayBraceLayout:
89+
Enabled: true
90+
EnforcedStyle: new_line
91+
92+
Layout/MultilineHashBraceLayout:
93+
Enabled: true
94+
EnforcedStyle: new_line
95+
96+
Layout/MultilineMethodCallBraceLayout:
97+
Enabled: true
98+
EnforcedStyle: symmetrical
99+
30100
Layout/SpaceAfterComma:
31101
Enabled: true
32102

@@ -128,3 +198,15 @@ Style/StringLiterals:
128198
Style/StringLiteralsInInterpolation:
129199
Enabled: true
130200
EnforcedStyle: double_quotes
201+
202+
Style/TrailingCommaInArguments:
203+
Enabled: true
204+
EnforcedStyleForMultiline: diff_comma
205+
206+
Style/TrailingCommaInArrayLiteral:
207+
Enabled: true
208+
EnforcedStyleForMultiline: consistent_comma
209+
210+
Style/TrailingCommaInHashLiteral:
211+
Enabled: true
212+
EnforcedStyleForMultiline: comma

‎lib/rb/Rakefile‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -84,18 +84,18 @@ end
8484

8585
desc "Build the native library"
8686
task build_ext: :'gen-rb' do
87-
next if defined?(RUBY_ENGINE) && RUBY_ENGINE == "jruby"
88-
next if ENV["SKIP_BUILD_EXT"] == "1"
89-
Dir::chdir(File::dirname("ext/extconf.rb")) do
90-
unless sh "ruby #{File::basename("ext/extconf.rb")}"
91-
$stderr.puts "Failed to run extconf"
92-
break
93-
end
94-
unless sh "make"
95-
$stderr.puts "make failed"
96-
break
97-
end
87+
next if defined?(RUBY_ENGINE) && RUBY_ENGINE == "jruby"
88+
next if ENV["SKIP_BUILD_EXT"] == "1"
89+
Dir::chdir(File::dirname("ext/extconf.rb")) do
90+
unless sh "ruby #{File::basename("ext/extconf.rb")}"
91+
$stderr.puts "Failed to run extconf"
92+
break
9893
end
94+
unless sh "make"
95+
$stderr.puts "make failed"
96+
break
97+
end
98+
end
9999
end
100100

101101
desc "Run the compiler tests (requires full thrift checkout)"
@@ -137,5 +137,5 @@ end
137137
CLEAN.include [
138138
".bundle", "benchmark/gen-rb", "coverage", "ext/*.{o,bundle,so,dll}", "ext/mkmf.log",
139139
"ext/Makefile", "ext/conftest.dSYM", "ext/thrift_native.bundle.dSYM", "mkmf.log",
140-
"pkg", "spec/gen-rb", "test/debug_proto/gen-rb", "thrift-*.gem"
140+
"pkg", "spec/gen-rb", "test/debug_proto/gen-rb", "thrift-*.gem",
141141
]

‎lib/rb/benchmark/benchmark.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ def report_output
235235
blue: 34,
236236
magenta: 35,
237237
cyan: 36,
238-
white: 37
238+
white: 37,
239239
}
240240

241241
def tabulate(fmt, *labels_and_values)

‎lib/rb/lib/thrift/client.rb‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,22 +57,22 @@ def validate_message_begin(fname, mtype, rseqid, expected_name)
5757
if mtype != MessageTypes::REPLY
5858
raise ApplicationException.new(
5959
ApplicationException::INVALID_MESSAGE_TYPE,
60-
"#{expected_name} failed: invalid message type"
60+
"#{expected_name} failed: invalid message type",
6161
)
6262
end
6363

6464
if fname != expected_name
6565
raise ApplicationException.new(
6666
ApplicationException::WRONG_METHOD_NAME,
67-
"#{expected_name} failed: wrong method name"
67+
"#{expected_name} failed: wrong method name",
6868
)
6969
end
7070

7171
return if !expected_seqid.nil? && rseqid == expected_seqid
7272

7373
raise ApplicationException.new(
7474
ApplicationException::BAD_SEQUENCE_ID,
75-
"#{expected_name} failed: out of sequence response"
75+
"#{expected_name} failed: out of sequence response",
7676
)
7777
end
7878

0 commit comments

Comments
 (0)