Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: use var_{id} as local variable name to avoid the name conflict with tuple struct #266

Merged
merged 2 commits into from
Sep 18, 2024
Merged

fix: use var_{id} as local variable name to avoid the name conflict with tuple struct #266

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c985662
fix: use var_{id} as local variable name to avoid the name conflict w…
Ggiggle Sep 18, 2024
d823c9a
fix: use rust_name of field in error hint
Ggiggle Sep 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pilota-build/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "pilota-build"
version = "0.11.16"
version = "0.11.17"
edition = "2021"
description = "Compile thrift and protobuf idl into rust code at compile-time."
documentation = "https://docs.rs/pilota-build"
Expand Down
54 changes: 29 additions & 25 deletions pilota-build/src/codegen/thrift/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,26 +186,6 @@ impl ThriftBackend {
keep: bool,
is_arg: bool,
) -> String {
let mut fields = s.fields.iter().map(|f| self.rust_name(f.did)).join(",");

if keep {
if !fields.is_empty() {
fields.push_str(", ");
}
if !helper.is_async {
fields.push_str("_unknown_fields");
} else {
fields.push_str("_unknown_fields: ::pilota::LinkedBytes::new()");
}
}

let required_without_default_fields = s
.fields
.iter()
.filter(|f| !f.is_optional() && self.default_val(f).is_none())
.map(|f| self.rust_name(f.did))
.collect_vec();

let def_fields_num = if keep && is_arg && !helper.is_async {
"let mut __pilota_fields_num = 0;"
} else {
Expand All @@ -216,7 +196,7 @@ impl ThriftBackend {
.fields
.iter()
.map(|f| {
let field_name = self.rust_name(f.did);
let field_name = f.local_var_name();
let mut v = "None".into();

if let Some((default, is_const)) = self.cx.default_val(f) {
Expand Down Expand Up @@ -245,7 +225,7 @@ impl ThriftBackend {
.fields
.iter()
.filter_map(|f| {
let field_name = self.rust_name(f.did);
let field_name = f.local_var_name();
if let Some((default, is_const)) = self.cx.default_val(f) {
if !is_const {
if f.is_optional() {
Expand All @@ -272,11 +252,18 @@ impl ThriftBackend {
let read_struct_end = helper.codegen_read_struct_end();
let read_fields = self.codegen_decode_fields(helper, &s.fields, keep, is_arg);

let required_without_default_fields = s
.fields
.iter()
.filter(|f| !f.is_optional() && self.default_val(f).is_none())
.map(|f| (self.rust_name(f.did), f.local_var_name()))
.collect_vec();

let verify_required_fields = required_without_default_fields
.iter()
.map(|s| {
.map(|(s, v)| {
format!(
r#"let Some({s}) = {s} else {{
r#"let Some({v}) = {v} else {{
return ::std::result::Result::Err(
::pilota::thrift::new_protocol_exception(
::pilota::thrift::ProtocolExceptionKind::InvalidData,
Expand Down Expand Up @@ -306,6 +293,23 @@ impl ThriftBackend {

let format_msg = format!("decode struct `{}` field(#{{}}) failed", s.name);

let mut fields = s
.fields
.iter()
.map(|f| format!("{}: {}", self.rust_name(f.did), f.local_var_name()))
.join(",");

if keep {
if !fields.is_empty() {
fields.push_str(", ");
}
if !helper.is_async {
fields.push_str("_unknown_fields");
} else {
fields.push_str("_unknown_fields: ::pilota::LinkedBytes::new()");
}
}

format! {
r#"
{def_fields_num}
Expand Down Expand Up @@ -364,7 +368,7 @@ impl ThriftBackend {
let match_fields = fields
.iter()
.map(|f| {
let field_ident = self.rust_name(f.did);
let field_ident = f.local_var_name();
let ttype = self.ttype(&f.ty);
let mut read_field = self.codegen_decode_ty(helper, &f.ty);
let field_id = f.id as i16;
Expand Down
4 changes: 4 additions & 0 deletions pilota-build/src/middle/rir.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ impl Field {
pub fn is_optional(&self) -> bool {
matches!(self.kind, FieldKind::Optional)
}

pub fn local_var_name(&self) -> String {
format!("var_{}", self.id)
}
}

#[derive(Clone, Debug, PartialEq, Eq, Hash)]
Expand Down
12 changes: 6 additions & 6 deletions pilota-build/test_data/must_gen_items.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub mod must_gen_items {
#[allow(unused_imports)]
use ::pilota::{thrift::TLengthProtocolExt, Buf};

let mut a = None;
let mut var_1 = None;

let mut __pilota_decoding_field_id = None;

Expand All @@ -47,7 +47,7 @@ pub mod must_gen_items {
__pilota_decoding_field_id = field_ident.id;
match field_ident.id {
Some(1) if field_ident.field_type == ::pilota::thrift::TType::I32 => {
a = Some(__protocol.read_i32()?);
var_1 = Some(__protocol.read_i32()?);
}
_ => {
__protocol.skip(field_ident.field_type)?;
Expand All @@ -69,7 +69,7 @@ pub mod must_gen_items {
};
__protocol.read_struct_end()?;

let data = Self { a };
let data = Self { a: var_1 };
::std::result::Result::Ok(data)
}

Expand All @@ -84,7 +84,7 @@ pub mod must_gen_items {
>,
> {
::std::boxed::Box::pin(async move {
let mut a = None;
let mut var_1 = None;

let mut __pilota_decoding_field_id = None;

Expand All @@ -101,7 +101,7 @@ pub mod must_gen_items {
Some(1)
if field_ident.field_type == ::pilota::thrift::TType::I32 =>
{
a = Some(__protocol.read_i32().await?);
var_1 = Some(__protocol.read_i32().await?);
}
_ => {
__protocol.skip(field_ident.field_type).await?;
Expand All @@ -124,7 +124,7 @@ pub mod must_gen_items {
};
__protocol.read_struct_end().await?;

let data = Self { a };
let data = Self { a: var_1 };
::std::result::Result::Ok(data)
})
}
Expand Down
28 changes: 14 additions & 14 deletions pilota-build/test_data/plugin/serde.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ pub mod serde {
#[allow(unused_imports)]
use ::pilota::{thrift::TLengthProtocolExt, Buf};

let mut a = None;
let mut b = None;
let mut var_1 = None;
let mut var_2 = None;

let mut __pilota_decoding_field_id = None;

Expand All @@ -63,10 +63,10 @@ pub mod serde {
Some(1)
if field_ident.field_type == ::pilota::thrift::TType::Binary =>
{
a = Some(__protocol.read_faststr()?);
var_1 = Some(__protocol.read_faststr()?);
}
Some(2) if field_ident.field_type == ::pilota::thrift::TType::I32 => {
b = Some(__protocol.read_i32()?);
var_2 = Some(__protocol.read_i32()?);
}
_ => {
__protocol.skip(field_ident.field_type)?;
Expand All @@ -88,20 +88,20 @@ pub mod serde {
};
__protocol.read_struct_end()?;

let Some(a) = a else {
let Some(var_1) = var_1 else {
return ::std::result::Result::Err(::pilota::thrift::new_protocol_exception(
::pilota::thrift::ProtocolExceptionKind::InvalidData,
"field a is required".to_string(),
));
};
let Some(b) = b else {
let Some(var_2) = var_2 else {
return ::std::result::Result::Err(::pilota::thrift::new_protocol_exception(
::pilota::thrift::ProtocolExceptionKind::InvalidData,
"field b is required".to_string(),
));
};

let data = Self { a, b };
let data = Self { a: var_1, b: var_2 };
::std::result::Result::Ok(data)
}

Expand All @@ -116,8 +116,8 @@ pub mod serde {
>,
> {
::std::boxed::Box::pin(async move {
let mut a = None;
let mut b = None;
let mut var_1 = None;
let mut var_2 = None;

let mut __pilota_decoding_field_id = None;

Expand All @@ -135,12 +135,12 @@ pub mod serde {
if field_ident.field_type
== ::pilota::thrift::TType::Binary =>
{
a = Some(__protocol.read_faststr().await?);
var_1 = Some(__protocol.read_faststr().await?);
}
Some(2)
if field_ident.field_type == ::pilota::thrift::TType::I32 =>
{
b = Some(__protocol.read_i32().await?);
var_2 = Some(__protocol.read_i32().await?);
}
_ => {
__protocol.skip(field_ident.field_type).await?;
Expand All @@ -163,15 +163,15 @@ pub mod serde {
};
__protocol.read_struct_end().await?;

let Some(a) = a else {
let Some(var_1) = var_1 else {
return ::std::result::Result::Err(
::pilota::thrift::new_protocol_exception(
::pilota::thrift::ProtocolExceptionKind::InvalidData,
"field a is required".to_string(),
),
);
};
let Some(b) = b else {
let Some(var_2) = var_2 else {
return ::std::result::Result::Err(
::pilota::thrift::new_protocol_exception(
::pilota::thrift::ProtocolExceptionKind::InvalidData,
Expand All @@ -180,7 +180,7 @@ pub mod serde {
);
};

let data = Self { a, b };
let data = Self { a: var_1, b: var_2 };
::std::result::Result::Ok(data)
})
}
Expand Down
Loading
Loading