[torque] Collect lint errors for later processing

This CL changes lint errors to not be printed directly to stderr.
Instead, they are collected in a list that gets surfaced via
the TorqueCompilerResult. This is done so they can be presented
to language server clients.

This change also removes the "abort_on_lint_errors" option.
API users can now decide for themselves what to do, depending on
the presence of lint errors in the returned list.

R=sigurds@chromium.org, tebbi@chromium.org

Bug: v8:8880
Change-Id: I44601010491aafcf4c8609fd8c115219317506a4
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1581608Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60983}

src/torque/ls/message-handler.cc

@@ -125,7 +125,6 @@ void RecompileTorque(MessageWriter writer) {
   options.output_directory = "";
   options.verbose = false;
   options.collect_language_server_data = true;
-  options.abort_on_lint_errors = false;
 
   TorqueCompilerResult result = CompileTorque(TorqueFileList::Get(), options);
 

src/torque/torque-compiler.cc

@@ -83,14 +83,13 @@ void CompileCurrentAst(TorqueCompilerOptions options) {
       implementation_visitor.GenerateImplementation(output_directory, n);
     }
   }
-
-  if (LintErrorStatus::HasLintErrors()) std::abort();
 }
 
 TorqueCompilerResult CollectResultFromContextuals() {
   TorqueCompilerResult result;
   result.source_file_map = SourceFileMap::Get();
   result.language_server_data = LanguageServerData::Get();
+  result.lint_errors = LintErrors::Get();
   return result;
 }
 
