add more fiddle hash checks
Convert some of bookmaker to use real json instead
of rolling its own. Also check to see if all
hashes are read.
TBR=jcgregario@google.com
Docs-Preview: https://skia.org/?cl=142166
Bug: skia:8151
Change-Id: Ib35ecd69648faec3522903e0b552d37b04b73f8b
Reviewed-on: https://skia-review.googlesource.com/142166
Commit-Queue: Cary Clark <caryclark@skia.org>
Auto-Submit: Cary Clark <caryclark@skia.org>
Reviewed-by: Cary Clark <caryclark@skia.org>
diff --git a/tools/bookmaker/bookmaker.cpp b/tools/bookmaker/bookmaker.cpp
index ffd3012..1b8e76e 100644
--- a/tools/bookmaker/bookmaker.cpp
+++ b/tools/bookmaker/bookmaker.cpp
@@ -703,6 +703,55 @@
return nullptr;
}
+static bool check_example_hashes(Definition* def) {
+ if (MarkType::kExample == def->fMarkType) {
+ if (def->fHash.length()) {
+ return true;
+ }
+ for (auto child : def->fChildren) {
+ if (MarkType::kPlatform == child->fMarkType) {
+ if (string::npos != string(child->fContentStart, child->length()).find("!fiddle")) {
+ return true;
+ }
+ }
+ }
+ return def->reportError<bool>("missing hash");
+ }
+ for (auto& child : def->fChildren) {
+ if (!check_example_hashes(child)) {
+ return false;
+ }
+ }
+ return true;
+}
+
+bool BmhParser::checkExampleHashes() const {
+ for (const auto& topic : fTopicMap) {
+ if (!topic.second->fParent && !check_example_hashes(topic.second)) {
+ return false;
+ }
+ }
+ return true;
+}
+
+static void reset_example_hashes(Definition* def) {
+ if (MarkType::kExample == def->fMarkType) {
+ def->fHash.clear();
+ return;
+ }
+ for (auto& child : def->fChildren) {
+ reset_example_hashes(child);
+ }
+}
+
+void BmhParser::resetExampleHashes() {
+ for (const auto& topic : fTopicMap) {
+ if (!topic.second->fParent) {
+ reset_example_hashes(topic.second);
+ }
+ }
+}
+
static void find_examples(const Definition& def, vector<string>* exampleNames) {
if (MarkType::kExample == def.fMarkType) {
exampleNames->push_back(def.fFiddle);
@@ -2640,7 +2689,7 @@
}
if (!done && !FLAGS_fiddle.isEmpty() && FLAGS_examples.isEmpty()) {
FiddleParser fparser(&bmhParser);
- if (!fparser.parseFile(FLAGS_fiddle[0], ".txt", ParserCommon::OneFile::kNo)) {
+ if (!fparser.parseFromFile(FLAGS_fiddle[0])) {
return -1;
}
}
diff --git a/tools/bookmaker/bookmaker.h b/tools/bookmaker/bookmaker.h
index c695cff..19ea86b 100644
--- a/tools/bookmaker/bookmaker.h
+++ b/tools/bookmaker/bookmaker.h
@@ -1328,19 +1328,29 @@
string fName;
};
-class StatusIter : public ParserCommon {
+class JsonCommon : public ParserCommon {
+public:
+ bool empty() { return fStack.empty(); }
+ bool parseFromFile(const char* path) override;
+
+ void reset() override {
+ fStack.clear();
+ INHERITED::resetCommon();
+ }
+
+ vector<JsonStatus> fStack;
+ Json::Value fRoot;
+private:
+ typedef ParserCommon INHERITED;
+};
+
+class StatusIter : public JsonCommon {
public:
StatusIter(const char* statusFile, const char* suffix, StatusFilter);
~StatusIter() override {}
string baseDir();
- bool empty() { return fStack.empty(); }
bool next(string* file);
-protected:
- bool parseFromFile(const char* path) override;
- void reset() override;
private:
- vector<JsonStatus> fStack;
- Json::Value fRoot;
const char* fSuffix;
StatusFilter fFilter;
};
@@ -1415,6 +1425,7 @@
bool checkParamReturn(const Definition* definition) const;
bool dumpExamples(FILE* fiddleOut, Definition& def, bool* continuation) const;
bool dumpExamples(const char* fiddleJsonFileName) const;
+ bool checkExampleHashes() const;
bool childOf(MarkType markType) const;
string className(MarkType markType);
bool collectExternals();
@@ -1441,6 +1452,7 @@
bool popParentStack(Definition* definition);
void reportDuplicates(const Definition& def, string dup) const;
+ void resetExampleHashes();
void reset() override {
INHERITED::resetCommon();
@@ -2084,10 +2096,10 @@
typedef IncludeParser INHERITED;
};
-class FiddleBase : public ParserCommon {
+class FiddleBase : public JsonCommon {
protected:
- FiddleBase(BmhParser* bmh) : ParserCommon()
- , fBmhParser(bmh)
+ FiddleBase(BmhParser* bmh)
+ : fBmhParser(bmh)
, fContinuation(false)
, fTextOut(false)
, fPngOut(false)
@@ -2096,7 +2108,7 @@
}
void reset() override {
- INHERITED::resetCommon();
+ INHERITED::reset();
}
Definition* findExample(string name) const { return fBmhParser->findExample(name); }
@@ -2111,7 +2123,7 @@
bool fTextOut;
bool fPngOut;
private:
- typedef ParserCommon INHERITED;
+ typedef JsonCommon INHERITED;
};
class FiddleParser : public FiddleBase {
@@ -2121,10 +2133,14 @@
}
bool parseFromFile(const char* path) override {
- if (!INHERITED::parseSetup(path)) {
+ if (!INHERITED::parseFromFile(path)) {
return false;
}
- return parseFiddles();
+ fBmhParser->resetExampleHashes();
+ if (!INHERITED::parseFiddles()) {
+ return false;
+ }
+ return fBmhParser->checkExampleHashes();
}
private:
diff --git a/tools/bookmaker/cataloger.cpp b/tools/bookmaker/cataloger.cpp
index 0e1e38a..c6aae74 100644
--- a/tools/bookmaker/cataloger.cpp
+++ b/tools/bookmaker/cataloger.cpp
@@ -79,14 +79,13 @@
}
bool Catalog::parseFromFile(const char* path) {
- if (!INHERITED::parseSetup(path)) {
+ if (!INHERITED::parseFromFile(path)) {
return false;
}
fIndent = 4;
this->writeString("var text = {");
this->lf(1);
fTextOut = true;
- TextParserSave save(this);
if (!parseFiddles()) {
return false;
}
@@ -96,7 +95,8 @@
this->writeString("var pngs = {");
fTextOut = false;
fPngOut = true;
- save.restore();
+ JsonStatus* status = &fStack.back();
+ status->fIter = status->fObject.begin();
fContinuation = false;
return parseFiddles();
}
diff --git a/tools/bookmaker/fiddleParser.cpp b/tools/bookmaker/fiddleParser.cpp
index 524f89d..990ff20 100644
--- a/tools/bookmaker/fiddleParser.cpp
+++ b/tools/bookmaker/fiddleParser.cpp
@@ -8,106 +8,73 @@
#include "bookmaker.h"
bool FiddleBase::parseFiddles() {
- if (!this->skipExact("{\n")) {
+ if (fStack.empty()) {
return false;
}
- while (!this->eof()) {
- if (!this->skipExact(" \"")) {
- return false;
+ JsonStatus* status = &fStack.back();
+ while (status->fIter != status->fObject.end()) {
+ const char* blockName = status->fIter.memberName();
+ Definition* example = nullptr;
+ string textString;
+ if (!status->fObject.isObject()) {
+ return this->reportError<bool>("expected object");
}
- const char* nameLoc = fChar;
- if (!this->skipToEndBracket("\"")) {
- return false;
- }
- string name(nameLoc, fChar - nameLoc);
- if (!this->skipExact("\": {\n")) {
- return false;
- }
- if (!this->skipExact(" \"compile_errors\": [")) {
- return false;
- }
- if (']' != this->peek()) {
- // report compiler errors
- int brackets = 1;
- do {
- if ('[' == this->peek()) {
- ++brackets;
- } else if (']' == this->peek()) {
- --brackets;
+ for (auto iter = status->fIter->begin(); status->fIter->end() != iter; ++iter) {
+ const char* memberName = iter.memberName();
+ if (!strcmp("compile_errors", memberName)) {
+ if (!iter->isArray()) {
+ return this->reportError<bool>("expected array");
}
- } while (!this->eof() && this->next() && brackets > 0);
- this->reportError("fiddle compile error");
+ if (iter->size()) {
+ return this->reportError<bool>("fiddle compiler error");
+ }
+ continue;
+ }
+ if (!strcmp("runtime_error", memberName)) {
+ if (!iter->isString()) {
+ return this->reportError<bool>("expected string 1");
+ }
+ if (iter->asString().length()) {
+ return this->reportError<bool>("fiddle runtime error");
+ }
+ continue;
+ }
+ if (!strcmp("fiddleHash", memberName)) {
+ if (!iter->isString()) {
+ return this->reportError<bool>("expected string 2");
+ }
+ example = this->findExample(blockName);
+ if (!example) {
+ return this->reportError<bool>("missing example");
+ }
+ if (example->fHash.length() && example->fHash != iter->asString()) {
+ return example->reportError<bool>("mismatched hash");
+ }
+ example->fHash = iter->asString();
+ continue;
+ }
+ if (!strcmp("text", memberName)) {
+ if (!iter->isString()) {
+ return this->reportError<bool>("expected string 3");
+ }
+ textString = iter->asString();
+ continue;
+ }
+ return this->reportError<bool>("unexpected key");
}
- if (!this->skipExact("],\n")) {
- return false;
+ if (!example) {
+ return this->reportError<bool>("missing fiddleHash");
}
- if (!this->skipExact(" \"runtime_error\": \"")) {
- return false;
- }
- if ('"' != this->peek()) {
- if (!this->skipToEndBracket('"')) {
+ size_t strLen = textString.length();
+ if (strLen) {
+ if (fTextOut
+ && !this->textOut(example, textString.c_str(), textString.c_str() + strLen)) {
return false;
}
- this->reportError("fiddle runtime error");
- }
- if (!this->skipExact("\",\n")) {
+ } else if (fPngOut && !this->pngOut(example)) {
return false;
}
- if (!this->skipExact(" \"fiddleHash\": \"")) {
- return false;
- }
- const char* hashStart = fChar;
- if (!this->skipToEndBracket('"')) {
- return false;
- }
- Definition* example = this->findExample(name);
- if (!example) {
- this->reportError("missing example");
- }
- string hash(hashStart, fChar - hashStart);
- if (example) {
- example->fHash = hash;
- }
- if (!this->skipExact("\",\n")) {
- return false;
- }
- if (!this->skipExact(" \"text\": \"")) {
- return false;
- }
- if ('"' != this->peek()) {
- const char* stdOutStart = fChar;
- do {
- if ('\\' == this->peek()) {
- this->next();
- } else if ('"' == this->peek()) {
- break;
- }
- } while (!this->eof() && this->next());
- const char* stdOutEnd = fChar;
- if (example && fTextOut) {
- if (!this->textOut(example, stdOutStart, stdOutEnd)) {
- return false;
- }
- }
- } else {
- if (example && fPngOut) {
- if (!this->pngOut(example)) {
- return false;
- }
- }
- }
- if (!this->skipExact("\"\n")) {
- return false;
- }
- if (!this->skipExact(" }")) {
- return false;
- }
- if ('\n' == this->peek()) {
- break;
- }
- if (!this->skipExact(",\n")) {
- return false;
- }
+ status->fIter++;
}
return true;
}
diff --git a/tools/bookmaker/includeParser.cpp b/tools/bookmaker/includeParser.cpp
index fc378dd..d6594ea 100644
--- a/tools/bookmaker/includeParser.cpp
+++ b/tools/bookmaker/includeParser.cpp
@@ -2464,6 +2464,9 @@
if (!this->checkForWord()) {
return false;
}
+ if (!fParent->fTokens.size()) {
+ break;
+ }
{
const Definition& lastToken = fParent->fTokens.back();
if (lastToken.fType != Definition::Type::kWord) {
diff --git a/tools/bookmaker/parserCommon.cpp b/tools/bookmaker/parserCommon.cpp
index 33bf149..fb00b16 100644
--- a/tools/bookmaker/parserCommon.cpp
+++ b/tools/bookmaker/parserCommon.cpp
@@ -392,7 +392,7 @@
return true;
}
-bool StatusIter::parseFromFile(const char* path) {
+bool JsonCommon::parseFromFile(const char* path) {
sk_sp<SkData> json(SkData::MakeFromFileName(path));
if (!json) {
SkDebugf("file %s:\n", path);
@@ -400,7 +400,7 @@
}
Json::Reader reader;
const char* data = (const char*)json->data();
- if (!reader.parse(data, data+json->size(), fRoot)) {
+ if (!reader.parse(data, data + json->size(), fRoot)) {
SkDebugf("file %s:\n", path);
return this->reportError<bool>("file not parsable");
}
@@ -409,6 +409,3 @@
return true;
}
-void StatusIter::reset() {
- fStack.clear();
-}