add subtopics to all methods

add self-check looking for #In markup on every method, pointing
to an existing #Subtopic to reference the method.

Docs-Preview: https://skia.org/?cl=104325
Bug: skia:6898
Change-Id: I749a25b9a43033ae68d193249b2c0b810dcf8fc8
Reviewed-on: https://skia-review.googlesource.com/104325
Commit-Queue: Cary Clark <caryclark@skia.org>
Reviewed-by: Cary Clark <caryclark@skia.org>

5 files changed, 183 insertions, 64 deletions

diff --git a/tools/bookmaker/bookmaker.h b/tools/bookmaker/bookmaker.h
index 96372756a1..d1b6382320 100644
--- a/tools/bookmaker/bookmaker.h
+++ b/tools/bookmaker/bookmaker.h
@@ -767,6 +767,11 @@ public:
         kSubtractFrom,
     };
 
+    enum class Format {
+        kIncludeReturn,
+        kOmitReturn,
+    };
+
     Definition() {}
 
     Definition(const char* start, const char* end, int line, Definition* parent)
@@ -843,7 +848,7 @@ public:
     string extractText(TrimExtract trimExtract) const;
     string fiddleName() const;
     const Definition* findClone(string match) const;
-    string formatFunction() const;
+    string formatFunction(Format format) const;
     const Definition* hasChild(MarkType markType) const;
     bool hasMatch(const string& name) const;
     const Definition* hasParam(const string& ref) const;