@@ -108,7 +107,7 @@ TorqueCompilerResult CompileTorque(const std::string& source,
   SourceFileMap::Scope source_map_scope;
   CurrentSourceFile::Scope no_file_scope(SourceFileMap::AddSource("<torque>"));
   CurrentAst::Scope ast_scope;
-  LintErrorStatus::Scope lint_error_status_scope;
+  LintErrors::Scope lint_errors_scope;
   LanguageServerData::Scope server_data_scope;
 
   try {
@@ -126,7 +125,7 @@ TorqueCompilerResult CompileTorque(std::vector<std::string> files,
   SourceFileMap::Scope source_map_scope;
   CurrentSourceFile::Scope unknown_source_file_scope(SourceId::Invalid());
   CurrentAst::Scope ast_scope;
-  LintErrorStatus::Scope lint_error_status_scope;
+  LintErrors::Scope lint_errors_scope;
   LanguageServerData::Scope server_data_scope;
 
   try {

src/torque/torque-compiler.h

@@ -19,7 +19,6 @@ struct TorqueCompilerOptions {
   std::string output_directory;
   bool verbose;
   bool collect_language_server_data;
-  bool abort_on_lint_errors;
 };
 
 struct TorqueCompilerResult {
@@ -32,6 +31,10 @@ struct TorqueCompilerResult {
   // Set the corresponding options flag to enable.
   LanguageServerData language_server_data;
 
+  // Lint errors collected during compilation. These are
+  // mainly naming convention violations.
+  std::vector<LintError> lint_errors;
+
   // If any error occurred during either parsing or compilation,
   // this field will be set.
   base::Optional<TorqueError> error;

src/torque/torque-parser.cc

@@ -197,7 +197,7 @@ void CheckNotDeferredStatement(Statement* statement) {
   CurrentSourcePosition::Scope source_position(statement->pos);
   if (BlockStatement* block = BlockStatement::DynamicCast(statement)) {
     if (block->deferred) {
-      LintError(
+      ReportLintError(
           "cannot use deferred with a statement block here, it will have no "
           "effect");
     }

src/torque/torque.cc

@@ -33,20 +33,27 @@ int WrappedMain(int argc, const char** argv) {
   options.output_directory = output_directory;
   options.verbose = verbose;
   options.collect_language_server_data = false;
-  options.abort_on_lint_errors = true;
 
   TorqueCompilerResult result = CompileTorque(files, options);
-  if (result.error) {
-    // PositionAsString requires the SourceFileMap to be set to
-    // resolve the file name.
-    SourceFileMap::Scope source_file_map_scope(result.source_file_map);
 
+  // PositionAsString requires the SourceFileMap to be set to
+  // resolve the file name. Needed to report errors and lint warnings.
+  SourceFileMap::Scope source_file_map_scope(result.source_file_map);
+
+  if (result.error) {
     TorqueError& error = *result.error;
     if (error.position) std::cerr << PositionAsString(*error.position) << ": ";
     std::cerr << "Torque error: " << error.message << "\n";
     v8::base::OS::Abort();
   }
 
+  for (const LintError& error : result.lint_errors) {
+    std::cerr << PositionAsString(error.position)
+              << ": Lint error: " << error.message << "\n";
+  }
+
+  if (!result.lint_errors.empty()) v8::base::OS::Abort();
+
   return 0;
 }
 

src/torque/utils.cc

@@ -15,6 +15,8 @@ namespace v8 {
 namespace internal {
 namespace torque {
 
+DEFINE_CONTEXTUAL_VARIABLE(LintErrors)
+
 std::string StringLiteralUnquote(const std::string& s) {
   DCHECK(('"' == s.front() && '"' == s.back()) ||
          ('\'' == s.front() && '\'' == s.back()));
@@ -122,8 +124,6 @@ std::string CurrentPositionAsString() {
   return PositionAsString(CurrentSourcePosition::Get());
 }
 
-DEFINE_CONTEXTUAL_VARIABLE(LintErrorStatus)
-
 [[noreturn]] void ThrowTorqueError(const std::string& message,
                                    bool include_position) {
   TorqueError error(message);
@@ -131,9 +131,8 @@ DEFINE_CONTEXTUAL_VARIABLE(LintErrorStatus)
   throw error;
 }
 
-void LintError(const std::string& error) {
-  LintErrorStatus::SetLintError();
-  std::cerr << CurrentPositionAsString() << ": Lint error: " << error << "\n";
+void ReportLintError(const std::string& error) {
+  LintErrors::Get().push_back({error, CurrentSourcePosition::Get()});
 }
 
 void NamingConventionError(const std::string& type, const std::string& name,
@@ -141,7 +140,7 @@ void NamingConventionError(const std::string& type, const std::string& name,
   std::stringstream sstream;
   sstream << type << " \"" << name << "\" doesn't follow \"" << convention
           << "\" naming convention.";
-  LintError(sstream.str());
+  ReportLintError(sstream.str());
 }
 
 namespace {

src/torque/utils.h

@@ -28,18 +28,13 @@ std::string StringLiteralQuote(const std::string& s);
 V8_EXPORT_PRIVATE base::Optional<std::string> FileUriDecode(
     const std::string& s);
 
-class LintErrorStatus : public ContextualClass<LintErrorStatus> {
- public:
-  LintErrorStatus() : has_lint_errors_(false) {}
-
-  static bool HasLintErrors() { return Get().has_lint_errors_; }
-  static void SetLintError() { Get().has_lint_errors_ = true; }
-
- private:
-  bool has_lint_errors_;
+struct LintError {
+  std::string message;
+  SourcePosition position;
 };
+DECLARE_CONTEXTUAL_VARIABLE(LintErrors, std::vector<LintError>);
 
-void LintError(const std::string& error);
+void ReportLintError(const std::string& error);
 
 // Prints a LintError with the format "{type} '{name}' doesn't follow
 // '{convention}' naming convention".

test/unittests/torque/ls-server-data-unittest.cc

@@ -18,7 +18,6 @@ struct TestCompiler {
 
   void Compile(const std::string& source) {
     TorqueCompilerOptions options;
-    options.abort_on_lint_errors = false;
     options.output_directory = "";
     options.verbose = false;
     options.collect_language_server_data = true;

test/unittests/torque/torque-unittest.cc

@@ -2,8 +2,10 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "src/torque/torque-compiler.h"
 #include "src/torque/utils.h"
 #include "test/unittests/test-utils.h"
+#include "testing/gmock-support.h"
 
 namespace v8 {
 namespace internal {
@@ -16,6 +18,26 @@ TEST(Torque, StackDeleteRange) {
   ASSERT_TRUE(stack == result);
 }
 
+using ::testing::HasSubstr;
+TEST(Torque, TypeNamingConventionLintError) {
+  std::string source = R"(
+    type void;
+    type never;
+
+    type foo generates 'TNode<Foo>';
+  )";
+
+  TorqueCompilerOptions options;
+  options.output_directory = "";
+  options.verbose = false;
+  options.collect_language_server_data = false;
+
+  const TorqueCompilerResult result = CompileTorque(source, options);
+
+  ASSERT_EQ(result.lint_errors.size(), static_cast<size_t>(1));
+  EXPECT_THAT(result.lint_errors[0].message, HasSubstr("\"foo\""));
+}
+
 }  // namespace torque
 }  // namespace internal
 }  // namespace v8