Merge pull request #5255 from rmosolgo/enum-value-methods-optional

Make Enum value_methods optional

Author: rmosolgo

lib/graphql/schema/build_from_definition.rb

@@ -298,7 +298,6 @@ def build_enum_type(enum_type_definition, type_resolver)
                 description: enum_value_definition.description,
                 directives: builder.prepare_directives(enum_value_definition, type_resolver),
                 ast_node: enum_value_definition,
-                value_method: GraphQL::Schema::Enum.respond_to?(enum_value_definition.name.downcase) ? false : nil,
               )
             end
           end

lib/graphql/schema/enum.rb

@@ -70,7 +70,9 @@ def value(*args, value_method: nil, **kwargs, &block)
           kwargs[:owner] = self
           value = enum_value_class.new(*args, **kwargs, &block)
 
-          generate_value_method(value, value_method)
+          if value_method || (value_methods && value_method != false)
+            generate_value_method(value, value_method)
+          end
 
           key = value.graphql_name
           prev_value = own_values[key]
@@ -159,6 +161,18 @@ def enum_value_class(new_enum_value_class = nil)
           end
         end
 
+        def value_methods(new_value = NOT_CONFIGURED)
+          if NOT_CONFIGURED.equal?(new_value)
+            if @value_methods != nil
+              @value_methods
+            else
+              find_inherited_value(:value_methods, false)
+            end
+          else
+            @value_methods = new_value
+          end
+        end
+
         def kind
           GraphQL::TypeKinds::ENUM
         end
@@ -220,6 +234,7 @@ def inherited(child_class)
             # because they would end up with names like `#<Class0x1234>::UnresolvedValueError` which messes up bug trackers
             child_class.const_set(:UnresolvedValueError, Class.new(Schema::Enum::UnresolvedValueError))
           end
+          child_class.class_eval { @value_methods = nil }
           super
         end
 

lib/graphql/schema/input_object.rb

@@ -156,7 +156,7 @@ def argument(*args, **kwargs, &block)
               def #{method_name}
                 self[#{method_name.inspect}]
               end
-              alias_method :#{method_name}, :#{method_name}
+              alias_method #{method_name.inspect}, #{method_name.inspect}
             RUBY
           end
           argument_defn

lib/graphql/schema/loader.rb

@@ -108,7 +108,6 @@ def define_type(type, type_resolver)
                   enum_value["name"],
                   description: enum_value["description"],
                   deprecation_reason: enum_value["deprecationReason"],
-                  value_method: respond_to?(enum_value["name"].downcase) ? false : nil
                 )
               end
             end

spec/graphql/schema/dynamic_members_spec.rb

@@ -175,7 +175,7 @@ class Thing < LegacyThing
 
     class Language < BaseEnum
       value "RUBY"
-      value "PERL6", deprecation_reason: "Use RAKU instead", future_schema: true, value_method: false
+      value "PERL6", deprecation_reason: "Use RAKU instead", future_schema: true
       value "PERL6", future_schema: false
       value "RAKU", future_schema: true
       value "COFFEE_SCRIPT", future_schema: false
@@ -1026,7 +1026,7 @@ class BaseEnumValue < GraphQL::Schema::EnumValue
       class DuplicateEnumValue < GraphQL::Schema::Enum
         enum_value_class(BaseEnumValue)
         value "ONE", description: "second definition", allow_for: [2, 3]
-        value "ONE", description: "first definition", allow_for: [1, 2], value_method: false
+        value "ONE", description: "first definition", allow_for: [1, 2]
       end
 
       class DuplicateFieldObject < GraphQL::Schema::Object

spec/graphql/schema/enum_spec.rb

@@ -11,12 +11,21 @@
   end
 
   describe "value methods" do
-    it "defines default methods to fetch graphql names" do
-      assert_equal enum.string, "STRING"
-      assert_equal enum.woodwind, "WOODWIND"
-      assert_equal enum.brass, "BRASS"
-      assert_equal enum.didgeridoo, "DIDGERIDOO"
-      assert_equal enum.keys, "KEYS"
+    class EnumWithValueMethods < GraphQL::Schema::Enum
+      value_methods(true)
+      value :SOMETHING
+      value :SOMETHING_ELSE, value_method: false
+      value :SOMETHING_CUSTOM, value_method: :custom
+    end
+    it "defines methods to fetch graphql names when configured" do
+      assert_equal "SOMETHING", EnumWithValueMethods.something
+      assert_equal "SOMETHING", EnumWithValueMethods.something
+    end
+
+    it "inherits a value_methods config" do
+      new_enum = Class.new(EnumWithValueMethods)
+      new_enum.value(:NEW_VALUE)
+      assert_equal "NEW_VALUE", new_enum.new_value
     end
 
     describe "when value_method is configured" do
@@ -27,23 +36,29 @@
     end
 
     describe "when value_method conflicts with existing method" do
+      class ConflictEnum < GraphQL::Schema::Enum
+        value_methods(true)
+      end
       it "does not define method and emits warning" do
         expected_message = "Failed to define value method for :value, because ConflictEnum already responds to that method. Use `value_method:` to override the method name or `value_method: false` to disable Enum value method generation.\n"
         assert_warns(expected_message) do
-          conflict_enum = Class.new(GraphQL::Schema::Enum)
-          Object.const_set("ConflictEnum", conflict_enum)
-          already_defined_method = conflict_enum.method(:value)
-          conflict_enum.value "VALUE", "Makes conflict"
-          assert_equal conflict_enum.method(:value), already_defined_method
+          already_defined_method = ConflictEnum.method(:value)
+          ConflictEnum.value "VALUE", "Makes conflict"
+          assert_equal ConflictEnum.method(:value), already_defined_method
         end
       end
     end
 
     describe "when value_method = false" do
       it "does not define method" do
-        assert_equal enum.respond_to?(:silence), false
+        assert_equal EnumWithValueMethods.respond_to?(:something_else), false
       end
     end
+    it "doesn't define value methods by default" do
+      enum = Class.new(GraphQL::Schema::Enum) { graphql_name("SomeEnum") }
+      enum.value("SOME_VALUE")
+      refute enum.respond_to?(:some_value)
+    end
   end
 
   describe "type info" do
@@ -145,7 +160,7 @@ def value
     class MultipleNameTestEnum < GraphQL::Schema::Enum
       value "A"
       value "B", value: :a
-      value "B", value: :b, value_method: false
+      value "B", value: :b
     end
 
     it "doesn't allow it from enum_values" do

spec/graphql/schema/loader_spec.rb

@@ -19,8 +19,7 @@
 
       value "FOO", value: :foo
       value "BAR", deprecation_reason: "Don't use BAR"
-      value "NAME", value_method: false
-      value "foo", value_method: false
+      value "foo"
     end
 
     sub_input_type = Class.new(GraphQL::Schema::InputObject) do

spec/support/jazz.rb

@@ -246,7 +246,7 @@ class Family < BaseEnum
     value "KEYS" do
       description "Neither here nor there, really"
     end
-    value "SILENCE", "Makes no sound", value: false, value_method: false
+    value "SILENCE", "Makes no sound", value: false
   end
 
   class InstrumentType < BaseObject