Avoid a few unnecessary href calculations and loops (#2545)

* Avoid a few unnecessary href calculations and loops

* Minor other string cleanup

* Address feedback

Author: parlough

lib/dartdoc.dart

@@ -273,12 +273,8 @@ class Dartdoc {
       referredFromElements.removeWhere((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.
-        warnOnElement = warnOnElements.isEmpty ? null : warnOnElements.first;
-      }
+      warnOnElement = warnOnElements.firstWhere((e) => e.isCanonical,
+          orElse: () => warnOnElements.isEmpty ? null : warnOnElements.first);
     }
 
     if (referredFromElements.isEmpty && referredFrom == 'index.html') {
@@ -321,8 +317,7 @@ class Dartdoc {
         } else {
           // Error messages are orphaned by design and do not appear in the search
           // index.
-          if (<String>['__404error.html', 'categories.json']
-              .contains(fullPath)) {
+          if (const {'__404error.html', 'categories.json'}.contains(fullPath)) {
             _warn(packageGraph, PackageWarning.orphanedFile, fullPath,
                 normalOrigin);
           }
@@ -342,7 +337,7 @@ class Dartdoc {
   // This is extracted to save memory during the check; be careful not to hang
   // on to anything referencing the full file and doc tree.
   Tuple2<Iterable<String>, String> _getStringLinksAndHref(String fullPath) {
-    var file = config.resourceProvider.getFile('$fullPath');
+    var file = config.resourceProvider.getFile(fullPath);
     if (!file.exists) {
       return null;
     }
@@ -368,7 +363,7 @@ class Dartdoc {
       PackageGraph packageGraph, String origin, Set<String> visited) {
     var fullPath = path.joinAll([origin, 'index.json']);
     var indexPath = path.joinAll([origin, 'index.html']);
-    var file = config.resourceProvider.getFile('$fullPath');
+    var file = config.resourceProvider.getFile(fullPath);
     if (!file.exists) {
       return null;
     }
@@ -403,10 +398,7 @@ class Dartdoc {
   void _doCheck(PackageGraph packageGraph, String origin, Set<String> visited,
       String pathToCheck,
       [String source, String fullPath]) {
-    if (fullPath == null) {
-      fullPath = path.joinAll([origin, pathToCheck]);
-      fullPath = path.normalize(fullPath);
-    }
+    fullPath ??= path.normalize(path.joinAll([origin, pathToCheck]));
 
     var stringLinksAndHref = _getStringLinksAndHref(fullPath);
     if (stringLinksAndHref == null) {
@@ -430,14 +422,9 @@ class Dartdoc {
     var toVisit = <Tuple2<String, String>>{};
 
     final ignoreHyperlinks = RegExp(r'^(https:|http:|mailto:|ftp:)');
-    for (var href in stringLinks) {
+    for (final href in stringLinks) {
       if (!href.startsWith(ignoreHyperlinks)) {
-        Uri uri;
-        try {
-          uri = Uri.parse(href);
-        } on FormatException {
-          // ignore
-        }
+        final uri = Uri.tryParse(href);
 
         if (uri == null || !uri.hasAuthority && !uri.hasFragment) {
           String full;
@@ -446,9 +433,10 @@ class Dartdoc {
           } else {
             full = '${path.dirname(pathToCheck)}/$href';
           }
-          var newPathToCheck = path.normalize(full);
-          var newFullPath = path.joinAll([origin, newPathToCheck]);
-          newFullPath = path.normalize(newFullPath);
+
+          final newPathToCheck = path.normalize(full);
+          final newFullPath =
+              path.normalize(path.joinAll([origin, newPathToCheck]));
           if (!visited.contains(newFullPath)) {
             toVisit.add(Tuple2(newPathToCheck, newFullPath));
             visited.add(newFullPath);
@@ -472,9 +460,8 @@ class Dartdoc {
     _hrefs = packageGraph.allHrefs;
 
     final visited = <String>{};
-    final start = 'index.html';
     logInfo('Validating docs...');
-    _doCheck(packageGraph, origin, visited, start);
+    _doCheck(packageGraph, origin, visited, 'index.html');
     _doOrphanCheck(packageGraph, origin, visited);
     _doSearchIndexCheck(packageGraph, origin, visited);
   }

lib/src/model/package_graph.dart

@@ -254,10 +254,8 @@ class PackageGraph {
 
   Package _defaultPackage;
 
-  Package get defaultPackage {
-    _defaultPackage ??= Package.fromPackageMeta(packageMeta, this);
-    return _defaultPackage;
-  }
+  Package get defaultPackage =>
+      _defaultPackage ??= Package.fromPackageMeta(packageMeta, this);
 
   final bool hasEmbedderSdk;
 
@@ -572,27 +570,27 @@ class PackageGraph {
   /// on more than just [allLocalModelElements] to make the error messages
   /// comprehensive.
   Map<String, Set<ModelElement>> get allHrefs {
-    var hrefMap = <String, Set<ModelElement>>{};
+    final hrefMap = <String, Set<ModelElement>>{};
     // TODO(jcollins-g ): handle calculating hrefs causing new elements better
     //                    than toList().
-    for (var modelElement in allConstructedModelElements.values.toList()) {
+    for (final 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 is Dynamic) continue;
       // TODO: see [Accessor.enclosingCombo]
       if (modelElement is Accessor) continue;
-      if (modelElement.href == null) continue;
-      hrefMap.putIfAbsent(modelElement.href, () => {});
-      hrefMap[modelElement.href].add(modelElement);
+      final href = modelElement.href;
+      if (href == null) continue;
+
+      hrefMap.putIfAbsent(href, () => {}).add(modelElement);
     }
-    for (var package in packageMap.values) {
-      for (var library in package.libraries) {
-        if (library.href == null) continue;
-        hrefMap.putIfAbsent(library.href, () => {});
-        hrefMap[library.href].add(library);
-      }
+
+    for (final library in allLibraries.values) {
+      final href = library.href;
+      if (href == null) continue;
+      hrefMap.putIfAbsent(href, () => {}).add(library);
     }
     return hrefMap;
   }