Skip to content

Commit

Permalink
Refactor Node#initialize to not allocate a Hash
Browse files Browse the repository at this point in the history
  • Loading branch information
@rmosolgo
rmosolgo committed Dec 11, 2023
1 parent 11db91f commit 545083f
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 61 deletions.
120 changes: 68 additions & 52 deletions lib/graphql/language/nodes.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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|
Expand All @@ -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
Expand Down Expand Up @@ -353,7 +360,6 @@ class DirectiveLocation < NameOnlyNode
end

class DirectiveDefinition < AbstractNode
include DefinitionNode
attr_reader :description
scalar_methods :name, :repeatable
children_methods(
Expand Down Expand Up @@ -382,17 +388,27 @@ class Field < AbstractNode
# @!attribute selections
# @return [Array<Nodes::Field>] 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`
Expand All @@ -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)
Expand Down Expand Up @@ -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,
Expand All @@ -596,7 +621,6 @@ class SchemaExtension < AbstractNode
end

class ScalarTypeDefinition < AbstractNode
include DefinitionNode
attr_reader :description
scalar_methods :name
children_methods({
Expand All @@ -614,7 +638,6 @@ class ScalarTypeExtension < AbstractNode
end

class InputValueDefinition < AbstractNode
include DefinitionNode
attr_reader :description
scalar_methods :name, :type, :default_value
children_methods({
Expand All @@ -624,7 +647,6 @@ class InputValueDefinition < AbstractNode
end

class FieldDefinition < AbstractNode
include DefinitionNode
attr_reader :description
scalar_methods :name, :type
children_methods({
Expand All @@ -645,7 +667,6 @@ def merge(new_options)
end

class ObjectTypeDefinition < AbstractNode
include DefinitionNode
attr_reader :description
scalar_methods :name, :interfaces
children_methods({
Expand All @@ -665,7 +686,6 @@ class ObjectTypeExtension < AbstractNode
end

class InterfaceTypeDefinition < AbstractNode
include DefinitionNode
attr_reader :description
scalar_methods :name
children_methods({
Expand All @@ -687,7 +707,6 @@ class InterfaceTypeExtension < AbstractNode
end

class UnionTypeDefinition < AbstractNode
include DefinitionNode
attr_reader :description, :types
scalar_methods :name
children_methods({
Expand All @@ -706,7 +725,6 @@ class UnionTypeExtension < AbstractNode
end

class EnumValueDefinition < AbstractNode
include DefinitionNode
attr_reader :description
scalar_methods :name
children_methods({
Expand All @@ -716,7 +734,6 @@ class EnumValueDefinition < AbstractNode
end

class EnumTypeDefinition < AbstractNode
include DefinitionNode
attr_reader :description
scalar_methods :name
children_methods({
Expand All @@ -736,7 +753,6 @@ class EnumTypeExtension < AbstractNode
end

class InputObjectTypeDefinition < AbstractNode
include DefinitionNode
attr_reader :description
scalar_methods :name
children_methods({
Expand Down
8 changes: 5 additions & 3 deletions lib/graphql/language/parser.rb
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions lib/graphql/language/parser.y
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down Expand Up @@ -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
2 changes: 1 addition & 1 deletion lib/graphql/language/recursive_descent_parser.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 0 additions & 3 deletions spec/graphql/language/recursive_descent_parser_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 545083f

Please sign in to comment.