Skip to content

Commit 405b401

Browse files
committed
Encapsulate canonical_path_result
I want to change canonical_path_result's representation, but its guts are exposed everywhere. Encapsulate canonical_path_result by hiding its private variables and exposing accessor functions. This commit should not change behavior.
1 parent e9a5ca6 commit 405b401

File tree

5 files changed

+60
-25
lines changed

5 files changed

+60
-25
lines changed

src/configuration-loader.cpp

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include <quick-lint-js/warning.h>
1111
#include <string_view>
1212
#include <unordered_map>
13+
#include <utility>
1314

1415
using namespace std::literals::string_view_literals;
1516

@@ -25,7 +26,7 @@ configuration* configuration_loader::load_for_file(const file_to_lint& file) {
2526
configuration* configuration_loader::load_config_file(const char* config_path) {
2627
canonical_path_result canonical_config_path = canonicalize_path(config_path);
2728
if (!canonical_config_path.ok()) {
28-
this->last_error_ = std::move(canonical_config_path.error);
29+
this->last_error_ = std::move(canonical_config_path).error();
2930
return nullptr;
3031
}
3132

@@ -38,8 +39,12 @@ configuration* configuration_loader::load_config_file(const char* config_path) {
3839
this->last_error_ = std::move(config_json.error);
3940
return nullptr;
4041
}
41-
configuration* config =
42-
&this->loaded_config_files_[canonical_config_path.path];
42+
auto [config_it, inserted] = this->loaded_config_files_.emplace(
43+
std::piecewise_construct,
44+
std::forward_as_tuple(canonical_config_path.path()),
45+
std::forward_as_tuple());
46+
QLJS_ASSERT(inserted);
47+
configuration* config = &config_it->second;
4348
config->set_config_file_path(canonical_config_path.c_str());
4449
config->load_from_json(&config_json.content);
4550
return config;
@@ -53,13 +58,13 @@ configuration* configuration_loader::find_and_load_config_file(
5358
canonical_path_result canonical_input_path =
5459
canonicalize_path(input_path ? input_path : ".");
5560
if (!canonical_input_path.ok()) {
56-
this->last_error_ = std::move(canonical_input_path.error);
61+
this->last_error_ = std::move(canonical_input_path).error();
5762
return nullptr;
5863
}
5964

6065
std::string parent_directory =
61-
input_path ? parent_path(std::move(canonical_input_path.path))
62-
: std::move(canonical_input_path.path);
66+
input_path ? parent_path(std::move(canonical_input_path).path())
67+
: std::move(canonical_input_path).path();
6368

6469
// TODO(strager): Directory -> config to reduce lookups in cases like the
6570
// following:

src/file.cpp

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -277,14 +277,33 @@ void write_file(const char *path, string8_view content) {
277277
std::fclose(file);
278278
}
279279

280+
canonical_path_result::canonical_path_result() {}
281+
282+
canonical_path_result::canonical_path_result(const char *path) : path_(path) {}
283+
284+
std::string_view canonical_path_result::path() const &noexcept {
285+
QLJS_ASSERT(this->ok());
286+
return this->path_;
287+
}
288+
289+
std::string &&canonical_path_result::path() && noexcept {
290+
QLJS_ASSERT(this->ok());
291+
return std::move(this->path_);
292+
}
293+
280294
const char *canonical_path_result::c_str() const noexcept {
281295
QLJS_ASSERT(this->ok());
282-
return this->path.c_str();
296+
return this->path_.c_str();
297+
}
298+
299+
std::string &&canonical_path_result::error() && noexcept {
300+
QLJS_ASSERT(!this->ok());
301+
return std::move(this->error_);
283302
}
284303

285304
canonical_path_result canonical_path_result::failure(std::string &&error) {
286305
canonical_path_result result;
287-
result.error = std::move(error);
306+
result.error_ = std::move(error);
288307
QLJS_ASSERT(!result.ok());
289308
return result;
290309
}
@@ -298,18 +317,15 @@ canonical_path_result canonicalize_path(const char *path) {
298317
std::string("failed to canonicalize path ") + path + ": " +
299318
error.message());
300319
}
301-
canonical_path_result result;
302-
result.path = canonical.string();
303-
return result;
320+
return canonical_path_result(canonical.string().c_str());
304321
#elif QLJS_HAVE_REALPATH
305322
char *allocated_canonical = ::realpath(path, nullptr);
306323
if (!allocated_canonical) {
307324
return canonical_path_result::failure(
308325
std::string("failed to canonicalize path ") + path + ": " +
309326
std::strerror(errno));
310327
}
311-
canonical_path_result result;
312-
result.path = allocated_canonical;
328+
canonical_path_result result(allocated_canonical);
313329
std::free(allocated_canonical);
314330
return result;
315331
#else

src/quick-lint-js/file.h

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,25 @@ read_file_result read_stdin(void);
3030
void write_file(const std::string &path, string8_view content);
3131
void write_file(const char *path, string8_view content);
3232

33-
struct canonical_path_result {
34-
std::string path;
35-
std::string error;
36-
37-
bool ok() const noexcept { return this->error.empty(); }
33+
class canonical_path_result {
34+
public:
35+
explicit canonical_path_result(const char *path);
3836

37+
std::string_view path() const &noexcept;
38+
std::string &&path() && noexcept;
3939
const char *c_str() const noexcept;
4040

41+
std::string &&error() && noexcept;
42+
43+
bool ok() const noexcept { return this->error_.empty(); }
44+
4145
static canonical_path_result failure(std::string &&error);
46+
47+
private:
48+
explicit canonical_path_result();
49+
50+
std::string path_;
51+
std::string error_;
4252
};
4353

4454
canonical_path_result canonicalize_path(const char *path);

test/test-configuration-loader.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,12 @@ QLJS_WARNING_IGNORE_GCC("-Wmissing-field-initializers")
3333
#define EXPECT_SAME_FILE(path_a, path_b) \
3434
do { \
3535
canonical_path_result path_a_canonical_ = canonicalize_path((path_a)); \
36-
ASSERT_TRUE(path_a_canonical_.ok()) << path_a_canonical_.error; \
36+
ASSERT_TRUE(path_a_canonical_.ok()) \
37+
<< std::move(path_a_canonical_).error(); \
3738
canonical_path_result path_b_canonical_ = canonicalize_path((path_b)); \
38-
ASSERT_TRUE(path_b_canonical_.ok()) << path_b_canonical_.error; \
39-
EXPECT_EQ(path_a_canonical_.path, path_b_canonical_.path) \
39+
ASSERT_TRUE(path_b_canonical_.ok()) \
40+
<< std::move(path_b_canonical_).error(); \
41+
EXPECT_EQ(path_a_canonical_.path(), path_b_canonical_.path()) \
4042
<< (path_a) << " should be the same file as " << (path_b); \
4143
} while (false)
4244

@@ -208,7 +210,8 @@ TEST_F(test_configuration_loader, quick_lint_js_config_directory_fails) {
208210
});
209211

210212
EXPECT_FALSE(config);
211-
EXPECT_THAT(loader.error(), HasSubstr(canonicalize_path(config_file).path));
213+
EXPECT_THAT(loader.error(),
214+
HasSubstr(canonicalize_path(config_file).c_str()));
212215
EXPECT_THAT(
213216
loader.error(),
214217
AnyOf(HasSubstr("Is a directory"),

test/test-file.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ TEST_F(test_file, canonical_path_to_regular_file) {
206206
write_file(temp_file_path, u8"hello\nworld!\n");
207207

208208
canonical_path_result canonical = canonicalize_path(temp_file_path);
209-
ASSERT_TRUE(canonical.ok()) << canonical.error;
209+
ASSERT_TRUE(canonical.ok()) << std::move(canonical).error();
210210

211211
read_file_result file_content = read_file(canonical.c_str());
212212
ASSERT_TRUE(file_content.ok()) << file_content.error;
@@ -219,8 +219,9 @@ TEST_F(test_file, canonical_path_to_non_existing_file) {
219219

220220
canonical_path_result canonical = canonicalize_path(temp_file_path);
221221
EXPECT_FALSE(canonical.ok());
222-
EXPECT_THAT(canonical.error, HasSubstr("does-not-exist.js"));
223-
EXPECT_THAT(canonical.error,
222+
std::string error = std::move(canonical).error();
223+
EXPECT_THAT(error, HasSubstr("does-not-exist.js"));
224+
EXPECT_THAT(error,
224225
AnyOf(HasSubstr("No such file"), HasSubstr("cannot find")));
225226
}
226227
}

0 commit comments

Comments
 (0)