Naively change the case of method identifiers to comply with PSR-1

[vodik]

I'd be surprised if the patch will be taken in this form, but I'm willing to put the effort in to getting this in should there be interest.

Rust loves using snake_case for functions, but to comply with PSR-1, methods should be in camelCase. This patch tries to change the case of methods generated through the php_impl macro.

As for precedence for changing the case of identifiers via macros, both juniper and async-graphql do this.

[vodik]

I didn't realize that PHP itself isn't consistent with the casing of magic methods (__set_state is an exception) so I figured it's probably safest to just hardcode in a list.

[davidcole1340]

I think this could be useful, but there should probably be a way to opt-in or opt-out using an attribute. I can help you with that if you want (it's basically the same as the rest of the impl attributes, see here).

Let me know what you guys think.

[vodik]

@davidcole1340 I'll give it a shot. Its a good idea.

[vodik]

Added an attribute that allows for some limited control over casing. Only implemented camelCase and snake_case:

#[php_impl]
impl Test {
    #[rename("snake_case")]
    pub fn get_first_name(&self) -> String {
        // ...
    }

    #[rename("camelCase")]
    pub fn get_last_name(&self) -> String {
        // ...
    }
}

Looks like ident_name can't properly convert camel case identifiers into snake case though. I don't know how much of a problem this is in practice. I picked this library only because its already pulled in by another dep, but there are other implementations.

If this is acceptable, let me know and I'll put a blurb in the docs as well.

[Lctrs]

I honestly don't like the naming. A new user will think that their method will be renamed to literally snake_case or camelCase at first glance.

Also, I would have preferred a global attribute for that. I doubt that users will want to switch case to only a few methods.

[vodik]

Honestly, I think I agree. Not sure how a global attribute would look as I don't think I've come across a library where I've used them.

But as I was putting this together I was thinking just having rename be a bit more straightforward could be better: #[rename("my_identifier_of_choice")] instead of using it to control how to mangle the identifier.

[davidcole1340]

Yeah, rename with an identifier is good. Could also do #[rename(snake)] and #[rename(camel)]? What do you think?

[vodik]

I kept it with "snake_case" and "camcelCase" to keep it consistent with other rust packages (e.g. serde, async-graphql).

That said, as much as I've just appealed to how others do this, they don't do it with a rename attribute per say. I think what could work:

• Put a rename_methods parameter on php_impl: #[php_impl(rename_methods="snake_case")]
• Change rename to directly take an identifier and allow directly overridding the exported name of a function.

[davidcole1340]

Thanks!