Update error tests, improve Parser lifecycle and organization, fix mo…

…re SDL

benchmark/run.rb

@@ -37,19 +37,19 @@ def self.run(task)
         x.report("validate - big query") { BIG_SCHEMA.validate(BIG_QUERY) }
         x.report("validate - fields will merge") { FIELDS_WILL_MERGE_SCHEMA.validate(FIELDS_WILL_MERGE_QUERY) }
       when "scan"
-        x.report("scan c - introspection") { GraphQL::Language::CLexer.tokenize(QUERY_STRING) }
-        x.report("scan - introspection") { GraphQL.scan(QUERY_STRING) }
-        x.report("scan c - fragments") { GraphQL::Language::CLexer.tokenize(ABSTRACT_FRAGMENTS_2_QUERY_STRING) }
-        x.report("scan - fragments") { GraphQL.scan(ABSTRACT_FRAGMENTS_2_QUERY_STRING) }
-        x.report("scan c - big query") { GraphQL::Language::CLexer.tokenize(BIG_QUERY_STRING) }
-        x.report("scan - big query") { GraphQL.scan(BIG_QUERY_STRING) }
+        x.report("scan c - introspection") { GraphQL.scan_with_c(QUERY_STRING) }
+        x.report("scan - introspection") { GraphQL.scan_with_ruby(QUERY_STRING) }
+        x.report("scan c - fragments") { GraphQL.scan_with_c(ABSTRACT_FRAGMENTS_2_QUERY_STRING) }
+        x.report("scan - fragments") { GraphQL.scan_with_ruby(ABSTRACT_FRAGMENTS_2_QUERY_STRING) }
+        x.report("scan c - big query") { GraphQL.scan_with_c(BIG_QUERY_STRING) }
+        x.report("scan - big query") { GraphQL.scan_with_ruby(BIG_QUERY_STRING) }
       when "parse"
         x.report("parse c - introspection") { GraphQL.parse_with_c(QUERY_STRING) }
-        x.report("parse - introspection") { GraphQL.parse(QUERY_STRING) }
+        x.report("parse - introspection") { GraphQL.parse_with_racc(QUERY_STRING) }
         x.report("parse c - fragments") { GraphQL.parse_with_c(ABSTRACT_FRAGMENTS_2_QUERY_STRING) }
-        x.report("parse - fragments") { GraphQL.parse(ABSTRACT_FRAGMENTS_2_QUERY_STRING) }
+        x.report("parse - fragments") { GraphQL.parse_with_racc(ABSTRACT_FRAGMENTS_2_QUERY_STRING) }
         x.report("parse c - big query") { GraphQL.parse_with_c(BIG_QUERY_STRING) }
-        x.report("parse - big query") { GraphQL.parse(BIG_QUERY_STRING) }
+        x.report("parse - big query") { GraphQL.parse_with_racc(BIG_QUERY_STRING) }
       else
         raise("Unexpected task #{task}")
       end

graphql-c_parser/ext/graphql_c_parser_ext/graphql_c_parser_ext.c

@@ -1,40 +1,22 @@
 #include "graphql_c_parser_ext.h"
 
