diff --git a/docs/SkColor_Reference.bmh b/docs/SkColor_Reference.bmh index e3efad4ece..b32eebeb67 100644 --- a/docs/SkColor_Reference.bmh +++ b/docs/SkColor_Reference.bmh @@ -597,7 +597,7 @@ hsv[2] contains HSV_Value, a value from zero to one. # ------------------------------------------------------------------------------ -#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 @@ Out of range hsv values are pinned. # ------------------------------------------------------------------------------ -#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 bf425d703a..d831d94ed0 100644 --- a/docs/SkImageInfo_Reference.bmh +++ b/docs/SkImageInfo_Reference.bmh @@ -189,7 +189,7 @@ in the same order. #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 50ada6793c..a14055dee6 100644 --- a/docs/SkRegion_Reference.bmh +++ b/docs/SkRegion_Reference.bmh @@ -269,7 +269,7 @@ rect={1,2,3,4} #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 68fdc4b86a..e2c3951479 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 @@ FT_Load_Glyph ## #Topic Create_Color_Space_Xform_Canvas -#Method std::unique_ptr SK_API SkCreateColorSpaceXformCanvas(SkCanvas* target, - sk_sp targetCS) +#Method std::unique_ptr SkCreateColorSpaceXformCanvas(SkCanvas* target, + sk_sp targetCS) ## ## @@ -217,7 +216,7 @@ FT_Load_Glyph ## #Topic Debugging -#Method SK_API void SkDebugf(const char format[], ...) +#Method void SkDebugf(const char format[], ...) ## ## @@ -772,7 +771,7 @@ FT_Load_Glyph #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 0868f150f9..ca11aeda3e 100644 --- a/site/user/api/SkColor_Reference.md +++ b/site/user/api/SkColor_Reference.md @@ -569,7 +569,7 @@ Converts RGB to its HSV components. ---
-void SkColorToHSV(SkColor color, SkScalar hsv[3])
+static void SkColorToHSV(SkColor color, SkScalar hsv[3])
 
Converts ARGB to its HSV components. Alpha in ARGB is ignored. diff --git a/site/user/api/SkRegion_Reference.md b/site/user/api/SkRegion_Reference.md index f766134201..98c4ff954b 100644 --- a/site/user/api/SkRegion_Reference.md +++ b/site/user/api/SkRegion_Reference.md @@ -389,7 +389,7 @@ iterated SkRegion ---
-    class SK_API Cliperator {
+    class Cliperator {
     public:
         Cliperator(const SkRegion& region, const SkIRect& clip);
         bool done();
diff --git a/tools/bookmaker/definition.cpp b/tools/bookmaker/definition.cpp
index 8055d698fb..41407f810e 100644
--- a/tools/bookmaker/definition.cpp
+++ b/tools/bookmaker/definition.cpp
@@ -185,12 +185,9 @@ bool Definition::parseOperator(size_t doubleColons, string& result) {
     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 @@ const char* Definition::methodEnd() const {
     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 @@ bool Definition::crossCheckInside(const char* start, const char* end,
     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 @@ bool RootDefinition::dumpUnVisited() {
             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 8aace77623..c882ccc869 100644
--- a/tools/bookmaker/definition.h
+++ b/tools/bookmaker/definition.h
@@ -197,6 +197,8 @@ public:
         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 a9ec3db4d4..c86a18719f 100644
--- a/tools/bookmaker/includeParser.cpp
+++ b/tools/bookmaker/includeParser.cpp
@@ -634,6 +634,321 @@ void IncludeParser::writeCodeBlock() {
 #include 
 #include 
 
+void IncludeParser::checkTokens(list& 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 @@ bool IncludeParser::crossCheck(BmhParser& bmhParser) {
                 }
             }
         }
-        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 @@ void IncludeParser::RemoveOneFile(const char* docs, const char* includesFile) {
     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 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 f132525663..34e6f61701 100644
--- a/tools/bookmaker/includeParser.h
+++ b/tools/bookmaker/includeParser.h
@@ -36,6 +36,11 @@ public:
         kYes,
     };
 
+    enum class Suggest {
+        kMethodMissing,
+        kMethodDiffers,
+    };
+
     struct CheckCode {
         enum class State {
             kNone,
@@ -107,6 +112,8 @@ public:
     void checkForMissingParams(const vector& methodParams,
                                const vector& foundParams);
     bool checkForWord();
+    void checkTokens(list& tokens, string key, string className,
+            RootDefinition* root, BmhParser& bmhParser);
     string className() const;
 
     string codeBlock(const Definition& def, bool inProgress) const {
@@ -261,6 +268,8 @@ public:
         fInCharCommentString = fInChar || fInComment || fInString;
     }
 
+    void suggestFix(Suggest suggest, const Definition& iDef, const RootDefinition* root,
+            const Definition* bDef);
     Bracket topBracket() const;
 
     template