bookmaker error handling
Bookmaker will generate instructions on how to
fix detected errors in a few cases:
- if class function is missing description
- if global function is missing description
- if function parameters don't match doxygen
- if function parameters don't match bmh
(The last case above won't happen if bmh #Method
uses #Populate to retrieve parameter descriptions
from the include.)
Adding this revealed that globals weren't always
accounted for in bookmaker's cross-check; fix
that as well.
TBR=reed@google.com
Docs-Preview: https://skia.org/?cl=171224
Bug: skia:
Change-Id: Ic1b41d4722954fa8a42685a8fe7266b8a860c362
Reviewed-on: https://skia-review.googlesource.com/c/171224
Reviewed-by: Cary Clark <caryclark@skia.org>
Commit-Queue: Cary Clark <caryclark@skia.org>
Auto-Submit: Cary Clark <caryclark@skia.org>
diff --git a/docs/SkColor_Reference.bmh b/docs/SkColor_Reference.bmh
index e3efad4..b32eebe 100644
--- a/docs/SkColor_Reference.bmh
+++ b/docs/SkColor_Reference.bmh
@@ -597,7 +597,7 @@
# ------------------------------------------------------------------------------
-#Method void SkColorToHSV(SkColor color, SkScalar hsv[3])
+#Method static void SkColorToHSV(SkColor color, SkScalar hsv[3])
#In Functions
#Line # converts RGB to HSV ##
@@ -673,7 +673,7 @@
# ------------------------------------------------------------------------------
-#Method SkColor SkHSVToColor(const SkScalar hsv[3])
+#Method static SkColor SkHSVToColor(const SkScalar hsv[3])
#In Functions
#Line # converts HSV to RGB ##
diff --git a/docs/SkImageInfo_Reference.bmh b/docs/SkImageInfo_Reference.bmh
index bf425d7..d831d94 100644
--- a/docs/SkImageInfo_Reference.bmh
+++ b/docs/SkImageInfo_Reference.bmh
@@ -189,7 +189,7 @@
#Subtopic Unpremul ##
-#Method static inline bool SkAlphaTypeIsOpaque(SkAlphaType at)
+#Method static bool SkAlphaTypeIsOpaque(SkAlphaType at)
#In Property
#Line # returns if Alpha_Type equals kOpaque_SkAlphaType ##
#Populate
diff --git a/docs/SkRegion_Reference.bmh b/docs/SkRegion_Reference.bmh
index 50ada67..a14055d 100644
--- a/docs/SkRegion_Reference.bmh
+++ b/docs/SkRegion_Reference.bmh
@@ -269,7 +269,7 @@
#Line # iterator returning IRect within clip ##
#Code
- class SK_API Cliperator {
+ class Cliperator {
public:
Cliperator(const SkRegion& region, const SkIRect& clip);
bool done();
diff --git a/docs/undocumented.bmh b/docs/undocumented.bmh
index 68fdc4b..e2c3951 100644
--- a/docs/undocumented.bmh
+++ b/docs/undocumented.bmh
@@ -27,7 +27,6 @@
SkXXX_Reference # ditto
Skia # ditto
SK_ABORT # ditto
- SK_API # ditto
SK_DEBUG # ditto
SK_RELEASE # ditto
SK_USE_FREETYPE_EMBOLDEN # ditto
@@ -189,8 +188,8 @@
##
#Topic Create_Color_Space_Xform_Canvas
-#Method std::unique_ptr<SkCanvas> SK_API SkCreateColorSpaceXformCanvas(SkCanvas* target,
- sk_sp<SkColorSpace> targetCS)
+#Method std::unique_ptr<SkCanvas> SkCreateColorSpaceXformCanvas(SkCanvas* target,
+ sk_sp<SkColorSpace> targetCS)
##
##
@@ -217,7 +216,7 @@
##
#Topic Debugging
-#Method SK_API void SkDebugf(const char format[], ...)
+#Method void SkDebugf(const char format[], ...)
##
##
@@ -772,7 +771,7 @@
#Topic PathOps
#Enum SkPathOp
##
- #Method bool SK_API Op(const SkPath& one, const SkPath& two, SkPathOp op, SkPath* result)
+ #Method bool Op(const SkPath& one, const SkPath& two, SkPathOp op, SkPath* result)
##
#Topic ##
diff --git a/site/user/api/SkColor_Reference.md b/site/user/api/SkColor_Reference.md
index 0868f15..ca11aed 100644
--- a/site/user/api/SkColor_Reference.md
+++ b/site/user/api/SkColor_Reference.md
@@ -569,7 +569,7 @@
---
<pre style="padding: 1em 1em 1em 1em; width: 62.5em;background-color: #f0f0f0">
-void <a href='SkColor_Reference#SkColorToHSV'>SkColorToHSV</a>(<a href='SkColor_Reference#SkColor'>SkColor</a> <a href='SkColor_Reference#Color'>color</a>, <a href='undocumented#SkScalar'>SkScalar</a> hsv[3])
+static void <a href='SkColor_Reference#SkColorToHSV'>SkColorToHSV</a>(<a href='SkColor_Reference#SkColor'>SkColor</a> <a href='SkColor_Reference#Color'>color</a>, <a href='undocumented#SkScalar'>SkScalar</a> hsv[3])
</pre>
Converts ARGB to its HSV components. <a href='SkColor_Reference#Alpha'>Alpha</a> in ARGB is ignored.
diff --git a/site/user/api/SkRegion_Reference.md b/site/user/api/SkRegion_Reference.md
index f766134..98c4ff9 100644
--- a/site/user/api/SkRegion_Reference.md
+++ b/site/user/api/SkRegion_Reference.md
@@ -389,7 +389,7 @@
---
<pre style="padding: 1em 1em 1em 1em;width: 62.5em; background-color: #f0f0f0">
- class SK_API <a href='#SkRegion_Cliperator'>Cliperator</a> {
+ class <a href='#SkRegion_Cliperator'>Cliperator</a> {
public:
<a href='#SkRegion_Cliperator'>Cliperator</a>(const <a href='SkRegion_Reference#SkRegion'>SkRegion</a>& <a href='SkRegion_Reference#Region'>region</a>, const <a href='SkIRect_Reference#SkIRect'>SkIRect</a>& clip);
bool <a href='#SkRegion_Cliperator_done'>done()</a>;
diff --git a/tools/bookmaker/definition.cpp b/tools/bookmaker/definition.cpp
index 8055d69..41407f8 100644
--- a/tools/bookmaker/definition.cpp
+++ b/tools/bookmaker/definition.cpp
@@ -185,12 +185,9 @@
string className(fName, 0, doubleColons - 2);
TextParser iParser(fFileName, fStart, fContentStart, fLineCount);
SkAssertResult(iParser.skipWord("#Method"));
- iParser.skipExact("SK_API");
iParser.skipWhiteSpace();
bool isStatic = iParser.skipExact("static");
iParser.skipWhiteSpace();
- iParser.skipExact("SK_API");
- iParser.skipWhiteSpace();
bool returnsConst = iParser.skipExact("const");
if (returnsConst) {
SkASSERT(0); // incomplete
@@ -644,11 +641,7 @@
return tokenEnd;
}
-bool Definition::crossCheckInside(const char* start, const char* end,
- const Definition& includeToken) const {
- TextParser def(fFileName, start, end, fLineCount);
- const char* tokenEnd = includeToken.methodEnd();
- TextParser inc("", includeToken.fContentStart, tokenEnd, 0);
+bool Definition::SkipImplementationWords(TextParser& inc) {
if (inc.startsWith("SK_API")) {
inc.skipWord("SK_API");
}
@@ -661,7 +654,23 @@
if (inc.startsWith("SK_API")) {
inc.skipWord("SK_API");
}
- inc.skipExact("SkDEBUGCODE(");
+ return inc.skipExact("SkDEBUGCODE(");
+}
+
+bool Definition::crossCheckInside(const char* start, const char* end,
+ const Definition& includeToken) const {
+ TextParser def(fFileName, start, end, fLineCount);
+ const char* tokenEnd = includeToken.methodEnd();
+ TextParser inc("", includeToken.fContentStart, tokenEnd, 0);
+ if (inc.startsWith("static")) {
+ def.skipWhiteSpace();
+ if (!def.startsWith("static")) {
+ return false;
+ }
+ inc.skipWord("static");
+ def.skipWord("static");
+ }
+ (void) Definition::SkipImplementationWords(inc);
do {
bool defEof;
bool incEof;
@@ -1196,10 +1205,6 @@
if ("SkBitmap::validate()" == leaf.first) {
continue;
}
- // SkPath::pathRefIsValid in #ifdef ; prefer to remove chrome dependency to fix
- if ("SkPath::pathRefIsValid" == leaf.first) {
- continue;
- }
// FIXME: end of long tail bugs
SkDebugf("defined in bmh but missing in include: %s\n", leaf.first.c_str());
success = false;
diff --git a/tools/bookmaker/definition.h b/tools/bookmaker/definition.h
index 8aace77..c882ccc 100644
--- a/tools/bookmaker/definition.h
+++ b/tools/bookmaker/definition.h
@@ -197,6 +197,8 @@
fParentIndex = fParent ? (int) fParent->fTokens.size() : -1;
}
+ static bool SkipImplementationWords(TextParser& inc);
+
string simpleName() {
size_t doubleColon = fName.rfind("::");
return string::npos == doubleColon ? fName : fName.substr(doubleColon + 2);
diff --git a/tools/bookmaker/includeParser.cpp b/tools/bookmaker/includeParser.cpp
index a9ec3db..c86a187 100644
--- a/tools/bookmaker/includeParser.cpp
+++ b/tools/bookmaker/includeParser.cpp
@@ -634,6 +634,321 @@
#include <sstream>
#include <iostream>
+void IncludeParser::checkTokens(list<Definition>& tokens, string key, string className,
+ RootDefinition* root, BmhParser& bmhParser) {
+ for (const auto& token : tokens) {
+ if (token.fPrivate) {
+ continue;
+ }
+ string fullName = key + "::" + token.fName;
+ const Definition* def = nullptr;
+ if (root) {
+ def = root->find(fullName, RootDefinition::AllowParens::kYes);
+ }
+ switch (token.fMarkType) {
+ case MarkType::kMethod: {
+ if (this->isInternalName(token)) {
+ continue;
+ }
+ if (!root) {
+ if (token.fUndocumented) {
+ break;
+ }
+ auto methIter = bmhParser.fMethodMap.find(token.fName);
+ if (bmhParser.fMethodMap.end() != methIter) {
+ def = &methIter->second;
+ if (def->crossCheck2(token)) {
+ def->fVisited = true;
+ } else {
+ this->suggestFix(Suggest::kMethodDiffers, token, root, def);
+ fFailed = true;
+ }
+ } else {
+ this->suggestFix(Suggest::kMethodMissing, token, root, nullptr);
+ fFailed = true;
+ }
+ break;
+ }
+ if (!def) {
+ string paramName = className + "::";
+ paramName += string(token.fContentStart,
+ token.fContentEnd - token.fContentStart);
+ if (string::npos != paramName.find('\n')) {
+ paramName.erase(std::remove(paramName.begin(), paramName.end(), '\n'),
+ paramName.end());
+ }
+ def = root->find(paramName, RootDefinition::AllowParens::kYes);
+ if (!def && 0 == token.fName.find("operator")) {
+ string operatorName = className + "::";
+ TextParser oper("", token.fStart, token.fContentEnd, 0);
+ const char* start = oper.strnstr("operator", token.fContentEnd);
+ SkASSERT(start);
+ oper.skipTo(start);
+ oper.skipToEndBracket('(');
+ int parens = 0;
+ do {
+ if ('(' == oper.peek()) {
+ ++parens;
+ } else if (')' == oper.peek()) {
+ --parens;
+ }
+ } while (!oper.eof() && oper.next() && parens > 0);
+ operatorName += string(start, oper.fChar - start);
+ def = root->find(operatorName, RootDefinition::AllowParens::kYes);
+ }
+ }
+ if (!def) {
+ int skip = !strncmp(token.fContentStart, "explicit ", 9) ? 9 : 0;
+ skip = !strncmp(token.fContentStart, "virtual ", 8) ? 8 : skip;
+ const char* tokenEnd = token.methodEnd();
+ string constructorName = className + "::";
+ constructorName += string(token.fContentStart + skip,
+ tokenEnd - token.fContentStart - skip);
+ def = root->find(constructorName, RootDefinition::AllowParens::kYes);
+ }
+ if (!def && 0 == token.fName.find("SK_")) {
+ string incName = token.fName + "()";
+ string macroName = className + "::" + incName;
+ def = root->find(macroName, RootDefinition::AllowParens::kYes);
+ if (def) {
+ if (def->fName == incName) {
+ def->fVisited = true;
+ if ("SK_TO_STRING_NONVIRT" == token.fName) {
+ def = root->find(className + "::toString",
+ RootDefinition::AllowParens::kYes);
+ if (def) {
+ def->fVisited = true;
+ } else {
+ SkDebugf("missing toString bmh: %s\n", fullName.c_str());
+ fFailed = true;
+ }
+ }
+ break;
+ } else {
+ SkDebugf("method macro differs from bmh: %s\n", fullName.c_str());
+ fFailed = true;
+ }
+ }
+ }
+ if (!def) {
+ bool allLower = true;
+ for (size_t index = 0; index < token.fName.length(); ++index) {
+ if (!islower(token.fName[index])) {
+ allLower = false;
+ break;
+ }
+ }
+ if (allLower) {
+ string lowerName = className + "::" + token.fName + "()";
+ def = root->find(lowerName, RootDefinition::AllowParens::kYes);
+ }
+ }
+ if (!def) {
+ if (0 == token.fName.find("SkDEBUGCODE")) {
+ break;
+ }
+ }
+ if (!def) {
+ // simple method names inside nested classes have a bug and are missing trailing parens
+ string withParens = fullName + "()"; // FIXME: this shouldn't be necessary
+ def = root->find(withParens, RootDefinition::AllowParens::kNo);
+ }
+ if (!def) {
+ if (!token.fUndocumented) {
+ this->suggestFix(Suggest::kMethodMissing, token, root, nullptr);
+ fFailed = true;
+ }
+ break;
+ }
+ if (token.fUndocumented) {
+ // we can't report an error yet; if bmh documents this unnecessarily,
+ // we'll detect that later. It may be that def points to similar
+ // documented function.
+ break;
+ }
+ if (def->crossCheck2(token)) {
+ def->fVisited = true;
+ } else {
+ SkDebugf("method differs from bmh: %s\n", fullName.c_str());
+ fFailed = true;
+ }
+ } break;
+ case MarkType::kComment:
+ break;
+ case MarkType::kEnumClass:
+ case MarkType::kEnum: {
+ if (!def) {
+ // work backwards from first word to deduce #Enum name
+ TextParser firstMember("", token.fStart, token.fContentEnd, 0);
+ SkAssertResult(firstMember.skipName("enum"));
+ SkAssertResult(firstMember.skipToEndBracket('{'));
+ firstMember.next();
+ firstMember.skipWhiteSpace();
+ SkASSERT('k' == firstMember.peek());
+ const char* savePos = firstMember.fChar;
+ firstMember.skipToNonName();
+ const char* wordEnd = firstMember.fChar;
+ firstMember.fChar = savePos;
+ const char* lastUnderscore = nullptr;
+ do {
+ if (!firstMember.skipToEndBracket('_')) {
+ break;
+ }
+ if (firstMember.fChar > wordEnd) {
+ break;
+ }
+ lastUnderscore = firstMember.fChar;
+ } while (firstMember.next());
+ if (lastUnderscore) {
+ ++lastUnderscore;
+ string enumName(lastUnderscore, wordEnd - lastUnderscore);
+ if (root) {
+ string anonName = className + "::" + enumName + 's';
+ def = root->find(anonName, RootDefinition::AllowParens::kYes);
+ } else {
+ auto enumIter = bmhParser.fEnumMap.find(enumName);
+ if (bmhParser.fEnumMap.end() != enumIter) {
+ RootDefinition* rootDef = &enumIter->second;
+ def = rootDef;
+ }
+ }
+ }
+ if (!def && !root) {
+ auto enumIter = bmhParser.fEnumMap.find(token.fName);
+ if (bmhParser.fEnumMap.end() != enumIter) {
+ def = &enumIter->second;
+ }
+ if (!def) {
+ auto enumClassIter = bmhParser.fClassMap.find(token.fName);
+ if (bmhParser.fClassMap.end() != enumClassIter) {
+ def = &enumClassIter->second;
+ }
+ }
+ }
+ if (!def) {
+ if (!token.fUndocumented) {
+ SkDebugf("enum missing from bmh: %s\n", fullName.c_str());
+ fFailed = true;
+ }
+ break;
+ }
+ }
+ def->fVisited = true;
+ bool hasCode = false;
+ bool hasPopulate = true;
+ for (auto& child : def->fChildren) {
+ if (MarkType::kCode == child->fMarkType) {
+ hasPopulate = std::any_of(child->fChildren.begin(),
+ child->fChildren.end(), [](auto grandChild){
+ return MarkType::kPopulate == grandChild->fMarkType; });
+ if (!hasPopulate) {
+ def = child;
+ }
+ hasCode = true;
+ break;
+ }
+ }
+ if (!hasCode && !root) {
+ const Definition* topic = def->topicParent();
+ hasCode = std::any_of(topic->fChildren.begin(), topic->fChildren.end(),
+ [](Definition* def){ return MarkType::kCode == def->fMarkType
+ && def->fChildren.size() > 0 && MarkType::kPopulate ==
+ def->fChildren.front()->fMarkType; });
+ }
+ if (!hasCode) {
+ SkDebugf("enum code missing from bmh: %s\n", fullName.c_str());
+ fFailed = true;
+ break;
+ }
+ if (!hasPopulate) {
+ if (def->crossCheck(token)) {
+ def->fVisited = true;
+ } else {
+ SkDebugf("enum differs from bmh: %s\n", def->fName.c_str());
+ fFailed = true;
+ }
+ }
+ for (auto& member : token.fTokens) {
+ if (MarkType::kMember != member.fMarkType) {
+ continue;
+ }
+ string constName = MarkType::kEnumClass == token.fMarkType ?
+ fullName : className;
+ if (root) {
+ constName += "::" + member.fName;
+ def = root->find(constName, RootDefinition::AllowParens::kYes);
+ } else {
+ auto enumMapper = bmhParser.fEnumMap.find(token.fName);
+ if (bmhParser.fEnumMap.end() != enumMapper) {
+ auto& enumDoc = enumMapper->second;
+ auto memberIter = enumDoc.fLeaves.find(member.fName);
+ if (enumDoc.fLeaves.end() != memberIter) {
+ def = &memberIter->second;
+ }
+ }
+ }
+ if (!def) {
+ string innerName = key + "::" + member.fName;
+ def = root->find(innerName, RootDefinition::AllowParens::kYes);
+ }
+ if (!def) {
+ if (!member.fUndocumented) {
+ SkDebugf("const missing from bmh: %s\n", constName.c_str());
+ fFailed = true;
+ }
+ } else {
+ def->fVisited = true;
+ }
+ }
+ } break;
+ case MarkType::kMember:
+ if (def) {
+ def->fVisited = true;
+ } else {
+ SkDebugf("member missing from bmh: %s\n", fullName.c_str());
+ fFailed = true;
+ }
+ break;
+ case MarkType::kTypedef:
+ if (!def && !root) {
+ auto typedefIter = bmhParser.fTypedefMap.find(token.fName);
+ if (bmhParser.fTypedefMap.end() != typedefIter) {
+ def = &typedefIter->second;
+ }
+ }
+ if (def) {
+ def->fVisited = true;
+ } else {
+ SkDebugf("typedef missing from bmh: %s\n", fullName.c_str());
+ fFailed = true;
+ }
+ break;
+ case MarkType::kConst:
+ if (!def && !root) {
+ auto constIter = bmhParser.fConstMap.find(token.fName);
+ if (bmhParser.fConstMap.end() != constIter) {
+ def = &constIter->second;
+ }
+ }
+ if (def) {
+ def->fVisited = true;
+ } else {
+ if (!token.fUndocumented) {
+ SkDebugf("const missing from bmh: %s\n", fullName.c_str());
+ fFailed = true;
+ }
+ }
+ break;
+ case MarkType::kDefine:
+ // TODO: incomplete
+ break;
+ default:
+ SkASSERT(0); // unhandled
+ break;
+ }
+ }
+}
+
bool IncludeParser::crossCheck(BmhParser& bmhParser) {
for (auto& classMapper : fIClassMap) {
string className = classMapper.first;
@@ -674,242 +989,10 @@
}
}
}
- auto& classMap = classMapper.second;
- auto& tokens = classMap.fTokens;
- for (const auto& token : tokens) {
- if (token.fPrivate) {
- continue;
- }
- string fullName = classMapper.first + "::" + token.fName;
- const Definition* def = root->find(fullName, RootDefinition::AllowParens::kYes);
- switch (token.fMarkType) {
- case MarkType::kMethod: {
- if (this->isInternalName(token)) {
- continue;
- }
- if (!def) {
- string paramName = className + "::";
- paramName += string(token.fContentStart,
- token.fContentEnd - token.fContentStart);
- if (string::npos != paramName.find('\n')) {
- paramName.erase(std::remove(paramName.begin(), paramName.end(), '\n'),
- paramName.end());
- }
- def = root->find(paramName, RootDefinition::AllowParens::kYes);
- if (!def && 0 == token.fName.find("operator")) {
- string operatorName = className + "::";
- TextParser oper("", token.fStart, token.fContentEnd, 0);
- const char* start = oper.strnstr("operator", token.fContentEnd);
- SkASSERT(start);
- oper.skipTo(start);
- oper.skipToEndBracket('(');
- int parens = 0;
- do {
- if ('(' == oper.peek()) {
- ++parens;
- } else if (')' == oper.peek()) {
- --parens;
- }
- } while (!oper.eof() && oper.next() && parens > 0);
- operatorName += string(start, oper.fChar - start);
- def = root->find(operatorName, RootDefinition::AllowParens::kYes);
- }
- }
- if (!def) {
- int skip = !strncmp(token.fContentStart, "explicit ", 9) ? 9 : 0;
- skip = !strncmp(token.fContentStart, "virtual ", 8) ? 8 : skip;
- const char* tokenEnd = token.methodEnd();
- string constructorName = className + "::";
- constructorName += string(token.fContentStart + skip,
- tokenEnd - token.fContentStart - skip);
- def = root->find(constructorName, RootDefinition::AllowParens::kYes);
- }
- if (!def && 0 == token.fName.find("SK_")) {
- string incName = token.fName + "()";
- string macroName = className + "::" + incName;
- def = root->find(macroName, RootDefinition::AllowParens::kYes);
- if (def) {
- if (def->fName == incName) {
- def->fVisited = true;
- if ("SK_TO_STRING_NONVIRT" == token.fName) {
- def = root->find(className + "::toString",
- RootDefinition::AllowParens::kYes);
- if (def) {
- def->fVisited = true;
- } else {
- SkDebugf("missing toString bmh: %s\n", fullName.c_str());
- fFailed = true;
- }
- }
- break;
- } else {
- SkDebugf("method macro differs from bmh: %s\n", fullName.c_str());
- fFailed = true;
- }
- }
- }
- if (!def) {
- bool allLower = true;
- for (size_t index = 0; index < token.fName.length(); ++index) {
- if (!islower(token.fName[index])) {
- allLower = false;
- break;
- }
- }
- if (allLower) {
- string lowerName = className + "::" + token.fName + "()";
- def = root->find(lowerName, RootDefinition::AllowParens::kYes);
- }
- }
- if (!def) {
- if (0 == token.fName.find("SkDEBUGCODE")) {
- break;
- }
- }
- if (!def) {
- // simple method names inside nested classes have a bug and are missing trailing parens
- string withParens = fullName + "()"; // FIXME: this shouldn't be necessary
- def = root->find(withParens, RootDefinition::AllowParens::kNo);
- }
- if (!def) {
- if (!token.fUndocumented) {
- SkDebugf("method missing from bmh: %s\n", fullName.c_str());
- fFailed = true;
- }
- break;
- }
- if (token.fUndocumented) {
- break;
- }
- if (def->crossCheck2(token)) {
- def->fVisited = true;
- } else {
- SkDebugf("method differs from bmh: %s\n", fullName.c_str());
- fFailed = true;
- }
- } break;
- case MarkType::kComment:
- break;
- case MarkType::kEnumClass:
- case MarkType::kEnum: {
- if (!def) {
- // work backwards from first word to deduce #Enum name
- TextParser firstMember("", token.fStart, token.fContentEnd, 0);
- SkAssertResult(firstMember.skipName("enum"));
- SkAssertResult(firstMember.skipToEndBracket('{'));
- firstMember.next();
- firstMember.skipWhiteSpace();
- SkASSERT('k' == firstMember.peek());
- const char* savePos = firstMember.fChar;
- firstMember.skipToNonName();
- const char* wordEnd = firstMember.fChar;
- firstMember.fChar = savePos;
- const char* lastUnderscore = nullptr;
- do {
- if (!firstMember.skipToEndBracket('_')) {
- break;
- }
- if (firstMember.fChar > wordEnd) {
- break;
- }
- lastUnderscore = firstMember.fChar;
- } while (firstMember.next());
- if (lastUnderscore) {
- ++lastUnderscore;
- string anonName = className + "::" + string(lastUnderscore,
- wordEnd - lastUnderscore) + 's';
- def = root->find(anonName, RootDefinition::AllowParens::kYes);
- }
- if (!def) {
- if (!token.fUndocumented) {
- SkDebugf("enum missing from bmh: %s\n", fullName.c_str());
- fFailed = true;
- }
- break;
- }
- }
- def->fVisited = true;
- bool hasCode = false;
- bool hasPopulate = true;
- for (auto& child : def->fChildren) {
- if (MarkType::kCode == child->fMarkType) {
- hasPopulate = std::any_of(child->fChildren.begin(),
- child->fChildren.end(), [](auto grandChild){
- return MarkType::kPopulate == grandChild->fMarkType; });
- if (!hasPopulate) {
- def = child;
- }
- hasCode = true;
- break;
- }
- }
- if (!hasCode) {
- SkDebugf("enum code missing from bmh: %s\n", fullName.c_str());
- fFailed = true;
- break;
- }
- if (!hasPopulate) {
- if (def->crossCheck(token)) {
- def->fVisited = true;
- } else {
- SkDebugf("enum differs from bmh: %s\n", def->fName.c_str());
- fFailed = true;
- }
- }
- for (auto& member : token.fTokens) {
- if (MarkType::kMember != member.fMarkType) {
- continue;
- }
- string constName = MarkType::kEnumClass == token.fMarkType ?
- fullName : className;
- constName += "::" + member.fName;
- def = root->find(constName, RootDefinition::AllowParens::kYes);
- if (!def) {
- string innerName = classMapper.first + "::" + member.fName;
- def = root->find(innerName, RootDefinition::AllowParens::kYes);
- }
- if (!def) {
- if (!member.fUndocumented) {
- SkDebugf("const missing from bmh: %s\n", constName.c_str());
- fFailed = true;
- }
- } else {
- def->fVisited = true;
- }
- }
- } break;
- case MarkType::kMember:
- if (def) {
- def->fVisited = true;
- } else {
- SkDebugf("member missing from bmh: %s\n", fullName.c_str());
- fFailed = true;
- }
- break;
- case MarkType::kTypedef:
- if (def) {
- def->fVisited = true;
- } else {
- SkDebugf("typedef missing from bmh: %s\n", fullName.c_str());
- fFailed = true;
- }
- break;
- case MarkType::kConst:
- if (def) {
- def->fVisited = true;
- } else {
- if (!token.fUndocumented) {
- SkDebugf("const missing from bmh: %s\n", fullName.c_str());
- fFailed = true;
- }
- }
- break;
- default:
- SkASSERT(0); // unhandled
- break;
- }
- }
+ this->checkTokens(classMapper.second.fTokens, classMapper.first, className, root,
+ bmhParser);
}
+ this->checkTokens(fGlobals, "", "", nullptr, bmhParser);
int crossChecks = 0;
string firstCheck;
for (auto& classMapper : fIClassMap) {
@@ -3578,6 +3661,132 @@
remove(fullName.c_str());
}
+static const char kMethodMissingStr[] =
+ "If the method requires documentation, add to "
+ "%s at minimum:\n" // path to bmh file
+ "\n"
+ "#Method %s\n" // method declaration less implementation details
+ "#In SomeSubtopicName\n"
+ "#Line # add a one line description here ##\n"
+ "#Populate\n"
+ "#NoExample\n"
+ "// or better yet, use #Example and put C++ code here\n"
+ "##\n"
+ "#SeeAlso optional related symbols\n"
+ "#Method ##\n"
+ "\n"
+ "Add to %s, at minimum:\n" // path to include
+ "\n"
+ "/** (description) Starts with present tense action verb\n"
+ " and end with a period.\n"
+ "%s" // @param, @return if needed go here
+ "*/\n"
+ "%s ...\n" // method declaration
+ "\n"
+ "If the method does not require documentation,\n"
+ "add \"private\" or \"experimental\", as in:\n"
+ "\n"
+ "/** Experimental, do not use. And so on...\n"
+ "*/\n"
+ "%s ...\n" // method declaration
+ "\n"
+ ;
+
+// bDef does not have #Populate
+static const char kMethodDiffersNoPopStr[] =
+ "In %s:\n" // path to bmh file
+ "#Method %s\n" // method declaration less implementation details
+ "does not match doxygen comment of:\n"
+ "%s.\n" // method declaration
+ "\n"
+ ;
+
+static const char kMethodDiffersStr[] =
+ "In %s:\n" // path to include
+ "%s\n" // method declaration
+ "does not match doxygen comment.\n"
+ "\n"
+ ;
+
+void IncludeParser::suggestFix(Suggest suggest, const Definition& iDef,
+ const RootDefinition* root, const Definition* bDef) {
+ string methodNameStr(iDef.fContentStart, iDef.length());
+ const char* methodName = methodNameStr.c_str();
+ TextParser lessImplParser(&iDef);
+ if (lessImplParser.skipExact("static")) {
+ lessImplParser.skipWhiteSpace();
+ }
+ // TODO : handle debug wrapper
+ /* bool inDebugWrapper = */ Definition::SkipImplementationWords(lessImplParser);
+ string lessImplStr(lessImplParser.fChar, lessImplParser.fEnd - lessImplParser.fChar);
+ const char* methodNameLessImpl = lessImplStr.c_str();
+ // return result, if any is substr from 0 to location of iDef.fName
+ size_t namePos = methodNameStr.find(iDef.fName);
+ SkASSERT(string::npos != namePos);
+ size_t funcEnd = namePos;
+ while (funcEnd > 0 && ' ' >= methodNameStr[funcEnd - 1]) {
+ funcEnd -= 1;
+ }
+ string funcRet = methodNameStr.substr(0, funcEnd);
+// parameters, if any, are delimited by () and separate by ,
+ TextParser parser(&iDef);
+ parser.fChar += namePos + iDef.fName.length();
+ const char* start = parser.fChar;
+ vector<string> paramStrs;
+ if ('(' == start[0]) {
+ parser.skipToBalancedEndBracket('(', ')');
+ TextParser params(&iDef);
+ params.fChar = start + 1;
+ params.fEnd = parser.fChar;
+ while (!params.eof()) {
+ const char* paramEnd = params.anyOf("=,)");
+ const char* paramStart = paramEnd;
+ while (paramStart > params.fChar && ' ' >= paramStart[-1]) {
+ paramStart -= 1;
+ }
+ while (paramStart > params.fChar && (isalnum(paramStart[-1])
+ || '_' == paramStart[-1])) {
+ paramStart -= 1;
+ }
+ string param(paramStart, paramEnd - paramStart);
+ paramStrs.push_back(param);
+ params.fChar = params.anyOf(",)") + 1;
+ }
+ }
+ string bmhFile = root ? root->fFileName : bDef ? bDef->fFileName : "a *.bmh file";
+ bool hasFuncReturn = "" != funcRet && "void" != funcRet;
+ switch(suggest) {
+ case Suggest::kMethodMissing: {
+ // if include @param, @return are missing, request them as well
+ string paramDox;
+ bool firstParam = true;
+ for (auto paramStr : paramStrs) {
+ if (firstParam) {
+ paramDox += "\n";
+ firstParam = false;
+ }
+ paramDox += " @param " + paramStr + " descriptive phrase\n";
+ }
+ if (hasFuncReturn) {
+ paramDox += "\n";
+ paramDox += " @return descriptive phrase\n";
+ }
+ SkDebugf(kMethodMissingStr, bmhFile.c_str(), methodNameLessImpl, iDef.fFileName.c_str(),
+ paramDox.c_str(), methodName, methodName);
+ } break;
+ case Suggest::kMethodDiffers: {
+ bool hasPop = std::any_of(bDef->fChildren.begin(), bDef->fChildren.end(),
+ [](Definition* def) { return MarkType::kPopulate == def->fMarkType; });
+ if (!hasPop) {
+ SkDebugf(kMethodDiffersNoPopStr, bmhFile.c_str(), methodNameLessImpl, methodName);
+ }
+ SkDebugf(kMethodDiffersStr, iDef.fFileName.c_str(), methodName);
+ } break;
+ default:
+ SkASSERT(0);
+ }
+}
+
Bracket IncludeParser::topBracket() const {
Definition* parent = this->parentBracket(fParent);
return parent ? parent->fBracket : Bracket::kNone;
diff --git a/tools/bookmaker/includeParser.h b/tools/bookmaker/includeParser.h
index f132525..34e6f61 100644
--- a/tools/bookmaker/includeParser.h
+++ b/tools/bookmaker/includeParser.h
@@ -36,6 +36,11 @@
kYes,
};
+ enum class Suggest {
+ kMethodMissing,
+ kMethodDiffers,
+ };
+
struct CheckCode {
enum class State {
kNone,
@@ -107,6 +112,8 @@
void checkForMissingParams(const vector<string>& methodParams,
const vector<string>& foundParams);
bool checkForWord();
+ void checkTokens(list<Definition>& tokens, string key, string className,
+ RootDefinition* root, BmhParser& bmhParser);
string className() const;
string codeBlock(const Definition& def, bool inProgress) const {
@@ -261,6 +268,8 @@
fInCharCommentString = fInChar || fInComment || fInString;
}
+ void suggestFix(Suggest suggest, const Definition& iDef, const RootDefinition* root,
+ const Definition* bDef);
Bracket topBracket() const;
template <typename T>
