Skip to content

Commit ceb0f7b

Browse files
tmikovfacebook-github-bot
tmikov
authored and
facebook-github-bot
committed
pass SourceErrorManager to source map parser
Summary: Previously the source map parser just printed errors to standard error, which is not suitable for a library function. Change its signature to accept a SourceErrorManager. Reviewed By: avp Differential Revision: D26204178 fbshipit-source-id: 903b6cef267feb21cc492e11dbe40cfcd257c986
1 parent e179c66 commit ceb0f7b

6 files changed

Lines changed: 60 additions & 25 deletions

File tree

‎include/hermes/SourceMap/SourceMapParser.h‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,17 @@ class SourceMapParser {
2121
public:
2222
/// Parse input \p sourceMap and return parsed SourceMap.
2323
/// On failure if malformed, prints an error message and returns nullptr.
24-
static std::unique_ptr<SourceMap> parse(llvh::MemoryBufferRef sourceMap);
24+
static std::unique_ptr<SourceMap> parse(
25+
llvh::MemoryBufferRef sourceMap,
26+
SourceErrorManager &sm);
2527

2628
/// Parse input \p sourceMapContent and return parsed SourceMap.
2729
/// Set the filename of the map file to "<source map>".
2830
/// On failure if malformed, prints an error message and returns nullptr.
29-
static std::unique_ptr<SourceMap> parse(llvh::StringRef sourceMapContent) {
30-
return parse(llvh::MemoryBufferRef(sourceMapContent, "<source map>"));
31+
static std::unique_ptr<SourceMap> parse(
32+
llvh::StringRef sourceMapContent,
33+
SourceErrorManager &sm) {
34+
return parse(llvh::MemoryBufferRef(sourceMapContent, "<source map>"), sm);
3135
}
3236

3337
private:

‎lib/CompilerDriver/CompilerDriver.cpp‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1548,7 +1548,9 @@ bool generateIRForSourcesAsCJSModules(
15481548
// This is the first time we're generating IR for this module.
15491549
sources.push_back(fileBuf->getBufferIdentifier());
15501550
if (moduleInSegment.sourceMap) {
1551-
auto inputMap = SourceMapParser::parse(*moduleInSegment.sourceMap);
1551+
SourceErrorManager sm;
1552+
auto inputMap =
1553+
SourceMapParser::parse(*moduleInSegment.sourceMap, sm);
15521554
if (!inputMap) {
15531555
// parse() returns nullptr on failure and reports its own errors.
15541556
return false;
@@ -1824,7 +1826,8 @@ CompileResult processSourceFiles(
18241826
auto &mainFileBuf = fileBufs[0][0];
18251827
std::unique_ptr<SourceMap> sourceMap{nullptr};
18261828
if (mainFileBuf.sourceMap) {
1827-
sourceMap = SourceMapParser::parse(*mainFileBuf.sourceMap);
1829+
SourceErrorManager sm;
1830+
sourceMap = SourceMapParser::parse(*mainFileBuf.sourceMap, sm);
18281831
if (!sourceMap) {
18291832
// parse() returns nullptr on failure and reports its own errors.
18301833
return InputFileError;

‎lib/SourceMap/SourceMapParser.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,11 @@ using namespace hermes::parser;
1919
namespace hermes {
2020

2121
std::unique_ptr<SourceMap> SourceMapParser::parse(
22-
llvh::MemoryBufferRef sourceMap) {
22+
llvh::MemoryBufferRef sourceMap,
23+
SourceErrorManager &sm) {
2324
std::shared_ptr<parser::JSLexer::Allocator> alloc =
2425
std::make_shared<parser::JSLexer::Allocator>();
2526
parser::JSONFactory factory(*alloc);
26-
SourceErrorManager sm;
2727
parser::JSONParser jsonParser(factory, sourceMap, sm);
2828

2929
llvh::Optional<JSONValue *> parsedMap = jsonParser.parse();

‎tools/emhermesc/emhermesc.cpp‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "hermes/BCGen/HBC/HBC.h"
2020
#include "hermes/SourceMap/SourceMapParser.h"
2121
#include "hermes/Support/Algorithms.h"
22+
#include "hermes/Support/SimpleDiagHandler.h"
2223

2324
#include "llvh/Support/SHA1.h"
2425

@@ -127,9 +128,12 @@ extern "C" CompileResult *hermesCompileToBytecode(
127128
return compileRes.release();
128129
}
129130

130-
sourceMap = SourceMapParser::parse({sourceMapData, sourceMapSize - 1});
131+
SourceErrorManager sm;
132+
SimpleDiagHandlerRAII diagHandler(sm);
133+
sourceMap = SourceMapParser::parse({sourceMapData, sourceMapSize - 1}, sm);
131134
if (!sourceMap) {
132-
compileRes->error_ = "Failed to parse source map";
135+
compileRes->error_ =
136+
"Failed to parse source map:" + diagHandler.getErrorString();
133137
return compileRes.release();
134138
}
135139
}

‎tools/hbcdump/hbcdump.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -491,7 +491,8 @@ int main(int argc, char **argv) {
491491
<< sourceMapBufOrErr.getError().message() << "\n";
492492
return -1;
493493
}
494-
sourceMap = SourceMapParser::parse(*sourceMapBufOrErr.get().get());
494+
SourceErrorManager sm;
495+
sourceMap = SourceMapParser::parse(*sourceMapBufOrErr.get().get(), sm);
495496
if (!sourceMap) {
496497
llvh::errs() << "Error loading source map: " << SourceMapFilename << "\n";
497498
return -1;

‎unittests/SourceMap/SourceMapTest.cpp‎

Lines changed: 38 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "hermes/SourceMap/SourceMapGenerator.h"
1010
#include "hermes/SourceMap/SourceMapParser.h"
1111
#include "hermes/Support/Base64vlq.h"
12+
#include "hermes/Support/SimpleDiagHandler.h"
1213

1314
#include "llvh/Support/MemoryBuffer.h"
1415
#include "llvh/Support/raw_ostream.h"
@@ -151,7 +152,9 @@ TEST(SourceMap, Basic) {
151152
OS.str(),
152153
R"#({"version":3,"sources":["file1","file2"],"mappings":"AAAC,EACA,CACA,C,CAAC;ACGI,CACJ,EAAC,EACF;","x_hermes_function_offsets":{"0":[20,23,50,789],"1":[1,255,300,500]}})#");
153154

154-
std::unique_ptr<SourceMap> sourceMap = SourceMapParser::parse(storage);
155+
SourceErrorManager sm;
156+
SimpleDiagHandlerRAII diagHandler(sm);
157+
std::unique_ptr<SourceMap> sourceMap = SourceMapParser::parse(storage, sm);
155158
for (uint32_t line = 0; line < sizeof(segmentsList) / sizeof(segmentsList[0]);
156159
++line) {
157160
const auto &segments = segmentsList[line];
@@ -163,7 +166,10 @@ TEST(SourceMap, Basic) {
163166
}
164167

165168
TEST(SourceMap, InvalidJsonMapTest) {
166-
std::unique_ptr<SourceMap> sourceMap = SourceMapParser::parse(InvalidJsonMap);
169+
SourceErrorManager sm;
170+
SimpleDiagHandlerRAII diagHandler(sm);
171+
std::unique_ptr<SourceMap> sourceMap =
172+
SourceMapParser::parse(InvalidJsonMap, sm);
167173
EXPECT_TRUE(sourceMap == nullptr);
168174
};
169175

@@ -172,8 +178,10 @@ TEST(SourceMap, InvalidJsonMapTest) {
172178
TEST(SourceMap, SourcesField) {
173179
auto verifySources = [](const char *sourceMapContent,
174180
const std::vector<std::string> &expected) {
181+
SourceErrorManager sm;
182+
SimpleDiagHandlerRAII diagHandler(sm);
175183
std::unique_ptr<SourceMap> sourceMap =
176-
SourceMapParser::parse(sourceMapContent);
184+
SourceMapParser::parse(sourceMapContent, sm);
177185
std::vector<std::string> sources = sourceMap->getAllFullPathSources();
178186
EXPECT_EQ(sources.size(), expected.size());
179187
for (uint32_t i = 0; i < expected.size(); ++i) {
@@ -188,7 +196,9 @@ TEST(SourceMap, SourcesField) {
188196
/// "test that the source root is reflected in a mapping's source field" from
189197
/// https://github.com/mozilla/source-map/blob/master/test/test-source-map-consumer.js
190198
TEST(SourceMap, SourceRoot) {
191-
std::unique_ptr<SourceMap> sourceMap = SourceMapParser::parse(TestMap);
199+
SourceErrorManager sm;
200+
SimpleDiagHandlerRAII diagHandler(sm);
201+
std::unique_ptr<SourceMap> sourceMap = SourceMapParser::parse(TestMap, sm);
192202

193203
llvh::Optional<SourceMapTextLocation> locOpt =
194204
sourceMap->getLocationForAddress(2, 2);
@@ -200,7 +210,7 @@ TEST(SourceMap, SourceRoot) {
200210
EXPECT_EQ(locOpt.getValue().fileName, "/the/root/one.js");
201211

202212
std::unique_ptr<SourceMap> sourceMap2 =
203-
SourceMapParser::parse(TestMapNoSourceRoot);
213+
SourceMapParser::parse(TestMapNoSourceRoot, sm);
204214

205215
locOpt = sourceMap2->getLocationForAddress(2, 2);
206216
EXPECT_TRUE(locOpt.hasValue());
@@ -211,7 +221,7 @@ TEST(SourceMap, SourceRoot) {
211221
EXPECT_EQ(locOpt.getValue().fileName, "one.js");
212222

213223
std::unique_ptr<SourceMap> sourceMap3 =
214-
SourceMapParser::parse(TestMapEmptySourceRoot);
224+
SourceMapParser::parse(TestMapEmptySourceRoot, sm);
215225

216226
locOpt = sourceMap3->getLocationForAddress(2, 2);
217227
EXPECT_TRUE(locOpt.hasValue());
@@ -225,7 +235,9 @@ TEST(SourceMap, SourceRoot) {
225235
/// "test mapping tokens back exactly" from
226236
/// https://github.com/mozilla/source-map/blob/master/test/test-source-map-consumer.js
227237
TEST(SourceMap, ExactMappings) {
228-
std::unique_ptr<SourceMap> sourceMap = SourceMapParser::parse(TestMap);
238+
SourceErrorManager sm;
239+
SimpleDiagHandlerRAII diagHandler(sm);
240+
std::unique_ptr<SourceMap> sourceMap = SourceMapParser::parse(TestMap, sm);
229241

230242
std::vector<std::string> sources = {"/the/root/one.js", "/the/root/two.js"};
231243

@@ -257,7 +269,9 @@ TEST(SourceMap, ExactMappings) {
257269
/// "test mapping tokens fuzzy" from
258270
/// https://github.com/mozilla/source-map/blob/master/test/test-source-map-consumer.js
259271
TEST(SourceMap, FuzzyMappings) {
260-
std::unique_ptr<SourceMap> sourceMap = SourceMapParser::parse(TestMap);
272+
SourceErrorManager sm;
273+
SimpleDiagHandlerRAII diagHandler(sm);
274+
std::unique_ptr<SourceMap> sourceMap = SourceMapParser::parse(TestMap, sm);
261275

262276
std::vector<std::string> sources = {"/the/root/one.js", "/the/root/two.js"};
263277

@@ -268,12 +282,15 @@ TEST(SourceMap, FuzzyMappings) {
268282

269283
/// Test to make sure we can parse mappings with no represented location
270284
TEST(SourceMap, NoRepresentedLocation) {
285+
SourceErrorManager sm;
286+
SimpleDiagHandlerRAII diagHandler(sm);
271287
std::unique_ptr<SourceMap> sourceMap = SourceMapParser::parse(
272288
R"#({
273289
"version": 3,
274290
"sources": ["a.js", "b.js"],
275291
"mappings": "CACC,E,G;A,A,CCCC"
276-
})#");
292+
})#",
293+
sm);
277294

278295
std::vector<std::string> sources = {"a.js", "b.js"};
279296

@@ -329,8 +346,10 @@ TEST(SourceMap, MergedWithInputSourceMaps) {
329346
})#";
330347

331348
std::vector<std::unique_ptr<SourceMap>> inputSourceMaps{};
332-
inputSourceMaps.push_back(SourceMapParser::parse(file1MapJson));
333-
inputSourceMaps.push_back(SourceMapParser::parse(file2MapJson));
349+
SourceErrorManager sm;
350+
SimpleDiagHandlerRAII diagHandler(sm);
351+
inputSourceMaps.push_back(SourceMapParser::parse(file1MapJson, sm));
352+
inputSourceMaps.push_back(SourceMapParser::parse(file2MapJson, sm));
334353

335354
std::vector<std::string> sources = {"file1", "file2"};
336355
SourceMap::SegmentList segments = {
@@ -374,7 +393,7 @@ TEST(SourceMap, MergedWithInputSourceMaps) {
374393
R"#({"version":3,"sources":["file1orig","\/foo\/file2orig"],)#"
375394
R"#("mappings":"A,EAAA,CAAA,C,C,C,CCCA,CAAA,CACC;"})#");
376395

377-
std::unique_ptr<SourceMap> sourceMap = SourceMapParser::parse(storage);
396+
std::unique_ptr<SourceMap> sourceMap = SourceMapParser::parse(storage, sm);
378397
for (uint32_t i = 0; i < expectedSegments.size(); ++i) {
379398
verifySegment(
380399
*sourceMap, /*generatedLine*/ 1, expectedSources, expectedSegments[i]);
@@ -438,8 +457,10 @@ TEST(SourceMap, PropagateFbMetadataFromInputs) {
438457
}, 42])#");
439458

440459
std::vector<std::unique_ptr<SourceMap>> inputSourceMaps{};
441-
inputSourceMaps.push_back(SourceMapParser::parse(file1MapJson));
442-
inputSourceMaps.push_back(SourceMapParser::parse(file2MapJson));
460+
SourceErrorManager sm;
461+
SimpleDiagHandlerRAII diagHandler(sm);
462+
inputSourceMaps.push_back(SourceMapParser::parse(file1MapJson, sm));
463+
inputSourceMaps.push_back(SourceMapParser::parse(file2MapJson, sm));
443464

444465
SourceMap::SegmentList segments = {
445466
loc(0, 0, 1, 0), // addr 1:0 -> file1:1:0 (unmapped in file1orig)
@@ -500,8 +521,10 @@ TEST(SourceMap, GenerateWithFbMetadata) {
500521

501522
/// Test to make sure we can properly parse empty lines.
502523
TEST(SourceMap, EmptyLines) {
524+
SourceErrorManager sm;
525+
SimpleDiagHandlerRAII diagHandler(sm);
503526
std::unique_ptr<SourceMap> sourceMap =
504-
SourceMapParser::parse(TestMapEmptyLines);
527+
SourceMapParser::parse(TestMapEmptyLines, sm);
505528

506529
std::vector<std::string> sources = {"one.js", "two.js"};
507530

0 commit comments

Comments
 (0)