OpenAPITools / openapi-generator

OpenAPI Generator allows generation of API client libraries (SDK generation), server stubs, documentation and configuration automatically given an OpenAPI Spec (v2, v3)
https://openapi-generator.tech
Apache License 2.0
22k stars 6.6k forks source link

[REQ][Rust] Cleanup code-generation, remove `local_var_` prefix #20198

Open xMAC94x opened 1 day ago

xMAC94x commented 1 day ago

Is your feature request related to a problem? Please describe.

I'm always frustrated when I try to read the api code and the only thing i see is local_var_ everywhere, makes it really hard to focus on the code.

Describe the solution you'd like

get rid of local_var_

Additional context

The majority of languages supported don't have local_var_. So it seems like most of this coding is 3-4 years old, I found one specific issue linked to this: #10421 , It's kinda hard to reproduce whats the original error, because the referenced logs are long gone. but from they change: #10419 I guess that have a parameter called configuration and then wanted to generate the source code with the flag useSingleRequestParameter .

That results in a problem when unboxing the single parameter here: https://github.com/OpenAPITools/openapi-generator/blob/47665aaa97cb1975c5382165f9048eeb9918034f/modules/openapi-generator/src/main/resources/rust/reqwest/api.mustache#L88

The solution kinda works, because the internal variable is no longer called configuration but now local_var_configuration. However this makes it very hard to read and will fails as soon as someone tries to insert a parameter local_var_configuration.

Knowing that most of the code is ~3 years old I can imagine the borrow checker having problems back then which needed this unboxing. However we could prob completely avoid it, and thus avoid any problems due to parameter names.

Note: as you have seen, this issue could also be interpreted as [BUG]. For me its a feature, because I want to read the code, but if you want me to rephrase it, just tell me and I will open another ticket.

Example

pub async fn get_auth_token(configuration: &configuration::Configuration, username: &str, get_auth_token_request: models::GetAuthTokenRequest) -> Result<String, Error<GetAuthTokenError>> {
    let local_var_configuration = configuration;

    let local_var_client = &local_var_configuration.client;

    let local_var_uri_str = format!("{}/auth/v1/{username}/token", local_var_configuration.base_path, username=crate::apis::urlencode(username));
    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());
    }
    local_var_req_builder = local_var_req_builder.json(&get_auth_token_request);

    let local_var_req = local_var_req_builder.build()?;
    let local_var_resp = local_var_client.execute(local_var_req).await?;

    let local_var_status = local_var_resp.status();
    let local_var_content = local_var_resp.text().await?;

    if !local_var_status.is_client_error() && !local_var_status.is_server_error() {
        serde_json::from_str(&local_var_content).map_err(Error::from)
    } else {
        let local_var_entity: Option<GetAuthTokenError> = 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))
    }
}

vs


pub async fn get_auth_token(configuration: &configuration::Configuration, username: &str, get_auth_token_request: models::GetAuthTokenRequest) -> Result<String, Error<GetAuthTokenError>> {

    let uri_str = format!("{}/auth/v1/{username}/token", configuration.base_path, username=crate::apis::urlencode(username));
    let mut req_builder = configuration.client.request(reqwest::Method::POST, uri_str.as_str());

    if let Some(ref user_agent) = configuration.user_agent {
        req_builder = req_builder.header(reqwest::header::USER_AGENT, user_agent.clone());
    }
    req_builder = req_builder.json(&get_auth_token_request);

    let req = req_builder.build()?;
    let resp = configuration.client.execute(req).await?;

    let status = resp.status();
    let content = resp.text().await?;

    if !status.is_client_error() && !status.is_server_error() {
        serde_json::from_str(&content).map_err(Error::from)
    } else {
        let entity: Option<GetAuthTokenError> = serde_json::from_str(&content).ok();
        Err(Error::ResponseError(ResponseContent { status, content, entity }))
    }
}
``
xMAC94x commented 1 day ago

I have a local branch with a solution, however it slaps a lot of

{{#vendorExtensions.x-group-parameters}}
params.{{paramName}}
{{/vendorExtensions.x-group-parameters}}
{{^vendorExtensions.x-group-parameters}}
{{paramName}}
{{/vendorExtensions.x-group-parameters}}

all over the place.

acording to some mustache documentation, stuff like this should prob handled on the logic side. E.g. we could add a paramNameWithPrefix somewhere and just use that. however in the java code this seems to be handled in the CodegenParameter.java file, And i dont want to touch the base class for all languages in my PR, what do you recommend ?

Any help here for a good place to add that parameter ? ideally so that it only affects rust generation ? Thanks for the help