Search index checker (#1442)

* Add checking of the search index

* Rearrangement makes broken link errors nicer.

Author: jcollins-g

lib/dartdoc.dart

@@ -6,6 +6,7 @@
 library dartdoc;
 
 import 'dart:async';
+import 'dart:convert';
 import 'dart:io';
 
 import 'package:analyzer/dart/element/element.dart' show LibraryElement;
@@ -214,45 +215,61 @@ class DartDoc {
     return new DartDocResults(packageMeta, package, outputDir);
   }
 
-  void _warn(Package package, PackageWarning kind, String p, String origin,
-      {String source}) {
+  /// Warn on file paths.
+  void _warn(Package package, PackageWarning kind, String warnOn, String origin,
+      {String referredFrom}) {
     // Ordinarily this would go in [Package.warn], but we don't actually know what
     // ModelElement to warn on yet.
-    Locatable referenceElement;
-    Set<Locatable> referenceElements;
+    Locatable referredFromElement;
+    Locatable warnOnElement;
+    Set<Locatable> referredFromElements;
+    Set<Locatable> warnOnElements;
 
     // Make all paths relative to origin.
-    if (path.isWithin(origin, p)) {
-      p = path.relative(p, from: origin);
+    if (path.isWithin(origin, warnOn)) {
+      warnOn = path.relative(warnOn, from: origin);
     }
-    if (source != null) {
-      if (path.isWithin(origin, source)) {
-        source = path.relative(source, from: origin);
+    if (referredFrom != null) {
+      if (path.isWithin(origin, referredFrom)) {
+        referredFrom = path.relative(referredFrom, from: origin);
       }
       // Source paths are always relative.
-      referenceElements = package.allHrefs[source];
-    } else {
-      referenceElements = package.allHrefs[p];
+      referredFromElements = _hrefs[referredFrom];
+    }
+    warnOnElements = _hrefs[warnOn];
+
+    if (referredFromElements != null) {
+      if (referredFromElements.any((e) => e.isCanonical)) {
+        referredFromElement =
+            referredFromElements.firstWhere((e) => e.isCanonical);
+      } else {
+        // If we don't have a canonical element, just pick one.
+        referredFromElement =
+            referredFromElements.isEmpty ? null : referredFromElements.first;
+      }
     }
-    if (referenceElements != null) {
-      if (referenceElements.any((e) => e.isCanonical)) {
-        referenceElement = referenceElements.firstWhere((e) => e.isCanonical);
+    if (warnOnElements != null) {
+      if (warnOnElements.any((e) => e.isCanonical)) {
+        warnOnElement = warnOnElements.firstWhere((e) => e.isCanonical);
       } else {
         // If we don't have a canonical element, just pick one.
-        referenceElement =
-            referenceElements.isEmpty ? null : referenceElements.first;
+        warnOnElement = warnOnElements.isEmpty ? null : warnOnElements.first;
       }
     }
-    if (referenceElement == null && source == 'index.html')
-      referenceElement = package;
-    package.warnOnElement(referenceElement, kind, message: p);
+
+    if (referredFromElement == null && referredFrom == 'index.html')
+      referredFromElement = package;
+    String message = warnOn;
+    if (referredFrom == 'index.json') message = '$warnOn (from index.json)';
+    package.warnOnElement(warnOnElement, kind,
+        message: message, referredFrom: referredFromElement);
   }
 
   void _doOrphanCheck(Package package, String origin, Set<String> visited) {
     String normalOrigin = path.normalize(origin);
     String staticAssets = path.joinAll([normalOrigin, 'static-assets', '']);
     String indexJson = path.joinAll([normalOrigin, 'index.json']);
-    bool foundIndex = false;
+    bool foundIndexJson = false;
     for (FileSystemEntity f
         in new Directory(normalOrigin).listSync(recursive: true)) {
       var fullPath = path.normalize(f.path);
@@ -263,7 +280,7 @@ class DartDoc {
         continue;
       }
       if (fullPath == indexJson) {
-        foundIndex = true;
+        foundIndexJson = true;
         _onCheckProgress.add(fullPath);
         continue;
       }
@@ -277,7 +294,7 @@ class DartDoc {
       _onCheckProgress.add(fullPath);
     }
 
-    if (!foundIndex) {
+    if (!foundIndexJson) {
       _warn(package, PackageWarning.brokenLink, indexJson, normalOrigin);
       _onCheckProgress.add(indexJson);
     }
@@ -305,6 +322,42 @@ class DartDoc {
     return new Tuple2(stringLinks, baseHref);
   }
 
+  void _doSearchIndexCheck(
+      Package package, String origin, Set<String> visited) {
+    String fullPath = path.joinAll([origin, 'index.json']);
+    String indexPath = path.joinAll([origin, 'index.html']);
+    File file = new File("$fullPath");
+    if (!file.existsSync()) {
+      return null;
+    }
+    JsonDecoder decoder = new JsonDecoder();
+    List jsonData = decoder.convert(file.readAsStringSync());
+
+    Set<String> found = new Set();
+    found.add(fullPath);
+    // The package index isn't supposed to be in the search, so suppress the
+    // warning.
+    found.add(indexPath);
+    for (Map<String, String> entry in jsonData) {
+      if (entry.containsKey('href')) {
+        String entryPath = path.joinAll([origin, entry['href']]);
+        if (!visited.contains(entryPath)) {
+          _warn(package, PackageWarning.brokenLink, entryPath,
+              path.normalize(origin),
+              referredFrom: fullPath);
+        }
+        found.add(entryPath);
+      }
+    }
+    // Missing from search index
+    Set<String> missing_from_search = visited.difference(found);
+    for (String s in missing_from_search) {
+      _warn(package, PackageWarning.missingFromSearchIndex, s,
+          path.normalize(origin),
+          referredFrom: fullPath);
+    }
+  }
+
   void _doCheck(
       Package package, String origin, Set<String> visited, String pathToCheck,
       [String source, String fullPath]) {
@@ -313,15 +366,15 @@ class DartDoc {
       fullPath = path.normalize(fullPath);
     }
 
-    visited.add(fullPath);
     Tuple2 stringLinksAndHref = _getStringLinksAndHref(fullPath);
     if (stringLinksAndHref == null) {
       _warn(package, PackageWarning.brokenLink, pathToCheck,
           path.normalize(origin),
-          source: source);
+          referredFrom: source);
       _onCheckProgress.add(pathToCheck);
       return null;
     }
+    visited.add(fullPath);
     Iterable<String> stringLinks = stringLinksAndHref.item1;
     String baseHref = stringLinksAndHref.item2;
 
@@ -355,10 +408,10 @@ class DartDoc {
 
     final Set<String> visited = new Set();
     final String start = 'index.html';
-    visited.add(start);
     stdout.write('\nvalidating docs...');
     _doCheck(package, origin, visited, start);
     _doOrphanCheck(package, origin, visited);
+    _doSearchIndexCheck(package, origin, visited);
   }
 
   List<LibraryElement> _parseLibraries(

lib/src/model.dart

@@ -2747,6 +2747,7 @@ enum PackageWarning {
   brokenLink,
   orphanedFile,
   unknownFile,
+  missingFromSearchIndex,
   typeAsHtml,
 }
 
@@ -2789,6 +2790,10 @@ Map<PackageWarning, List<String>> packageWarningText = {
     "unknownFile",
     "A leftover file exists in the tree that dartdoc did not write in this pass"
   ],
+  PackageWarning.missingFromSearchIndex: [
+    "missingFromSearchIndex",
+    "A file generated by dartdoc is not present in the generated index.json"
+  ],
   PackageWarning.typeAsHtml: [
     "typeAsHtml",
     "Use of <> in a comment for type parameters is being treated as HTML by markdown"
@@ -3094,6 +3099,8 @@ class Package implements Nameable, Documentable {
         break;
       case PackageWarning.brokenLink:
         warningMessage = 'dartdoc generated a broken link to: ${message}';
+        warnablePrefix = 'to element';
+        referredFromPrefix = 'linked to from';
         break;
       case PackageWarning.orphanedFile:
         warningMessage = 'dartdoc generated a file orphan: ${message}';
@@ -3102,6 +3109,10 @@ class Package implements Nameable, Documentable {
         warningMessage =
             'dartdoc detected an unknown file in the doc tree: ${message}';
         break;
+      case PackageWarning.missingFromSearchIndex:
+        warningMessage =
+            'dartdoc generated a file not in the search index: ${message}';
+        break;
       case PackageWarning.typeAsHtml:
         // The message for this warning can contain many punctuation and other symbols,
         // so bracket with a triple quote for defense.
@@ -3233,10 +3244,19 @@ class Package implements Nameable, Documentable {
     //                    than toList().
     for (ModelElement modelElement
         in _allConstructedModelElements.values.toList()) {
+      // Technically speaking we should be able to use canonical model elements
+      // only here, but since the warnings that depend on this debug
+      // canonicalization problems, don't limit ourselves in case an href is
+      // generated for something non-canonical.
       if (modelElement.href == null) continue;
       hrefMap.putIfAbsent(modelElement.href, () => new Set());
       hrefMap[modelElement.href].add(modelElement);
     }
+    for (Library library in _libraries) {
+      if (library.href == null) continue;
+      hrefMap.putIfAbsent(library.href, () => new Set());
+      hrefMap[library.href].add(library);
+    }
     return hrefMap;
   }