Skip to content

Commit 3e4d9fe

Browse files
layuslayus
committed
Let client error out on incomplete bazelrc lines
1 parent 6d30214 commit 3e4d9fe

File tree

2 files changed

+103
-79
lines changed

2 files changed

+103
-79
lines changed

‎src/main/cpp/rc_file.cc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,13 @@ RcFile::ParseError RcFile::ParseFile(const string& filename,
144144
} else {
145145
auto words_it = words.begin();
146146
words_it++; // Advance past command.
147+
if (words_it == words.end()) {
148+
blaze_util::StringPrintf(
149+
error_text,
150+
"Incomplete line in .blazerc file '%s': '%s'",
151+
canonical_filename.c_str(), line.c_str());
152+
return ParseError::INVALID_FORMAT;
153+
}
147154
for (; words_it != words.end(); words_it++) {
148155
options_[command].push_back({*words_it, rcfile_index});
149156
}

‎src/test/cpp/rc_options_test.cc

Lines changed: 96 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,18 @@ class RcOptionsTest : public ::testing::Test {
9797
}
9898
}
9999
}
100+
101+
void FailToParseRcWithErrorAndRegex(
102+
const string& filename,
103+
const RcFile::ParseError expected_error,
104+
const string& expected_error_regex) {
105+
RcFile::ParseError error;
106+
string error_text;
107+
std::unique_ptr<RcFile> rc = Parse(filename, &error, &error_text);
108+
// Test the text first, as the error code is hard to debug.
109+
ASSERT_THAT(error_text, MatchesRegex(expected_error_regex));
110+
ASSERT_EQ(error, expected_error);
111+
}
100112
};
101113

