Skip to content

Commit

Permalink
Report Prism parse errors
Browse files Browse the repository at this point in the history
  • Loading branch information
st0012 committed Nov 4, 2024
1 parent 6eb5a76 commit 3612c21
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 9 deletions.
3 changes: 2 additions & 1 deletion main/pipeline/pipeline.cc
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,8 @@ unique_ptr<parser::Node> runPrismParser(core::GlobalState &gs, core::FileRef fil
return std::unique_ptr<parser::Node>();
}

auto nodes = Prism::Translator(parser, gs, file).translate(std::move(root));
auto errors = parser.errors();
auto nodes = Prism::Translator(parser, errors, gs, file).translate(std::move(root));

if (print.ParseTree.enabled) {
print.ParseTree.fmt("{}\n", nodes->toStringWithTabs(gs, 0));
Expand Down
18 changes: 18 additions & 0 deletions parser/prism/Parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,22 @@ std::string_view Parser::extractString(pm_string_t *string) {
return std::string_view(reinterpret_cast<const char *>(pm_string_source(string)), pm_string_length(string));
}

std::vector<ParseError> Parser::errors() const {
std::vector<ParseError> errors;

pm_list_node_t *node = storage->parser.error_list.head;

while (node != nullptr) {
pm_diagnostic_t *error = reinterpret_cast<pm_diagnostic_t *>(node);
pm_error_level_t level = static_cast<pm_error_level_t>(error->level);

ParseError parseError(pm_diagnostic_id_human(error->diag_id),
std::string(reinterpret_cast<const char *>(error->message)), error->location, level);

errors.push_back(parseError);
node = node->next;
}

return errors;
}
}; // namespace sorbet::parser::Prism
14 changes: 14 additions & 0 deletions parser/prism/Parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include <memory>
#include <string>
#include <vector>

extern "C" {
#include "prism.h"
Expand All @@ -14,6 +15,17 @@ namespace sorbet::parser::Prism {

class Node;

class ParseError {
public:
ParseError(const char *id, const std::string &message, pm_location_t location, pm_error_level_t level)
: id(id), message(message), location(location), level(level) {}

const char *id;
std::string message;
pm_location_t location;
pm_error_level_t level;
};

// A backing implemenation detail of `Parser`, which stores a Prism parser and its options in a single allocation.
struct ParserStorage {
// The version of Ruby syntax that we're parsing with Prism. This determines what syntax is supported or not.
Expand Down Expand Up @@ -55,6 +67,8 @@ class Parser final {
std::string_view resolveConstant(pm_constant_id_t constant_id);
std::string_view extractString(pm_string_t *string);

std::vector<ParseError> errors() const;

private:
pm_parser_t *get_raw_parser_pointer();
};
Expand Down
9 changes: 7 additions & 2 deletions parser/prism/Translator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1207,7 +1207,12 @@ unique_ptr<parser::Node> Translator::translate(pm_node_t *node) {

case PM_IMPLICIT_NODE:
case PM_MISSING_NODE:
reportError(location, "unexpected token");
// For now, we only report errors when we hit a missing node because we don't want to always report dynamic
// constant assignment errors
// TODO: We will improve this in the future when we handle more errored cases
for (auto &error : parseErrors) {
reportError(translateLoc(error.location), error.message);
}
return make_unique<parser::Const>(location, nullptr, core::Names::Constants::ErrorNode());
}
}
Expand Down Expand Up @@ -1759,7 +1764,7 @@ template <typename PrismNode> std::unique_ptr<parser::Mlhs> Translator::translat
// Context management methods
Translator Translator::enterMethodDef() {
auto isInMethodDef = true;
return Translator(parser, gs, file, isInMethodDef);
return Translator(parser, parseErrors, gs, file, isInMethodDef);
}

void Translator::reportError(core::LocOffsets loc, const std::string &message) {
Expand Down
11 changes: 7 additions & 4 deletions parser/prism/Translator.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ namespace sorbet::parser::Prism {
class Translator final {
Parser parser;

std::vector<ParseError> parseErrors;

// The functions in Pipeline.cc pass around a reference to the global state as a parameter,
// but don't have explicit ownership over it. We take a temporary reference to it, but we can't
// escape that scope, which is why Translator objects can't be copied, or even moved.
Expand All @@ -32,16 +34,17 @@ class Translator final {
Translator &operator=(Translator &&) = delete; // Move assignment
Translator &operator=(const Translator &) = delete; // Copy assignment
public:
Translator(Parser parser, core::GlobalState &gs, core::FileRef file)
: parser(std::move(parser)), gs(gs), file(file) {}
Translator(Parser parser, std::vector<ParseError> parseErrors, core::GlobalState &gs, core::FileRef file)
: parser(std::move(parser)), parseErrors(parseErrors), gs(gs), file(file) {}

// Translates the given AST from Prism's node types into the equivalent AST in Sorbet's legacy parser node types.
std::unique_ptr<parser::Node> translate(pm_node_t *node);
std::unique_ptr<parser::Node> translate(const Node &node);

private:
Translator(Parser parser, core::GlobalState &gs, core::FileRef file, bool isInMethodDef)
: parser(parser), gs(gs), file(file), isInMethodDef(isInMethodDef) {}
Translator(Parser parser, std::vector<ParseError> parseErrors, core::GlobalState &gs, core::FileRef file,
bool isInMethodDef)
: parser(parser), parseErrors(parseErrors), gs(gs), file(file), isInMethodDef(isInMethodDef) {}
void reportError(core::LocOffsets loc, const std::string &message);

core::LocOffsets translateLoc(pm_location_t loc);
Expand Down
5 changes: 3 additions & 2 deletions test/prism_regression/error_recovery/assign.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# typed: false
# Should still see at least method def (not body)
def test_bad_assign(x)
x = # error: unexpected token
end
x =
# ^ error: expected an expression after `=`
end # error: unexpected 'end', assuming it is closing the parent method definition

0 comments on commit 3612c21

Please sign in to comment.