wip

core/ErrorQueue.cc

@@ -8,6 +8,21 @@ namespace sorbet::core {
 
 using namespace std;
 
+ErrorQueueMessage ErrorQueueMessage::clone() {
+    ErrorQueueMessage newMsg;
+
+    newMsg.kind = this->kind;
+    newMsg.whatFile = this->whatFile;
+    newMsg.text = this->text;
+    if (this->error) {
+        newMsg.error = make_unique<Error>(*this->error);
+    }
+    if (this->queryResponse) {
+        newMsg.queryResponse = make_unique<lsp::QueryResponse>(*this->queryResponse);
+    }
+    return newMsg;
+}
+
 ErrorQueue::ErrorQueue(spdlog::logger &logger, spdlog::logger &tracer, shared_ptr<ErrorFlusher> errorFlusher)
     : errorFlusher(errorFlusher), owner(this_thread::get_id()), logger(logger), tracer(tracer){};
 
@@ -32,7 +47,6 @@ bool ErrorQueue::wouldFlushErrorsForFile(FileRef file) const {
 void ErrorQueue::flushErrorsForFile(const GlobalState &gs, FileRef file) {
     checkOwned();
 
-    filesFlushedCount.fetch_add(1);
     Timer timeit(tracer, "ErrorQueue::flushErrorsForFile");
 
     core::ErrorQueueMessage msg;
@@ -43,6 +57,27 @@ void ErrorQueue::flushErrorsForFile(const GlobalState &gs, FileRef file) {
     errorFlusher->flushErrors(logger, gs, file, move(collected[file]));
 }
 
+void ErrorQueue::flushButRetainErrorsForFile(const GlobalState &gs, FileRef file) {
+    checkOwned();
+
+    Timer timeit(tracer, "ErrorQueue::flushButRetainErrorsForFile");
+
+
+    core::ErrorQueueMessage msg;
+    for (auto result = queue.try_pop(msg); result.gotItem(); result = queue.try_pop(msg)) {
+        if (!result.gotItem()) {
+            continue;
+        }
+        collected[msg.whatFile].emplace_back(make_unique<ErrorQueueMessage>(move(msg)));
+    }
+
+    std::vector<std::unique_ptr<ErrorQueueMessage>> toFlush;
+    for (auto &errMsg : collected[file]) {
+        toFlush.emplace_back(make_unique<ErrorQueueMessage>(errMsg.get()->clone()));
+    }
+    errorFlusher->flushErrors(logger, gs, file, move(toFlush));
+};
+
 void ErrorQueue::pushError(const core::GlobalState &gs, unique_ptr<core::Error> error) {
     core::ErrorQueueMessage msg;
     msg.kind = core::ErrorQueueMessage::Kind::Error;

core/ErrorQueue.h

@@ -26,7 +26,6 @@ class ErrorQueue {
     spdlog::logger &tracer;
     std::atomic<bool> hadCritical{false};
     std::atomic<int> nonSilencedErrorCount{0};
-    std::atomic<int> filesFlushedCount{0};
 
     ErrorQueue(spdlog::logger &logger, spdlog::logger &tracer,
                std::shared_ptr<ErrorFlusher> errorFlusher = std::make_shared<ErrorFlusherStdout>());
@@ -40,6 +39,9 @@ class ErrorQueue {
     void flushErrorsForFile(const GlobalState &gs, FileRef file);
     bool wouldFlushErrorsForFile(FileRef file) const;
 
+    /** reports errors, but doesn't remove them from internal storage, so they can be re-reported later*/
+    void flushButRetainErrorsForFile(const GlobalState &gs, FileRef file);
+
     /** Checks if the queue is empty. Is approximate if there are any concurrent dequeue/enqueue operations */
     bool queueIsEmptyApprox() const;
 };

core/ErrorQueueMessage.h

@@ -15,6 +15,8 @@ struct ErrorQueueMessage {
     std::optional<std::string> text;
     std::unique_ptr<Error> error;
     std::unique_ptr<lsp::QueryResponse> queryResponse;
+
+    ErrorQueueMessage clone();
 };
 
 } // namespace sorbet::core

main/lsp/LSPTypechecker.cc

@@ -315,6 +315,7 @@ vector<core::FileRef> LSPTypechecker::runFastPath(LSPFileUpdates &updates, Worke
         shouldRunIncrementalNamer
             ? pipeline::incrementalResolve(*gs, move(updatedIndexed), std::move(oldFoundHashesForFiles), config->opts)
             : pipeline::incrementalResolve(*gs, move(updatedIndexed), nullopt, config->opts);
+
     auto sorted = sortParsedFiles(*gs, *errorReporter, move(resolved));
     const auto presorted = true;
     const auto cancelable = false;
@@ -389,6 +390,7 @@ bool LSPTypechecker::copyIndexed(WorkerPool &workers, const UnorderedSet<int> &i
     return !epochManager.wasTypecheckingCanceled();
 }
 
+// TODO(iz) add extra flushes
 bool LSPTypechecker::runSlowPath(LSPFileUpdates updates, WorkerPool &workers,
                                  shared_ptr<core::ErrorFlusher> errorFlusher, bool cancelable) {
     ENFORCE(this_thread::get_id() == typecheckerThreadId,

main/pipeline/pipeline.cc

@@ -275,6 +275,9 @@ incrementalResolve(core::GlobalState &gs, vector<ast::ParsedFile> what,
             ENFORCE(result.hasResult());
             what = move(result.result());
 
+            for (auto &file : what) {
+                gs.errorQueue->flushButRetainErrorsForFile(gs, file.file);
+            }
             // Required for autogen tests, which need to control which phase to stop after.
             if (opts.stopAfterPhase == options::Phase::NAMER) {
                 return what;
@@ -292,6 +295,9 @@ incrementalResolve(core::GlobalState &gs, vector<ast::ParsedFile> what,
             ENFORCE(result.hasResult());
             what = move(result.result());
 
+            for (auto &file : what) {
+                gs.errorQueue->flushButRetainErrorsForFile(gs, file.file);
+            }
             // Required for autogen tests, which need to control which phase to stop after.
             if (opts.stopAfterPhase == options::Phase::RESOLVER) {
                 return what;
@@ -870,6 +876,9 @@ ast::ParsedFilesOrCancelled resolve(unique_ptr<core::GlobalState> &gs, vector<as
         }
         what = move(result.result());
 
+        for (auto &file : what) {
+            gs->errorQueue->flushButRetainErrorsForFile(*gs, file.file);
+        }
         for (auto &named : what) {
             if (opts.print.NameTree.enabled) {
                 opts.print.NameTree.fmt("{}\n", named.tree.toStringWithTabs(*gs, 0));
@@ -899,6 +908,9 @@ ast::ParsedFilesOrCancelled resolve(unique_ptr<core::GlobalState> &gs, vector<as
             }
 #endif
 
+            for (auto &file : what) {
+                gs->errorQueue->flushButRetainErrorsForFile(*gs, file.file);
+            }
             if (opts.stressIncrementalResolver) {
                 auto symbolsBefore = gs->symbolsUsedTotal();
                 for (auto &f : what) {
@@ -1045,9 +1057,6 @@ void typecheck(const core::GlobalState &gs, vector<ast::ParsedFile> what, const
                WorkerPool &workers, bool cancelable,
                optional<shared_ptr<core::lsp::PreemptionTaskManager>> preemptionManager, bool presorted,
                bool intentionallyLeakASTs) {
-    // Unless the error queue had a critical error, only typecheck should flush errors to the client, otherwise we will
-    // drop errors in LSP mode.
-    ENFORCE(gs.hadCriticalError() || gs.errorQueue->filesFlushedCount == 0);
 
     const auto &epochManager = *gs.epochManager;
     // Record epoch at start of typechecking before any preemption occurs.
@@ -1170,9 +1179,6 @@ void typecheck(const core::GlobalState &gs, vector<ast::ParsedFile> what, const
             extension->finishTypecheck(gs);
         }
 
-        // Error queue is re-used across runs, so reset the flush count to ignore files flushed during typecheck.
-        gs.errorQueue->filesFlushedCount = 0;
-
         return;
     }
 }