@@ -1181,11 +1186,11 @@ public:
     };
 
     enum class Resolvable {
-        kNo,    // neither resolved nor output
-        kYes,   // resolved, output
-        kOut,   // not resolved, but output
+        kNo,      // neither resolved nor output
+        kYes,     // resolved, output
+        kOut,     // not resolved, but output
         kLiteral, // output untouched (FIXME: is this really different from kOut?)
-		kClone, // resolved, output, with references to clones as well
+		kClone,   // resolved, output, with references to clones as well
     };
 
     enum class Exemplary {
diff --git a/tools/bookmaker/definition.cpp b/tools/bookmaker/definition.cpp
index 757169b1a9..25d154386d 100644
--- a/tools/bookmaker/definition.cpp
+++ b/tools/bookmaker/definition.cpp
@@ -897,7 +897,7 @@ bool Definition::crossCheckInside(const char* start, const char* end,
     return false;
 }
 
-string Definition::formatFunction() const {
+string Definition::formatFunction(Format format) const {
     const char* end = fContentStart;
     while (end > fStart && ' ' >= end[-1]) {
         --end;
@@ -913,6 +913,9 @@ string Definition::formatFunction() const {
     const char* nameInParser = methodParser.strnstr(name.c_str(), methodParser.fEnd);
     methodParser.skipTo(nameInParser);
     const char* lastEnd = methodParser.fChar;
+    if (Format::kOmitReturn == format) {
+        lastStart = lastEnd;
+    }
     const char* paren = methodParser.strnchr('(', methodParser.fEnd);
     size_t indent;
     if (paren) {
@@ -983,8 +986,10 @@ string Definition::formatFunction() const {
         if (delimiter) {
             if (nextEnd - nextStart >= (ptrdiff_t) (limit - written)) {
                 written = indent;
-                methodStr += '\n';
-                methodStr += string(indent, ' ');
+                if (Format::kIncludeReturn == format) {
+                    methodStr += '\n';
+                    methodStr += string(indent, ' ');
+                }
             }
             methodParser.skipTo(delimiter);
         }
@@ -1214,7 +1219,76 @@ string Definition::NormalizedName(string name) {
     return normalizedName;
 }
 
-bool Definition::paramsMatch(const string& match, const string& name) const {
+static string unpreformat(const string& orig) {
+    string result;
+    int amp = 0;
+    for (auto c : orig) {
+        switch (amp) {
+        case 0:
+            if ('&' == c) {
+                amp = 1;
+            } else {
+                amp = 0;
+                result += c;
+            }
+            break;
+        case 1:
+            if ('l' == c) {
+                amp = 2;
+            } else if ('g' == c) {
+                amp = 3;
+            } else {
+                amp = 0;
+                result += "&";
+                result += c;
+            }
+            break;
+        case 2:
+            if ('t' == c) {
+                amp = 4;
+            } else {
+                amp = 0;
+                result += "&l";
+                result += c;
+            }
+            break;
+        case 3:
+            if ('t' == c) {
+                amp = 5;
+            } else {
+                amp = 0;
+                result += "&g";
+                result += c;
+            }
+            break;
+        case 4:
+            if (';' == c) {
+                result += '<';
+            } else {
+                result += "&lt";
+                result += c;
+            }
+            amp = 0;
+            break;
+        case 5:
+            if (';' == c) {
+                result += '>';
+            } else {
+                result += "&gt";
+                result += c;
+            }
+            amp = 0;
+            break;
+        }
+    }
+    return result;
+}
+
+bool Definition::paramsMatch(const string& matchFormatted, const string& name) const {
+    if (string::npos != matchFormatted.find("readPixels")) {
+        SkDebugf("");
+    }
+    string match = unpreformat(matchFormatted);
     TextParser def(fFileName, fStart, fContentStart, fLineCount);
     const char* dName = def.strnstr(name.c_str(), fContentStart);
     if (!dName) {
diff --git a/tools/bookmaker/mdOut.cpp b/tools/bookmaker/mdOut.cpp
index 7bc70af669..31f8ef4f6f 100644
--- a/tools/bookmaker/mdOut.cpp
+++ b/tools/bookmaker/mdOut.cpp
@@ -357,6 +357,7 @@ bool MdOut::buildRefFromFile(const char* name, const char* outDir) {
             this->lfAlways(1);
             FPRINTF("===");
         }
+        fPopulators.clear();
         fPopulators[kClassesAndStructs].fDescription = "embedded struct and class members";
         fPopulators[kConstants].fDescription = "enum and enum class, const values";
         fPopulators[kConstructors].fDescription = "functions that construct";
@@ -907,7 +908,7 @@ void MdOut::markTypeOut(Definition* def) {
             } break;
         case MarkType::kMethod: {
             string method_name = def->methodName();
-            string formattedStr = def->formatFunction();
+            string formattedStr = def->formatFunction(Definition::Format::kIncludeReturn);
 
 			this->lfAlways(2);
 			FPRINTF("<a name=\"%s\"></a>", def->fFiddle.c_str());
@@ -972,6 +973,9 @@ void MdOut::markTypeOut(Definition* def) {
         case MarkType::kPopulate: {
             SkASSERT(MarkType::kSubtopic == def->fParent->fMarkType);
             string name = def->fParent->fName;
+            if ("Bitmap_Related_Function" == def->fParent->fFiddle) {
+                SkDebugf("");
+            }
             if (kSubtopics == name) {
                 this->subtopicsOut();
             } else {
@@ -1183,13 +1187,20 @@ void MdOut::mdHeaderOutLF(int depth, int lf) {
 
 void MdOut::populateTables(const Definition* def) {
     const Definition* csParent = this->csParent();
+    if (!csParent) {
+        return;
+    }
     for (auto child : def->fChildren) {
-        if (string::npos != child->fName.find("Rect_Set")) {
+        if (string::npos != child->fFiddle.find("Bitmap_Set")) {
             SkDebugf("");
         }
         if (MarkType::kTopic == child->fMarkType || MarkType::kSubtopic == child->fMarkType) {
-            bool legacyTopic = fPopulators.end() != fPopulators.find(child->fName);
-            if (!legacyTopic && child->fName != kOverview) {
+            string name = child->fName;
+            bool builtInTopic = name == kClassesAndStructs || name == kConstants
+                    || name == kConstructors || name == kMemberFunctions || name == kMembers
+                    || name == kOperators || name == kOverview || name == kRelatedFunctions
+                    || name == kSubtopics;
+            if (!builtInTopic && child->fName != kOverview) {
                 this->populator(kRelatedFunctions).fMembers.push_back(child);
             }
             this->populateTables(child);
@@ -1370,6 +1381,9 @@ void MdOut::subtopicOut(const TableContents& tableContents) {
         items[name] = entry;
     }
     for (auto entry : items) {
+        if (string::npos != entry.second->fName.find("SkRect::set")) {
+            SkDebugf("");
+        }
         if (entry.second->fDeprecated) {
             continue;
         }
@@ -1380,15 +1394,9 @@ void MdOut::subtopicOut(const TableContents& tableContents) {
                 break;
             }
         }
-        if (!oneLiner) {
-            SkDebugf("");
-        }
         SkASSERT(oneLiner);
         this->rowOut(entry.first.c_str(), string(oneLiner->fContentStart,
             oneLiner->fContentEnd - oneLiner->fContentStart));
-        if (string::npos != entry.second->fName.find("SkRect::set")) {
-            SkDebugf("");
-        }
         if (tableContents.fShowClones && entry.second->fCloned) {
             int cloneNo = 2;
             string builder = entry.second->fName;
@@ -1396,13 +1404,15 @@ void MdOut::subtopicOut(const TableContents& tableContents) {
                 builder = builder.substr(0, builder.length() - 2);
             }
             builder += '_';
+            this->rowOut("",
+                    preformat(entry.second->formatFunction(Definition::Format::kOmitReturn)));
             do {
                 string match = builder + to_string(cloneNo);
                 auto child = csParent->findClone(match);
                 if (!child) {
                     break;
                 }
-                this->rowOut("", child->methodName());
+                this->rowOut("", preformat(child->formatFunction(Definition::Format::kOmitReturn)));
             } while (++cloneNo);
         }
     }
diff --git a/tools/bookmaker/selfCheck.cpp b/tools/bookmaker/selfCheck.cpp
index 3392945a9f..5f1eb38ece 100644
--- a/tools/bookmaker/selfCheck.cpp
+++ b/tools/bookmaker/selfCheck.cpp
@@ -11,6 +11,27 @@
 #include <windows.h>
 #endif
 
+
+ /* SkDebugf works in both visual studio and git shell, but
+ in git shell output is not piped to grep.
+ printf does not generate output in visual studio, but
+ does in git shell and can be piped.
+ */
+#ifdef SK_BUILD_FOR_WIN
+#define PRINTF(...)                 \
+do {                                \
+    if (IsDebuggerPresent()) {      \
+        SkDebugf(__VA_ARGS__);      \
+    } else {                        \
+        printf(__VA_ARGS__);        \
+    }                               \
+} while (false)
+#else
+#define PRINTF(...)                 \
+        printf(__VA_ARGS__)
+#endif
+
+
 // Check that mutiple like-named methods are under one Subtopic
 
 // Check that SeeAlso reference each other
@@ -47,52 +68,62 @@ public:
 
 protected:
 
-	bool checkRelatedFunctions() {
-		const Definition* cs = this->classOrStruct();
-		vector<string> methodNames;
-		if (cs) {
-			string prefix = cs->fName + "::";
-			for (auto& csChild : cs->fChildren) {
-				if (MarkType::kMethod != csChild->fMarkType) {
-					// only check methods for now
-					continue;
-				}
-				if (Definition::MethodType::kConstructor == csChild->fMethodType) {
-					continue;
-				}
-				if (Definition::MethodType::kDestructor == csChild->fMethodType) {
-					continue;
-				}
-				if (Definition::MethodType::kOperator == csChild->fMethodType) {
-					continue;
-				}
-				if (csChild->fClone) {
-					// FIXME: check to see if all cloned methods are in table
-					// since format of clones is in flux, defer this check for now
-					continue;
-				}
-                bool containsMarkTypeIn = csChild->fDeprecated;  // no markup for deprecated
-                for (auto child : csChild->fChildren) {
-                    if (MarkType::kIn == child->fMarkType) {
-                        containsMarkTypeIn = true;
-                        break;
+    void checkMethod(string topic, const Definition* csChild, vector<string>* reported) {
+        if (MarkType::kSubtopic == csChild->fMarkType) {
+            for (auto child : csChild->fChildren) {
+                checkMethod(topic, child, reported);
+            }
+            return;
+        } else if (MarkType::kMethod != csChild->fMarkType) {
+            // only check methods for now
+            return;
+        }
+        bool containsMarkTypeIn = csChild->fDeprecated  // no markup for deprecated
+                || Definition::MethodType::kConstructor == csChild->fMethodType
+                || Definition::MethodType::kDestructor == csChild->fMethodType
+                || Definition::MethodType::kOperator == csChild->fMethodType
+                || csChild->fClone;
+        for (auto child : csChild->fChildren) {
+            if (MarkType::kIn == child->fMarkType) {
+                containsMarkTypeIn = true;
+                string subtopic(child->fContentStart,
+                    child->fContentEnd - child->fContentStart);
+                string fullname = topic + '_' + subtopic;
+                auto topEnd = fBmhParser.fTopicMap.end();
+                auto topFind = fBmhParser.fTopicMap.find(fullname);
+                auto reportEnd = reported->end();
+                auto reportFind = std::find(reported->begin(), reported->end(), subtopic);
+                if (topEnd == topFind) {
+                    if (reportEnd == reportFind) {
+                        reported->push_back(subtopic);
                     }
                 }
-                if (!containsMarkTypeIn) {
-#ifdef SK_BUILD_FOR_WIN
-                    /* SkDebugf works in both visual studio and git shell, but
-                       in git shell output is not piped to grep.
-                       printf does not generate output in visual studio, but
-                       does in git shell and can be piped.
-                     */
-                    if (IsDebuggerPresent()) {
-                        SkDebugf("No #In: %s\n", csChild->fName.c_str());
-                    } else
-#endif
-                    printf("No #In: %s\n", csChild->fName.c_str());
-                }
-			}
+            }
+        }
+        if (!containsMarkTypeIn) {
+            PRINTF("No #In: %s\n", csChild->fName.c_str());
+        }
+    }
+
+	bool checkRelatedFunctions() {
+		const Definition* cs = this->classOrStruct();
+        if (!cs) {
+            return true;
+        }
+        const Definition* topic = cs->fParent;
+        SkASSERT(topic);
+        SkASSERT(MarkType::kTopic == topic->fMarkType);
+        string topicName = topic->fName;
+        vector<string> methodNames;
+        vector<string> reported;
+		string prefix = cs->fName + "::";
+		for (auto& csChild : cs->fChildren) {
+            checkMethod(topicName, csChild, &reported);
 		}
+        for (auto missing : reported) {
+            string fullname = topicName + '_' + missing;
+            PRINTF("No #Subtopic: %s\n", fullname.c_str());
+        }
 		return true;
 	}
 
diff --git a/tools/bookmaker/spellCheck.cpp b/tools/bookmaker/spellCheck.cpp
index efc79ca528..bafc969ef4 100644
--- a/tools/bookmaker/spellCheck.cpp
+++ b/tools/bookmaker/spellCheck.cpp
@@ -214,7 +214,6 @@ bool SpellCheck::check(Definition* def) {
             if (all_lower(method_name)) {
                 method_name += "()";
             }
-            string formattedStr = def->formatFunction();
             if (!def->isClone() && Definition::MethodType::kOperator != def->fMethodType) {
                 this->wordCheck(method_name);
             }