102114
TEST_F(RcOptionsTest, Empty) {
@@ -124,17 +136,21 @@ TEST_F(RcOptionsTest, CommentedStartup) {
124136
TEST_F(RcOptionsTest, EmptyStartupLine) {
125137
WriteRc("empty_startup_line.bazelrc",
126138
"startup");
127-
unordered_map<string, vector<string>> no_expected_args;
128-
SuccessfullyParseRcWithExpectedArgs("empty_startup_line.bazelrc",
129-
no_expected_args);
139+
FailToParseRcWithErrorAndRegex(
140+
"empty_startup_line.bazelrc",
141+
RcFile::ParseError::INVALID_FORMAT,
142+
"Incomplete line in \\.blazerc file '.*empty_startup_line\\.bazelrc': "
143+
"'startup'");
130144
}
131145

132146
TEST_F(RcOptionsTest, StartupWithOnlyCommentedArg) {
133147
WriteRc("startup_with_comment.bazelrc",
134148
"startup # bar");
135-
unordered_map<string, vector<string>> no_expected_args;
136-
SuccessfullyParseRcWithExpectedArgs("startup_with_comment.bazelrc",
137-
no_expected_args);
149+
FailToParseRcWithErrorAndRegex(
150+
"startup_with_comment.bazelrc",
151+
RcFile::ParseError::INVALID_FORMAT,
152+
"Incomplete line in \\.blazerc file '.*startup_with_comment\\.bazelrc': "
153+
"'startup # bar'");
138154
}
139155

140156
TEST_F(RcOptionsTest, SingleStartupArg) {
@@ -355,17 +371,13 @@ TEST_F(RcOptionsTest, ImportCycleFails) {
355371
WriteRc("import_cycle_2.bazelrc",
356372
"import %workspace%/import_cycle_1.bazelrc");
357373

358-
RcFile::ParseError error;
359-
string error_text;
360-
std::unique_ptr<RcFile> rc =
361-
Parse("import_cycle_1.bazelrc", &error, &error_text);
362-
EXPECT_EQ(error, RcFile::ParseError::IMPORT_LOOP);
363-
ASSERT_THAT(
364-
error_text,
365-
MatchesRegex("Import loop detected:\n"
366-
" .*import_cycle_1.bazelrc\n"
367-
" .*import_cycle_2.bazelrc\n"
368-
" .*import_cycle_1.bazelrc\n"));
374+
FailToParseRcWithErrorAndRegex(
375+
"import_cycle_1.bazelrc",
376+
RcFile::ParseError::IMPORT_LOOP,
377+
"Import loop detected:\n"
378+
" .*import_cycle_1.bazelrc\n"
379+
" .*import_cycle_2.bazelrc\n"
380+
" .*import_cycle_1.bazelrc\n");
369381
}
370382

371383
TEST_F(RcOptionsTest, LongImportCycleFails) {
@@ -382,53 +394,62 @@ TEST_F(RcOptionsTest, LongImportCycleFails) {
382394
WriteRc("import_cycle_2.bazelrc",
383395
"import %workspace%/import_cycle_1.bazelrc");
384396

385-
RcFile::ParseError error;
386-
string error_text;
387-
std::unique_ptr<RcFile> rc =
388-
Parse("chain_to_cycle_1.bazelrc", &error, &error_text);
389-
EXPECT_EQ(error, RcFile::ParseError::IMPORT_LOOP);
390-
ASSERT_THAT(
391-
error_text,
392-
MatchesRegex("Import loop detected:\n"
393-
" .*chain_to_cycle_1.bazelrc\n"
394-
" .*chain_to_cycle_2.bazelrc\n"
395-
" .*chain_to_cycle_3.bazelrc\n"
396-
" .*chain_to_cycle_4.bazelrc\n"
397-
" .*import_cycle_1.bazelrc\n"
398-
" .*import_cycle_2.bazelrc\n"
399-
" .*import_cycle_1.bazelrc\n"));
397+
FailToParseRcWithErrorAndRegex(
398+
"chain_to_cycle_1.bazelrc",
399+
RcFile::ParseError::IMPORT_LOOP,
400+
"Import loop detected:\n"
401+
" .*chain_to_cycle_1.bazelrc\n"
402+
" .*chain_to_cycle_2.bazelrc\n"
403+
" .*chain_to_cycle_3.bazelrc\n"
404+
" .*chain_to_cycle_4.bazelrc\n"
405+
" .*import_cycle_1.bazelrc\n"
406+
" .*import_cycle_2.bazelrc\n"
407+
" .*import_cycle_1.bazelrc\n");
408+
}
409+
410+
TEST_F(RcOptionsTest, MissingCommandFails) {
411+
WriteRc("missing_command.bazelrc",
412+
"--debug_client");
413+
414+
FailToParseRcWithErrorAndRegex(
415+
"missing_command.bazelrc",
416+
RcFile::ParseError::INVALID_FORMAT,
417+
"Incomplete line in \\.blazerc file '.*missing_command\\.bazelrc': "
418+
"'--debug_client'");
419+
}
420+
421+
TEST_F(RcOptionsTest, EmptyOptionsFails) {
422+
WriteRc("empty_options.bazelrc",
423+
"build:debug # --debug_client");
424+
425+
FailToParseRcWithErrorAndRegex(
426+
"empty_options.bazelrc",
427+
RcFile::ParseError::INVALID_FORMAT,
428+
"Incomplete line in \\.blazerc file '.*empty_options\\.bazelrc': "
429+
"'build:debug # --debug_client'");
400430
}
401431

402432
TEST_F(RcOptionsTest, FileDoesNotExist) {
403-
RcFile::ParseError error;
404-
string error_text;
405-
std::unique_ptr<RcFile> rc = Parse("not_a_file.bazelrc", &error, &error_text);
406-
EXPECT_EQ(error, RcFile::ParseError::UNREADABLE_FILE);
407-
ASSERT_THAT(
408-
error_text,
409-
MatchesRegex(kIsWindows
410-
? "Unexpected error reading \\.blazerc file '.*not_a_file\\.bazelrc':.*"
411-
: "Unexpected error reading \\.blazerc file '.*not_a_file\\.bazelrc': "
412-
"\\(error: 2\\): No such file or directory"));
433+
FailToParseRcWithErrorAndRegex(
434+
"not_a_file.bazelrc",
435+
RcFile::ParseError::UNREADABLE_FILE,
436+
kIsWindows
437+
? "Unexpected error reading \\.blazerc file '.*not_a_file\\.bazelrc':.*"
438+
: "Unexpected error reading \\.blazerc file '.*not_a_file\\.bazelrc': "
439+
"\\(error: 2\\): No such file or directory");
413440
}
414441

415442
TEST_F(RcOptionsTest, ImportedFileDoesNotExist) {
416443
WriteRc("import_fake_file.bazelrc",
417444
"import somefile");
418445

419-
RcFile::ParseError error;
420-
string error_text;
421-
std::unique_ptr<RcFile> rc =
422-
Parse("import_fake_file.bazelrc", &error, &error_text);
423-
EXPECT_EQ(error, RcFile::ParseError::UNREADABLE_FILE);
424-
if (kIsWindows) {
425-
ASSERT_THAT(error_text, MatchesRegex(
426-
"Unexpected error reading \\.blazerc file 'somefile':.*"));
427-
} else {
428-
ASSERT_EQ(error_text,
429-
"Unexpected error reading .blazerc file 'somefile': (error: 2): No such "
430-
"file or directory");
431-
}
446+
FailToParseRcWithErrorAndRegex(
447+
"import_fake_file.bazelrc",
448+
RcFile::ParseError::UNREADABLE_FILE,
449+
kIsWindows
450+
? "Unexpected error reading \\.blazerc file 'somefile':.*"
451+
: "Unexpected error reading .blazerc file 'somefile': \\(error: 2\\): "
452+
"No such file or directory");
432453
}
433454

434455
TEST_F(RcOptionsTest, TryImportedFileDoesNotExist) {
@@ -443,29 +464,23 @@ TEST_F(RcOptionsTest, ImportHasTooManyArgs) {
443464
WriteRc("bad_import.bazelrc",
444465
"import somefile bar");
445466

446-
RcFile::ParseError error;
447-
string error_text;
448-
std::unique_ptr<RcFile> rc = Parse("bad_import.bazelrc", &error, &error_text);
449-
EXPECT_EQ(error, RcFile::ParseError::INVALID_FORMAT);
450-
ASSERT_THAT(
451-
error_text,
452-
MatchesRegex("Invalid import declaration in .blazerc file "
453-
"'.*bad_import.bazelrc': 'import somefile bar' \\(are you "
454-
"in your source checkout/WORKSPACE\\?\\)"));
467+
FailToParseRcWithErrorAndRegex(
468+
"bad_import.bazelrc",
469+
RcFile::ParseError::INVALID_FORMAT,
470+
"Invalid import declaration in .blazerc file "
471+
"'.*bad_import.bazelrc': 'import somefile bar' \\(are you "
472+
"in your source checkout/WORKSPACE\\?\\)");
455473
}
456474

457475
TEST_F(RcOptionsTest, TryImportHasTooManyArgs) {
458476
WriteRc("bad_import.bazelrc", "try-import somefile bar");
459477

460-
RcFile::ParseError error;
461-
string error_text;
462-
std::unique_ptr<RcFile> rc = Parse("bad_import.bazelrc", &error, &error_text);
463-
EXPECT_EQ(error, RcFile::ParseError::INVALID_FORMAT);
464-
ASSERT_THAT(
465-
error_text,
466-
MatchesRegex("Invalid import declaration in .blazerc file "
467-
"'.*bad_import.bazelrc': 'try-import somefile bar' \\(are "
468-
"you in your source checkout/WORKSPACE\\?\\)"));
478+
FailToParseRcWithErrorAndRegex(
479+
"bad_import.bazelrc",
480+
RcFile::ParseError::INVALID_FORMAT,
481+
"Invalid import declaration in .blazerc file "
482+
"'.*bad_import.bazelrc': 'try-import somefile bar' \\(are "
483+
"you in your source checkout/WORKSPACE\\?\\)");
469484
}
470485

471486
// TODO(b/34811299) The tests below identify ways that '\' used as a line
@@ -482,9 +497,11 @@ TEST_F(RcOptionsTest, BadStartupLineContinuation_HasWhitespaceAfterSlash) {
482497
WriteRc("bad_startup_line_continuation.bazelrc",
483498
"startup foo \\ \n"
484499
"bar");
485-
SuccessfullyParseRcWithExpectedArgs(
500+
FailToParseRcWithErrorAndRegex(
486501
"bad_startup_line_continuation.bazelrc",
487-
{{"startup", {"foo"}}}); // Does not contain "bar" from the next line.
502+
RcFile::ParseError::INVALID_FORMAT,
503+
"Incomplete line in \\.blazerc file '.*bad_startup_line_continuation\\.bazelrc': "
504+
"'bar'");
488505
}
489506

490507
TEST_F(RcOptionsTest, BadStartupLineContinuation_HasErroneousSlash) {
@@ -500,11 +517,11 @@ TEST_F(RcOptionsTest, BadStartupLineContinuation_HasCommentAfterSlash) {
500517
WriteRc("bad_startup_line_continuation.bazelrc",
501518
"startup foo \\ # comment\n"
502519
"bar");
503-
SuccessfullyParseRcWithExpectedArgs(
520+
FailToParseRcWithErrorAndRegex(
504521
"bad_startup_line_continuation.bazelrc",
505-
// Whitespace between the slash and comment gets counted as a new token,
506-
// and the bar on the next line is ignored (it's an argumentless command).
507-
{{"startup", {"foo", " "}}});
522+
RcFile::ParseError::INVALID_FORMAT,
523+
"Incomplete line in \\.blazerc file '.*bad_startup_line_continuation\\.bazelrc': "
524+
"'bar'");
508525
}
509526

510527
TEST_F(RcOptionsTest, BadStartupLineContinuation_InterpretsNextLineAsNewline) {

0 commit comments

Comments
 (0)