From 545083ff2d237565cd33d600f9d00a352bc14d15 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Mon, 11 Dec 2023 12:29:40 -0500 Subject: [PATCH] Refactor Node#initialize to not allocate a Hash --- lib/graphql/language/nodes.rb | 120 ++++++++++-------- lib/graphql/language/parser.rb | 8 +- lib/graphql/language/parser.y | 4 +- .../language/recursive_descent_parser.rb | 2 +- .../language/recursive_descent_parser_spec.rb | 3 - 5 files changed, 76 insertions(+), 61 deletions(-) diff --git a/lib/graphql/language/nodes.rb b/lib/graphql/language/nodes.rb index efc0254365..c1362f50b3 100644 --- a/lib/graphql/language/nodes.rb +++ b/lib/graphql/language/nodes.rb @@ -16,9 +16,9 @@ module DefinitionNode # @return [Integer] The first line of the definition (not the description) attr_reader :definition_line - def initialize(options = {}) - @definition_line = options.delete(:definition_line) - super(options) + def initialize(definition_line: nil, **_rest) + @definition_line = definition_line + super(**_rest) end end @@ -38,27 +38,6 @@ def col end end - # Initialize a node by extracting its position, - # then calling the class's `initialize_node` method. - # @param options [Hash] Initial attributes for this node - def initialize(options = {}) - if options.key?(:position_source) - position_source = options.delete(:position_source) - @line = position_source[1] - @col = position_source[2] - else - @line = options.delete(:line) - @col = options.delete(:col) - end - - @pos = options.delete(:pos) - - @filename = options.delete(:filename) - @source_string = options.delete(:source_string) - - initialize_node(**options) - end - # Value equality # @return [Boolean] True if `self` is equivalent to `other` def ==(other) @@ -213,8 +192,8 @@ def children_methods(children_of_type) module_eval <<-RUBY, __FILE__, __LINE__ # Singular method: create a node with these options # and return a new `self` which includes that node in this list. - def merge_#{method_name.to_s.sub(/s$/, "")}(node_opts) - merge(#{method_name}: #{method_name} + [#{node_type.name}.new(node_opts)]) + def merge_#{method_name.to_s.sub(/s$/, "")}(**node_opts) + merge(#{method_name}: #{method_name} + [#{node_type.name}.new(**node_opts)]) end RUBY end @@ -243,13 +222,14 @@ def children end if defined?(@scalar_methods) - if !method_defined?(:initialize_node) - generate_initialize_node + if !@initialize_was_generated + @initialize_was_generated = true + generate_initialize else # This method was defined manually end else - raise "Can't generate_initialize_node because scalar_methods wasn't called; call it before children_methods" + raise "Can't generate_initialize because scalar_methods wasn't called; call it before children_methods" end end @@ -278,7 +258,16 @@ def scalars end end - def generate_initialize_node + DEFAULT_INITIALIZE_OPTIONS = [ + "line: nil", + "col: nil", + "position_source: nil", + "pos: nil", + "filename: nil", + "source_string: nil", + ] + + def generate_initialize scalar_method_names = @scalar_methods # TODO: These probably should be scalar methods, but `types` returns an array [:types, :description].each do |extra_method| @@ -294,16 +283,34 @@ def generate_initialize_node return else arguments = scalar_method_names.map { |m| "#{m}: nil"} + - @children_methods.keys.map { |m| "#{m}: NO_CHILDREN" } + @children_methods.keys.map { |m| "#{m}: NO_CHILDREN" } + + DEFAULT_INITIALIZE_OPTIONS assignments = scalar_method_names.map { |m| "@#{m} = #{m}"} + @children_methods.keys.map { |m| "@#{m} = #{m}.freeze" } + if name.end_with?("Definition") && name != "FragmentDefinition" + arguments << "definition_line: nil" + assignments << "@definition_line = definition_line" + attr_reader :definition_line + end + keywords = scalar_method_names.map { |m| "#{m}: #{m}"} + @children_methods.keys.map { |m| "#{m}: #{m}" } module_eval <<-RUBY, __FILE__, __LINE__ - def initialize_node #{arguments.join(", ")} + def initialize(#{arguments.join(", ")}) + if position_source + @line = position_source[1] + @col = position_source[2] + else + @line = line + @col = col + end + + @pos = pos + @filename = filename + @source_string = source_string #{assignments.join("\n")} end @@ -353,7 +360,6 @@ class DirectiveLocation < NameOnlyNode end class DirectiveDefinition < AbstractNode - include DefinitionNode attr_reader :description scalar_methods :name, :repeatable children_methods( @@ -382,17 +388,27 @@ class Field < AbstractNode # @!attribute selections # @return [Array] Selections on this object (or empty array if this is a scalar field) - def initialize_node(attributes) - @name = attributes[:name] - @arguments = attributes[:arguments] || NONE - @directives = attributes[:directives] || NONE - @selections = attributes[:selections] || NONE + def initialize(name: nil, arguments: NONE, directives: NONE, selections: NONE, field_alias: nil, position_source: nil, line: nil, col: nil, pos: nil, filename: nil, source_string: nil) + @name = name + @arguments = arguments || NONE + @directives = directives || NONE + @selections = selections || NONE # oops, alias is a keyword: - @alias = attributes[:alias] + @alias = field_alias + if position_source + @line = position_source[1] + @col = position_source[2] + else + @line = line + @col = col + end + @pos = pos + @filename = filename + @source_string = source_string end - def self.from_a(filename, line, col, graphql_alias, name, arguments, directives, selections) # rubocop:disable Metrics/ParameterLists - self.new(filename: filename, line: line, col: col, alias: graphql_alias, name: name, arguments: arguments, directives: directives, selections: selections) + def self.from_a(filename, line, col, field_alias, name, arguments, directives, selections) # rubocop:disable Metrics/ParameterLists + self.new(filename: filename, line: line, col: col, field_alias: field_alias, name: name, arguments: arguments, directives: directives, selections: selections) end # Override this because default is `:fields` @@ -406,11 +422,21 @@ class FragmentDefinition < AbstractNode # @!attribute type # @return [String] the type condition for this fragment (name of type which it may apply to) - def initialize_node(name: nil, type: nil, directives: [], selections: []) + def initialize(name: nil, type: nil, directives: NONE, selections: NONE, filename: nil, pos: nil, source_string: nil, line: nil, col: nil, position_source: nil) @name = name @type = type @directives = directives @selections = selections + @filename = filename + @pos = pos + @source_string = source_string + if position_source + @line = position_source[1] + @col = position_source[2] + else + @line = line + @col = col + end end def self.from_a(filename, line, col, name, type, directives, selections) @@ -579,7 +605,6 @@ class VariableIdentifier < NameOnlyNode end class SchemaDefinition < AbstractNode - include DefinitionNode scalar_methods :query, :mutation, :subscription children_methods({ directives: GraphQL::Language::Nodes::Directive, @@ -596,7 +621,6 @@ class SchemaExtension < AbstractNode end class ScalarTypeDefinition < AbstractNode - include DefinitionNode attr_reader :description scalar_methods :name children_methods({ @@ -614,7 +638,6 @@ class ScalarTypeExtension < AbstractNode end class InputValueDefinition < AbstractNode - include DefinitionNode attr_reader :description scalar_methods :name, :type, :default_value children_methods({ @@ -624,7 +647,6 @@ class InputValueDefinition < AbstractNode end class FieldDefinition < AbstractNode - include DefinitionNode attr_reader :description scalar_methods :name, :type children_methods({ @@ -645,7 +667,6 @@ def merge(new_options) end class ObjectTypeDefinition < AbstractNode - include DefinitionNode attr_reader :description scalar_methods :name, :interfaces children_methods({ @@ -665,7 +686,6 @@ class ObjectTypeExtension < AbstractNode end class InterfaceTypeDefinition < AbstractNode - include DefinitionNode attr_reader :description scalar_methods :name children_methods({ @@ -687,7 +707,6 @@ class InterfaceTypeExtension < AbstractNode end class UnionTypeDefinition < AbstractNode - include DefinitionNode attr_reader :description, :types scalar_methods :name children_methods({ @@ -706,7 +725,6 @@ class UnionTypeExtension < AbstractNode end class EnumValueDefinition < AbstractNode - include DefinitionNode attr_reader :description scalar_methods :name children_methods({ @@ -716,7 +734,6 @@ class EnumValueDefinition < AbstractNode end class EnumTypeDefinition < AbstractNode - include DefinitionNode attr_reader :description scalar_methods :name children_methods({ @@ -736,7 +753,6 @@ class EnumTypeExtension < AbstractNode end class InputObjectTypeDefinition < AbstractNode - include DefinitionNode attr_reader :description scalar_methods :name children_methods({ diff --git a/lib/graphql/language/parser.rb b/lib/graphql/language/parser.rb index c7ff65fb76..1ddb625481 100644 --- a/lib/graphql/language/parser.rb +++ b/lib/graphql/language/parser.rb @@ -1,6 +1,6 @@ # # DO NOT MODIFY!!!! -# This file is automatically generated by Racc 1.6.2 +# This file is automatically generated by Racc 1.7.1 # from Racc grammar file "". # @@ -119,7 +119,7 @@ def make_node(node_name, assigns) assigns[:filename] = @filename - GraphQL::Language::Nodes.const_get(node_name).new(assigns) + GraphQL::Language::Nodes.const_get(node_name).new(**assigns) end ...end parser.y/module_eval... ##### State transition tables begin ### @@ -863,6 +863,7 @@ def make_node(node_name, assigns) racc_shift_n, racc_reduce_n, racc_use_result_var ] +Ractor.make_shareable(Racc_arg) if defined?(Ractor) Racc_token_to_s_table = [ "$end", @@ -990,6 +991,7 @@ def make_node(node_name, assigns) "field_definition_list", "directive_repeatable_opt", "directive_locations" ] +Ractor.make_shareable(Racc_token_to_s_table) if defined?(Ractor) Racc_debug_parser = false @@ -1234,7 +1236,7 @@ def _reduce_37(val, _values, result) def _reduce_38(val, _values, result) result = make_node( :Field, { - alias: val[0], + field_alias: val[0], name: val[2], arguments: val[3], directives: val[4], diff --git a/lib/graphql/language/parser.y b/lib/graphql/language/parser.y index 25630e6402..87d3e8c633 100644 --- a/lib/graphql/language/parser.y +++ b/lib/graphql/language/parser.y @@ -119,7 +119,7 @@ rule | name COLON name arguments_opt directives_list_opt selection_set_opt { result = make_node( :Field, { - alias: val[0], + field_alias: val[0], name: val[2], arguments: val[3], directives: val[4], @@ -556,5 +556,5 @@ def make_node(node_name, assigns) assigns[:filename] = @filename - GraphQL::Language::Nodes.const_get(node_name).new(assigns) + GraphQL::Language::Nodes.const_get(node_name).new(**assigns) end diff --git a/lib/graphql/language/recursive_descent_parser.rb b/lib/graphql/language/recursive_descent_parser.rb index f0e17a9f29..a476a9b44d 100644 --- a/lib/graphql/language/recursive_descent_parser.rb +++ b/lib/graphql/language/recursive_descent_parser.rb @@ -498,7 +498,7 @@ def selection_set directives = at?(:DIR_SIGN) ? parse_directives : nil selection_set = at?(:LCURLY) ? self.selection_set : nil - Nodes::Field.new(pos: loc, alias: field_alias, name: name, arguments: arguments, directives: directives, selections: selection_set, filename: @filename, source_string: @graphql_str) + Nodes::Field.new(pos: loc, field_alias: field_alias, name: name, arguments: arguments, directives: directives, selections: selection_set, filename: @filename, source_string: @graphql_str) end end expect_token(:RCURLY) diff --git a/spec/graphql/language/recursive_descent_parser_spec.rb b/spec/graphql/language/recursive_descent_parser_spec.rb index 03d6dd1392..88622695d0 100644 --- a/spec/graphql/language/recursive_descent_parser_spec.rb +++ b/spec/graphql/language/recursive_descent_parser_spec.rb @@ -29,9 +29,6 @@ subject.parse(query_str).to_query_string ) end - it "parses comments" do - skip "TODO" - end it "parses schemas" do schema_str = <<~GRAPHQL