dry-rb / dry-effects

Algebraic effects in Ruby
https://dry-rb.org/gems/dry-effects/
MIT License
112 stars 20 forks source link

fix resolving dependencies via nested names #81

Closed ermolaev closed 3 years ago

timriley commented 3 years ago

My only thought here would that it'd be worth following a similar approach to dry-auto_inject's name detection code, so we have consistent handling across the two. For that, see https://github.com/dry-rb/dry-auto_inject/blob/045cbd46be4338ad37ac4278b5cd72ad37d76dc8/lib/dry/auto_inject/dependency_map.rb#L42-L47:

  VALID_NAME = /([a-z_][a-zA-Z_0-9]*)$/

  # ...

      def name_for(identifier)
        matched = VALID_NAME.match(identifier.to_s)
        raise DependencyNameInvalid, "name +#{identifier}+ is not a valid Ruby identifier" unless matched

        matched[0]
      end

This approach has two benefits:

ermolaev commented 3 years ago

Thanks for the clarification, I updated code

solnic commented 3 years ago

FYI Codacy got stuck analysing this PR so let's just ignore it here 🤷🏻

flash-gordon commented 3 years ago

Thank you for this, I'll be merging and doing a few tweaks. For instance, we usually have errors in a separate module and have their names ending with Error (ruby convention more or less)