Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 22 additions & 15 deletions src/configuration.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,22 +28,19 @@ inline auto find_configuration(const std::filesystem::path &path)
return sourcemeta::blaze::Configuration::find(path);
}

inline auto read_configuration(
inline auto load_configuration(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: load_configuration does not wrap JSON parsing failures from parse_json, so malformed configuration files can now surface without consistent file-context error wrapping on the newly centralized call paths.

(Based on your team's feedback about reusing centralized wrapper errors.) .

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/configuration.h, line 31:

<comment>`load_configuration` does not wrap JSON parsing failures from `parse_json`, so malformed configuration files can now surface without consistent file-context error wrapping on the newly centralized call paths.

(Based on your team's feedback about reusing centralized wrapper errors.) .</comment>

<file context>
@@ -28,22 +28,19 @@ inline auto find_configuration(const std::filesystem::path &path)
 }
 
-inline auto read_configuration(
+inline auto load_configuration(
     const sourcemeta::core::Options &options,
-    const std::optional<std::filesystem::path> &configuration_path,
</file context>

const sourcemeta::core::Options &options,
const std::optional<std::filesystem::path> &configuration_path,
const std::optional<std::filesystem::path> &schema_path = std::nullopt)
const std::optional<std::filesystem::path> &configuration_path)
-> const std::optional<sourcemeta::blaze::Configuration> & {
using CacheKey = std::optional<std::filesystem::path>;
static std::map<CacheKey, std::optional<sourcemeta::blaze::Configuration>>
configuration_cache;

// Check if configuration is already cached for this path
auto iterator{configuration_cache.find(configuration_path)};
if (iterator != configuration_cache.end()) {
return iterator->second;
}

// Compute and cache the configuration
std::optional<sourcemeta::blaze::Configuration> result{std::nullopt};
if (configuration_path.has_value()) {
LOG_DEBUG(options) << "Using configuration file: "
Expand Down Expand Up @@ -95,23 +92,33 @@ inline auto read_configuration(
}
}
}

if (schema_path.has_value() &&
!result.value().applies_to(schema_path.value())) {
LOG_DEBUG(options)
<< "Ignoring configuration file given extensions mismatch: "
<< sourcemeta::core::weakly_canonical(configuration_path.value())
.string()
<< "\n";
result = std::nullopt;
}
}

auto [inserted_iterator, inserted] =
configuration_cache.emplace(configuration_path, std::move(result));
return inserted_iterator->second;
}

inline auto read_configuration(
const sourcemeta::core::Options &options,
const std::optional<std::filesystem::path> &configuration_path,
const std::optional<std::filesystem::path> &schema_path = std::nullopt)
-> const std::optional<sourcemeta::blaze::Configuration> & {
const auto &configuration{load_configuration(options, configuration_path)};
if (configuration.has_value() && schema_path.has_value() &&
!configuration.value().applies_to(schema_path.value())) {
LOG_DEBUG(options)
<< "Ignoring configuration file given extensions mismatch: "
<< sourcemeta::core::weakly_canonical(configuration_path.value())
.string()
<< "\n";
static const std::optional<sourcemeta::blaze::Configuration> empty{
std::nullopt};
return empty;
}
return configuration;
}

} // namespace sourcemeta::jsonschema

#endif
36 changes: 11 additions & 25 deletions src/input.h
Original file line number Diff line number Diff line change
Expand Up @@ -125,17 +125,12 @@ inline auto
merge_configuration_ignore(const std::filesystem::path &configuration_path,
std::set<std::filesystem::path> &blacklist,
const sourcemeta::core::Options &options) -> void {
try {
const auto configuration{sourcemeta::blaze::Configuration::read_json(
configuration_path, sourcemeta::core::read_file_to_string<>)};
for (const auto &ignore_path : configuration.ignore) {
LOG_VERBOSE(options) << "Ignoring path from configuration: "
<< ignore_path << "\n";
blacklist.insert(ignore_path);
}
} catch (const sourcemeta::blaze::ConfigurationParseError &error) {
throw sourcemeta::core::FileError<
sourcemeta::blaze::ConfigurationParseError>(configuration_path, error);
const auto &configuration{load_configuration(options, configuration_path)};
assert(configuration.has_value());
for (const auto &ignore_path : configuration.value().ignore) {
LOG_VERBOSE(options) << "Ignoring path from configuration: " << ignore_path
<< "\n";
blacklist.insert(ignore_path);
}
}

Expand Down Expand Up @@ -486,27 +481,18 @@ inline auto for_each_json(const std::vector<std::string_view> &arguments,
}

for (const auto &entry : arguments) {
std::optional<sourcemeta::blaze::Configuration> entry_configuration{
std::optional<std::filesystem::path> entry_configuration_path{
std::nullopt};
if (entry != "-") {
const auto entry_path{
sourcemeta::core::weakly_canonical(std::filesystem::path{entry})};
const auto configuration_path{
entry_configuration_path =
find_configuration(std::filesystem::is_directory(entry_path)
? entry_path
: entry_path.parent_path())};
if (configuration_path.has_value()) {
try {
entry_configuration = sourcemeta::blaze::Configuration::read_json(
configuration_path.value(),
sourcemeta::core::read_file_to_string<>);
} catch (const sourcemeta::blaze::ConfigurationParseError &error) {
throw sourcemeta::core::FileError<
sourcemeta::blaze::ConfigurationParseError>(
configuration_path.value(), error);
}
}
: entry_path.parent_path());
}
const auto &entry_configuration{
load_configuration(options, entry_configuration_path)};
const auto &extensions{parse_extensions(options, entry_configuration)};
const auto before{result.size()};
handle_json_entry(entry, blacklist, extensions, result, options);
Expand Down
Loading