From a4539a47a626b4a972bc8d78476f54ad70abd8f6 Mon Sep 17 00:00:00 2001 From: Varun Gandhi Date: Tue, 1 Nov 2022 20:01:02 +0800 Subject: [PATCH] chore: Move gem metadata logic into separate TU. --- scip_indexer/BUILD | 2 + scip_indexer/SCIPGemMetadata.cc | 172 ++++++++++++++++++++++++++++++++ scip_indexer/SCIPGemMetadata.h | 76 ++++++++++++++ scip_indexer/SCIPIndexer.cc | 1 + scip_indexer/SCIPSymbolRef.cc | 159 +---------------------------- scip_indexer/SCIPSymbolRef.h | 58 +---------- test/scip_test_runner.cc | 11 +- 7 files changed, 255 insertions(+), 224 deletions(-) create mode 100644 scip_indexer/SCIPGemMetadata.cc create mode 100644 scip_indexer/SCIPGemMetadata.h diff --git a/scip_indexer/BUILD b/scip_indexer/BUILD index d8b251681..5e5330dee 100644 --- a/scip_indexer/BUILD +++ b/scip_indexer/BUILD @@ -32,6 +32,8 @@ cc_library( "Debug.h", "SCIPFieldResolve.cc", "SCIPFieldResolve.h", + "SCIPGemMetadata.cc", + "SCIPGemMetadata.h", "SCIPIndexer.cc", "SCIPProtoExt.cc", "SCIPProtoExt.h", diff --git a/scip_indexer/SCIPGemMetadata.cc b/scip_indexer/SCIPGemMetadata.cc new file mode 100644 index 000000000..27a20894e --- /dev/null +++ b/scip_indexer/SCIPGemMetadata.cc @@ -0,0 +1,172 @@ +#include +#include +#include +#include +#include +#include +#include +#include + +#include "absl/strings/str_replace.h" +#include "scip_indexer/SCIPGemMetadata.h" + +using namespace std; + +namespace sorbet::scip_indexer { + +using GMEKind = GemMetadataError::Kind; +GemMetadataError configNotFoundError = + GemMetadataError{GMEKind::Error, "Failed to find .gemspec file for identifying gem version"}; +GemMetadataError multipleGemspecWarning = GemMetadataError{ + GMEKind::Warning, "Found multiple .gemspec files when trying to infer Gem name and version; picking the first one " + "lexicographically. Consider passing --gem-metadata name@version explicitly instead"}; +GemMetadataError failedToParseGemspecWarning = GemMetadataError{ + GMEKind::Warning, "Failed to parse .gemspec file for inferring gem name and version. Consider passing " + "--gem-metadata name@version explicitly instead"}; +GemMetadataError failedToParseNameFromGemspecWarning = GemMetadataError{ + GMEKind::Warning, "Failed to parse gem name from .gemspec file; using the .gemspec's base name as a proxy"}; +GemMetadataError failedToParseVersionFromGemspecWarning = + GemMetadataError{GMEKind::Warning, "Failed to parse gem version from .gemspec file"}; +GemMetadataError failedToParseGemfileLockWarning{GMEKind::Warning, + "Failed to extract name and version from Gemfile.lock"}; + +pair> GemMetadata::readFromGemfileLock(const string &contents) { + istringstream lines(contents); + bool sawPATH = false; + bool sawSpecs = false; + optional name; + optional version; + vector errors; + // PATH + // remote: . + // specs: + // my_gem_name (M.N.P) + for (string line; getline(lines, line);) { + if (absl::StartsWith(line, "PATH")) { + sawPATH = true; + continue; + } + if (sawPATH && absl::StrContains(line, "specs:")) { + sawSpecs = true; + continue; + } + if (sawSpecs) { + std::regex specLineRegex(R"END(\s+([A-Za-z0-9_\-]+)\s*\((.+)\)\s*)END"); + std::smatch matches; + if (std::regex_match(line, matches, specLineRegex)) { + name = matches[1].str(); + version = matches[2].str(); + } + break; + } + } + if (!name.has_value()) { + errors.push_back(failedToParseGemfileLockWarning); + } + return {GemMetadata{name.value_or(""), version.value_or("")}, errors}; +} + +pair> GemMetadata::readFromGemspec(const string &contents) { + optional name; + optional version; + vector errors; + + std::regex stringKeyRegex(R"END(\s*["'](.+)["'](.freeze)?)END"); + const auto readValue = [&](const string_view line) -> string { + vector entries = absl::StrSplit(line, '='); + if (entries.size() != 2) { + return ""; + } + std::smatch matches; + if (std::regex_match(entries[1], matches, stringKeyRegex)) { + return matches[1].str(); + } + return ""; + }; + istringstream lines(contents); + for (string line; std::getline(lines, line);) { + if (name.has_value() && version.has_value()) { + break; + } + if (!name.has_value() && absl::StrContains(line, ".name =")) { + name = readValue(line); + if (name->empty()) { + errors.push_back(failedToParseNameFromGemspecWarning); + continue; + } + } + // NOTE: In some cases, the version may be stored symbolically, + // in which case this parsing will fail. + if (!version.has_value() && absl::StrContains(line, ".version =")) { + version = readValue(line); + if (version->empty()) { + errors.push_back(failedToParseVersionFromGemspecWarning); + continue; + } + } + } + if (!name.has_value() || !version.has_value()) { + errors.push_back(failedToParseGemspecWarning); + } + return {GemMetadata{name.value_or(""), version.value_or("")}, errors}; +} + +pair> GemMetadata::readFromConfig(const FileSystem &fs) { + UnorderedSet extensions{".lock", ".gemspec"}; + auto paths = fs.listFilesInDir(".", extensions, /*recursive*/ false, {}, {}); + vector errors; + auto currentDirName = [&fs]() -> std::string { + auto currentDirPath = fs.getCurrentDir(); + ENFORCE(!currentDirPath.empty()); + while (currentDirPath.back() == '/') { + currentDirPath.pop_back(); + ENFORCE(!currentDirPath.empty()); + } + return std::filesystem::path(move(currentDirPath)).filename(); + }; + if (paths.empty()) { + errors.push_back(configNotFoundError); + return {GemMetadata(currentDirName(), "latest"), errors}; + } + optional name{}; + optional version{}; + auto copyState = [&](auto &m, auto &errs) { + name = m.name().empty() ? name : m.name(); + version = m.version().empty() ? version : m.version(); + absl::c_copy(errs, std::back_inserter(errors)); + }; + for (auto &path : paths) { + if (!absl::EndsWith(path, "Gemfile.lock")) { + continue; + } + auto [gemMetadata, parseErrors] = GemMetadata::readFromGemfileLock(fs.readFile(path)); + if (!gemMetadata.name().empty() && !gemMetadata.version().empty()) { + return {gemMetadata, {}}; + } + copyState(gemMetadata, parseErrors); + break; + } + string gemspecPath{}; + for (auto &filename : paths) { + if (!absl::EndsWith(filename, ".gemspec")) { + continue; + } + gemspecPath = filename; + auto [gemMetadata, parseErrors] = GemMetadata::readFromGemspec(fs.readFile(filename)); + if (!gemMetadata.name().empty() && !gemMetadata.version().empty()) { + return {gemMetadata, {}}; + } + copyState(gemMetadata, parseErrors); + break; + } + if (name.has_value() && version.has_value()) { + errors.clear(); + } + if (!name.has_value() && !gemspecPath.empty()) { + vector components = absl::StrSplit(gemspecPath, '/'); + name = string(absl::StripSuffix(components.back(), ".gemspec")); + } + return {GemMetadata(name.value_or(currentDirName()), version.value_or("latest")), errors}; +} + +} // namespace sorbet::scip_indexer diff --git a/scip_indexer/SCIPGemMetadata.h b/scip_indexer/SCIPGemMetadata.h new file mode 100644 index 000000000..4407969ba --- /dev/null +++ b/scip_indexer/SCIPGemMetadata.h @@ -0,0 +1,76 @@ +#ifndef SORBET_SCIP_GEM_METADATA +#define SORBET_SCIP_GEM_METADATA + +#include +#include +#include + +#include "absl/strings/str_split.h" + +#include "common/FileSystem.h" + +namespace sorbet::scip_indexer { + +struct GemMetadataError { + enum class Kind { Error, Warning } kind; + std::string message; + + template friend H AbslHashValue(H h, const GemMetadataError &x) { + return H::combine(std::move(h), x.kind, x.message); + } + + bool operator==(const GemMetadataError &other) const { + return this->kind == other.kind && this->message == other.message; + } +}; + +extern GemMetadataError configNotFoundError, multipleGemspecWarning, failedToParseGemspecWarning, + failedToParseGemspecWarning, failedToParseNameFromGemspecWarning, failedToParseVersionFromGemspecWarning, + failedToParseGemfileLockWarning; + +class GemMetadata final { + std::string _name; + std::string _version; + + GemMetadata(std::string name, std::string version) : _name(name), _version(version) {} + +public: + GemMetadata() = default; + GemMetadata &operator=(const GemMetadata &) = default; + + // Don't call this method outside test code! + static GemMetadata forTest(std::string name, std::string version) { + return GemMetadata(name, version); + } + + static std::optional tryParse(const std::string &nameAndVersion) { + std::vector v = absl::StrSplit(nameAndVersion, '@'); + if (v.size() != 2 || v[0].empty() || v[1].empty()) { + return std::nullopt; + } + return GemMetadata{v[0], v[1]}; + } + + const std::string &name() const { + return this->_name; + } + + const std::string &version() const { + return this->_version; + } + + bool operator==(const GemMetadata &other) const { + return this->name() == other.name() && this->version() == other.version(); + } + + // HACK: Do a best-effort parse of any config files to extract the name and version. + static std::pair> readFromConfig(const FileSystem &fs); + +private: + static std::pair> readFromGemfileLock(const std::string &); + static std::pair> readFromGemspec(const std::string &); +}; + +} // namespace sorbet::scip_indexer + +#endif // SORBET_SCIP_GEM_METADATA diff --git a/scip_indexer/SCIPIndexer.cc b/scip_indexer/SCIPIndexer.cc index 4692ba25e..29657662c 100644 --- a/scip_indexer/SCIPIndexer.cc +++ b/scip_indexer/SCIPIndexer.cc @@ -35,6 +35,7 @@ #include "scip_indexer/Debug.h" #include "scip_indexer/SCIPFieldResolve.h" +#include "scip_indexer/SCIPGemMetadata.h" #include "scip_indexer/SCIPProtoExt.h" #include "scip_indexer/SCIPSymbolRef.h" #include "scip_indexer/SCIPUtils.h" diff --git a/scip_indexer/SCIPSymbolRef.cc b/scip_indexer/SCIPSymbolRef.cc index 2f6c98cf6..5f3b3ec39 100644 --- a/scip_indexer/SCIPSymbolRef.cc +++ b/scip_indexer/SCIPSymbolRef.cc @@ -1,11 +1,8 @@ // NOTE: Protobuf headers should go first since they use poisoned functions. #include "proto/SCIP.pb.h" -#include #include -#include #include -#include #include #include "absl/status/status.h" @@ -19,6 +16,7 @@ #include "main/lsp/LSPLoop.h" #include "scip_indexer/Debug.h" +#include "scip_indexer/SCIPGemMetadata.h" #include "scip_indexer/SCIPProtoExt.h" #include "scip_indexer/SCIPSymbolRef.h" @@ -34,161 +32,6 @@ string showRawRelationshipsMap(const core::GlobalState &gs, const RelationshipsM }); } -using GMEKind = GemMetadataError::Kind; -GemMetadataError configNotFoundError = - GemMetadataError{GMEKind::Error, "Failed to find .gemspec file for identifying gem version"}; -GemMetadataError multipleGemspecWarning = GemMetadataError{ - GMEKind::Warning, "Found multiple .gemspec files when trying to infer Gem name and version; picking the first one " - "lexicographically. Consider passing --gem-metadata name@version explicitly instead"}; -GemMetadataError failedToParseGemspecWarning = GemMetadataError{ - GMEKind::Warning, "Failed to parse .gemspec file for inferring gem name and version. Consider passing " - "--gem-metadata name@version explicitly instead"}; -GemMetadataError failedToParseNameFromGemspecWarning = GemMetadataError{ - GMEKind::Warning, "Failed to parse gem name from .gemspec file; using the .gemspec's base name as a proxy"}; -GemMetadataError failedToParseVersionFromGemspecWarning = - GemMetadataError{GMEKind::Warning, "Failed to parse gem version from .gemspec file"}; -GemMetadataError failedToParseGemfileLockWarning{GMEKind::Warning, - "Failed to extract name and version from Gemfile.lock"}; - -pair> GemMetadata::readFromGemfileLock(const string &contents) { - istringstream lines(contents); - bool sawPATH = false; - bool sawSpecs = false; - optional name; - optional version; - vector errors; - // PATH - // remote: . - // specs: - // my_gem_name (M.N.P) - for (string line; getline(lines, line);) { - if (absl::StartsWith(line, "PATH")) { - sawPATH = true; - continue; - } - if (sawPATH && absl::StrContains(line, "specs:")) { - sawSpecs = true; - continue; - } - if (sawSpecs) { - std::regex specLineRegex(R"END(\s+([A-Za-z0-9_\-]+)\s*\((.+)\)\s*)END"); - std::smatch matches; - if (std::regex_match(line, matches, specLineRegex)) { - name = matches[1].str(); - version = matches[2].str(); - } - break; - } - } - if (!name.has_value()) { - errors.push_back(failedToParseGemfileLockWarning); - } - return {GemMetadata{name.value_or(""), version.value_or("")}, errors}; -} - -pair> GemMetadata::readFromGemspec(const string &contents) { - optional name; - optional version; - vector errors; - - std::regex stringKeyRegex(R"END(\s*["'](.+)["'](.freeze)?)END"); - const auto readValue = [&](const string_view line) -> string { - vector entries = absl::StrSplit(line, '='); - if (entries.size() != 2) { - return ""; - } - std::smatch matches; - if (std::regex_match(entries[1], matches, stringKeyRegex)) { - return matches[1].str(); - } - return ""; - }; - istringstream lines(contents); - for (string line; std::getline(lines, line);) { - if (name.has_value() && version.has_value()) { - break; - } - if (!name.has_value() && absl::StrContains(line, ".name =")) { - name = readValue(line); - if (name->empty()) { - errors.push_back(failedToParseNameFromGemspecWarning); - continue; - } - } - // NOTE: In some cases, the version may be stored symbolically, - // in which case this parsing will fail. - if (!version.has_value() && absl::StrContains(line, ".version =")) { - version = readValue(line); - if (version->empty()) { - errors.push_back(failedToParseVersionFromGemspecWarning); - continue; - } - } - } - if (!name.has_value() || !version.has_value()) { - errors.push_back(failedToParseGemspecWarning); - } - return {GemMetadata{name.value_or(""), version.value_or("")}, errors}; -} - -pair> GemMetadata::readFromConfig(const FileSystem &fs) { - UnorderedSet extensions{".lock", ".gemspec"}; - auto paths = fs.listFilesInDir(".", extensions, /*recursive*/ false, {}, {}); - vector errors; - auto currentDirName = [&fs]() -> std::string { - auto currentDirPath = fs.getCurrentDir(); - ENFORCE(!currentDirPath.empty()); - while (currentDirPath.back() == '/') { - currentDirPath.pop_back(); - ENFORCE(!currentDirPath.empty()); - } - return std::filesystem::path(move(currentDirPath)).filename(); - }; - if (paths.empty()) { - errors.push_back(configNotFoundError); - return {GemMetadata(currentDirName(), "latest"), errors}; - } - optional name{}; - optional version{}; - auto copyState = [&](auto &m, auto &errs) { - name = m.name().empty() ? name : m.name(); - version = m.version().empty() ? version : m.version(); - absl::c_copy(errs, std::back_inserter(errors)); - }; - for (auto &path : paths) { - if (!absl::EndsWith(path, "Gemfile.lock")) { - continue; - } - auto [gemMetadata, parseErrors] = GemMetadata::readFromGemfileLock(fs.readFile(path)); - if (!gemMetadata.name().empty() && !gemMetadata.version().empty()) { - return {gemMetadata, {}}; - } - copyState(gemMetadata, parseErrors); - break; - } - string gemspecPath{}; - for (auto &filename : paths) { - if (!absl::EndsWith(filename, ".gemspec")) { - continue; - } - gemspecPath = filename; - auto [gemMetadata, parseErrors] = GemMetadata::readFromGemspec(fs.readFile(filename)); - if (!gemMetadata.name().empty() && !gemMetadata.version().empty()) { - return {gemMetadata, {}}; - } - copyState(gemMetadata, parseErrors); - break; - } - if (name.has_value() && version.has_value()) { - errors.clear(); - } - if (!name.has_value() && !gemspecPath.empty()) { - vector components = absl::StrSplit(gemspecPath, '/'); - name = string(absl::StripSuffix(components.back(), ".gemspec")); - } - return {GemMetadata(name.value_or(currentDirName()), version.value_or("latest")), errors}; -} - // Try to compute a scip::Symbol for this value. absl::Status UntypedGenericSymbolRef::symbolForExpr(const core::GlobalState &gs, const GemMetadata &metadata, optional loc, scip::Symbol &symbol) const { diff --git a/scip_indexer/SCIPSymbolRef.h b/scip_indexer/SCIPSymbolRef.h index c19e46f9f..21086c009 100644 --- a/scip_indexer/SCIPSymbolRef.h +++ b/scip_indexer/SCIPSymbolRef.h @@ -17,6 +17,7 @@ #include "core/TypePtr.h" #include "scip_indexer/SCIPFieldResolve.h" +#include "scip_indexer/SCIPGemMetadata.h" namespace scip { // Avoid needlessly including protobuf header here. class Symbol; @@ -27,63 +28,6 @@ namespace sorbet::scip_indexer { template using SmallVec = InlinedVector; -struct GemMetadataError { - enum class Kind { Error, Warning } kind; - std::string message; - - template friend H AbslHashValue(H h, const GemMetadataError &x) { - return H::combine(std::move(h), x.kind, x.message); - } - - bool operator==(const GemMetadataError &other) const { - return this->kind == other.kind && this->message == other.message; - } -}; - -extern GemMetadataError configNotFoundError, multipleGemspecWarning, failedToParseGemspecWarning, - failedToParseGemspecWarning, failedToParseNameFromGemspecWarning, failedToParseVersionFromGemspecWarning, - failedToParseGemfileLockWarning; - -struct GemMetadataInferenceTestCase; - -class GemMetadata final { - std::string _name; - std::string _version; - - GemMetadata(std::string name, std::string version) : _name(name), _version(version) {} - - friend GemMetadataInferenceTestCase; - -public: - GemMetadata() = default; - GemMetadata &operator=(const GemMetadata &) = default; - - static std::optional tryParse(const std::string &nameAndVersion) { - std::vector v = absl::StrSplit(nameAndVersion, '@'); - if (v.size() != 2 || v[0].empty() || v[1].empty()) { - return std::nullopt; - } - return GemMetadata{v[0], v[1]}; - } - - const std::string &name() const { - return this->_name; - } - - const std::string &version() const { - return this->_version; - } - - bool operator==(const GemMetadata &other) const { - return this->name() == other.name() && this->version() == other.version(); - } - - // HACK: Do a best-effort parse of any config files to extract the name and version. - static std::pair> readFromConfig(const FileSystem &fs); - static std::pair> readFromGemfileLock(const std::string &); - static std::pair> readFromGemspec(const std::string &); -}; - class UntypedGenericSymbolRef; using RelationshipsMap = UnorderedMap; diff --git a/test/scip_test_runner.cc b/test/scip_test_runner.cc index f4f0dacd2..525fe72e8 100644 --- a/test/scip_test_runner.cc +++ b/test/scip_test_runner.cc @@ -44,8 +44,8 @@ #include "resolver/resolver.h" #include "rewriter/rewriter.h" #include "scip_indexer/Debug.h" +#include "scip_indexer/SCIPGemMetadata.h" #include "scip_indexer/SCIPIndexer.h" -#include "scip_indexer/SCIPSymbolRef.h" #include "test/helpers/MockFileSystem.h" #include "test/helpers/expectations.h" #include "test/helpers/position_assertions.h" @@ -59,10 +59,6 @@ struct GemMetadataInferenceTestCase { GemMetadata expectedMetadata; std::vector expectedErrors; - static GemMetadata makeMetadata(string name, string version) { - return GemMetadata(name, version); - } - GemMetadataInferenceTestCase(string fileName, GemMetadata metadata, std::vector expectedErrors, string content) : fileName(fileName), content(content), expectedMetadata(metadata), expectedErrors(expectedErrors) {} @@ -83,10 +79,7 @@ TEST_CASE("GemMetadataInference") { return; } using namespace scip_indexer; - auto metadata = [](auto &name, auto &version) { return GemMetadataInferenceTestCase::makeMetadata(name, version); }; - auto emptyNameGem = GemMetadataInferenceTestCase::makeMetadata("", "0.1"); - auto emptyVersionGem = GemMetadataInferenceTestCase::makeMetadata("sciptest", ""); - auto bothEmptyGem = GemMetadataInferenceTestCase::makeMetadata("", ""); + auto metadata = [](auto &name, auto &version) { return GemMetadata::forTest(name, version); }; // TODO: Create a MockFilesystem here, add a file to that, and then test readFromConfig instead. std::vector testCases{ GemMetadataInferenceTestCase("Gemfile.lock", metadata("sciptest", "0.2"), {}, R"(