Skip to content

Commit ddb0d89

Browse files
authored
Merge pull request #14580 from ethereum/fix-isoltest-info-color
Fix coloring of info messages in isoltest
2 parents 810c446 + 0da44ca commit ddb0d89

File tree

9 files changed

+101
-51
lines changed

9 files changed

+101
-51
lines changed

liblangutil/Exceptions.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,26 @@
2929
using namespace solidity;
3030
using namespace solidity::langutil;
3131

32+
std::map<Error::Type, std::string> const Error::m_errorTypeNames = {
33+
{Error::Type::IOError, "IOError"},
34+
{Error::Type::FatalError, "FatalError"},
35+
{Error::Type::JSONError, "JSONError"},
36+
{Error::Type::InternalCompilerError, "InternalCompilerError"},
37+
{Error::Type::CompilerError, "CompilerError"},
38+
{Error::Type::Exception, "Exception"},
39+
{Error::Type::CodeGenerationError, "CodeGenerationError"},
40+
{Error::Type::DeclarationError, "DeclarationError"},
41+
{Error::Type::DocstringParsingError, "DocstringParsingError"},
42+
{Error::Type::ParserError, "ParserError"},
43+
{Error::Type::SyntaxError, "SyntaxError"},
44+
{Error::Type::TypeError, "TypeError"},
45+
{Error::Type::UnimplementedFeatureError, "UnimplementedFeatureError"},
46+
{Error::Type::YulException, "YulException"},
47+
{Error::Type::SMTLogicException, "SMTLogicException"},
48+
{Error::Type::Warning, "Warning"},
49+
{Error::Type::Info, "Info"},
50+
};
51+
3252
Error::Error(
3353
ErrorId _errorId, Error::Type _type,
3454
std::string const& _description,

liblangutil/Exceptions.h

Lines changed: 15 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,11 @@
3333
#include <boost/preprocessor/facilities/overload.hpp>
3434
#include <boost/algorithm/string/case_conv.hpp>
3535

36+
#include <optional>
3637
#include <string>
3738
#include <utility>
38-
#include <vector>
39-
#include <memory>
4039
#include <variant>
40+
#include <vector>
4141

4242
namespace solidity::langutil
4343
{
@@ -269,27 +269,17 @@ class Error: virtual public util::Exception
269269

270270
static std::string formatErrorType(Type _type)
271271
{
272-
switch (_type)
273-
{
274-
case Type::IOError: return "IOError";
275-
case Type::FatalError: return "FatalError";
276-
case Type::JSONError: return "JSONError";
277-
case Type::InternalCompilerError: return "InternalCompilerError";
278-
case Type::CompilerError: return "CompilerError";
279-
case Type::Exception: return "Exception";
280-
case Type::CodeGenerationError: return "CodeGenerationError";
281-
case Type::DeclarationError: return "DeclarationError";
282-
case Type::DocstringParsingError: return "DocstringParsingError";
283-
case Type::ParserError: return "ParserError";
284-
case Type::SyntaxError: return "SyntaxError";
285-
case Type::TypeError: return "TypeError";
286-
case Type::UnimplementedFeatureError: return "UnimplementedFeatureError";
287-
case Type::YulException: return "YulException";
288-
case Type::SMTLogicException: return "SMTLogicException";
289-
case Type::Warning: return "Warning";
290-
case Type::Info: return "Info";
291-
}
292-
util::unreachable();
272+
return m_errorTypeNames.at(_type);
273+
}
274+
275+
static std::optional<Type> parseErrorType(std::string _name)
276+
{
277+
static std::map<std::string, Error::Type> const m_errorTypesByName = util::invertMap(m_errorTypeNames);
278+
279+
if (m_errorTypesByName.count(_name) == 0)
280+
return std::nullopt;
281+
282+
return m_errorTypesByName.at(_name);
293283
}
294284

295285
static std::string formatTypeOrSeverity(std::variant<Error::Type, Error::Severity> _typeOrSeverity)
@@ -309,6 +299,8 @@ class Error: virtual public util::Exception
309299
private:
310300
ErrorId m_errorId;
311301
Type m_type;
302+
303+
static std::map<Type, std::string> const m_errorTypeNames;
312304
};
313305

314306
}

liblangutil/SourceReferenceFormatter.cpp

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,28 @@ std::string SourceReferenceFormatter::formatErrorInformation(Error const& _error
5555
);
5656
}
5757

