Skip to content

Commit

Permalink
CLI: print and stop on config errors
Browse files Browse the repository at this point in the history
configuration objects store a list of errors encountered during loading.
In the CLI, print these errors to the console, and don't lint any files
if a config file has problems.
  • Loading branch information
strager committed Jul 19, 2021
1 parent bfdecc5 commit 91f23fd
Show file tree
Hide file tree
Showing 4 changed files with 121 additions and 13 deletions.
5 changes: 5 additions & 0 deletions src/configuration.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,11 @@ void configuration::set_config_file_path(const canonical_path& path) {

void configuration::report_errors(error_reporter* reporter) {
this->errors_.copy_into(reporter);
this->errors_were_reported_ = true;
}

bool configuration::errors_were_reported() const noexcept {
return this->errors_were_reported_;
}

void configuration::reset() {
Expand Down
49 changes: 36 additions & 13 deletions src/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -178,28 +178,51 @@ void handle_options(quick_lint_js::options o) {
std::exit(EXIT_FAILURE);
}

configuration_loader config_loader(
basic_configuration_filesystem::instance());
quick_lint_js::any_error_reporter reporter =
quick_lint_js::any_error_reporter::make(o.output_format, &o.exit_fail_on);
for (const quick_lint_js::file_to_lint &file : o.files_to_lint) {

configuration_loader config_loader(
basic_configuration_filesystem::instance());
for (const file_to_lint &file : o.files_to_lint) {
auto config_result = config_loader.load_for_file(file);
if (!config_result.ok()) {
std::fprintf(stderr, "error: %s\n",
config_result.error_to_string().c_str());
std::exit(1);
}
configuration *config = *config_result ? &(*config_result)->config
: config_loader.get_default_config();
result<padded_string, read_file_io_error> source =
file.is_stdin ? quick_lint_js::read_stdin()
: quick_lint_js::read_file(file.path);
if (!source.ok()) {
source.error().print_and_exit();
loaded_config_file *config_file = *config_result;
if (config_file && !config_file->config.errors_were_reported()) {
if (const std::optional<canonical_path> &config_path =
config_file->config.config_file_path()) {
reporter.set_source(&config_file->file_content,
file_to_lint{
.path = config_path->c_str(),
.config_file = nullptr,
.is_stdin = false,
.vim_bufnr = std::nullopt,
});
config_file->config.report_errors(reporter.get());
}
}
}

if (!reporter.get_error()) {
for (const quick_lint_js::file_to_lint &file : o.files_to_lint) {
auto config_result = config_loader.load_for_file(file);
QLJS_ASSERT(config_result.ok());
configuration *config = *config_result
? &(*config_result)->config
: config_loader.get_default_config();
result<padded_string, read_file_io_error> source =
file.is_stdin ? quick_lint_js::read_stdin()
: quick_lint_js::read_file(file.path);
if (!source.ok()) {
source.error().print_and_exit();
}
reporter.set_source(&*source, file);
quick_lint_js::process_file(&*source, *config, reporter.get(),
o.print_parser_visits);
}
reporter.set_source(&*source, file);
quick_lint_js::process_file(&*source, *config, reporter.get(),
o.print_parser_visits);
}
reporter.finish();

Expand Down
2 changes: 2 additions & 0 deletions src/quick-lint-js/configuration.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class configuration {
void set_config_file_path(canonical_path&&);

void report_errors(error_reporter*);
bool errors_were_reported() const noexcept;

void reset();

Expand All @@ -59,6 +60,7 @@ class configuration {
bool add_global_group_ecmascript_ = true;
monotonic_allocator string_allocator_;
buffering_error_reporter errors_;
bool errors_were_reported_ = false;

string8_view save_string(std::string_view s);
};
Expand Down
78 changes: 78 additions & 0 deletions test/test-cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,84 @@ def test_automatically_find_config_file(self) -> None:
self.assertEqual(result.stdout, "")
self.assertEqual(result.returncode, 0)

def test_config_file_parse_error_prevents_lint(self) -> None:
with tempfile.TemporaryDirectory() as test_directory:
test_file = pathlib.Path(test_directory) / "test.js"
test_file.write_text("console.log(myGlobalVariable);")

config_file = pathlib.Path(test_directory) / "quick-lint-js.config"
config_file.write_text('INVALID JSON')

result = subprocess.run(
[
get_quick_lint_js_executable_path(),
str(test_file),
],
capture_output=True,
encoding="utf-8",
)
self.assertEqual(result.returncode, 1)

# test.js shouldn't be linted.
self.assertNotIn("myGlobalVariable", result.stderr)
self.assertNotIn("E057", result.stderr)

# quick-lint-js.config should have errors.
self.assertIn("quick-lint-js.config", result.stderr)
self.assertIn("E164", result.stderr)

def test_config_error_for_multiple_js_files_is_printed_only_once(self) -> None:
with tempfile.TemporaryDirectory() as test_directory:
test_file_1 = pathlib.Path(test_directory) / "test1.js"
test_file_1.write_text("")
test_file_2 = pathlib.Path(test_directory) / "test2.js"
test_file_2.write_text("")

config_file = pathlib.Path(test_directory) / "quick-lint-js.config"
config_file.write_text('INVALID JSON')

result = subprocess.run(
[
get_quick_lint_js_executable_path(),
str(test_file_1),
str(test_file_2),
],
capture_output=True,
encoding="utf-8",
)
self.assertEqual(result.returncode, 1)
self.assertEqual(result.stderr.count("E164"), 1)

def test_errors_for_all_config_files_are_printed(self) -> None:
with tempfile.TemporaryDirectory() as test_directory:
test_dir_1 = pathlib.Path(test_directory) / "dir1"
test_dir_1.mkdir()
test_file_1 = test_dir_1 / "test.js"
test_file_1.write_text("")
config_file_1 = test_dir_1 / "quick-lint-js.config"
config_file_1.write_text('INVALID JSON')

test_dir_2 = pathlib.Path(test_directory) / "dir2"
test_dir_2.mkdir()
test_file_2 = test_dir_2 / "test.js"
test_file_2.write_text("")
config_file_2 = test_dir_2 / "quick-lint-js.config"
config_file_2.write_text('INVALID JSON')

result = subprocess.run(
[
get_quick_lint_js_executable_path(),
str(test_file_1),
str(test_file_2),
],
capture_output=True,
encoding="utf-8",
)
self.assertEqual(result.returncode, 1)
self.assertIn("dir1", result.stderr)
self.assertIn("dir2", result.stderr)
self.assertEqual(result.stderr.count("E164"), 2)


if __name__ == "__main__":
unittest.main()
Expand Down

0 comments on commit 91f23fd

Please sign in to comment.