dry-rb / dry-container

A simple, configurable object container implemented in Ruby
https://dry-rb.org/gems/dry-container
MIT License
335 stars 41 forks source link

Raise KeyError for missing key, with DidYouMean integration #82

Closed cllns closed 2 years ago

cllns commented 2 years ago

We could go further to check to make sure actual suggestions are offered, but that's like testing the external library. The last spec example, the instance check, is actually enough (without checking key and provider), but it'll raise an error without the other two values set, so we might as well check those first.

I think we'll want to extract some proper Dry::Container::Errors, possibly our own Dry::Container::KeyError but these specs will ensure we don't regress this neat functionality that Ruby gives us

After this PR:

> require 'dry/container'
> c = Dry::Container.new
> c.register(:foo, 3)
> c[:floof]
# KeyError: KeyError
# Did you mean?  "foo"
flash-gordon commented 2 years ago

Can we make the following:

module Dry
  module Container
    class KeyError < ::KeyError
    end

    Error = KeyError

    deprecate_constant(:Error)
  end
end

?

It would preserve backward compatibility and add support for DidYouMean I guess

cllns commented 2 years ago

@flash-gordon Done, though I had to adapt it. There are quite a few other Errors so aliasing it to ::KeyError isn't accurate.

Likewise the deprecate_constant adds a number of warnings. I did add that, but I think it might make more sense to wait on that until we can extract all the appropriate Dry::Container::Errors.

> c[:floof]
Dry::Container::KeyError: key not found: "floof"
Did you mean?  "foo"

I also added the "key not found" message, to keep the message the same as Hash. (Very annoying it's not pulled from key keyword argument by default)