58+
char const* SourceReferenceFormatter::errorTextColor(Error::Severity _severity)
59+
{
60+
switch (_severity)
61+
{
62+
case Error::Severity::Error: return RED;
63+
case Error::Severity::Warning: return YELLOW;
64+
case Error::Severity::Info: return WHITE;
65+
}
66+
util::unreachable();
67+
}
68+
69+
char const* SourceReferenceFormatter::errorHighlightColor(Error::Severity _severity)
70+
{
71+
switch (_severity)
72+
{
73+
case Error::Severity::Error: return RED_BACKGROUND;
74+
case Error::Severity::Warning: return ORANGE_BACKGROUND_256;
75+
case Error::Severity::Info: return GRAY_BACKGROUND;
76+
}
77+
util::unreachable();
78+
}
79+
5880
AnsiColorized SourceReferenceFormatter::normalColored() const
5981
{
6082
return AnsiColorized(m_stream, m_colored, {WHITE});
@@ -67,18 +89,7 @@ AnsiColorized SourceReferenceFormatter::frameColored() const
6789

6890
AnsiColorized SourceReferenceFormatter::errorColored(std::ostream& _stream, bool _colored, Error::Severity _severity)
6991
{
70-
// We used to color messages of any severity as errors so this seems like a good default
71-
// for cases where severity cannot be determined.
72-
char const* textColor = RED;
73-
74-
switch (_severity)
75-
{
76-
case Error::Severity::Error: textColor = RED; break;
77-
case Error::Severity::Warning: textColor = YELLOW; break;
78-
case Error::Severity::Info: textColor = WHITE; break;
79-
}
80-
81-
return AnsiColorized(_stream, _colored, {BOLD, textColor});
92+
return AnsiColorized(_stream, _colored, {BOLD, errorTextColor(_severity)});
8293
}
8394

8495
AnsiColorized SourceReferenceFormatter::messageColored(std::ostream& _stream, bool _colored)

liblangutil/SourceReferenceFormatter.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,16 @@ class SourceReferenceFormatter
127127
bool _withErrorIds = false
128128
);
129129

130+
/// The default text color for printing error messages of a given severity in the terminal.
131+
/// Assumes a dark background color.
132+
static char const* errorTextColor(Error::Severity _severity);
133+
134+
/// The default background color for highlighting source fragments corresponding to an error
135+
/// of a given severity in the terminal. Assumes a light text color.
136+
/// @note This is *not* meant to be used for the same text in combination with @a errorTextColor().
137+
/// It's an alternative way to highlight it, while preserving the original text color.
138+
static char const* errorHighlightColor(Error::Severity _severity);
139+
130140
private:
131141
util::AnsiColorized normalColored() const;
132142
util::AnsiColorized frameColored() const;

libsolutil/AnsiColorized.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ static constexpr char const* BLUE_BACKGROUND = "\033[44m";
5252
static constexpr char const* MAGENTA_BACKGROUND = "\033[45m";
5353
static constexpr char const* CYAN_BACKGROUND = "\033[46m";
5454
static constexpr char const* WHITE_BACKGROUND = "\033[47m";
55+
static constexpr char const* GRAY_BACKGROUND = "\033[100m";
5556

5657
// 256-bit-colors (incomplete set)
5758
static constexpr char const* RED_BACKGROUND_256 = "\033[48;5;160m";

