Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add --fatal_warnings flag to treat warnings as errors #8131

Merged
merged 2 commits into from Mar 9, 2021
Merged

Add --fatal_warnings flag to treat warnings as errors #8131

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7110293
Add --fatal_warnings flag to treat warnings as errors
benesch Feb 25, 2018
0092910
Fixes to make the --fatal_warnings flag work with current master
perlun Dec 9, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 19 additions & 3 deletions src/google/protobuf/compiler/command_line_interface.cc
View file
Open in desktop
Expand Up @@ -288,7 +288,7 @@ class CommandLineInterface::ErrorPrinter
public DescriptorPool::ErrorCollector {
public:
ErrorPrinter(ErrorFormat format, DiskSourceTree* tree = NULL)
: format_(format), tree_(tree), found_errors_(false) {}
: format_(format), tree_(tree), found_errors_(false), found_warnings_(false) {}
~ErrorPrinter() {}

// implements MultiFileErrorCollector ------------------------------
Expand All @@ -300,6 +300,7 @@ class CommandLineInterface::ErrorPrinter

void AddWarning(const std::string& filename, int line, int column,
const std::string& message) {
found_warnings_ = true;
AddErrorOrWarning(filename, line, column, message, "warning", std::clog);
}

Expand Down Expand Up @@ -327,6 +328,8 @@ class CommandLineInterface::ErrorPrinter

bool FoundErrors() const { return found_errors_; }

bool FoundWarnings() const { return found_warnings_; }

private:
void AddErrorOrWarning(const std::string& filename, int line, int column,
const std::string& message, const std::string& type,
Expand Down Expand Up @@ -365,6 +368,7 @@ class CommandLineInterface::ErrorPrinter
const ErrorFormat format_;
DiskSourceTree* tree_;
bool found_errors_;
bool found_warnings_;
};

// -------------------------------------------------------------------
Expand Down Expand Up @@ -1117,7 +1121,8 @@ int CommandLineInterface::Run(int argc, const char* const argv[]) {
}
}

if (error_collector->FoundErrors()) {
if (error_collector->FoundErrors() ||
(fatal_warnings_ && error_collector->FoundWarnings())) {
return 1;
}

Expand Down Expand Up @@ -1630,7 +1635,8 @@ bool CommandLineInterface::ParseArgument(const char* arg, std::string* name,
*name == "--version" || *name == "--decode_raw" ||
*name == "--print_free_field_numbers" ||
*name == "--experimental_allow_proto3_optional" ||
*name == "--deterministic_output") {
*name == "--deterministic_output" ||
*name == "--fatal_warnings") {
// HACK: These are the only flags that don't take a value.
// They probably should not be hard-coded like this but for now it's
// not worth doing better.
Expand Down Expand Up @@ -1883,6 +1889,12 @@ CommandLineInterface::InterpretArgument(const std::string& name,
return PARSE_ARGUMENT_FAIL;
}

} else if (name == "--fatal_warnings") {
if (fatal_warnings_) {
std::cerr << name << " may only be passed once." << std::endl;
return PARSE_ARGUMENT_FAIL;
}
fatal_warnings_ = true;
} else if (name == "--plugin") {
if (plugin_prefix_.empty()) {
std::cerr << "This compiler does not support plugins." << std::endl;
Expand Down Expand Up @@ -2042,6 +2054,10 @@ Parse PROTO_FILES and generate output based on the options given:
--error_format=FORMAT Set the format in which to print errors.
FORMAT may be 'gcc' (the default) or 'msvs'
(Microsoft Visual Studio format).
--fatal_warnings Make warnings be fatal (similar to -Werr in
gcc). This flag will make protoc return
with a non-zero exit code if any warnings
are generated.
--print_free_field_numbers Print the free field numbers of the messages
defined in the given proto files. Groups share
the same field number space with the parent
Expand Down
3 changes: 3 additions & 0 deletions src/google/protobuf/compiler/command_line_interface.h
View file
Open in desktop
Expand Up @@ -392,6 +392,9 @@ class PROTOC_EXPORT CommandLineInterface {

ErrorFormat error_format_ = ERROR_FORMAT_GCC;

// True if we should treat warnings as errors that fail the compilation.
bool fatal_warnings_ = false;

std::vector<std::pair<std::string, std::string> >
proto_path_; // Search path for proto files.
std::vector<std::string> input_files_; // Names of the input proto files.
Expand Down
36 changes: 34 additions & 2 deletions src/google/protobuf/compiler/command_line_interface_unittest.cc
View file
Open in desktop
Expand Up @@ -157,6 +157,11 @@ class CommandLineInterfaceTest : public testing::Test {
void ExpectCapturedStdoutSubstringWithZeroReturnCode(
const std::string& expected_substring);

// Checks that Run() returned zero and the stderr contains the given
// substring.
void ExpectCapturedStderrSubstringWithZeroReturnCode(
const std::string& expected_substring);

#if defined(_WIN32) && !defined(__CYGWIN__)
// Returns true if ExpectErrorSubstring(expected_substring) would pass, but
// does not fail otherwise.
Expand Down Expand Up @@ -426,8 +431,8 @@ void CommandLineInterfaceTest::ExpectErrorSubstring(

void CommandLineInterfaceTest::ExpectWarningSubstring(
const std::string& expected_substring) {
EXPECT_EQ(0, return_code_);
EXPECT_PRED_FORMAT2(testing::IsSubstring, expected_substring, error_text_);
EXPECT_EQ(0, return_code_);
}

#if defined(_WIN32) && !defined(__CYGWIN__)
Expand Down Expand Up @@ -515,6 +520,13 @@ void CommandLineInterfaceTest::ExpectCapturedStdoutSubstringWithZeroReturnCode(
captured_stdout_);
}

void CommandLineInterfaceTest::ExpectCapturedStderrSubstringWithZeroReturnCode(
const std::string& expected_substring) {
EXPECT_EQ(0, return_code_);
EXPECT_PRED_FORMAT2(testing::IsSubstring, expected_substring,
error_text_);
}

void CommandLineInterfaceTest::ExpectFileContent(const std::string& filename,
const std::string& content) {
std::string path = temp_directory_ + "/" + filename;
Expand Down Expand Up @@ -2303,7 +2315,7 @@ TEST_F(CommandLineInterfaceTest, MsvsFormatErrors) {
}

TEST_F(CommandLineInterfaceTest, InvalidErrorFormat) {
// Test --error_format=msvs
// Test invalid --error_format

CreateTempFile("foo.proto",
"syntax = \"proto2\";\n"
Expand All @@ -2315,6 +2327,26 @@ TEST_F(CommandLineInterfaceTest, InvalidErrorFormat) {
ExpectErrorText("Unknown error format: invalid\n");
}

TEST_F(CommandLineInterfaceTest, Warnings) {
// Test --fatal_warnings.

CreateTempFile("foo.proto",
"syntax = \"proto2\";\n"
"import \"bar.proto\";\n");
CreateTempFile("bar.proto",
"syntax = \"proto2\";\n");

Run("protocol_compiler --test_out=$tmpdir "
"--proto_path=$tmpdir foo.proto");
ExpectCapturedStderrSubstringWithZeroReturnCode(
"foo.proto:2:1: warning: Import bar.proto is unused.");

Run("protocol_compiler --test_out=$tmpdir --fatal_warnings "
"--proto_path=$tmpdir foo.proto");
ExpectErrorSubstring(
"foo.proto:2:1: warning: Import bar.proto is unused.");
}

// -------------------------------------------------------------------
// Flag parsing tests

Expand Down