alloy-rs / core

High-performance, well-tested & documented core libraries for Ethereum, in Rust
https://alloy.rs
Apache License 2.0
794 stars 155 forks source link

[Bug] `syn-solidity` Semver Incompatibility #736

Closed ratankaliani closed 2 months ago

ratankaliani commented 2 months ago

Component

syn-solidity

What version of Alloy are you on?

0.8.0

Operating System

None

Describe the bug

Background The syn-solidity crate recently introduced a breaking change in version 0.8.3, which was not reflected in its semantic versioning. This change has caused compilation issues for downstream consumers, highlighting the importance of adhering to Semver principles.

Issue Details Alloy crates were at version 0.8.0 Cargo automatically bumped syn-solidity to version 0.8.3 Version 0.8.3 introduced a breaking change in the API for indexed_by_hash This change was implemented in PR: https://github.com/alloy-rs/core/pull/735

The error logs can be seen here:

error[E0061]: this method takes 1 argument but 0 arguments were supplied
   --> /Users/ratankaliani/.cargo/registry/src/index.crates.io-6f17d22bba15001f/alloy-sol-macro-expander-0.8.0/src/expand/event.rs:84:14
    |
84  |         if p.indexed_as_hash() {
    |              ^^^^^^^^^^^^^^^-- an argument is missing
    |
note: method defined here
   --> /Users/ratankaliani/.cargo/registry/src/index.crates.io-6f17d22bba15001f/syn-solidity-0.8.3/src/item/event.rs:259:12
    |
259 |     pub fn indexed_as_hash(&self, custom_is_value_type: impl Fn(&SolPath) -> bool) -> bool {
    |            ^^^^^^^^^^^^^^^
help: provide the argument
    |
84  |         if p.indexed_as_hash(/* custom_is_value_type */) {
    |                             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~

   Compiling alloy-trie v0.5.1
error[E0599]: no method named `is_abi_dynamic` found for reference `&EventParameter` in the current scope
   --> /Users/ratankaliani/.cargo/registry/src/index.crates.io-6f17d22bba15001f/alloy-sol-macro-expander-0.8.0/src/expand/event.rs:228:14
    |
228 |     if param.is_abi_dynamic() {
    |              ^^^^^^^^^^^^^^ method not found in `&EventParameter`
    |
help: one of the expressions' fields has a method of the same name
    |
228 |     if param.ty.is_abi_dynamic() {
    |              +++

error[E0061]: this method takes 1 argument but 0 arguments were supplied
   --> /Users/ratankaliani/.cargo/registry/src/index.crates.io-6f17d22bba15001f/alloy-sol-macro-expander-0.8.0/src/expand/event.rs:242:23
    |
242 |     let ty = if param.indexed_as_hash() {
    |                       ^^^^^^^^^^^^^^^-- an argument is missing
    |
note: method defined here
   --> /Users/ratankaliani/.cargo/registry/src/index.crates.io-6f17d22bba15001f/syn-solidity-0.8.3/src/item/event.rs:259:12
    |
259 |     pub fn indexed_as_hash(&self, custom_is_value_type: impl Fn(&SolPath) -> bool) -> bool {
    |            ^^^^^^^^^^^^^^^
help: provide the argument
    |
242 |     let ty = if param.indexed_as_hash(/* custom_is_value_type */) {

Impact The Semver incompatibility has resulted in compilation failures for projects that depend on syn-solidity and Alloy. This situation forces downstream consumers to manually intervene and precisely downgrade syn-solidity which is a bad developer experience. I'd recommend using a tool like https://github.com/MarcoIeni/release-plz to catch API breaking changes.

DaniPopes commented 2 months ago

Bumping a single alloy crate is generally not supported, especially when it's a crate that's tightly coupled with others, and when the breakage is for a bug fix