Enum value accessors should be opt-in

[gmac]

Describe the bug

I'm going to upgrade this discussion to a formal bug report. The changes made in #5206 are both niche and problematic and should not be a default library feature. Here's an excellent example of why:

StrictWarning::Offense: Failed to define value method for :name, because LabelOrderField already responds to that method. Use `value_method:` to override the method name or `value_method: false` to disable Enum value method generation.

So, an Enum can't include a NAME value without conflicting with Ruby, or requires manual intervention to reconcile the conflict. Now apply this to all other common methods that Ruby implements that are probably also common enum values (class, etc). This feature is not valuable enough to be on by default.

IMO the base library should specify value_method: false, then do something like this:

class BaseEnum < GraphQL::Schema::Enum
  value_methods(true)
end

As a side note, this feature doesn't make a lot of sense to me because the only logical remediation for the NAME conflict would be to change the Enum value, lest the accessor become inconsistent – which kind of defeats the purpose. I'm not sure what we really made better here.

Versions

2.4.9

[rmosolgo]

Yeah, that makes sense. I'll make them opt-in in #5255.

[wuarmin]

Hey @rmosolgo,
please checkout: github-community-projects/graphql-client#62

Will this also solve this issue?
thanks

[gmac]

@wuarmin – yes, that's this same problem. I'd suggest sticking to v2.4.8 for now and upgrade directly to v2.4.11 that will make the conflicting behavior an opt-in, see #5255.

[wuarmin]

Thanks @gmac !

[rmosolgo]

🚢 2.4.11! Thanks again for sharing your thoughts on this.

[humphreyja]

The docs need to be updated to reflect this by the way. Additionally, in the changelog this should really be considered a breaking change not a bug fix. I bumped versions to pick up the CVE fix and found out this broke a few things.