Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Search index checker #1442

Merged
merged 2 commits into from
Jun 1, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 79 additions & 26 deletions lib/dartdoc.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
library dartdoc;

import 'dart:async';
import 'dart:convert';
import 'dart:io';

import 'package:analyzer/dart/element/element.dart' show LibraryElement;
Expand Down Expand Up @@ -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);
Expand All @@ -263,7 +280,7 @@ class DartDoc {
continue;
}
if (fullPath == indexJson) {
foundIndex = true;
foundIndexJson = true;
_onCheckProgress.add(fullPath);
continue;
}
Expand All @@ -277,7 +294,7 @@ class DartDoc {
_onCheckProgress.add(fullPath);
}

if (!foundIndex) {
if (!foundIndexJson) {
_warn(package, PackageWarning.brokenLink, indexJson, normalOrigin);
_onCheckProgress.add(indexJson);
}
Expand Down Expand Up @@ -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]) {
Expand All @@ -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;

Expand Down Expand Up @@ -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(
Expand Down
20 changes: 20 additions & 0 deletions lib/src/model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2745,6 +2745,7 @@ enum PackageWarning {
brokenLink,
orphanedFile,
unknownFile,
missingFromSearchIndex,
typeAsHtml,
}

Expand Down Expand Up @@ -2787,6 +2788,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"
Expand Down Expand Up @@ -3092,6 +3097,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';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

linked to from => referenced from?

break;
case PackageWarning.orphanedFile:
warningMessage = 'dartdoc generated a file orphan: ${message}';
Expand All @@ -3100,6 +3107,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.
Expand Down Expand Up @@ -3231,10 +3242,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;
}

Expand Down