openethereum / pwasm-abi

Parity WASM Abi (Legacy and new)
Apache License 2.0
29 stars 17 forks source link

Update syn quote #73

Closed Robbepop closed 6 years ago

Robbepop commented 6 years ago

This PR updates dependencies syn from 0.11.x to 0.15.x and quote from 0.3.x to 0.6.x.

Improvements to the documentation have been made. Also restructured and simplified some code.

Robbepop commented 6 years ago

Note that Travis CI fails because of some unstable features used in stable context. I currently do not know why this happens.

pepyakin commented 6 years ago

Before continuing work on that I would really love to see CI working again.

Robbepop commented 6 years ago

Before continuing work on that I would really love to see CI working again.

Master doesn't work either as far as I have tested. But it should be when adding +nightly to cargo command. I will test that.

debris commented 6 years ago

I see that there are no unit tests for generated token stream. As debugging syn && quote is extremely painful I suggest adding some tests for generated token streams 😉

e.g. we have something like this in ethabi

https://github.com/paritytech/ethabi/blob/523cb5bcb41607737e52da051d46e223cc54b212/derive/src/event.rs#L322-L344

    #[test]
    fn test_log_with_one_field() {
        let ethabi_event = ethabi::Event {
            name: "one".into(),
            inputs: vec![ethabi::EventParam {
                name: "foo".into(),
                kind: ethabi::ParamType::Address,
                indexed: false
            }],
            anonymous: false,
        };

        let e = Event::from(&ethabi_event);

        let expected = quote! {
            #[derive(Debug, Clone, PartialEq)]
            pub struct One {
                pub foo: ethabi::Address
            }
        };

        assert_eq!(expected.to_string(), e.generate_log().to_string());
    }
Robbepop commented 6 years ago

I see that there are no unit tests for generated token stream. As debugging syn && quote is extremely painful I suggest adding some tests for generated token streams wink

e.g. we have something like this in ethabi

https://github.com/paritytech/ethabi/blob/523cb5bcb41607737e52da051d46e223cc54b212/derive/src/event.rs#L322-L344

  #[test]
  fn test_log_with_one_field() {
      let ethabi_event = ethabi::Event {
          name: "one".into(),
          inputs: vec![ethabi::EventParam {
              name: "foo".into(),
              kind: ethabi::ParamType::Address,
              indexed: false
          }],
          anonymous: false,
      };

      let e = Event::from(&ethabi_event);

      let expected = quote! {
          #[derive(Debug, Clone, PartialEq)]
          pub struct One {
              pub foo: ethabi::Address
          }
      };

      assert_eq!(expected.to_string(), e.generate_log().to_string());
  }

Thank you very much for hinting that to me! I actually didn't know about this testing functionality but it sounds awesome.

Maybe we should implement the tests in another PR as this is already too big.

Robbepop commented 6 years ago

@fckt I hope I resolved the formatting issues to your satisfaction in the most recent commits.

Besides that I have added some proper documentation for the items defined in items.rs as well. I hope we can merge this asap so that this PR doesn't get even bigger.

Robbepop commented 6 years ago

Thank you all for the reviews and feedback! :)

Robbepop commented 6 years ago

@NikVolf I think we can also delete the syn-upgrade branch.