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

[Rust] harden against name collisions while generate cleaner rust code, fix #20337 #20396

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

[Rust] harden against name collisions while generate cleaner rust code, fix #20337 #20396

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6d06003
prevent all name clashes by using a prefix for all parameters, this w…
xMAC94x Jan 2, 2025
46f1379
get rid of "local_var" prefix, now that we no longer clash with param…
xMAC94x Jan 2, 2025
b695b68
fix a typo and remove the r# generated to the paramName when we creat…
xMAC94x Jan 3, 2025
6bb4f1a
update rust samples
xMAC94x Jan 2, 2025
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: 2 additions & 0 deletions ...enapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractRustCodegen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ public abstract class AbstractRustCodegen extends DefaultCodegen implements Code

private final Logger LOGGER = LoggerFactory.getLogger(AbstractRustCodegen.class);

protected static final String VENDOR_EXTENSION_PARAM_IDENTIFIER = "x-rust-param-identifier";

protected List<String> charactersToAllow = Collections.singletonList("_");
protected Set<String> keywordsThatDoNotSupportRawIdentifiers = new HashSet<>(
Arrays.asList("super", "self", "Self", "extern", "crate"));
Expand Down
23 changes: 22 additions & 1 deletion ...openapi-generator/src/main/java/org/openapitools/codegen/languages/RustClientCodegen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,10 @@
import org.openapitools.codegen.CodegenModel;
import org.openapitools.codegen.CodegenOperation;
import org.openapitools.codegen.CodegenProperty;
import org.openapitools.codegen.CodegenParameter;
import org.openapitools.codegen.CodegenType;
import org.openapitools.codegen.SupportingFile;
import org.openapitools.codegen.VendorExtension;
import org.openapitools.codegen.meta.features.ClientModificationFeature;
import org.openapitools.codegen.meta.features.DocumentationFeature;
import org.openapitools.codegen.meta.features.GlobalFeature;
Expand Down Expand Up @@ -602,6 +604,25 @@ public void postProcessModelProperty(CodegenModel model, CodegenProperty propert
}
}

@Override
public void postProcessParameter(CodegenParameter parameter) {
super.postProcessParameter(parameter);
// in order to avoid name conflicts, we map parameters inside the functions
String in_function_identifier = "";
if (this.useSingleRequestParameter) {
in_function_identifier = "params." + parameter.paramName;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we rename in_function_identifier to inFunctionIdentifier instead to conform to java variable naming convention?

} else {
if (parameter.paramName.startsWith("r#")) {
in_function_identifier = "p_" + parameter.paramName.substring(2);
} else {
in_function_identifier = "p_" + parameter.paramName;
}
}
if (!parameter.vendorExtensions.containsKey(this.VENDOR_EXTENSION_PARAM_IDENTIFIER)) { // allow to overwrite this value
parameter.vendorExtensions.put(this.VENDOR_EXTENSION_PARAM_IDENTIFIER, in_function_identifier);
}
}

@Override
public OperationsMap postProcessOperationsWithModels(OperationsMap objs, List<ModelMap> allModels) {
OperationMap objectMap = objs.getOperations();
Expand Down Expand Up @@ -699,7 +720,7 @@ public String toDefaultValue(Schema p) {
@Override
protected ImmutableMap.Builder<String, Lambda> addMustacheLambdas() {
return super.addMustacheLambdas()
// Convert variable names to lifetime names.
// Convert variable names to lifetime names.
// Generally they are the same, but `#` is not valid in lifetime names.
// Rust uses `r#` prefix for variables that are also keywords.
.put("lifetimeName", new ReplaceAllLambda("^r#", "r_"));
Expand Down
206 changes: 100 additions & 106 deletions modules/openapi-generator/src/main/resources/rust/reqwest/api.mustache
View file
Open in desktop

Large diffs are not rendered by default.

30 changes: 13 additions & 17 deletions samples/client/others/rust/reqwest-regression-16119/src/apis/default_api.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,30 +24,26 @@ pub enum ReproError {


pub fn repro(configuration: &configuration::Configuration, ) -> Result<models::Parent, Error<ReproError>> {
let local_var_configuration = configuration;

let local_var_client = &local_var_configuration.client;
let uri_str = format!("{}/repro", configuration.base_path);
let mut req_builder = configuration.client.request(reqwest::Method::POST, &uri_str);

let local_var_uri_str = format!("{}/repro", local_var_configuration.base_path);
let mut local_var_req_builder = local_var_client.request(reqwest::Method::POST, local_var_uri_str.as_str());

if let Some(ref local_var_user_agent) = local_var_configuration.user_agent {
local_var_req_builder = local_var_req_builder.header(reqwest::header::USER_AGENT, local_var_user_agent.clone());
if let Some(ref user_agent) = configuration.user_agent {
req_builder = req_builder.header(reqwest::header::USER_AGENT, user_agent.clone());
}

let local_var_req = local_var_req_builder.build()?;
let local_var_resp = local_var_client.execute(local_var_req)?;
let req = req_builder.build()?;
let resp = configuration.client.execute(req)?;

let local_var_status = local_var_resp.status();
let status = resp.status();

if !local_var_status.is_client_error() && !local_var_status.is_server_error() {
let local_var_content = local_var_resp.text()?;
serde_json::from_str(&local_var_content).map_err(Error::from)
if !status.is_client_error() && !status.is_server_error() {
let content = resp.text()?;
serde_json::from_str(&content).map_err(Error::from)
} else {
let local_var_content = local_var_resp.text()?;
let local_var_entity: Option<ReproError> = serde_json::from_str(&local_var_content).ok();
let local_var_error = ResponseContent { status: local_var_status, content: local_var_content, entity: local_var_entity };
Err(Error::ResponseError(local_var_error))
let content = resp.text()?;
let entity: Option<ReproError> = serde_json::from_str(&content).ok();
Err(Error::ResponseError(ResponseContent { status, content, entity }))
}
}

28 changes: 13 additions & 15 deletions samples/client/others/rust/reqwest/api-with-ref-param/src/apis/default_api.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,29 +24,27 @@ pub enum DemoColorGetError {


pub async fn demo_color_get(configuration: &configuration::Configuration, color: models::Color) -> Result<(), Error<DemoColorGetError>> {
let local_var_configuration = configuration;
// add a prefix to parameters to efficiently prevent name collisions
let p_color = color;

let local_var_client = &local_var_configuration.client;
let uri_str = format!("{}/demo/{color}", configuration.base_path, color=p_color.to_string());
let mut req_builder = configuration.client.request(reqwest::Method::GET, &uri_str);

let local_var_uri_str = format!("{}/demo/{color}", local_var_configuration.base_path, color=color.to_string());
let mut local_var_req_builder = local_var_client.request(reqwest::Method::GET, local_var_uri_str.as_str());

if let Some(ref local_var_user_agent) = local_var_configuration.user_agent {
local_var_req_builder = local_var_req_builder.header(reqwest::header::USER_AGENT, local_var_user_agent.clone());
if let Some(ref user_agent) = configuration.user_agent {
req_builder = req_builder.header(reqwest::header::USER_AGENT, user_agent.clone());
}

let local_var_req = local_var_req_builder.build()?;
let local_var_resp = local_var_client.execute(local_var_req).await?;
let req = req_builder.build()?;
let resp = configuration.client.execute(req).await?;

let local_var_status = local_var_resp.status();
let status = resp.status();

if !local_var_status.is_client_error() && !local_var_status.is_server_error() {
if !status.is_client_error() && !status.is_server_error() {
Ok(())
} else {
let local_var_content = local_var_resp.text().await?;
let local_var_entity: Option<DemoColorGetError> = serde_json::from_str(&local_var_content).ok();
let local_var_error = ResponseContent { status: local_var_status, content: local_var_content, entity: local_var_entity };
Err(Error::ResponseError(local_var_error))
let content = resp.text().await?;
let entity: Option<DemoColorGetError> = serde_json::from_str(&content).ok();
Err(Error::ResponseError(ResponseContent { status, content, entity }))
}
}

60 changes: 27 additions & 33 deletions samples/client/others/rust/reqwest/composed-oneof/src/apis/default_api.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,58 +31,52 @@ pub enum GetStateError {


pub fn create_state(configuration: &configuration::Configuration, create_state_request: models::CreateStateRequest) -> Result<(), Error<CreateStateError>> {
let local_var_configuration = configuration;
// add a prefix to parameters to efficiently prevent name collisions
let p_create_state_request = create_state_request;

let local_var_client = &local_var_configuration.client;
let uri_str = format!("{}/state", configuration.base_path);
let mut req_builder = configuration.client.request(reqwest::Method::POST, &uri_str);

let local_var_uri_str = format!("{}/state", local_var_configuration.base_path);
let mut local_var_req_builder = local_var_client.request(reqwest::Method::POST, local_var_uri_str.as_str());

if let Some(ref local_var_user_agent) = local_var_configuration.user_agent {
local_var_req_builder = local_var_req_builder.header(reqwest::header::USER_AGENT, local_var_user_agent.clone());
if let Some(ref user_agent) = configuration.user_agent {
req_builder = req_builder.header(reqwest::header::USER_AGENT, user_agent.clone());
}
local_var_req_builder = local_var_req_builder.json(&create_state_request);
req_builder = req_builder.json(&p_create_state_request);

let local_var_req = local_var_req_builder.build()?;
let local_var_resp = local_var_client.execute(local_var_req)?;
let req = req_builder.build()?;
let resp = configuration.client.execute(req)?;

let local_var_status = local_var_resp.status();
let status = resp.status();

if !local_var_status.is_client_error() && !local_var_status.is_server_error() {
if !status.is_client_error() && !status.is_server_error() {
Ok(())
} else {
let local_var_content = local_var_resp.text()?;
let local_var_entity: Option<CreateStateError> = serde_json::from_str(&local_var_content).ok();
let local_var_error = ResponseContent { status: local_var_status, content: local_var_content, entity: local_var_entity };
Err(Error::ResponseError(local_var_error))
let content = resp.text()?;
let entity: Option<CreateStateError> = serde_json::from_str(&content).ok();
Err(Error::ResponseError(ResponseContent { status, content, entity }))
}
}

pub fn get_state(configuration: &configuration::Configuration, ) -> Result<models::GetState200Response, Error<GetStateError>> {
let local_var_configuration = configuration;

let local_var_client = &local_var_configuration.client;

let local_var_uri_str = format!("{}/state", local_var_configuration.base_path);
let mut local_var_req_builder = local_var_client.request(reqwest::Method::GET, local_var_uri_str.as_str());
let uri_str = format!("{}/state", configuration.base_path);
let mut req_builder = configuration.client.request(reqwest::Method::GET, &uri_str);

if let Some(ref local_var_user_agent) = local_var_configuration.user_agent {
local_var_req_builder = local_var_req_builder.header(reqwest::header::USER_AGENT, local_var_user_agent.clone());
if let Some(ref user_agent) = configuration.user_agent {
req_builder = req_builder.header(reqwest::header::USER_AGENT, user_agent.clone());
}

let local_var_req = local_var_req_builder.build()?;
let local_var_resp = local_var_client.execute(local_var_req)?;
let req = req_builder.build()?;
let resp = configuration.client.execute(req)?;

let local_var_status = local_var_resp.status();
let status = resp.status();

if !local_var_status.is_client_error() && !local_var_status.is_server_error() {
let local_var_content = local_var_resp.text()?;
serde_json::from_str(&local_var_content).map_err(Error::from)
if !status.is_client_error() && !status.is_server_error() {
let content = resp.text()?;
serde_json::from_str(&content).map_err(Error::from)
Copy link
Member

@wing328 wing328 Jan 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about adding a fake endpoint with the parameter content to the test spec to ensure the change works as expected?

test spec: modules/openapi-generator/src/test/resources/3_0/rust/petstore.yaml

an example of fake endpoint: https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/resources/3_0/rust/petstore.yaml#L585

} else {
let local_var_content = local_var_resp.text()?;
let local_var_entity: Option<GetStateError> = serde_json::from_str(&local_var_content).ok();
let local_var_error = ResponseContent { status: local_var_status, content: local_var_content, entity: local_var_entity };
Err(Error::ResponseError(local_var_error))
let content = resp.text()?;
let entity: Option<GetStateError> = serde_json::from_str(&content).ok();
Err(Error::ResponseError(ResponseContent { status, content, entity }))
}
}

30 changes: 13 additions & 17 deletions samples/client/others/rust/reqwest/emptyObject/src/apis/default_api.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,30 +24,26 @@ pub enum EndpointGetError {


pub fn endpoint_get(configuration: &configuration::Configuration, ) -> Result<models::EmptyObject, Error<EndpointGetError>> {
let local_var_configuration = configuration;

let local_var_client = &local_var_configuration.client;
let uri_str = format!("{}/endpoint", configuration.base_path);
let mut req_builder = configuration.client.request(reqwest::Method::GET, &uri_str);

let local_var_uri_str = format!("{}/endpoint", local_var_configuration.base_path);
let mut local_var_req_builder = local_var_client.request(reqwest::Method::GET, local_var_uri_str.as_str());

if let Some(ref local_var_user_agent) = local_var_configuration.user_agent {
local_var_req_builder = local_var_req_builder.header(reqwest::header::USER_AGENT, local_var_user_agent.clone());
if let Some(ref user_agent) = configuration.user_agent {
req_builder = req_builder.header(reqwest::header::USER_AGENT, user_agent.clone());
}

let local_var_req = local_var_req_builder.build()?;
let local_var_resp = local_var_client.execute(local_var_req)?;
let req = req_builder.build()?;
let resp = configuration.client.execute(req)?;

let local_var_status = local_var_resp.status();
let status = resp.status();

if !local_var_status.is_client_error() && !local_var_status.is_server_error() {
let local_var_content = local_var_resp.text()?;
serde_json::from_str(&local_var_content).map_err(Error::from)
if !status.is_client_error() && !status.is_server_error() {
let content = resp.text()?;
serde_json::from_str(&content).map_err(Error::from)
} else {
let local_var_content = local_var_resp.text()?;
let local_var_entity: Option<EndpointGetError> = serde_json::from_str(&local_var_content).ok();
let local_var_error = ResponseContent { status: local_var_status, content: local_var_content, entity: local_var_entity };
Err(Error::ResponseError(local_var_error))
let content = resp.text()?;
let entity: Option<EndpointGetError> = serde_json::from_str(&content).ok();
Err(Error::ResponseError(ResponseContent { status, content, entity }))
}
}

60 changes: 27 additions & 33 deletions samples/client/others/rust/reqwest/oneOf-array-map/src/apis/default_api.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,58 +31,52 @@ pub enum TestError {


pub fn root_get(configuration: &configuration::Configuration, ) -> Result<models::Fruit, Error<RootGetError>> {
let local_var_configuration = configuration;

let local_var_client = &local_var_configuration.client;
let uri_str = format!("{}/", configuration.base_path);
let mut req_builder = configuration.client.request(reqwest::Method::GET, &uri_str);

let local_var_uri_str = format!("{}/", local_var_configuration.base_path);
let mut local_var_req_builder = local_var_client.request(reqwest::Method::GET, local_var_uri_str.as_str());

if let Some(ref local_var_user_agent) = local_var_configuration.user_agent {
local_var_req_builder = local_var_req_builder.header(reqwest::header::USER_AGENT, local_var_user_agent.clone());
if let Some(ref user_agent) = configuration.user_agent {
req_builder = req_builder.header(reqwest::header::USER_AGENT, user_agent.clone());
}

let local_var_req = local_var_req_builder.build()?;
let local_var_resp = local_var_client.execute(local_var_req)?;
let req = req_builder.build()?;
let resp = configuration.client.execute(req)?;

let local_var_status = local_var_resp.status();
let status = resp.status();

if !local_var_status.is_client_error() && !local_var_status.is_server_error() {
let local_var_content = local_var_resp.text()?;
serde_json::from_str(&local_var_content).map_err(Error::from)
if !status.is_client_error() && !status.is_server_error() {
let content = resp.text()?;
serde_json::from_str(&content).map_err(Error::from)
} else {
let local_var_content = local_var_resp.text()?;
let local_var_entity: Option<RootGetError> = serde_json::from_str(&local_var_content).ok();
let local_var_error = ResponseContent { status: local_var_status, content: local_var_content, entity: local_var_entity };
Err(Error::ResponseError(local_var_error))
let content = resp.text()?;
let entity: Option<RootGetError> = serde_json::from_str(&content).ok();
Err(Error::ResponseError(ResponseContent { status, content, entity }))
}
}

pub fn test(configuration: &configuration::Configuration, body: Option<serde_json::Value>) -> Result<(), Error<TestError>> {
let local_var_configuration = configuration;

let local_var_client = &local_var_configuration.client;
// add a prefix to parameters to efficiently prevent name collisions
let p_body = body;

let local_var_uri_str = format!("{}/", local_var_configuration.base_path);
let mut local_var_req_builder = local_var_client.request(reqwest::Method::PUT, local_var_uri_str.as_str());
let uri_str = format!("{}/", configuration.base_path);
let mut req_builder = configuration.client.request(reqwest::Method::PUT, &uri_str);

if let Some(ref local_var_user_agent) = local_var_configuration.user_agent {
local_var_req_builder = local_var_req_builder.header(reqwest::header::USER_AGENT, local_var_user_agent.clone());
if let Some(ref user_agent) = configuration.user_agent {
req_builder = req_builder.header(reqwest::header::USER_AGENT, user_agent.clone());
}
local_var_req_builder = local_var_req_builder.json(&body);
req_builder = req_builder.json(&p_body);

let local_var_req = local_var_req_builder.build()?;
let local_var_resp = local_var_client.execute(local_var_req)?;
let req = req_builder.build()?;
let resp = configuration.client.execute(req)?;

let local_var_status = local_var_resp.status();
let status = resp.status();

if !local_var_status.is_client_error() && !local_var_status.is_server_error() {
if !status.is_client_error() && !status.is_server_error() {
Ok(())
} else {
let local_var_content = local_var_resp.text()?;
let local_var_entity: Option<TestError> = serde_json::from_str(&local_var_content).ok();
let local_var_error = ResponseContent { status: local_var_status, content: local_var_content, entity: local_var_entity };
Err(Error::ResponseError(local_var_error))
let content = resp.text()?;
let entity: Option<TestError> = serde_json::from_str(&content).ok();
Err(Error::ResponseError(ResponseContent { status, content, entity }))
}
}

Loading