Skip to content

Commit 766efb3

Browse files
cameelcameel
authored
Merge pull request #14590 from ethereum/severity-and-color-for-cli-error-messages
Consistent severity and color for CLI error messages
2 parents e98f174 + 23b7505 commit 766efb3

File tree

55 files changed

+138
-79
lines changed
  • Changelog.md
  • liblangutil
  • solc
  • test
    • cmdlineTests.sh
    • cmdlineTests
      • ast_json_import_wrong_evmVersion
      • debug_info_in_yul_and_evm_asm_print_all_and_none
      • debug_info_in_yul_and_evm_asm_print_snippet_only
      • linker_mode_invalid_option_no_optimize_yul
      • linker_mode_invalid_option_optimize_runs
      • linker_mode_invalid_option_optimize_yul
      • linker_mode_invalid_option_optimize
      • linker_mode_invalid_option_yul_optimizations
      • linker_mode_output_selection_invalid
      • linking_strict_assembly_duplicate_library_name
      • model_checker_bmc_loop_iterations_invalid_arg
      • model_checker_bmc_loop_iterations_no_argument
      • model_checker_contracts_contract_missing
      • model_checker_contracts_empty_contract
      • model_checker_contracts_empty_source
      • model_checker_contracts_one_contract_missing
      • model_checker_contracts_source_missing
      • model_checker_ext_calls_empty_arg
      • model_checker_ext_calls_wrong_arg
      • model_checker_invariants_wrong
      • model_checker_print_query_no_smtlib2_solver_bmc
      • model_checker_print_query_no_smtlib2_solver_chc
      • model_checker_print_query_superflous_solver
      • model_checker_solvers_wrong2
      • model_checker_solvers_wrong
      • model_checker_targets_error
      • no_cbor_metadata_with_metadata_hash
      • optimizer_enabled_invalid_yul_optimizer_enabled_and_disabled
      • standard_cli_output_selection_invalid
      • standard_file_not_found
      • standard_invalid_option_no_optimize_yul
      • standard_invalid_option_optimize_runs
      • standard_invalid_option_optimize_yul
      • standard_invalid_option_optimize
      • standard_invalid_option_yul_optimizations
      • stop_after_parsing_abi
      • strict_asm_debug_info_print_snippet_only
      • strict_asm_invalid_option_output_dir
      • strict_asm_optimizer_invalid_yul_optimizer_enabled_and_disabled
      • strict_asm_options_in_non_asm_mode
      • strict_asm_output_selection_invalid
      • yul_optimizer_steps_disabled
      • yul_optimizer_steps_invalid_abbreviation
      • yul_optimizer_steps_nesting_too_deep
      • yul_optimizer_steps_unbalanced_closing_bracket
      • yul_optimizer_steps_unbalanced_opening_bracket
      • ~unknown_options
    • solc

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

55 files changed

+138
-79
lines changed

‎Changelog.md

Lines changed: 1 addition & 0 deletions

‎liblangutil/SourceReferenceFormatter.cpp

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ AnsiColorized SourceReferenceFormatter::frameColored() const
6565
return AnsiColorized(m_stream, m_colored, {BOLD, BLUE});
6666
}
6767

68-
AnsiColorized SourceReferenceFormatter::errorColored(Error::Severity _severity) const
68+
AnsiColorized SourceReferenceFormatter::errorColored(std::ostream& _stream, bool _colored, Error::Severity _severity)
6969
{
7070
// We used to color messages of any severity as errors so this seems like a good default
7171
// for cases where severity cannot be determined.
@@ -78,12 +78,12 @@ AnsiColorized SourceReferenceFormatter::errorColored(Error::Severity _severity)
7878
case Error::Severity::Info: textColor = WHITE; break;
7979
}
8080

81-
return AnsiColorized(m_stream, m_colored, {BOLD, textColor});
81+
return AnsiColorized(_stream, _colored, {BOLD, textColor});
8282
}
8383

84-
AnsiColorized SourceReferenceFormatter::messageColored() const
84+
AnsiColorized SourceReferenceFormatter::messageColored(std::ostream& _stream, bool _colored)
8585
{
86-
return AnsiColorized(m_stream, m_colored, {BOLD, WHITE});
86+
return AnsiColorized(_stream, _colored, {BOLD, WHITE});
8787
}
8888

