[torque-ls] Port tests from cctest to unittest

Moving to gtest allows negative test cases as the current parser
implementation exits the process on a parser error. The CL adds two
small negative tests. The idea is less to get full coverage, but to
have a place for regression tests.

Drive-by-change: Lexer errors need a valid source position scope and
Json parser needs a valid SourceId, otherwise we read OOB when the
error message is generated.

R=petermarshall@chromium.org

Bug: v8:8880
Change-Id: I56c4b9e0a29c8333b2e5e44f8116e5178552d2f0
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1498472Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Commit-Queue: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60014}

BUILD.gn

@@ -3700,6 +3700,7 @@ if (is_component_build) {
 
     public_deps = [
       ":torque_base",
+      ":torque_ls_base",
       ":v8_base",
       ":v8_headers",
       ":v8_maybe_snapshot",
@@ -3728,6 +3729,7 @@ if (is_component_build) {
 
     public_deps = [
       ":torque_base",
+      ":torque_ls_base",
       ":v8_base",
       ":v8_maybe_snapshot",
     ]

src/torque/earley-parser.cc

@@ -126,6 +126,8 @@ LexerResult Lexer::RunLexer(const std::string& input) {
     InputPosition token_end = pos;
     line_column_tracker.Advance(token_start, token_end);
     if (!symbol) {
+      CurrentSourcePosition::Scope pos_scope(
+          line_column_tracker.ToSourcePosition());
       ReportError("Lexer Error: unknown token " +
                   StringLiteralQuote(std::string(
                       token_start, token_start + std::min<ptrdiff_t>(

src/torque/ls/json-parser.cc

@@ -183,7 +183,8 @@ class JsonGrammar : public Grammar {
 JsonValue ParseJson(const std::string& input) {
   // Torque needs a CurrentSourceFile scope during parsing.
   // As JSON lives in memory only, a unknown file scope is created.
-  CurrentSourceFile::Scope unkown_file(SourceId::Invalid());
+  SourceFileMap::Scope source_map_scope;
+  CurrentSourceFile::Scope unkown_file(SourceFileMap::AddSource("<json>"));
 
   return (*JsonGrammar().Parse(input)).Cast<JsonValue>();
 }

src/torque/ls/json-parser.h

@@ -5,6 +5,7 @@
 #ifndef V8_TORQUE_LS_JSON_PARSER_H_
 #define V8_TORQUE_LS_JSON_PARSER_H_
 
+#include "src/base/macros.h"
 #include "src/torque/ls/json.h"
 
 namespace v8 {
@@ -12,7 +13,7 @@ namespace internal {
 namespace torque {
 namespace ls {
 
-JsonValue ParseJson(const std::string& input);
+V8_EXPORT_PRIVATE JsonValue ParseJson(const std::string& input);
 
 }  // namespace ls
 }  // namespace torque

src/torque/ls/message-handler.h

@@ -5,6 +5,7 @@
 #ifndef V8_TORQUE_LS_MESSAGE_HANDLER_H_
 #define V8_TORQUE_LS_MESSAGE_HANDLER_H_
 
+#include "src/base/macros.h"
 #include "src/torque/ls/json.h"
 
 namespace v8 {
@@ -16,7 +17,7 @@ namespace ls {
 // To allow unit testing, the "sending" function is configurable.
 using MessageWriter = void (*)(JsonValue& message);
 
-void HandleMessage(JsonValue& raw_message, MessageWriter);
+V8_EXPORT_PRIVATE void HandleMessage(JsonValue& raw_message, MessageWriter);
 
 }  // namespace ls
 }  // namespace torque

src/torque/server-data.h

@@ -8,6 +8,7 @@
 #include <map>
 #include <vector>
 
+#include "src/base/macros.h"
 #include "src/base/optional.h"
 #include "src/torque/source-positions.h"
 
@@ -28,10 +29,11 @@ class LanguageServerData : public ContextualClass<LanguageServerData> {
  public:
   LanguageServerData() = default;
 
-  static void AddDefinition(SourcePosition token, SourcePosition definition);
+  V8_EXPORT_PRIVATE static void AddDefinition(SourcePosition token,
+                                              SourcePosition definition);
 
-  static base::Optional<SourcePosition> FindDefinition(SourceId source,
-                                                       LineAndColumn pos);
+  V8_EXPORT_PRIVATE static base::Optional<SourcePosition> FindDefinition(
+      SourceId source, LineAndColumn pos);
 
  private:
   DefinitionsMap definitions_map_;

src/torque/source-positions.h

@@ -67,6 +67,7 @@ class SourceFileMap : public ContextualClass<SourceFileMap> {
  public:
   SourceFileMap() = default;
   static const std::string& GetSource(SourceId source) {
+    CHECK(source.IsValid());
     return Get().sources_[source.id_];
   }
 

test/cctest/BUILD.gn

@@ -250,8 +250,6 @@ v8_source_set("cctest_sources") {
     "test-version.cc",
     "test-weakmaps.cc",
     "test-weaksets.cc",
-    "torque/test-torque-ls-json.cc",
-    "torque/test-torque-ls-message.cc",
     "trace-extension.cc",
     "trace-extension.h",
     "unicode-helpers.cc",
@@ -372,7 +370,6 @@ v8_source_set("cctest_sources") {
     ":cctest_headers",
     ":resources",
     "..:common_test_headers",
-    "../..:torque_ls_base",
     "../..:v8_initializers",
     "../..:v8_libbase",
     "../..:v8_libplatform",

test/unittests/BUILD.gn

@@ -198,6 +198,8 @@ v8_source_set("unittests_sources") {
     "test-utils.cc",
     "test-utils.h",
     "torque/earley-parser-unittest.cc",
+    "torque/ls-json-unittest.cc",
+    "torque/ls-message-unittest.cc",
     "torque/torque-unittest.cc",
     "unicode-unittest.cc",
     "utils-unittest.cc",

test/cctest/torque/test-torque-ls-json.cc → test/unittests/torque/ls-json-unittest.cc

@@ -4,90 +4,97 @@
 
 #include "src/torque/ls/json-parser.h"
 #include "src/torque/ls/json.h"
-#include "test/cctest/cctest.h"
+#include "src/torque/source-positions.h"
+#include "test/unittests/test-utils.h"
 
 namespace v8 {
 namespace internal {
 namespace torque {
 namespace ls {
 
-TEST(TestJsonPrimitives) {
+TEST(LanguageServerJson, TestJsonPrimitives) {
   const JsonValue true_result = ParseJson("true");
-  CHECK_EQ(true_result.tag, JsonValue::BOOL);
-  CHECK_EQ(true_result.ToBool(), true);
+  ASSERT_EQ(true_result.tag, JsonValue::BOOL);
+  EXPECT_EQ(true_result.ToBool(), true);
 
   const JsonValue false_result = ParseJson("false");
-  CHECK_EQ(false_result.tag, JsonValue::BOOL);
-  CHECK_EQ(false_result.ToBool(), false);
+  ASSERT_EQ(false_result.tag, JsonValue::BOOL);
+  EXPECT_EQ(false_result.ToBool(), false);
 
   const JsonValue null_result = ParseJson("null");
-  CHECK_EQ(null_result.tag, JsonValue::IS_NULL);
+  ASSERT_EQ(null_result.tag, JsonValue::IS_NULL);
 
   const JsonValue number = ParseJson("42");
-  CHECK_EQ(number.tag, JsonValue::NUMBER);
-  CHECK_EQ(number.ToNumber(), 42);
+  ASSERT_EQ(number.tag, JsonValue::NUMBER);
+  EXPECT_EQ(number.ToNumber(), 42);
 }
 
-TEST(TestJsonStrings) {
+TEST(LanguageServerJson, TestJsonStrings) {
   const JsonValue basic = ParseJson("\"basic\"");
-  CHECK_EQ(basic.tag, JsonValue::STRING);
-  CHECK_EQ(basic.ToString(), "basic");
+  ASSERT_EQ(basic.tag, JsonValue::STRING);
+  EXPECT_EQ(basic.ToString(), "basic");
 
   const JsonValue singleQuote = ParseJson("\"'\"");
-  CHECK_EQ(singleQuote.tag, JsonValue::STRING);
-  CHECK_EQ(singleQuote.ToString(), "'");
+  ASSERT_EQ(singleQuote.tag, JsonValue::STRING);
+  EXPECT_EQ(singleQuote.ToString(), "'");
 }
 
-TEST(TestJsonArrays) {
+TEST(LanguageServerJson, TestJsonArrays) {
   const JsonValue empty_array = ParseJson("[]");
-  CHECK_EQ(empty_array.tag, JsonValue::ARRAY);
-  CHECK_EQ(empty_array.ToArray().size(), 0);
+  ASSERT_EQ(empty_array.tag, JsonValue::ARRAY);
+  EXPECT_EQ(empty_array.ToArray().size(), (size_t)0);
 
   const JsonValue number_array = ParseJson("[1, 2, 3, 4]");
-  CHECK_EQ(number_array.tag, JsonValue::ARRAY);
+  ASSERT_EQ(number_array.tag, JsonValue::ARRAY);
 
   const JsonArray& array = number_array.ToArray();
-  CHECK_EQ(array.size(), 4);
-  CHECK_EQ(array[1].tag, JsonValue::NUMBER);
-  CHECK_EQ(array[1].ToNumber(), 2);
+  ASSERT_EQ(array.size(), (size_t)4);
+  ASSERT_EQ(array[1].tag, JsonValue::NUMBER);
+  EXPECT_EQ(array[1].ToNumber(), 2);
 
   const JsonValue string_array_object = ParseJson("[\"a\", \"b\"]");
-  CHECK_EQ(string_array_object.tag, JsonValue::ARRAY);
+  ASSERT_EQ(string_array_object.tag, JsonValue::ARRAY);
 
   const JsonArray& string_array = string_array_object.ToArray();
-  CHECK_EQ(string_array.size(), 2);
-  CHECK_EQ(string_array[1].tag, JsonValue::STRING);
-  CHECK_EQ(string_array[1].ToString(), "b");
+  ASSERT_EQ(string_array.size(), (size_t)2);
+  ASSERT_EQ(string_array[1].tag, JsonValue::STRING);
+  EXPECT_EQ(string_array[1].ToString(), "b");
 }
 
-TEST(TestJsonObjects) {
+TEST(LanguageServerJson, TestJsonObjects) {
   const JsonValue empty_object = ParseJson("{}");
-  CHECK_EQ(empty_object.tag, JsonValue::OBJECT);
-  CHECK_EQ(empty_object.ToObject().size(), 0);
+  ASSERT_EQ(empty_object.tag, JsonValue::OBJECT);
+  EXPECT_EQ(empty_object.ToObject().size(), (size_t)0);
 
   const JsonValue primitive_fields = ParseJson("{ \"flag\": true, \"id\": 5}");
-  CHECK_EQ(primitive_fields.tag, JsonValue::OBJECT);
+  EXPECT_EQ(primitive_fields.tag, JsonValue::OBJECT);
 
   const JsonValue& flag = primitive_fields.ToObject().at("flag");
-  CHECK_EQ(flag.tag, JsonValue::BOOL);
-  CHECK(flag.ToBool());
+  ASSERT_EQ(flag.tag, JsonValue::BOOL);
+  EXPECT_TRUE(flag.ToBool());
 
   const JsonValue& id = primitive_fields.ToObject().at("id");
-  CHECK_EQ(id.tag, JsonValue::NUMBER);
-  CHECK_EQ(id.ToNumber(), 5);
+  ASSERT_EQ(id.tag, JsonValue::NUMBER);
+  EXPECT_EQ(id.ToNumber(), 5);
 
   const JsonValue& complex_fields =
       ParseJson("{ \"array\": [], \"object\": { \"name\": \"torque\" } }");
-  CHECK_EQ(complex_fields.tag, JsonValue::OBJECT);
+  ASSERT_EQ(complex_fields.tag, JsonValue::OBJECT);
 
   const JsonValue& array = complex_fields.ToObject().at("array");
-  CHECK_EQ(array.tag, JsonValue::ARRAY);
-  CHECK_EQ(array.ToArray().size(), 0);
+  ASSERT_EQ(array.tag, JsonValue::ARRAY);
+  EXPECT_EQ(array.ToArray().size(), (size_t)0);
 
   const JsonValue& object = complex_fields.ToObject().at("object");
-  CHECK_EQ(object.tag, JsonValue::OBJECT);
-  CHECK_EQ(object.ToObject().at("name").tag, JsonValue::STRING);
-  CHECK_EQ(object.ToObject().at("name").ToString(), "torque");
+  ASSERT_EQ(object.tag, JsonValue::OBJECT);
+  ASSERT_EQ(object.ToObject().at("name").tag, JsonValue::STRING);
+  EXPECT_EQ(object.ToObject().at("name").ToString(), "torque");
+}
+
+TEST(LanguageServerJsonDeathTest, SyntaxError) {
+  ASSERT_DEATH(ParseJson("{]"), "Parser Error: unexpected token");
+  ASSERT_DEATH(ParseJson("{ noquoteskey: null }"),
+               "Lexer Error: unknown token");
 }
 
 }  // namespace ls

test/cctest/torque/test-torque-ls-message.cc → test/unittests/torque/ls-message-unittest.cc

@@ -7,14 +7,14 @@
 #include "src/torque/ls/message.h"
 #include "src/torque/server-data.h"
 #include "src/torque/source-positions.h"
-#include "test/cctest/cctest.h"
+#include "test/unittests/test-utils.h"
 
 namespace v8 {
 namespace internal {
 namespace torque {
 namespace ls {
 
-TEST(InitializeRequest) {
+TEST(LanguageServerMessage, InitializeRequest) {
   InitializeRequest request;
   request.set_id(5);
   request.set_method("initialize");
@@ -25,32 +25,33 @@ TEST(InitializeRequest) {
 
     // Check that the response id matches up with the request id, and that
     // the language server signals its support for definitions.
-    CHECK_EQ(response.id(), 5);
-    CHECK_EQ(response.result().capabilities().definitionProvider(), true);
+    EXPECT_EQ(response.id(), 5);
+    EXPECT_EQ(response.result().capabilities().definitionProvider(), true);
   });
 }
 
-TEST(RegisterDynamicCapabilitiesAfterInitializedNotification) {
+TEST(LanguageServerMessage,
+     RegisterDynamicCapabilitiesAfterInitializedNotification) {
   Request<bool> notification;
   notification.set_method("initialized");
 
   HandleMessage(notification.GetJsonValue(), [](JsonValue& raw_request) {
     RegistrationRequest request(raw_request);
 
-    CHECK_EQ(request.method(), "client/registerCapability");
-    CHECK_EQ(request.params().registrations_size(), 1);
+    ASSERT_EQ(request.method(), "client/registerCapability");
+    ASSERT_EQ(request.params().registrations_size(), (size_t)1);
 
     Registration registration = request.params().registrations(0);
-    CHECK_EQ(registration.method(), "workspace/didChangeWatchedFiles");
+    ASSERT_EQ(registration.method(), "workspace/didChangeWatchedFiles");
 
     auto options =
         registration
             .registerOptions<DidChangeWatchedFilesRegistrationOptions>();
-    CHECK_EQ(options.watchers_size(), 1);
+    ASSERT_EQ(options.watchers_size(), (size_t)1);
   });
 }
 
-TEST(GotoDefinitionUnkownFile) {
+TEST(LanguageServerMessage, GotoDefinitionUnkownFile) {
   SourceFileMap::Scope source_file_map_scope;
 
   GotoDefinitionRequest request;
@@ -60,12 +61,12 @@ TEST(GotoDefinitionUnkownFile) {
 
   HandleMessage(request.GetJsonValue(), [](JsonValue& raw_response) {
     GotoDefinitionResponse response(raw_response);
-    CHECK_EQ(response.id(), 42);
-    CHECK(response.IsNull("result"));
+    EXPECT_EQ(response.id(), 42);
+    EXPECT_TRUE(response.IsNull("result"));
   });
 }
 
-TEST(GotoDefinition) {
+TEST(LanguageServerMessage, GotoDefinition) {
   SourceFileMap::Scope source_file_map_scope;
   SourceId test_id = SourceFileMap::AddSource("test.tq");
   SourceId definition_id = SourceFileMap::AddSource("base.tq");
@@ -84,8 +85,8 @@ TEST(GotoDefinition) {
 
   HandleMessage(request.GetJsonValue(), [](JsonValue& raw_response) {
     GotoDefinitionResponse response(raw_response);
-    CHECK_EQ(response.id(), 42);
-    CHECK(response.IsNull("result"));
+    EXPECT_EQ(response.id(), 42);
+    EXPECT_TRUE(response.IsNull("result"));
   });
 
   // Second, check a known defintion.
@@ -98,15 +99,15 @@ TEST(GotoDefinition) {
 
   HandleMessage(request.GetJsonValue(), [](JsonValue& raw_response) {
     GotoDefinitionResponse response(raw_response);
-    CHECK_EQ(response.id(), 43);
-    CHECK(!response.IsNull("result"));
+    EXPECT_EQ(response.id(), 43);
+    ASSERT_FALSE(response.IsNull("result"));
 
     Location location = response.result();
-    CHECK_EQ(location.uri(), "file://base.tq");
-    CHECK_EQ(location.range().start().line(), 4);
-    CHECK_EQ(location.range().start().character(), 1);
-    CHECK_EQ(location.range().end().line(), 4);
-    CHECK_EQ(location.range().end().character(), 5);
+    EXPECT_EQ(location.uri(), "file://base.tq");
+    EXPECT_EQ(location.range().start().line(), 4);
+    EXPECT_EQ(location.range().start().character(), 1);
+    EXPECT_EQ(location.range().end().line(), 4);
+    EXPECT_EQ(location.range().end().character(), 5);
   });
 }