Skip to content

Commit 2a62b8a

Browse files
Mizuchimeta-codesync[bot]
authored andcommitted
Support skipping "// ..." and "/* ... */" comments
Summary: Implement JSON5 comment parsing. Now `skipWhitespace()` will invoke both `skipWhitespaceOnly()` and `skipComment` to skip both. Reviewed By: yfeldblum Differential Revision: D93452598 fbshipit-source-id: e212d62dd0f1055c492a627c6d85743bf7daaf53
1 parent a0bab93 commit 2a62b8a

3 files changed

Lines changed: 73 additions & 15 deletions

File tree

third-party/folly/src/folly/json/json.cpp

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,7 @@ struct Input {
353353
});
354354
}
355355

356-
void skipWhitespace() {
356+
bool skipWhitespaceOnly() {
357357
unsigned index = 0;
358358
while (true) {
359359
while (index < range_.size() && range_[index] == ' ') {
@@ -374,6 +374,48 @@ struct Input {
374374
}
375375
range_.advance(index);
376376
storeCurrent();
377+
return index;
378+
}
379+
380+
bool skipComment() {
381+
if (!opts_.allow_json5_experimental) {
382+
return false;
383+
}
384+
if (consume("//")) {
385+
// Single-line comment: skip until CR or LF.
386+
char prev = 0;
387+
skipWhile([&prev](char curr) {
388+
if (prev == '\r' || prev == '\n') {
389+
return false;
390+
}
391+
prev = curr;
392+
return true;
393+
});
394+
return true;
395+
}
396+
if (consume("/*")) {
397+
// Block comment: skip until closing "*/".
398+
char prev = 0;
399+
skipWhile([&prev](char curr) {
400+
if (prev == '*' && curr == '/') {
401+
return false;
402+
}
403+
prev = curr;
404+
return true;
405+
});
406+
if (consume("/")) {
407+
return true;
408+
}
409+
error("unterminated block comment");
410+
}
411+
return false;
412+
}
413+
414+
void skipWhitespace() {
415+
do {
416+
skipWhitespaceOnly();
417+
// Loop to handle adjacent comments, e.g. `/*a*//*b*/`.
418+
} while (skipComment());
377419
}
378420

379421
void expect(char c) {

third-party/folly/src/folly/json/test/Json5Test.cpp

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -67,14 +67,25 @@ constexpr auto kPassedTests = {
6767
"src/arrays/regular-array.json",
6868
"src/arrays/trailing-comma-array.json5",
6969
// Comments
70+
"src/comments/block-comment-following-array-element.json5",
71+
"src/comments/block-comment-following-top-level-value.json5",
7072
"src/comments/block-comment-in-string.json",
73+
"src/comments/block-comment-preceding-top-level-value.json5",
74+
"src/comments/block-comment-with-asterisks.json5",
75+
"src/comments/inline-comment-following-array-element.json5",
76+
"src/comments/inline-comment-following-top-level-value.json5",
7177
"src/comments/inline-comment-in-string.json",
78+
"src/comments/inline-comment-preceding-top-level-value.json5",
7279
"src/comments/top-level-block-comment.txt",
7380
"src/comments/top-level-inline-comment.txt",
7481
"src/comments/unterminated-block-comment.txt",
7582
// Misc
7683
"src/misc/empty.txt",
7784
"src/misc/npm-package.json",
85+
// New lines
86+
"src/new-lines/comment-cr.json5",
87+
"src/new-lines/comment-crlf.json5",
88+
"src/new-lines/comment-lf.json5",
7889
// Numbers
7990
"src/numbers/float.json",
8091
"src/numbers/float-leading-zero.json",
@@ -125,22 +136,11 @@ constexpr auto kPassedTests = {
125136
};
126137

127138
constexpr auto kFailedTests = {
128-
// Comments
129-
"src/comments/block-comment-following-array-element.json5",
130-
"src/comments/block-comment-following-top-level-value.json5",
131-
"src/comments/block-comment-preceding-top-level-value.json5",
132-
"src/comments/block-comment-with-asterisks.json5",
133-
"src/comments/inline-comment-following-array-element.json5",
134-
"src/comments/inline-comment-following-top-level-value.json5",
135-
"src/comments/inline-comment-preceding-top-level-value.json5",
136139
// Misc
137140
"src/misc/npm-package.json5",
138141
"src/misc/readme-example.json5",
139142
"src/misc/valid-whitespace.json5",
140143
// New lines
141-
"src/new-lines/comment-cr.json5",
142-
"src/new-lines/comment-crlf.json5",
143-
"src/new-lines/comment-lf.json5",
144144
"src/new-lines/escaped-cr.json5",
145145
"src/new-lines/escaped-crlf.json5",
146146
"src/new-lines/escaped-lf.json5",

third-party/folly/src/folly/json/test/JsonOtherTest.cpp

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,12 @@
1717
#include <folly/json/json.h>
1818
#include <folly/portability/GTest.h>
1919

20+
// Create a wrapper to suppress lint warnings
21+
folly::dynamic fromJson5(folly::StringPiece s) {
22+
// @lint-ignore CLANGTIDY clang-diagnostic-deprecated-declarations
23+
return folly::parseJson5(s);
24+
}
25+
2026
TEST(Json, StripComments) {
2127
auto testStr = folly::stripLeftMargin(R"JSON(
2228
{
@@ -29,8 +35,8 @@ TEST(Json, StripComments) {
2935
"test4": "foo /* bar", /* comment */
3036
"te//": "foo",
3137
"te/*": "bar",
32-
"\\\"": "\\" /* comment */
33-
}
38+
"\\\"": "\\" /* comment *//* adjacent comment */
39+
} // more comments
3440
)JSON");
3541
auto expectedStr = folly::stripLeftMargin(R"JSON(
3642
{
@@ -44,8 +50,18 @@ TEST(Json, StripComments) {
4450
"te//": "foo",
4551
"te/*": "bar",
4652
"\\\"": "\\"
47-
}
53+
}
4854
)JSON");
4955

5056
EXPECT_EQ(expectedStr, folly::json::stripComments(testStr));
57+
EXPECT_EQ(folly::parseJson(expectedStr), fromJson5(testStr));
58+
59+
// Without json5, comments should fail
60+
EXPECT_THROW(folly::parseJson(testStr), std::exception);
61+
62+
// Unterminated block comment should fail
63+
EXPECT_THROW(fromJson5(testStr + " /* unterminated"), std::exception);
64+
65+
EXPECT_EQ(fromJson5("42 /**/"), 42);
66+
EXPECT_THROW(fromJson5("42 /*/"), std::exception);
5167
}

0 commit comments

Comments
 (0)