jamboree / bustache

C++20 implementation of {{ mustache }}
82 stars 10 forks source link

Add line/column info to `format_error` #16

Closed mbeutel closed 5 years ago

mbeutel commented 5 years ago

The following commit adds line and column info to the format_error class, which makes locating errors in templates much easier.

https://github.com/mbeutel/bustache/commit/50d090450a386f64460cdc6da599911aad85ab71

Using the additional info, one can generate a helpful error message from a format_error as follows:

[[noreturn]] static void throwParseError(const std::filesystem::path& file, const bustache::format_error& error)
{
        // we use 1-based indexing here
    auto line = error.line() + 1,
         column = error.column() + 1;

    std::ostringstream sstr;
    sstr << "while parsing file '" << minunicode::path_to_utf8(file) << "':\n";
    auto indent = std::string_view{ "    " };
    auto curIndent = std::string(indent);
    if (line > 0 && column > 0)
    {
        sstr << curIndent;
        if (multiline)
            sstr << "in line " << line << ", column ";
        else
            sstr << "in column ";
        sstr << column << ":\n";
        curIndent += std::string(indent);
    }
    sstr << curIndent << bustache::error_type_to_message(error.code());
    std::throw_with_nested(std::runtime_error(sstr.str()));
}

The implementation approach is probably not ideal; I opted to be minimally invasive because I didn't want to change too many function signatures. Also, the error message returned by format_error::what() currently remains unchanged, and I added a slightly redundant error_type_to_message() function which omits the non-human-readable parts of the error message. A better approach might be to pass through the start of the range to all parsing functions and to generate a more helpful error message at the throw site rather than catching the plain error code and re-throwing another exception.

How would you like this to be implemented in order to be eligible for merging? Do you want format_error::what() to remain unchanged, or would it be acceptable to have the error message include line and column info?

jamboree commented 5 years ago

Bustache wasn't designed to be a lint, so I didn't consider rich error info. Probably the best we can do is to add a format_error::position() which returns the offset from the source, and let the users compute col/row and format the error msg themselves. I think the non-human-readable parts can be removed from format_error::what() (I probably mimics the MSVC STL style).

mbeutel commented 5 years ago

The pull request implements format_error::position() as you proposed.

Just in case anyone else needs this, I now use the following wrappers to augment formatting errors with line/column info by nesting exceptions:

// bustache-format.hpp

#ifndef INCLUDED_BUSTACHE_FORMAT_HPP_
#define INCLUDED_BUSTACHE_FORMAT_HPP_

#include <cstddef>   // for size_t, ptrdiff_t
#include <utility>   // for forward<>()
#include <stdexcept> // for runtime_error

#include <bustache/format.hpp>

enum class FormatTemplateType
{
    string,
    multilineString
};

class BustacheFormatError : public std::runtime_error
{
private:
    bustache::format_error error_;
    std::ptrdiff_t line_;
    std::ptrdiff_t column_;

public:
    BustacheFormatError(const bustache::format_error& _error);
    BustacheFormatError(const bustache::format_error& _error, std::ptrdiff_t _line, std::ptrdiff_t _column);

    std::ptrdiff_t line(void) const noexcept { return line_; }
    std::ptrdiff_t column(void) const noexcept { return column_; }
};

namespace detail
{

[[noreturn]] void throwBustacheFormatErrorWithPosition(FormatTemplateType templateType, const char* data, const bustache::format_error& error);

} // namespace detail

inline bustache::format bustacheFormat(const char* first, const char* last, FormatTemplateType templateType)
{
    try
    {
        return bustache::format(first, last);
    }
    catch (const bustache::format_error& error)
    {
        detail::throwBustacheFormatErrorWithPosition(templateType, first, error);
    }
}
template <typename SourceT>
    bustache::format bustacheFormat(SourceT&& source, FormatTemplateType templateType)
{
    try
    {
        return bustache::format(std::forward<SourceT>(source));
    }
    catch (const bustache::format_error& error)
    {
        detail::throwBustacheFormatErrorWithPosition(templateType, source.data(), error);
    }
}
template <std::size_t N>
    bustache::format bustacheFormat(const char (&source)[N], FormatTemplateType templateType)
{
    try
    {
        return bustache::format(source);
    }
    catch (const bustache::format_error& error)
    {
        detail::throwBustacheFormatErrorWithPosition(templateType, source, error);
    }
}

#endif // INCLUDED_BUSTACHE_FORMAT_HPP_
// bustache-format.cpp

#include <string>
#include <sstream>
#include <utility>   // for pair<>
#include <exception> // for throw_with_nested()

#include "bustache-format.hpp"

static std::string makePositionMessage(std::ptrdiff_t line, std::ptrdiff_t column)
{
        // We use 1-based indices in human-readable error messages.
    std::ostringstream sstr;
    if (line >= 0 && column >= 0)
        sstr << "in line " << (line + 1) << ", column " << (column + 1) << ":";
    else if (column >= 0)
        sstr << "at position " << (column + 1) << ":";
    else
        sstr << "at unknown position:";
    return sstr.str();
}

BustacheFormatError::BustacheFormatError(const bustache::format_error& _error)
    : std::runtime_error(makePositionMessage(-1, _error.position())), error_(_error), line_(-1), column_(_error.position())
{
}

BustacheFormatError::BustacheFormatError(const bustache::format_error& _error, std::ptrdiff_t _line, std::ptrdiff_t _column)
    : std::runtime_error(makePositionMessage(_line, _column)), error_(_error), line_(_line), column_(_column)
{
}

namespace detail
{

template <typename I, typename D>
    static std::pair<D, D> determineLineAndColumn(I data, D position)
{
    if (position < 0)
        return std::make_pair(-1, -1);

    D line = 0;
    I i0 = data;
    for (I i = data, e = data + position; i < e; ++i)
    {
        if (*i == '\n')
        {
            ++line;
            i0 = i + 1;
        }
    }
    D column = (data + position) - i0;
    return std::make_pair(line, column);
}

void throwBustacheFormatErrorWithPosition(FormatTemplateType templateType, const char* data, const bustache::format_error& error)
{
    if (error.position() < 0)
        throw;

    switch (templateType)
    {
    case FormatTemplateType::string:
        std::throw_with_nested(BustacheFormatError(error));
    case FormatTemplateType::multilineString:
        {
            auto [line, column] = determineLineAndColumn(data, error.position());
            std::throw_with_nested(BustacheFormatError(error, line, column));
        }
    }
    std::terminate();
}

} // namespace detail