[doc_checker] Adding filename to errors & check images
Adds a check for image sources.
Also adds the name of the yaml file being checked
making the error message more actionable.
Change-Id: I14762199cf05637dc070ab2a9342871bc99020ff
diff --git a/tools/doc_checker/bin/main.dart b/tools/doc_checker/bin/main.dart
index e92a8f9..c55558b 100644
--- a/tools/doc_checker/bin/main.dart
+++ b/tools/doc_checker/bin/main.dart
@@ -8,6 +8,7 @@
import 'package:args/args.dart';
import 'package:path/path.dart' as path;
+import 'package:doc_checker/errors.dart';
import 'package:doc_checker/graph.dart';
import 'package:doc_checker/link_scraper.dart';
import 'package:doc_checker/link_verifier.dart';
@@ -56,28 +57,6 @@
print('${errorToString(error.type).padRight(25)}: ${error.content}$location');
}
-enum ErrorType {
- unknownLocalFile,
- convertHttpToPath,
- brokenLink,
- unreachablePage,
- obsoleteProject,
- invalidMenu,
- invalidUri,
-}
-
-class Error {
- final ErrorType type;
- final String location;
- final String content;
-
- Error(this.type, this.location, this.content);
-
- Error.forProject(this.type, this.content) : location = null;
-
- bool get hasLocation => location != null;
-}
-
// Checks whether the URI points to the master branch of a Gerrit (i.e.,
// googlesource.com) project.
bool onGerritMaster(Uri uri) {
@@ -244,8 +223,10 @@
YamlChecker checker = YamlChecker(rootDir, rootYaml, yamls, docs);
await checker.check();
- for (var msg in checker.errors) {
- errors.add(Error(ErrorType.invalidMenu, null, msg));
+ List<Error> yamlErrors = checker.errors;
+
+ if (yamlErrors.isNotEmpty) {
+ errors.addAll(yamlErrors);
}
errors
diff --git a/tools/doc_checker/lib/errors.dart b/tools/doc_checker/lib/errors.dart
new file mode 100644
index 0000000..555bfdf
--- /dev/null
+++ b/tools/doc_checker/lib/errors.dart
@@ -0,0 +1,25 @@
+// Copyright 2019 The Fuchsia Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+enum ErrorType {
+ unknownLocalFile,
+ convertHttpToPath,
+ brokenLink,
+ unreachablePage,
+ obsoleteProject,
+ invalidMenu,
+ invalidUri,
+}
+
+class Error {
+ final ErrorType type;
+ final String location;
+ final String content;
+
+ Error(this.type, this.location, this.content);
+
+ Error.forProject(this.type, this.content) : location = null;
+
+ bool get hasLocation => location != null;
+}
diff --git a/tools/doc_checker/lib/link_scraper.dart b/tools/doc_checker/lib/link_scraper.dart
index c491a50..191a6b7 100644
--- a/tools/doc_checker/lib/link_scraper.dart
+++ b/tools/doc_checker/lib/link_scraper.dart
@@ -28,6 +28,8 @@
class _Visitor implements NodeVisitor {
static const String _key = 'href';
+ static const String _imgSrc = 'src';
+ static const String _imgTag = 'img';
final Set<String> links = <String>{};
@@ -35,6 +37,9 @@
bool visitElementBefore(Element element) {
if (element.attributes.containsKey(_key)) {
links.add(element.attributes[_key]);
+ } else if (element.tag == _imgTag &&
+ element.attributes.containsKey(_imgSrc)) {
+ links.add(element.attributes[_imgSrc]);
}
return true;
}
diff --git a/tools/doc_checker/lib/yaml_checker.dart b/tools/doc_checker/lib/yaml_checker.dart
index 7b6a178..80b0436 100644
--- a/tools/doc_checker/lib/yaml_checker.dart
+++ b/tools/doc_checker/lib/yaml_checker.dart
@@ -7,6 +7,8 @@
import 'package:yaml/yaml.dart';
+import 'package:doc_checker/errors.dart';
+
/// Loads yaml files to verify they are
/// compatible with fuchsia.dev hosting.
class YamlChecker {
@@ -16,7 +18,7 @@
String _rootDir;
// List of errors found.
- List<String> errors = <String>[];
+ List<Error> errors = <Error>[];
static const List<String> validStatusValues = <String>[
'alpha',
@@ -65,12 +67,14 @@
if (_rootYaml != null) {
File f = File(_rootYaml);
f.readAsString().then((String data) {
- parse(loadYamlDocuments(data));
+ parse(loadYamlDocuments(data), _rootYaml);
for (String s in _yamlSet) {
- errors.add('unreferenced yaml $s');
+ errors
+ .add(Error(ErrorType.invalidMenu, null, 'unreferenced yaml $s'));
}
for (String s in _mdSet) {
- errors.add('File $s not referenced in any yaml file');
+ errors.add(Error(ErrorType.invalidMenu, null,
+ 'File $s not referenced in any yaml file'));
}
completer.complete(errors.isEmpty);
});
@@ -82,14 +86,14 @@
}
/// Parses the yaml structure.
- void parse(List<YamlDocument> doc) {
+ void parse(List<YamlDocument> doc, String filename) {
for (YamlDocument d in doc) {
- validateTopLevel(d.contents.value);
+ validateTopLevel(d.contents.value, filename);
}
}
/// Validates the top level of the menu.
- void validateTopLevel(YamlMap map) {
+ void validateTopLevel(YamlMap map, String filename) {
for (String key in map.keys) {
switch (key) {
case 'guides':
@@ -98,12 +102,13 @@
case 'reference':
case 'toc':
{
- validateTocElement(map[key]);
+ validateTocElement(map[key], filename);
}
break;
default:
{
- errors.add('Unknown top level key: $key');
+ errors.add(Error(ErrorType.invalidMenu, filename,
+ 'Unknown top level key: $key'));
break;
}
}
@@ -111,28 +116,30 @@
}
/// Validates the toc element in the menu.
- void validateTocElement(YamlNode val) {
+ void validateTocElement(YamlNode val, String filename) {
// valid entries are documented at
if (val is YamlList) {
for (YamlNode node in val) {
if (node.runtimeType == YamlMap) {
- validateContentMap(node);
+ validateContentMap(node, filename);
} else {
- errors.add('Unexpected node of type: ${node.runtimeType} $node');
+ errors.add(Error(ErrorType.invalidMenu, filename,
+ 'Unexpected node of type: ${node.runtimeType} $node'));
}
}
} else {
- errors.add('Expected a list, got ${val.runtimeType}: $val');
+ errors.add(Error(ErrorType.invalidMenu, filename,
+ 'Expected a list, got ${val.runtimeType}: $val'));
}
}
/// Validates the content of toc.
- void validateContentMap(YamlMap map) {
+ void validateContentMap(YamlMap map, String filename) {
for (String key in map.keys) {
switch (key) {
case 'alternate_paths':
{
- validatePathList(map[key]);
+ validatePathList(map[key], filename);
}
break;
case 'break':
@@ -140,43 +147,46 @@
{
// only include if break : true.
if (map[key] != true) {
- errors.add('$key should only be included when set to true');
+ errors.add(Error(ErrorType.invalidMenu, filename,
+ '$key should only be included when set to true'));
}
}
break;
case 'section':
case 'contents':
{
- validateTocElement(map[key]);
+ validateTocElement(map[key], filename);
}
break;
case 'include':
{
- processIncludedFile(map[key]);
+ processIncludedFile(map[key], filename);
}
break;
case 'path':
{
String menuPath = map[key];
- if (validatePath(menuPath)) {
+ if (validatePath(menuPath, filename)) {
// If the path is to a file, check that the file exists.
// TODO(wilkinsonclay): check the URLs are valid.
if (!menuPath.startsWith('https://') &&
!menuPath.startsWith('//')) {
- checkFileExists(menuPath);
+ checkFileExists(menuPath, filename);
}
}
}
break;
case 'path_attributes':
{
- errors.add('path_attributes not supported on fuchsia.dev');
+ errors.add(Error(ErrorType.invalidMenu, filename,
+ 'path_attributes not supported on fuchsia.dev'));
}
break;
case 'status':
{
if (!validStatusValues.contains(map[key])) {
- errors.add('invalid status value of ${map[key]}');
+ errors.add(Error(ErrorType.invalidMenu, filename,
+ 'invalid status value of ${map[key]}'));
}
}
break;
@@ -184,28 +194,28 @@
{
// make sure there is no section in this group.
if (map.containsKey('section')) {
- errors.add(
- 'invalid use of \'step_group\'. Group cannot also contain \`section\`');
+ errors.add(Error(ErrorType.invalidMenu, filename,
+ 'invalid use of \'step_group\'. Group cannot also contain \`section\`'));
}
if (!(map[key] is String)) {
- errors.add(
- 'invalid value of \'step_group\'. Expected String got ${map[key].runtimeType} ${map[key]}');
+ errors.add(Error(ErrorType.invalidMenu, filename,
+ 'invalid value of \'step_group\'. Expected String got ${map[key].runtimeType} ${map[key]}'));
}
}
break;
case 'style':
{
if (map.containsKey('break') || map.containsKey('include')) {
- errors.add(
- 'invalid use of \'style\'. Group cannot also contain `break` nor `include`');
+ errors.add(Error(ErrorType.invalidMenu, filename,
+ 'invalid use of \'style\'. Group cannot also contain `break` nor `include`'));
}
if (map[key] != 'accordion' && map[key] != 'divider') {
- errors.add(
- 'invalid value of `style`. Expected `accordion` or `divider`');
+ errors.add(Error(ErrorType.invalidMenu, filename,
+ 'invalid value of `style`. Expected `accordion` or `divider`'));
}
if (!map.containsKey('heading') && !map.containsKey('section')) {
- errors.add(
- 'invalid use of \'style\'. Group must also have `heading` or `section`.');
+ errors.add(Error(ErrorType.invalidMenu, filename,
+ 'invalid use of \'style\'. Group must also have `heading` or `section`.'));
}
}
break;
@@ -214,36 +224,38 @@
case 'title':
{
if (map[key].runtimeType != String) {
- errors
- .add('Expected String for $key, got ${map[key].runtimeType}');
+ errors.add(Error(ErrorType.invalidMenu, filename,
+ 'Expected String for $key, got ${map[key].runtimeType}'));
}
}
break;
default:
- errors.add('Unknown Content Key $key');
+ errors.add(Error(
+ ErrorType.invalidMenu, filename, 'Unknown Content Key $key'));
}
}
}
/// Handles an included yaml file. It is validated completely then returns.
- void processIncludedFile(String menuPath) {
- if (validatePath(menuPath)) {
+ void processIncludedFile(String menuPath, String parentFilename) {
+ if (validatePath(menuPath, parentFilename)) {
String filePath = '$_rootDir$menuPath';
// parse in a try/catch to handle any syntax errors reading the included file.
try {
- parse(loadYamlDocuments(File(filePath).readAsStringSync()));
+ parse(loadYamlDocuments(File(filePath).readAsStringSync()), menuPath);
_yamlSet.remove(filePath);
} on Exception catch (exception) {
- errors.add(exception.toString());
+ errors.add(
+ Error(ErrorType.invalidMenu, parentFilename, exception.toString()));
}
}
}
/// Validates a list of paths are valid paths for the menu.
- bool validatePathList(List<String> paths) {
+ bool validatePathList(List<String> paths, String filename) {
for (String s in paths) {
- if (!validatePath(s)) {
+ if (!validatePath(s, filename)) {
return false;
}
}
@@ -256,14 +268,15 @@
/// http URLs. Exceptions are made for
/// CONTRIBUTING.md and CODE_OF_CONDUCT.md which
/// are in the root of the project.
- bool validatePath(String menuPath) {
+ bool validatePath(String menuPath, String filename) {
if (!menuPath.startsWith('/docs') &&
menuPath != '/CONTRIBUTING.md' &&
menuPath != '/CODE_OF_CONDUCT.md' &&
!menuPath.startsWith('http://') &&
!menuPath.startsWith('https://') &&
!menuPath.startsWith('//')) {
- errors.add('Path needs to start with \'/docs\', got $menuPath');
+ errors.add(Error(ErrorType.invalidMenu, filename,
+ 'Path needs to start with \'/docs\', got $menuPath'));
return false;
}
@@ -273,7 +286,7 @@
/// Check that the file pointed to by the path exists.
/// if it does, remove the file from the mdSet to keep track of
/// which files are referenced.
- void checkFileExists(String menuPath) {
+ void checkFileExists(String menuPath, String filename) {
String filePath = '$_rootDir$menuPath';
if (File(filePath).existsSync()) {
@@ -284,7 +297,8 @@
if (File(filePath).existsSync()) {
_mdSet.remove(filePath);
} else {
- errors.add('Invalid menu path $menuPath not found');
+ errors.add(Error(ErrorType.invalidMenu, filename,
+ 'Invalid menu path $menuPath not found'));
}
}
}
diff --git a/tools/doc_checker/test/link_scraper_test.dart b/tools/doc_checker/test/link_scraper_test.dart
index bb431d2..a05afca 100644
--- a/tools/doc_checker/test/link_scraper_test.dart
+++ b/tools/doc_checker/test/link_scraper_test.dart
@@ -16,11 +16,13 @@
final List<String> lines = [
'this text has no links',
'this one [does](link.md).',
- 'but not *this one*'
+ 'but not *this one*',
+ 'images are scraped '
];
Iterable<String> links = LinkScraper().scrapeLines(lines);
- expect(links, hasLength(1));
+ expect(links, hasLength(2));
expect(links.first, equals('link.md'));
+ expect(links.last, equals('image.png'));
});
});
}
diff --git a/tools/doc_checker/test/yaml_checker_test.dart b/tools/doc_checker/test/yaml_checker_test.dart
index f06dfe4..3cba4f2 100644
--- a/tools/doc_checker/test/yaml_checker_test.dart
+++ b/tools/doc_checker/test/yaml_checker_test.dart
@@ -84,8 +84,8 @@
bool ret = await checker.check();
expect(ret, equals(false));
- expect(checker.errors[0], equals('Unknown Content Key tile'));
- expect(checker.errors[1], equals('Unknown Content Key filepath'));
+ expect(checker.errors[0].content, equals('Unknown Content Key tile'));
+ expect(checker.errors[1].content, equals('Unknown Content Key filepath'));
expect(checker.errors.length, equals(2));
});
@@ -115,7 +115,7 @@
file.absolute.path, [file.absolute.path], []);
bool ret = await checker.check();
expect(ret, equals(false));
- expect(checker.errors[0],
+ expect(checker.errors[0].content,
equals('Path needs to start with \'/docs\', got /src/ref/1.cc'));
expect(checker.errors.length, equals(1));
});
@@ -168,7 +168,7 @@
file.absolute.path, [file.absolute.path], []);
bool ret = await checker.check();
expect(ret, equals(false));
- expect(checker.errors[0],
+ expect(checker.errors[0].content,
equals('Path needs to start with \'/docs\', got /src/sample/1.c'));
});
@@ -187,10 +187,9 @@
expect(ret, equals(false));
expect(checker.errors.length, equals(1));
expect(
- checker.errors[0],
+ checker.errors[0].content,
startsWith(
'FileSystemException: Cannot open file, path = \'${Directory.systemTemp.path}/docs/_included_notfound.yaml\''));
-
});
// Test when there is a directory with the same name and there is a README.md in that
