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.

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

Procedural macro for routing #19

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

Procedural macro for routing #19

wants to merge 10 commits into from

Conversation

Meralis40
Copy link
Contributor

This is the first part for procedural macro for routing, very basic, only GET and POST.
This is currently nightly-only (relies on procedural macro attributes, not stable yet).
It's exposed into shio and shio::prelude under the nightly feature.

See the hello_macros examples for how to use it for get request.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 79.762% when pulling 9119358 on Meralis40:shio-macro into 033a0b5 on mehcode:master.

@coveralls
Copy link

coveralls commented Sep 8, 2017

Coverage Status

Coverage remained the same at 79.762% when pulling 4d433c1 on Meralis40:shio-macro into 033a0b5 on mehcode:master.

workspace = ".."

[dependencies]
syn = {version="0.11.11", features=["full","parsing","printing"]}
Copy link
Owner

Choose a reason for hiding this comment

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

quote="0.3"

[lib]
proc-macro = true
Copy link
Owner

Choose a reason for hiding this comment

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

Make sure and leave an empty line at the end of files. I wouldn't mind an .editorconfig like crates.io.


fn impl_route_rewrite(meth: syn::Expr, opts: TokenStream, item: TokenStream) -> TokenStream {
let item = item.to_string();
let item = syn::parse_item(&item).expect("Unable to parse item associated to get attribute");
Copy link
Owner

Choose a reason for hiding this comment

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

The std lib uses lowercase-without-punctuation for all error messages.

let item = item.to_string();
let item = syn::parse_item(&item).expect("Unable to parse item associated to get attribute");

match &item.node {
Copy link
Owner

Choose a reason for hiding this comment

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

warning: you don't need to add `&` to both the expression and the patterns
  --> macros/src/lib.rs:25:5
   |
25 | /     match &item.node {
26 | |         &syn::ItemKind::Fn(_, _, _, _, _, _) => {}
27 | |         _ => panic!("get attribute is only for functions"),
28 | |     }
   | |_____^ help: try: `match item.node { .. }`
   |
   = note: #[warn(match_ref_pats)] on by default

Try running clippy over lib. Many of these warnings come up.

cargo +nightly-2017-08-30 clippy -p shio_macros --features nightly --all

Is how I run clippy. I pin a nightly version, install clippy, and then use it explicitly when I want it.

@mehcode
Copy link
Owner

mehcode commented Sep 12, 2017

#[allow(non_camel_case_types)]
struct __shio_route_hello {
}
impl Clone for __shio_route_hello {
    fn clone(&self) -> Self { *self }
}
impl Copy for __shio_route_hello { }
#[allow(non_upper_case_globals)]
static hello: __shio_route_hello = __shio_route_hello{};
impl Into<::shio::router::Route> for __shio_route_hello {
    fn into(self) -> ::shio::router::Route {
        (::shio::Method::Get, "/{name}", __shio_handler_hello).into()
    }
}

Neat solution to non-moveable statics with the newtype plus impl. I'd suggest shortening it up a bit with a ZST:

#[allow(non_camel_case_types)]
struct hello;
impl Into<::shio::router::Route> for hello {
    fn into(self) -> ::shio::router::Route {
        (::shio::Method::Get, "/{name}", __shio_handler_hello).into()
    }
}

@mehcode
Copy link
Owner

mehcode commented Sep 12, 2017

@Meralis40 Over all, nice work! Let's get this cleaned up a bit and we can merge it in.

I'd love to see the "standard" methods before this is released though:

  • get
  • post
  • patch
  • put
  • delete
  • options
  • head

@Meralis40
Copy link
Contributor Author

@mehcode All changes have been done, except complete clippy, because of "needless_pass_by_value" warning. Also, I've change the generated code as suggested & added the "standard" methods

@coveralls
Copy link

coveralls commented Sep 12, 2017

Coverage Status

Coverage remained the same at 79.762% when pulling cfd41a2 on Meralis40:shio-macro into 033a0b5 on mehcode:master.

@Meralis40
Copy link
Contributor Author

Rebased

@coveralls
Copy link

coveralls commented Sep 15, 2017

Coverage Status

Coverage remained the same at 80.699% when pulling 81786a5 on Meralis40:shio-macro into 724880d on mehcode:master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants