From d789550da4bf666ca11487ddcb5dd6b9b13ec3d7 Mon Sep 17 00:00:00 2001 From: Cary Clark Date: Wed, 18 Jul 2018 15:10:08 -0400 Subject: 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 Auto-Submit: Cary Clark Reviewed-by: Cary Clark --- tools/bookmaker/bookmaker.cpp | 51 +++++++++++++- tools/bookmaker/bookmaker.h | 44 ++++++++---- tools/bookmaker/cataloger.cpp | 6 +- tools/bookmaker/fiddleParser.cpp | 141 +++++++++++++++----------------------- tools/bookmaker/includeParser.cpp | 3 + tools/bookmaker/parserCommon.cpp | 7 +- 6 files changed, 142 insertions(+), 110 deletions(-) (limited to 'tools') diff --git a/tools/bookmaker/bookmaker.cpp b/tools/bookmaker/bookmaker.cpp index ffd3012e24..1b8e76ee33 100644 --- a/tools/bookmaker/bookmaker.cpp +++ b/tools/bookmaker/bookmaker.cpp @@ -703,6 +703,55 @@ Definition* BmhParser::findExample(string name) const { 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("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* exampleNames) { if (MarkType::kExample == def.fMarkType) { exampleNames->push_back(def.fFiddle); @@ -2640,7 +2689,7 @@ int main(int argc, char** const argv) { } 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 c695cff0e8..19ea86b68d 100644 --- a/tools/bookmaker/bookmaker.h +++ b/tools/bookmaker/bookmaker.h @@ -1328,19 +1328,29 @@ struct JsonStatus { 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 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 fStack; - Json::Value fRoot; const char* fSuffix; StatusFilter fFilter; }; @@ -1415,6 +1425,7 @@ public: 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 @@ public: bool popParentStack(Definition* definition); void reportDuplicates(const Definition& def, string dup) const; + void resetExampleHashes(); void reset() override { INHERITED::resetCommon(); @@ -2084,10 +2096,10 @@ private: 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 @@ protected: } void reset() override { - INHERITED::resetCommon(); + INHERITED::reset(); } Definition* findExample(string name) const { return fBmhParser->findExample(name); } @@ -2111,7 +2123,7 @@ protected: bool fTextOut; bool fPngOut; private: - typedef ParserCommon INHERITED; + typedef JsonCommon INHERITED; }; class FiddleParser : public FiddleBase { @@ -2121,10 +2133,14 @@ public: } bool parseFromFile(const char* path) override { - if (!INHERITED::parseSetup(path)) { + if (!INHERITED::parseFromFile(path)) { + return false; + } + fBmhParser->resetExampleHashes(); + if (!INHERITED::parseFiddles()) { return false; } - return parseFiddles(); + return fBmhParser->checkExampleHashes(); } private: diff --git a/tools/bookmaker/cataloger.cpp b/tools/bookmaker/cataloger.cpp index 0e1e38ac45..c6aae74c30 100644 --- a/tools/bookmaker/cataloger.cpp +++ b/tools/bookmaker/cataloger.cpp @@ -79,14 +79,13 @@ bool Catalog::closeCatalog() { } 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 @@ bool Catalog::parseFromFile(const char* path) { 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 524f89d020..990ff201fb 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; - } - 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; + 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("expected object"); + } + 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("expected array"); } - } while (!this->eof() && this->next() && brackets > 0); - this->reportError("fiddle compile error"); - } - if (!this->skipExact("],\n")) { - return false; - } - if (!this->skipExact(" \"runtime_error\": \"")) { - return false; - } - if ('"' != this->peek()) { - if (!this->skipToEndBracket('"')) { - return false; + if (iter->size()) { + return this->reportError("fiddle compiler error"); + } + continue; } - this->reportError("fiddle runtime error"); - } - if (!this->skipExact("\",\n")) { - 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; + if (!strcmp("runtime_error", memberName)) { + if (!iter->isString()) { + return this->reportError("expected string 1"); + } + if (iter->asString().length()) { + return this->reportError("fiddle runtime error"); } - } while (!this->eof() && this->next()); - const char* stdOutEnd = fChar; - if (example && fTextOut) { - if (!this->textOut(example, stdOutStart, stdOutEnd)) { - return false; + continue; + } + if (!strcmp("fiddleHash", memberName)) { + if (!iter->isString()) { + return this->reportError("expected string 2"); + } + example = this->findExample(blockName); + if (!example) { + return this->reportError("missing example"); + } + if (example->fHash.length() && example->fHash != iter->asString()) { + return example->reportError("mismatched hash"); } + example->fHash = iter->asString(); + continue; } - } else { - if (example && fPngOut) { - if (!this->pngOut(example)) { - return false; + if (!strcmp("text", memberName)) { + if (!iter->isString()) { + return this->reportError("expected string 3"); } + textString = iter->asString(); + continue; } + return this->reportError("unexpected key"); } - if (!this->skipExact("\"\n")) { - return false; - } - if (!this->skipExact(" }")) { - return false; - } - if ('\n' == this->peek()) { - break; + if (!example) { + return this->reportError("missing fiddleHash"); } - if (!this->skipExact(",\n")) { + size_t strLen = textString.length(); + if (strLen) { + if (fTextOut + && !this->textOut(example, textString.c_str(), textString.c_str() + strLen)) { + return false; + } + } else if (fPngOut && !this->pngOut(example)) { return false; } + status->fIter++; } return true; } diff --git a/tools/bookmaker/includeParser.cpp b/tools/bookmaker/includeParser.cpp index fc378ddb45..d6594ea72c 100644 --- a/tools/bookmaker/includeParser.cpp +++ b/tools/bookmaker/includeParser.cpp @@ -2464,6 +2464,9 @@ bool IncludeParser::parseChar() { 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 33bf14918e..fb00b167a8 100644 --- a/tools/bookmaker/parserCommon.cpp +++ b/tools/bookmaker/parserCommon.cpp @@ -392,7 +392,7 @@ bool StatusIter::next(string* str) { return true; } -bool StatusIter::parseFromFile(const char* path) { +bool JsonCommon::parseFromFile(const char* path) { sk_sp json(SkData::MakeFromFileName(path)); if (!json) { SkDebugf("file %s:\n", path); @@ -400,7 +400,7 @@ bool StatusIter::parseFromFile(const char* path) { } 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("file not parsable"); } @@ -409,6 +409,3 @@ bool StatusIter::parseFromFile(const char* path) { return true; } -void StatusIter::reset() { - fStack.clear(); -} -- cgit v1.2.3