test/CommonSyntaxTest.cpp

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,17 @@
1919
#include <test/CommonSyntaxTest.h>
2020
#include <test/Common.h>
2121
#include <test/TestCase.h>
22+
23+
#include <liblangutil/SourceReferenceFormatter.h>
24+
2225
#include <libsolutil/CommonIO.h>
2326
#include <libsolutil/StringUtils.h>
27+
2428
#include <boost/algorithm/string.hpp>
2529
#include <boost/algorithm/string/predicate.hpp>
2630
#include <boost/test/unit_test.hpp>
2731
#include <boost/throw_exception.hpp>
32+
2833
#include <fstream>
2934
#include <memory>
3035
#include <stdexcept>
@@ -115,15 +120,18 @@ void CommonSyntaxTest::printSource(ostream& _stream, string const& _linePrefix,
115120
{
116121
assert(static_cast<size_t>(error.locationStart) <= source.length());
117122
assert(static_cast<size_t>(error.locationEnd) <= source.length());
118-
bool isWarning = error.type == "Warning";
119123
for (int i = error.locationStart; i < error.locationEnd; i++)
120-
if (isWarning)
121-
{
122-
if (sourceFormatting[static_cast<size_t>(i)] == util::formatting::RESET)
123-
sourceFormatting[static_cast<size_t>(i)] = util::formatting::ORANGE_BACKGROUND_256;
124-
}
125-
else
126-
sourceFormatting[static_cast<size_t>(i)] = util::formatting::RED_BACKGROUND;
124+
{
125+
char const*& cellFormat = sourceFormatting[static_cast<size_t>(i)];
126+
char const* infoBgColor = SourceReferenceFormatter::errorHighlightColor(Error::Severity::Info);
127+
128+
if (
129+
(error.type != Error::Type::Warning && error.type != Error::Type::Info) ||
130+
(error.type == Error::Type::Warning && (cellFormat == RESET || cellFormat == infoBgColor)) ||
131+
(error.type == Error::Type::Info && cellFormat == RESET)
132+
)
133+
cellFormat = SourceReferenceFormatter::errorHighlightColor(Error::errorSeverity(error.type));
134+
}
127135
}
128136

129137
_stream << _linePrefix << sourceFormatting.front() << source.front();
@@ -190,8 +198,12 @@ void CommonSyntaxTest::printErrorList(
190198
for (auto const& error: _errorList)
191199
{
192200
{
193-
util::AnsiColorized scope(_stream, _formatted, {BOLD, (error.type == "Warning") ? YELLOW : RED});
194-
_stream << _linePrefix << error.type;
201+
util::AnsiColorized scope(
202+
_stream,
203+
_formatted,
204+
{BOLD, SourceReferenceFormatter::errorTextColor(Error::errorSeverity(error.type))}
205+
);
206+
_stream << _linePrefix << Error::formatErrorType(error.type);
195207
if (error.errorId.has_value())
196208
_stream << ' ' << error.errorId->error;
197209
_stream << ": ";
@@ -244,7 +256,11 @@ vector<SyntaxTestError> CommonSyntaxTest::parseExpectations(istream& _stream)
244256
auto typeBegin = it;
245257
while (it != line.end() && isalpha(*it, locale::classic()))
246258
++it;
247-
string errorType(typeBegin, it);
259+
260+
string errorTypeStr(typeBegin, it);
261+
optional<Error::Type> errorType = Error::parseErrorType(errorTypeStr);
262+
if (!errorType.has_value())
263+
BOOST_THROW_EXCEPTION(runtime_error("Invalid error type: " + errorTypeStr));
248264

249265
skipWhitespace(it, line.end());
250266

@@ -281,7 +297,7 @@ vector<SyntaxTestError> CommonSyntaxTest::parseExpectations(istream& _stream)
281297

282298
string errorMessage(it, line.end());
283299
expectations.emplace_back(SyntaxTestError{
284-
std::move(errorType),
300+
errorType.value(),
285301
std::move(errorId),
286302
std::move(errorMessage),
287303
std::move(sourceName),

test/CommonSyntaxTest.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ namespace solidity::test
3434

3535
struct SyntaxTestError
3636
{
37-
std::string type;
37+
langutil::Error::Type type;
3838
std::optional<langutil::ErrorId> errorId;
3939
std::string message;
4040
std::string sourceName;

test/libsolidity/SyntaxTest.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ void SyntaxTest::parseAndAnalyze()
9191
catch (UnimplementedFeatureError const& _e)
9292
{
9393
m_errorList.emplace_back(SyntaxTestError{
94-
"UnimplementedFeatureError",
94+
Error::Type::UnimplementedFeatureError,
9595
std::nullopt,
9696
errorMessage(_e),
9797
"",
@@ -140,7 +140,7 @@ void SyntaxTest::filterObtainedErrors()
140140
}
141141
}
142142
m_errorList.emplace_back(SyntaxTestError{
143-
Error::formatErrorType(currentError->type()),
143+
currentError->type(),
144144
currentError->errorId(),
145145
errorMessage(*currentError),
146146
sourceName,

test/libyul/SyntaxTest.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ void SyntaxTest::parseAndAnalyze()
6161
}
6262

6363
m_errorList.emplace_back(SyntaxTestError{
64-
Error::formatErrorType(error->type()),
64+
error->type(),
6565
error->errorId(),
6666
errorMessage(*error),
6767
name,

0 commit comments

Comments
 (0)