-VALUE GraphQL_Language_CLexer_tokenize(VALUE self, VALUE query_string) {
+VALUE GraphQL_CParser_Lexer_tokenize(VALUE self, VALUE query_string) {
   return tokenize(query_string);
 }
 
-VALUE call_tokenize(VALUE yield_arg, VALUE query_string, int argc, VALUE* argv, VALUE block_arg) {
-  return tokenize(query_string);
-}
-
-VALUE call_yyparse(VALUE yield_arg, VALUE parser, int argc, VALUE* argv, VALUE block_arg) {
-  yyparse(parser);
-  return rb_ivar_get(parser, rb_intern("@result"));
-}
-
-VALUE GraphQL_Language_CParser_parse(VALUE self, VALUE query_string, VALUE trace) {
-  VALUE opts = rb_hash_new();
-  rb_hash_aset(opts, ID2SYM(rb_intern("query_string")), query_string);
-  VALUE argv[] = {opts};
-  VALUE tokens = rb_block_call_kw(trace, rb_intern("lex"), 1, argv, call_tokenize, query_string, RB_PASS_KEYWORDS);
-
-  VALUE parser = rb_class_new(self);
-  rb_ivar_set(parser, rb_intern("@query_string"), query_string);
-  rb_ivar_set(parser, rb_intern("@tokens"), tokens);
-  rb_ivar_set(parser, rb_intern("@next_token_index"), INT2FIX(0));
-  rb_ivar_set(parser, rb_intern("@result"), Qnil);
-  return rb_block_call_kw(trace, rb_intern("parse"), 1, argv, call_yyparse, parser, RB_PASS_KEYWORDS);
+VALUE GraphQL_CParser_Parser_c_parse(VALUE self) {
+  yyparse(self);
+  return Qnil;
 }
 
 void Init_graphql_c_parser_ext() {
   VALUE GraphQL = rb_define_module("GraphQL");
-  VALUE Language = rb_define_module_under(GraphQL, "Language");
-  VALUE CLexer = rb_define_module_under(Language, "CLexer");
-  rb_define_singleton_method(CLexer, "tokenize", GraphQL_Language_CLexer_tokenize, 1);
+  VALUE CParser = rb_define_module_under(GraphQL, "CParser");
+  VALUE Lexer = rb_define_module_under(CParser, "Lexer");
+  rb_define_singleton_method(Lexer, "tokenize", GraphQL_CParser_Lexer_tokenize, 1);
   setup_static_token_variables();
 
-  VALUE CParser = rb_define_class_under(Language, "CParser", rb_cObject);
-  rb_define_singleton_method(CParser, "parse", GraphQL_Language_CParser_parse, 2);
+  VALUE Parser = rb_define_class_under(CParser, "Parser", rb_cObject);
+  rb_define_method(Parser, "c_parse", GraphQL_CParser_Parser_c_parse, 0);
   initialize_node_class_variables();
 }

graphql-c_parser/ext/graphql_c_parser_ext/parser.y

@@ -308,7 +308,7 @@ SETUP_NODE_CLASS_VARIABLE(SchemaDefinition)
   }
 
   object_value:
-    | LCURLY object_value_list_opt RCURLY {
+    LCURLY object_value_list_opt RCURLY {
       $$ = rb_funcall(GraphQL_Language_Nodes_InputObject, rb_intern("from_a"), 3,
         rb_ary_entry($1, 1),
         rb_ary_entry($1, 2),
@@ -743,20 +743,14 @@ int yylex (YYSTYPE *lvalp, VALUE parser) {
 }
 
 void yyerror(VALUE parser, const char *msg) {
-  VALUE next_token_idx = rb_ivar_get(parser, rb_intern("@next_token_index"));
-  int this_token_idx = FIX2INT(next_token_idx) - 1;
-  VALUE tokens = rb_ivar_get(parser, rb_intern("@tokens"));
-  VALUE this_token = rb_ary_entry(tokens, this_token_idx);
   VALUE mGraphQL = rb_const_get_at(rb_cObject, rb_intern("GraphQL"));
-  VALUE cParseError = rb_const_get_at(mGraphQL, rb_intern("ParseError"));
+  VALUE mCParser = rb_const_get_at(mGraphQL, rb_intern("CParser"));
+  VALUE rb_message = rb_str_new_cstr(msg);
   VALUE exception = rb_funcall(
-      cParseError, rb_intern("new"), 4,
-      rb_str_new_cstr(msg),
-      rb_ary_entry(this_token, 1),
-      rb_ary_entry(this_token, 2),
-      rb_ivar_get(parser, rb_intern("@query_string"))
+      mCParser, rb_intern("prepare_parse_error"), 2,
+      rb_message,
+      parser
   );
-
   rb_exc_raise(exception);
 }
 

graphql-c_parser/lib/graphql/c_parser.rb

@@ -6,9 +6,75 @@
 
 module GraphQL
   module CParser
+    def self.parse(query_str, filename, trace)
+      parser = Parser.new(query_str, filename, trace)
+      parser.result
+    end
+
+    def self.prepare_parse_error(message, parser)
+      token = parser.tokens[parser.next_token_index - 1]
+      line = token[1]
+      col = token[2]
+      if line && col
+        location_str = " at [#{line}, #{col}]"
+        if !message.include?(location_str)
+          message += location_str
+        end
+      end
+
+      message.sub!(/, unexpected ([a-zA-Z ]+),/, ", unexpected \\1 (#{token[3].inspect}),")
+
+      GraphQL::ParseError.new(message, line, col, parser.query_string, filename: parser.filename)
+    end
+
+    class Parser
+      def initialize(query_string, filename, trace)
+        @query_string = query_string
+        @filename = filename
+        @tokens = nil
+        @next_token_index = 0
+        @result = nil
+        @trace = trace
+      end
+
+      def result
+        if @result.nil?
+          @tokens = @trace.lex(query_string: @query_string) do
+            GraphQL::CParser::Lexer.tokenize(@query_string)
+          end
+          @trace.parse(query_string: @query_string) do
+            c_parse
+            @result
+          end
+        end
+        @result
+      end
+
+      attr_reader :tokens, :next_token_index, :query_string, :filename
+    end
   end
 
   def self.scan_with_c(graphql_string)
-    GraphQL::Language::CLexer.tokenize(graphql_string)
+    GraphQL::CParser::Lexer.tokenize(graphql_string)
+  end
+
+  def self.parse_with_c(string, filename: nil, trace: GraphQL::Tracing::NullTrace)
+    if string.nil?
+      raise GraphQL::ParseError.new("No query string was present", nil, nil, string)
+    end
+    document = GraphQL::CParser.parse(string, filename, trace)
+    if document.definitions.size == 0
+      raise GraphQL::ParseError.new("Unexpected end of document", 1, 1, string)
+    end
+    document
+  end
+
+  # Call CParser implementations by default:
+  def self.scan(str)
+    scan_with_c(str)
+  end
+
+  def self.parse(*args, **kwargs)
+    parse_with_c(*args, **kwargs)
   end
 end

lib/graphql.rb

@@ -67,24 +67,6 @@ def self.scan_with_ruby(graphql_string)
     GraphQL::Language::Lexer.tokenize(graphql_string)
   end
 
-  # TODO move this to Cparser gem
-  def self.scan_with_c(graphql_string)
-    GraphQL::Language::CLexer.tokenize(graphql_string)
-  end
-
-  # TODO move this to Cparser Gem
-  def self.parse_with_c(string, filename: nil, trace: GraphQL::Tracing::NullTrace)
-    # TODO handle other arguments here
-    if string.nil?
-      raise GraphQL::ParseError.new("No query string was present", nil, nil, string)
-    end
-    document = GraphQL::Language::CParser.parse(string, trace)
-    if document.definitions.size == 0
-      raise GraphQL::ParseError.new("Unexpected end of document", 1, 1, string)
-    end
-    document
-  end
-
   NOT_CONFIGURED = Object.new
   private_constant :NOT_CONFIGURED
 end

lib/graphql/parse_error.rb

@@ -7,13 +7,6 @@ def initialize(message, line, col, query, filename: nil)
         message += " (#{filename})"
       end
 
-      if line && col && query
-        message += "\n\n"
-        line_txt = query.split("\n", line + 1)[line - 1]
-        message += "\n    #{line_txt}"
-        message += "\n    #{" " * (col - 1)}^ at #{line}:#{col}\n"
-      end
-
       super(message)
       @line = line
       @col = col

spec/graphql/language/clexer_spec.rb

@@ -3,8 +3,8 @@
 require "graphql/c_parser"
 require_relative "./lexer_examples"
 
-describe GraphQL::Language::CLexer do
-  subject { GraphQL::Language::CLexer }
+describe GraphQL::CParser::Lexer do
+  subject { GraphQL::CParser::Lexer }
 
   it "makes tokens like the other lexer" do
     str = "{ f1(arg: \"str\") ...F2 }\nfragment F2 on SomeType { f2 }"

spec/graphql/language/cparser_spec.rb

@@ -3,10 +3,10 @@
 require "graphql/c_parser"
 
 # TODO all parsing tests here
-describe GraphQL::Language::CParser do
+describe GraphQL::CParser::Parser do
   it "does something" do
     # pp GraphQL::Language::CParser.parse("{ }")
-    pp GraphQL::Language::CParser.parse("{ a b: c d(e: \"F\", g: HIJ, k: 1, l: 2.3, m: $M, n: null, o: { p: $Q }) }")
+    pp GraphQL::CParser::Parser.parse("{ a b: c d(e: \"F\", g: HIJ, k: 1, l: 2.3, m: $M, n: null, o: { p: $Q }) }")
     # pp GraphQL::Language::CParser.parse("{ a @stuff(things: OK) }")
     # pp GraphQL::Language::CParser.parse("{ a { b c ... F } } fragment F on T { q }")
 

spec/graphql/language/parser_spec.rb

@@ -16,11 +16,13 @@
     err = assert_raises(GraphQL::ParseError) {
       GraphQL.parse('query 😘 { a b }')
     }
-    if ENV["GRAPHQL_CLEXER"]
-      assert_equal "Parse error on \"\\xF0\" (error) at [1, 7]", err.message
+    expected_msg = if USING_C_PARSER
+      "syntax error, unexpected invalid token (\"\\xF0\"), expecting LCURLY at [1, 7]"
     else
-      assert_equal "Parse error on \"😘\" (error) at [1, 7]", err.message
+      "Parse error on \"😘\" (error) at [1, 7]"
     end
+
+    assert_equal expected_msg, err.message
   end
 
   describe "anonymous fragment extension" do
@@ -300,7 +302,7 @@
       query_with_malformed_argument_value = '{ node(id:) { name } }'
 
       assert_raises(GraphQL::ParseError) {
-        subject.parse(query_with_malformed_argument_value)
+        pp subject.parse(query_with_malformed_argument_value)
       }
     end
   end

spec/graphql/schema/non_null_spec.rb

@@ -45,8 +45,13 @@
         find(id: $id) { __typename }
       }
       "
+      expected_err = if USING_C_PARSER
+        "syntax error, unexpected BANG, expecting RPAREN or VAR_SIGN at [2, 21]"
+      else
+        'Parse error on "!" (BANG) at [2, 21]'
+      end
 
-      assert_equal ['Parse error on "!" (BANG) at [2, 21]'], res["errors"].map { |e| e["message"] }
+      assert_equal [expected_err], res["errors"].map { |e| e["message"] }
     end
   end
 

spec/spec_helper.rb

@@ -8,18 +8,12 @@
 ENV["BACKTRACE"] = "1"
 
 require "graphql"
-if ENV["GRAPHQL_CLEXER"]
-  puts "Opting in to GraphQL::Language::CLexer"
+if ENV["GRAPHQL_CPARSER"]
+  USING_C_PARSER = true
+  puts "Opting in to GraphQL::CParser"
   require "graphql-c_parser"
-  module GraphQL
-    def self.scan(str)
-      scan_with_c(str)
-    end
-
-    def self.parse(*args, **kwargs)
-      parse_with_c(*args, **kwargs)
-    end
-  end
+else
+  USING_C_PARSER = false
 end
 
 require "rake"