8989
AnsiColorized SourceReferenceFormatter::secondaryColored() const
@@ -175,14 +175,26 @@ void SourceReferenceFormatter::printSourceLocation(SourceReference const& _ref)
175175
}
176176
}
177177

178-
void SourceReferenceFormatter::printExceptionInformation(SourceReferenceExtractor::Message const& _msg)
178+
void SourceReferenceFormatter::printPrimaryMessage(
179+
std::ostream& _stream,
180+
std::string _message,
181+
std::variant<Error::Type, Error::Severity> _typeOrSeverity,
182+
std::optional<ErrorId> _errorId,
183+
bool _colored,
184+
bool _withErrorIds
185+
)
179186
{
180-
errorColored(Error::errorSeverityOrType(_msg._typeOrSeverity)) << Error::formatTypeOrSeverity(_msg._typeOrSeverity);
187+
errorColored(_stream, _colored, Error::errorSeverityOrType(_typeOrSeverity)) << Error::formatTypeOrSeverity(_typeOrSeverity);
188+
189+
if (_withErrorIds && _errorId.has_value())
190+
errorColored(_stream, _colored, Error::errorSeverityOrType(_typeOrSeverity)) << " (" << _errorId.value().error << ")";
181191

182-
if (m_withErrorIds && _msg.errorId.has_value())
183-
errorColored(Error::errorSeverityOrType(_msg._typeOrSeverity)) << " (" << _msg.errorId.value().error << ")";
192+
messageColored(_stream, _colored) << ": " << _message << '\n';
193+
}
184194

185-
messageColored() << ": " << _msg.primary.message << '\n';
195+
void SourceReferenceFormatter::printExceptionInformation(SourceReferenceExtractor::Message const& _msg)
196+
{
197+
printPrimaryMessage(m_stream, _msg.primary.message, _msg._typeOrSeverity, _msg.errorId, m_colored, m_withErrorIds);
186198
printSourceLocation(_msg.primary);
187199

188200
for (auto const& secondary: _msg.secondary)

‎liblangutil/SourceReferenceFormatter.h

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,15 +118,27 @@ class SourceReferenceFormatter
118118

119119
static std::string formatErrorInformation(Error const& _error, CharStream const& _charStream);
120120

121+
static void printPrimaryMessage(
122+
std::ostream& _stream,
123+
std::string _message,
124+
std::variant<Error::Type, Error::Severity> _typeOrSeverity,
125+
std::optional<ErrorId> _errorId = std::nullopt,
126+
bool _colored = false,
127+
bool _withErrorIds = false
128+
);
129+
121130
private:
122131
util::AnsiColorized normalColored() const;
123132
util::AnsiColorized frameColored() const;
124-
util::AnsiColorized errorColored(langutil::Error::Severity _severity) const;
125-
util::AnsiColorized messageColored() const;
133+
util::AnsiColorized errorColored(Error::Severity _severity) const { return errorColored(m_stream, m_colored, _severity); }
134+
util::AnsiColorized messageColored() const { return messageColored(m_stream, m_colored); }
126135
util::AnsiColorized secondaryColored() const;
127136
util::AnsiColorized highlightColored() const;
128137
util::AnsiColorized diagColored() const;
129138

139+
static util::AnsiColorized errorColored(std::ostream& _stream, bool _colored, langutil::Error::Severity _severity);
140+
static util::AnsiColorized messageColored(std::ostream& _stream, bool _colored);
141+
130142
private:
131143
std::ostream& m_stream;
132144
CharStreamProvider const& m_charStreamProvider;

‎solc/CommandLineInterface.cpp

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -517,7 +517,7 @@ void CommandLineInterface::readInputFiles()
517517
if (!m_options.input.ignoreMissingFiles)
518518
solThrow(CommandLineValidationError, '"' + infile.string() + "\" is not found.");
519519
else
520-
serr() << infile << " is not found. Skipping." << std::endl;
520+
report(Error::Severity::Info, fmt::format("\"{}\" is not found. Skipping.", infile.string()));
521521

522522
continue;
523523
}
@@ -527,7 +527,7 @@ void CommandLineInterface::readInputFiles()
527527
if (!m_options.input.ignoreMissingFiles)
528528
solThrow(CommandLineValidationError, '"' + infile.string() + "\" is not a valid file.");
529529
else
530-
serr() << infile << " is not a valid file. Skipping." << std::endl;
530+
report(Error::Severity::Info, fmt::format("\"{}\" is not a valid file. Skipping.", infile.string()));
531531

