Tighten up the gen_docs.dart script. (#187)

Move some functionality into helper functions, to make the overall logic
easier to read. Extracted constants, added comments.

Make the script work when called from any directory (finds package
root relative to `Platform.script`, instead of assuming current
working directory is the root.)

Remove dependency on `package:collection` which was only used for
one function, in code that was restructured.

General functionality should still be the same (the generated output
and JSON cache haven't changed).

Author: lrhn

pubspec.yaml

@@ -16,7 +16,6 @@ environment:
 # dependencies:
 
 dev_dependencies:
-  collection: ^1.17.0
   http: ^1.0.0
   path: ^1.8.0
   yaml: ^3.1.0

tool/gen_docs.dart

@@ -5,98 +5,208 @@
 import 'dart:convert';
 import 'dart:io';
 
-import 'package:collection/collection.dart';
 import 'package:path/path.dart' as p;
 import 'package:yaml/yaml.dart' as yaml;
 import 'package:http/http.dart' as http;
 
+/// Source of truth for linter rules.
+const rulesUrl =
+    'https://raw.githubusercontent.com/dart-lang/site-www/main/src/_data/linter_rules.json';
+
+/// Local cache of linter rules from [rulesUrl].
+///
+/// Relative to package root.
+const rulesCacheFilePath = 'tool/rules.json';
+
+/// Generated rules documentation markdown file.
+///
+/// Relative to package root.
+const rulesMarkdownFilePath = 'rules.md';
+
+/// Fetches the [rulesUrl] JSON description of all lints, saves a cached
+/// summary of the relevant fields in [rulesCacheFilePath], and
+/// updates [rulesMarkdownFilePath] to
+///
+/// Passing any command line argument disables generating documentation,
+/// and makes this tool just verify that the doc is up-to-date with the
+/// [rulesCacheFilePath]. (Which it always should be, since the two
+/// are saved at the same time.)
 void main(List<String> args) async {
-  final justVerify = args.isNotEmpty;
-  final lintRules = <String, List<String>>{};
-
-  final rulesJsonFile = File('tool/rules.json');
-  final rulesUrl =
-      'https://raw.githubusercontent.com/dart-lang/site-www/main/src/_data/linter_rules.json';
-  if (!justVerify) {
-    rulesJsonFile.writeAsStringSync((await http.get(Uri.parse(rulesUrl))).body);
+  final verifyOnly = args.isNotEmpty;
+
+  // Read lint rules.
+  final rulesJson = await _fetchRulesJson(verifyOnly: verifyOnly);
+
+  // Read existing generated Markdown documentation.
+  final rulesMarkdownFile = _packageRelativeFile(rulesMarkdownFilePath);
+  final rulesMarkdownContent = rulesMarkdownFile.readAsStringSync();
+
+  if (verifyOnly) {
+    print('Validating that ${rulesMarkdownFile.path} is up-to-date ...');
+  } else {
+    print('Regenerating ${rulesMarkdownFile.path} ...');
   }
-  final rulesJson = (jsonDecode(rulesJsonFile.readAsStringSync()) as List)
-      .cast<Map<String, dynamic>>();
 
-  final rulesMdFile = File('rules.md');
-  final rulesMdContent = rulesMdFile.readAsStringSync();
+  // Generate new documentation.
+  var newRulesMarkdownContent =
+      _updateMarkdown(rulesMarkdownContent, rulesJson);
+
+  // If no documentation change, all is up-to-date.
+  if (newRulesMarkdownContent == rulesMarkdownContent) {
+    print('${rulesMarkdownFile.path} is up-to-date.');
+    return;
+  }
 
-  if (justVerify) {
-    print('Validating that ${rulesMdFile.path} is up-to-date ...');
+  /// Documentation has changed.
+  if (verifyOnly) {
+    print('${rulesMarkdownFile.path} is not up-to-date (lint tables need to be '
+        'regenerated).');
+    print('');
+    print("Run 'dart tool/gen_docs.dart' to re-generate.");
+    exit(1);
   } else {
-    print('Regenerating ${rulesMdFile.path} ...');
+    // Save [rulesMarkdownFilePath].
+    rulesMarkdownFile.writeAsStringSync(newRulesMarkdownContent);
+    print('Wrote ${rulesMarkdownFile.path}.');
   }
+}
 
-  for (var file in ['lib/core.yaml', 'lib/recommended.yaml']) {
-    var name = p.basenameWithoutExtension(file);
-    lintRules[name] = _parseRules(File(file));
+/// Fetches or load the JSON lint rules.
+///
+/// If [verifyOnly] is `false`, fetches JSON from [rulesUrl],
+/// extracts the needed information, and writes a summary to
+/// [rulesCacheFilePath].
+///
+/// If [verifyOnly] is `true`, only reads the cached data back from
+/// [rulesCacheFilePath].
+Future<Map<String, Map<String, String>>> _fetchRulesJson(
+    {required bool verifyOnly}) async {
+  final rulesJsonFile = _packageRelativeFile(rulesCacheFilePath);
+  if (verifyOnly) {
+    final rulesJsonText = rulesJsonFile.readAsStringSync();
+    return _readJson(rulesJsonText);
   }
+  final rulesJsonText = (await http.get(Uri.parse(rulesUrl))).body;
+  final rulesJson = _readJson(rulesJsonText);
 
-  var newContent = rulesMdContent;
+  // Re-save [rulesJsonFile] file.
+  var newRulesJson = [...rulesJson.values];
+  rulesJsonFile
+      .writeAsStringSync(JsonEncoder.withIndent('  ').convert(newRulesJson));
 
-  for (var ruleSetName in lintRules.keys) {
-    final comment = '<!-- $ruleSetName -->\n';
+  return rulesJson;
+}
 
-    newContent = newContent.replaceRange(
-      newContent.indexOf(comment) + comment.length,
-      newContent.lastIndexOf(comment),
-      _createRuleTable(lintRules[ruleSetName]!, rulesJson),
-    );
-  }
+/// Extracts relevant information from a list of JSON objects.
+///
+/// For each JSON object, includes only the relevant (string-typed) properties,
+/// then creates a map indexed by the `'name'` property of the objects.
+Map<String, Map<String, String>> _readJson(String rulesJsonText) {
+  /// Relevant keys in the JSON information about lints.
+  const relevantKeys = {'name', 'description', 'fixStatus'};
+  final rulesJson = jsonDecode(rulesJsonText) as List<dynamic>;
+  return {
+    for (Map<String, Object?> rule in rulesJson)
+      rule['name'] as String: {
+        for (var key in relevantKeys) key: rule[key] as String
+      }
+  };
+}
 
-  if (justVerify) {
-    if (newContent != rulesMdContent) {
-      print('${rulesMdFile.path} is not up-to-date (lint tables need to be '
-          'regenerated).');
-      print('');
-      print("Run 'dart tool/gen_docs.dart' to re-generate.");
-      exit(1);
-    } else {
-      print('${rulesMdFile.path} is up-to-date.');
-    }
-  } else {
-    // Re-save rules.json.
-    const retainKeys = {'name', 'description', 'fixStatus'};
-    for (var rule in rulesJson) {
-      rule.removeWhere((key, value) => !retainKeys.contains(key));
+/// Inserts new Markdown content for both rule sets into [content].
+///
+/// For both "core" and "recommended" rule sets,
+/// replaces the table between the two `<!-- core -->` and the two
+/// `<!-- recommended -->` markers with a new table generated from
+/// [rulesJson], based on the list of rules in `lib/core.yaml` and
+/// `lib/recommended.yaml`.
+String _updateMarkdown(
+    String content, Map<String, Map<String, String>> rulesJson) {
+  for (var ruleSetName in ['core', 'recommended']) {
+    var ruleFile = _packageRelativeFile(p.join('lib', '$ruleSetName.yaml'));
+    var ruleSet = _parseRules(ruleFile);
+
+    final rangeDelimiter = '<!-- $ruleSetName -->\n';
+    var rangeStart = content.indexOf(rangeDelimiter) + rangeDelimiter.length;
+    var rangeEnd = content.indexOf(rangeDelimiter, rangeStart);
+    if (rangeEnd < 0) {
+      stderr.writeln('Missing "$rangeDelimiter" in $rulesMarkdownFilePath.');
+      continue;
     }
-    rulesJsonFile
-        .writeAsStringSync(JsonEncoder.withIndent('  ').convert(rulesJson));
-
-    // Write out the rules md file.
-    rulesMdFile.writeAsStringSync(newContent);
-    print('Wrote ${rulesMdFile.path}.');
+    content = content.replaceRange(
+        rangeStart, rangeEnd, _createRuleTable(ruleSet, rulesJson));
   }
+  return content;
 }
 
+/// Parses analysis options YAML file, and extracts linter rules.
 List<String> _parseRules(File yamlFile) {
   var yamlData = yaml.loadYaml(yamlFile.readAsStringSync()) as Map;
-  return (yamlData['linter']['rules'] as List).toList().cast<String>();
+  var linterEntry = yamlData['linter'] as Map;
+  return List<String>.from(linterEntry['rules'] as List);
 }
 
+/// Creates markdown source for a table of lint rules.
 String _createRuleTable(
-    List<String> rules, List<Map<String, dynamic>> lintMeta) {
+    List<String> rules, Map<String, Map<String, String>> lintMeta) {
   rules.sort();
 
   final lines = [
     '| Lint Rules | Description | [Fix][] |',
     '| :--------- | :---------- | ------- |',
-    ...rules.map((rule) {
-      final ruleMeta =
-          lintMeta.firstWhereOrNull((meta) => meta['name'] == rule);
-
-      final description = ruleMeta?['description'] as String? ?? '';
-      final hasFix = ruleMeta?['fixStatus'] == 'hasFix';
-      final fixDesc = hasFix ? '✅' : '';
-
-      return '| [`$rule`](https://dart.dev/lints/$rule) | $description | $fixDesc |';
-    }),
+    for (var rule in rules) _createRuleTableRow(rule, lintMeta),
   ];
 
   return '${lines.join('\n')}\n';
 }
+
+/// Creates a line containing the markdown table row for a single lint rule.
+///
+/// Used by [_createRuleTable] for each row in the generated table.
+/// The row should have the same number of entires as the table format,
+/// and should be on a single line with no newline at the end.
+String _createRuleTableRow(
+    String rule, Map<String, Map<String, String>> lintMeta) {
+  final ruleMeta = lintMeta[rule];
+  if (ruleMeta == null) {
+    stderr.writeln("WARNING: Missing rule information for rule: $rule");
+  }
+  final description = ruleMeta?['description'] ?? '';
+  final hasFix = ruleMeta?['fixStatus'] == 'hasFix';
+  final fixDesc = hasFix ? '✅' : '';
+
+  return '| [`$rule`](https://dart.dev/lints/$rule) | '
+      '$description | $fixDesc |';
+}
+
+/// A path relative to the root of this package.
+///
+/// Works independently of the current working directory.
+/// Is based on the location of this script, through [Platform.script].
+File _packageRelativeFile(String packagePath) =>
+    File(p.join(_packageRoot, packagePath));
+
+/// Cached package root used by [_packageRelative].
+final String _packageRoot = _relativePackageRoot();
+
+/// A path to the package root from the current directory.
+///
+/// If the current directory is inside the package, the returned path is
+/// a relative path of a number of `..` segments.
+/// If the current directory is outside of the package, the returned path
+/// may be absolute.
+String _relativePackageRoot() {
+  var rootPath = p.dirname(p.dirname(Platform.script.path));
+  if (p.isRelative(rootPath)) return rootPath;
+  var baseDir = p.current;
+  if (rootPath == baseDir) return '';
+  if (baseDir.startsWith(rootPath)) {
+    var backSteps = <String>[];
+    do {
+      backSteps.add('..');
+      baseDir = p.dirname(baseDir);
+    } while (baseDir != rootPath);
+    return p.joinAll(backSteps);
+  }
+  return rootPath;
+}