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

Fail early when one defines an attribute that overlaps with any method in Attributor::Model #111

Closed
konstantin-dzreev opened this issue Mar 17, 2015 · 4 comments

Comments

@konstantin-dzreev
Copy link
Contributor

In the example below attribute ":attributes" overlaps with pre existing method "attributes" defined in Attributor::Model.

  class Data < Attributor::Model
    attributes do
      attribute :foo, String
      attribute :attributes, Hash
    end
  end

This compiles and runs. And you do not see any obvious issues until you try to load and validate your attributor model instance where it fails with a weird error:

Attributor::Hash can not validate object of type Hash for $.attributes 

It would be nice to fail earlier (ideally in the model definition) or at least to have a better error message.

@blanquer
Copy link
Contributor

yes, we do have that issue in general (as #99 sort of states).
This particular case of the attributes method, though, I think is a slightly different, as it is pertaining to a Model only, and it clashes with an instance method we explicitly set in the model instances:
https://github.com/rightscale/attributor/blob/master/lib/attributor/types/model.rb#L145-L147

We might be able to get rid of that method or rename (i.e., _attributes) it so it doesn't clash so directly. This would require a lot of care and attention though, as these inheritable types have dependencies on top that might be difficult to bend...

@konstantin-dzreev
Copy link
Contributor Author

It would be nice to have a fix that is not for 'attributes' only but for any method that may overlap with a defined attribute (any method that is defined on a model instance).

@blanquer
Copy link
Contributor

oh absolutely, I simply wanted to point out that this issue is "similar" to #99 , but it is quite different in the sense that has to do with instance-level methods (while #99 is mostly about class-level methods). But either way, a better story for non-overlapping methods would be great regardless.

@careo
Copy link
Contributor

careo commented Oct 30, 2015

Duplicate of #71, as that's really how we should end up fixing this.

@careo careo closed this as completed Oct 30, 2015
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

No branches or pull requests

3 participants