532532
continue;
533533
}
@@ -639,7 +639,7 @@ bool CommandLineInterface::run(int _argc, char const* const* _argv)
639639
// There might be no message in the exception itself if the error output is bulky and has
640640
// already been printed to stderr (this happens e.g. for compiler errors).
641641
if (_exception.what() != ""s)
642-
serr() << _exception.what() << std::endl;
642+
report(Error::Severity::Error, _exception.what());
643643

644644
return false;
645645
}
@@ -658,7 +658,19 @@ bool CommandLineInterface::parseArguments(int _argc, char const* const* _argv)
658658
return false;
659659
}
660660

661-
parser.parse(_argc, _argv);
661+
try
662+
{
663+
parser.parse(_argc, _argv);
664+
}
665+
catch (...)
666+
{
667+
// Even if the overall CLI parsing fails, the --color/--no-color options may have been
668+
// successfully parsed, and if so, should be taken into account when printing errors.
669+
// If no value is present, it's possible that --no-color is still there but parsing failed
670+
// due to other, unrecognized options so play it safe and disable color in that case.
671+
m_options.formatting.coloredOutput = parser.options().formatting.coloredOutput.value_or(false);
672+
throw;
673+
}
662674
m_options = parser.options();
663675

664676
return true;
@@ -817,7 +829,7 @@ void CommandLineInterface::compile()
817829
{
818830
if (_error.type() == Error::Type::DocstringParsingError)
819831
{
820-
serr() << *boost::get_error_info<errinfo_comment>(_error);
832+
report(Error::Severity::Error, *boost::get_error_info<errinfo_comment>(_error));
821833
solThrow(CommandLineExecutionError, "Documentation parsing failed.");
822834
}
823835
else
@@ -1020,7 +1032,10 @@ void CommandLineInterface::link()
10201032
copy(hexStr.begin(), hexStr.end(), it);
10211033
}
10221034
else
1023-
serr() << "Reference \"" << foundPlaceholder << "\" in file \"" << src.first << "\" still unresolved." << std::endl;
1035+
report(
1036+
Error::Severity::Warning,
1037+
fmt::format("Reference \"{}\" in file \"{}\" still unresolved.", foundPlaceholder, src.first)
1038+
);
10241039
it += placeholderSize;
10251040
}
10261041
// Remove hints for resolved libraries.
@@ -1136,7 +1151,7 @@ void CommandLineInterface::assembleYul(yul::YulStack::Language _language, yul::Y
11361151
if (object.bytecode)
11371152
sout() << object.bytecode->toHex() << std::endl;
11381153
else
1139-
serr() << "No binary representation found." << std::endl;
1154+
report(Error::Severity::Info, "No binary representation found.");
11401155
}
11411156
if (m_options.compiler.outputs.astCompactJson)
11421157
{
@@ -1150,7 +1165,7 @@ void CommandLineInterface::assembleYul(yul::YulStack::Language _language, yul::Y
11501165
if (!object.assembly.empty())
11511166
sout() << object.assembly << std::endl;
11521167
else
1153-
serr() << "No text representation found." << std::endl;
1168+
report(Error::Severity::Info, "No text representation found.");
11541169
}
11551170
}
11561171
}
@@ -1220,4 +1235,16 @@ void CommandLineInterface::outputCompilationResults()
12201235
}
12211236
}
12221237

1238+
void CommandLineInterface::report(langutil::Error::Severity _severity, std::string _message)
1239+
{
1240+
SourceReferenceFormatter::printPrimaryMessage(
1241+
serr(),
1242+
_message,
1243+
_severity,
1244+
std::nullopt,
1245+
coloredOutput(m_options),
1246+
m_options.formatting.withErrorIds
1247+
);
1248+
}
1249+
12231250
}

