Skip to content

Commit 6c6d268

Browse files
fix: Handle path normalization in JSON map.
1 parent f07c00d commit 6c6d268

File tree

11 files changed

+59
-15
lines changed

11 files changed

+59
-15
lines changed

core/Error.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ string Error::toString(const GlobalState &gs) const {
124124
stringstream buf;
125125
buf << RESET_STYLE << FILE_POS_STYLE << loc.filePosToString(gs) << RESET_STYLE << ": " << ERROR_COLOR
126126
<< restoreColors(header, ERROR_COLOR) << RESET_COLOR;
127-
if (what.code != 25900) { // SCIPRubyDebug
127+
if (what != scip_indexer::errors::SCIPRubyDebug && what != scip_indexer::errors::SCIPRuby) {
128128
buf << LOW_NOISE_COLOR << " " << gs.errorUrlBase << what.code << RESET_COLOR;
129129
}
130130
if (loc.exists()) {

core/GlobalState.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2156,7 +2156,7 @@ bool GlobalState::shouldReportErrorOn(Loc loc, ErrorClass what) const {
21562156
return false;
21572157
}
21582158
if (this->isSCIPRuby && !this->unsilenceErrors) {
2159-
if (what.code != 25900) { // SCIPRubyDebug
2159+
if (what != scip_indexer::errors::SCIPRubyDebug && what != scip_indexer::errors::SCIPRuby) {
21602160
return false;
21612161
}
21622162
}

core/errors/errors.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,4 @@
1515
#include "core/errors/packager.h"
1616
#include "core/errors/parser.h"
1717
#include "core/errors/resolver.h"
18+
#include "core/errors/scip_ruby.h"

core/errors/scip_ruby.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
#ifndef SORBET_CORE_ERRORS_SCIP_RUBY_H
2+
#define SORBET_CORE_ERRORS_SCIP_RUBY_H
3+
#include "core/Error.h"
4+
5+
namespace sorbet::scip_indexer::errors {
6+
constexpr sorbet::core::ErrorClass SCIPRubyDebug{25900, sorbet::core::StrictLevel::False};
7+
constexpr sorbet::core::ErrorClass SCIPRuby{25901, sorbet::core::StrictLevel::False};
8+
} // namespace sorbet::scip_indexer::errors
9+
10+
#endif // SORBET_CORE_ERRORS_SCIP_RUBY_H

docs/scip-ruby/CLI.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,12 @@ If information about the gem being indexed is not present in the JSON file,
5656
and cannot be inferred from the filesystem, then you can supply
5757
the `--gem-metadata` argument as described earlier.
5858

59+
If you run into an error message where a path in the JSON file
60+
is not recognized by `scip-ruby`, you can re-run the indexing command
61+
with extra arguments `--log-recorded-filepaths --debug-log-file out.log`
62+
to identify differences between the JSON file
63+
and paths created by traversing directories.
64+
5965
## `--index-file <arg>`
6066

6167
The path for emitting the SCIP index. Defaults to `index.scip`.

scip_indexer/Debug.cc

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,14 @@
33

44
#include "core/GlobalState.h"
55
#include "core/Loc.h"
6+
#include "core/errors/scip_ruby.h"
67

78
#include "scip_indexer/Debug.h"
89

910
namespace sorbet::scip_indexer {
1011

11-
constexpr sorbet::core::ErrorClass SCIPRubyDebug{25900, sorbet::core::StrictLevel::False};
12-
1312
void _log_debug(const sorbet::core::GlobalState &gs, sorbet::core::Loc loc, std::string s) {
14-
if (auto e = gs.beginError(loc, SCIPRubyDebug)) {
13+
if (auto e = gs.beginError(loc, errors::SCIPRubyDebug)) {
1514
auto lines = absl::StrSplit(s, '\n');
1615
for (auto line = lines.begin(); line != lines.end(); line++) {
1716
auto text = std::string(line->begin(), line->length());

scip_indexer/Debug.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,6 @@ template <typename V, typename Fn> std::string showVec(const V &v, Fn f) {
5555
return out.str();
5656
}
5757

58-
extern const sorbet::core::ErrorClass SCIPRubyDebug;
59-
6058
void _log_debug(const sorbet::core::GlobalState &gs, sorbet::core::Loc loc, std::string s);
6159
} // namespace sorbet::scip_indexer
6260

scip_indexer/SCIPGemMetadata.cc

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,10 @@
99

1010
#include "absl/strings/str_replace.h"
1111

12+
#include "common/FileSystem.h"
1213
#include "common/common.h"
1314
#include "core/FileRef.h"
15+
#include "core/errors/scip_ruby.h"
1416

1517
// Uses ENFORCE as defined by common/common.h, so put this later
1618
#include "rapidjson/document.h"
@@ -201,8 +203,10 @@ optional<shared_ptr<GemMetadata>> GemMapping::lookupGemForFile(const core::Globa
201203
return nullopt;
202204
}
203205

204-
void GemMapping::populateFromNDJSON(const core::GlobalState &gs, const std::string &ndjsonText) {
205-
istringstream input(ndjsonText);
206+
void GemMapping::populateFromNDJSON(const core::GlobalState &gs, const FileSystem &fs, const std::string &ndjsonPath) {
207+
istringstream input(fs.readFile(ndjsonPath));
208+
auto currentDir = filesystem::path(fs.getCurrentDir());
209+
unsigned errorCount = 0;
206210
for (string line; getline(input, line);) {
207211
auto jsonLine = std::string(absl::StripAsciiWhitespace(line));
208212
if (jsonLine.empty()) {
@@ -215,9 +219,31 @@ void GemMapping::populateFromNDJSON(const core::GlobalState &gs, const std::stri
215219
}
216220
if (document.HasMember("path") && document["path"].IsString() && document.HasMember("gem") &&
217221
document["gem"].IsString()) {
218-
// TODO: handle path normalization
219-
auto fileRef = gs.findFileByPath(document["path"].GetString());
222+
auto jsonPath = document["path"].GetString();
223+
auto fileRef = gs.findFileByPath(jsonPath);
220224
if (!fileRef.exists()) {
225+
vector<string> alternatePaths;
226+
auto path = filesystem::path(jsonPath);
227+
auto normalizedPath = path.lexically_normal();
228+
alternatePaths.push_back(normalizedPath);
229+
if (normalizedPath.is_absolute()) {
230+
alternatePaths.push_back(string(normalizedPath.lexically_relative(currentDir).lexically_normal()));
231+
} else {
232+
alternatePaths.push_back(string((currentDir / normalizedPath).lexically_normal()));
233+
}
234+
for (auto &altPath : alternatePaths) {
235+
fileRef = gs.findFileByPath(altPath);
236+
if (fileRef.exists()) {
237+
break;
238+
}
239+
}
240+
errorCount++;
241+
if (errorCount < 5) {
242+
if (auto e = gs.beginError(core::Loc(), errors::SCIPRuby)) {
243+
e.setHeader("File path in JSON map doesn't match known paths: {}", jsonPath);
244+
e.addErrorNote("Use --debug-log-filepaths to check the paths being recorded by scip-ruby");
245+
}
246+
}
221247
continue;
222248
}
223249
auto gemMetadata = GemMetadata::tryParse(document["gem"].GetString());

scip_indexer/SCIPGemMetadata.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ class GemMapping final {
8787

8888
std::optional<std::shared_ptr<GemMetadata>> lookupGemForFile(const core::GlobalState &gs, core::FileRef file) const;
8989

90-
void populateFromNDJSON(const core::GlobalState &, const std::string &ndjsonContents);
90+
void populateFromNDJSON(const core::GlobalState &, const FileSystem &fs, const std::string &ndjsonPath);
9191

9292
void markCurrentGem(GemMetadata gem);
9393
};

scip_indexer/SCIPIndexer.cc

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "core/Loc.h"
3131
#include "core/SymbolRef.h"
3232
#include "core/Symbols.h"
33+
#include "core/errors/scip_ruby.h"
3334
#include "main/pipeline/semantic_extension/SemanticExtension.h"
3435
#include "sorbet_version/sorbet_version.h"
3536

@@ -507,6 +508,7 @@ class SCIPState {
507508
scip::Document document;
508509
// TODO(varun): Double-check the path code and maybe document it,
509510
// to make sure its guarantees match what SCIP expects.
511+
// FIXME: This can contain absolute paths if an absolute path was passed to scip-ruby.
510512
document.set_relative_path(string(file.data(gs).path()));
511513

512514
auto occurrences = this->occurrenceMap.find(file);
@@ -1184,7 +1186,7 @@ class SCIPSemanticExtension : public SemanticExtension {
11841186
auto [gem, errors] = scip_indexer::GemMetadata::readFromConfig(OSFileSystem());
11851187
currentGem = gem;
11861188
for (auto &error : errors) {
1187-
if (auto e = gs.beginError(core::Loc(), scip_indexer::SCIPRubyDebug)) {
1189+
if (auto e = gs.beginError(core::Loc(), scip_indexer::errors::SCIPRubyDebug)) {
11881190
e.setHeader("{}: {}",
11891191
error.kind == scip_indexer::GemMetadataError::Kind::Error ? "error" : "warning",
11901192
error.message);
@@ -1193,7 +1195,7 @@ class SCIPSemanticExtension : public SemanticExtension {
11931195
}
11941196
this->gemMap.markCurrentGem(currentGem);
11951197
if (!this->gemMapPath.empty()) {
1196-
this->gemMap.populateFromNDJSON(gs, OSFileSystem().readFile(this->gemMapPath));
1198+
this->gemMap.populateFromNDJSON(gs, OSFileSystem(), this->gemMapPath);
11971199
}
11981200
};
11991201

0 commit comments

Comments
 (0)