Build warnings in proto defs

[hntd187]

Bug Report

Version

0.1.1

Platform

All (happens on windows, osx and linux)

Description

I get build warnings like the following

warning: type `raft_requestSvc` should have an upper camel case name
   --> C:\Users\shcar\IdeaProjects\Toshi\target\debug\build\toshi-proto-979910a1e8f4b4da\out/clusterrpc.rs:574:28
    |
574 |                     struct raft_requestSvc<T: IndexService>(pub Arc<T>);
    |                            ^^^^^^^^^^^^^^^ help: convert the identifier to upper camel case: `RaftRequestSvc`

warning: type `joinSvc` should have an upper camel case name
   --> C:\Users\shcar\IdeaProjects\Toshi\target\debug\build\toshi-proto-979910a1e8f4b4da\out/clusterrpc.rs:601:28
    |
601 |                     struct joinSvc<T: IndexService>(pub Arc<T>);
    |                            ^^^^^^^ help: convert the identifier to upper camel case: `JoinSvc`

My proto def can be found here.
https://github.com/toshi-search/Toshi/blob/master/toshi-proto/proto/cluster.proto

[jen20]

Hi @hntd187! Thanks for opening a bug report. This reproduces easily using the latest release of tonic and tonic-build. The issue is caused by service_ident in the various generated files being derived from the gRPC method identifier, for example:

tonic/tonic-build/src/server.rs

Line 257 in f65cda1

let service_ident = quote::format_ident!("{}Svc", method.identifier());

There seem to be two approaches to resolving this:

• Transform the method identifier from the conventional gRPC lower snake case into the upper camel case expected by Rust type names when generating service_ident. My hunch is that this is OK, but I'm not 100% certain this could never lead to duplicate type names in a protocol buffers package with names constructed in some manner.

• Annotate the type names to allow non-conventional type names (using #[allow(non_camel_case_types)]) - since the structures in question are never exposed in public APIs anyway, this seems less invasive and could be build directly into the templates.

@LucioFranco Any thoughts on which approach you'd prefer here?

[LucioFranco]

@jen20 the allow solution should be fine 👍 its quick and easy.

[LucioFranco]

Also to note, this change will go out with 0.2 which is slightly on hold for the moment, so when this gets fixed on master I'd suggest users to work off of master for a bit.

[bmulder-innoseis]

this also happens in streaming calls.
proto:

service Foo {
    rpc bar (barRequest) returns (stream barResponse);
}

generated server code:

pub mod foo_server {
#![allow(unused_variables, dead_code, missing_docs, clippy::let_unit_value)]
    use tonic::codegen::*;
    ///Generated trait containing gRPC methods that should be implemented for use with FooServer.
    #[async_trait]
    pub trait Foo: Send + Sync + 'static {
        ///Server streaming response type for the bar method.
        type barStream: futures_core::Stream<
                Item = Result<super::BarResponse, tonic::Status>,
            >
            + Send
            + 'static;
        [...]
    }
}

when compiling:

    |
135 |         type barStream: futures_core::Stream<
    |              ^^^^^^^^^^^^^^^^^ help: convert the identifier to upper camel case: `BarStream`
    |
    = note: `#[warn(non_camel_case_types)]` on by default