‎solc/CommandLineInterface.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,8 @@ class CommandLineInterface
136136
/// stream has ever been used unless @arg _markAsUsed is set to false.
137137
std::ostream& serr(bool _markAsUsed = true);
138138

139+
void report(langutil::Error::Severity _severity, std::string _message);
140+
139141
std::istream& m_sin;
140142
std::ostream& m_sout;
141143
std::ostream& m_serr;

‎solc/CommandLineParser.cpp

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -896,6 +896,9 @@ void CommandLineParser::parseArgs(int _argc, char const* const* _argv)
896896
po::options_description allOptions = optionsDescription();
897897
po::positional_options_description filesPositions = positionalOptionsDescription();
898898

899+
m_options = {};
900+
m_args = {};
901+
899902
// parse the compiler arguments
900903
try
901904
{
@@ -914,6 +917,11 @@ void CommandLineParser::parseArgs(int _argc, char const* const* _argv)
914917

915918
void CommandLineParser::processArgs()
916919
{
920+
if (m_args.count(g_strNoColor) > 0)
921+
m_options.formatting.coloredOutput = false;
922+
else if (m_args.count(g_strColor) > 0)
923+
m_options.formatting.coloredOutput = true;
924+
917925
checkMutuallyExclusive({
918926
g_strHelp,
919927
g_strLicense,
@@ -1028,11 +1036,6 @@ void CommandLineParser::processArgs()
10281036
);
10291037
}
10301038

1031-
if (m_args.count(g_strColor) > 0)
1032-
m_options.formatting.coloredOutput = true;
1033-
else if (m_args.count(g_strNoColor) > 0)
1034-
m_options.formatting.coloredOutput = false;
1035-
10361039
m_options.formatting.withErrorIds = m_args.count(g_strErrorIds);
10371040

10381041
if (m_args.count(g_strRevertStrings))

‎test/cmdlineTests.sh

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -318,14 +318,14 @@ EOF
318318
## RUN
319319

320320
printTask "Testing passing files that are not found..."
321-
test_solc_behaviour "file_not_found.sol" "" "" "" 1 "" "\"file_not_found.sol\" is not found." "" ""
321+
test_solc_behaviour "file_not_found.sol" "" "" "" 1 "" "Error: \"file_not_found.sol\" is not found." "" ""
322322

323323
printTask "Testing passing files that are not files..."
324-
test_solc_behaviour "." "" "" "" 1 "" "\".\" is not a valid file." "" ""
324+
test_solc_behaviour "." "" "" "" 1 "" "Error: \".\" is not a valid file." "" ""
325325

326326
printTask "Testing passing empty remappings..."
327-
test_solc_behaviour "${0}" "=/some/remapping/target" "" "" 1 "" "Invalid remapping: \"=/some/remapping/target\"." "" ""
328-
test_solc_behaviour "${0}" "ctx:=/some/remapping/target" "" "" 1 "" "Invalid remapping: \"ctx:=/some/remapping/target\"." "" ""
327+
test_solc_behaviour "${0}" "=/some/remapping/target" "" "" 1 "" "Error: Invalid remapping: \"=/some/remapping/target\"." "" ""
328+
test_solc_behaviour "${0}" "ctx:=/some/remapping/target" "" "" 1 "" "Error: Invalid remapping: \"ctx:=/some/remapping/target\"." "" ""
329329

330330
printTask "Running general commandline tests..."
331331
(

‎test/cmdlineTests/ast_json_import_wrong_evmVersion/err

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
Failed to import AST: Imported tree evm version differs from configured evm version!
1+
Error: Failed to import AST: Imported tree evm version differs from configured evm version!

‎test/cmdlineTests/debug_info_in_yul_and_evm_asm_print_all_and_none/err

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
Invalid value for --debug-info option: location,all,none
1+
Error: Invalid value for --debug-info option: location,all,none

‎test/cmdlineTests/debug_info_in_yul_and_evm_asm_print_snippet_only/err

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
To use 'snippet' with --debug-info you must select also 'location'.
1+
Error: To use 'snippet' with --debug-info you must select also 'location'.

0 commit comments

Comments
 (0)