Quality of life developer tooling changes for dartdoc (#2070)

* Quality of life tooling changes for dartdoc

* Add a CONTRIBUTING comment

* Update appveyor call to grinder

* Strip fragile checks from the sdk-analyzer test hack

Author: jcollins-g

CONTRIBUTING.md

@@ -26,7 +26,8 @@ yet in the issue tracker, start by opening an issue. Thanks!
 3. Please include a test for your change.  `dartdoc` has both `package:test`-style unittests as well as integration tests.  To run the unittests, use `grind test`.
 4. For major changes, run `grind compare-sdk-warnings` and `grind compare-flutter-warnings`, and include the summary results in your pull request.
 5. Be sure to format your Dart code using `dartfmt -w`, otherwise travis will complain.
-6. Post your change via a pull request for review and integration!
+6. Use `grind presubmit` before creating a pull request to quickly check for common problems.
+7. Post your change via a pull request for review and integration!
 
 ## Testing
 

appveyor.yml

@@ -23,4 +23,4 @@ install:
 build: off
 
 test_script:
-  - pub run grinder buildbot-no-publish
+  - pub run grinder buildbot

lib/src/dartdoc_options.dart

@@ -1376,7 +1376,8 @@ class DartdocOptionContext extends DartdocOptionContextBase
 
   bool get injectHtml => optionSet['injectHtml'].valueAt(context);
 
-  bool get excludeFooterVersion => optionSet['excludeFooterVersion'].valueAt(context);
+  bool get excludeFooterVersion =>
+      optionSet['excludeFooterVersion'].valueAt(context);
 
   ToolConfiguration get tools => optionSet['tools'].valueAt(context);
 
@@ -1420,8 +1421,7 @@ class DartdocOptionContext extends DartdocOptionContextBase
   bool isPackageExcluded(String name) =>
       excludePackages.any((pattern) => name == pattern);
 
-  String get templatesDir =>
-      optionSet['templatesDir'].valueAt(context);
+  String get templatesDir => optionSet['templatesDir'].valueAt(context);
 }
 
 /// Instantiate dartdoc's configuration file and options parser with the
@@ -1624,8 +1624,12 @@ Future<List<DartdocOption>> createDartdocOptions() async {
             'exist. Executables for different platforms are specified by '
             'giving the platform name as a key, and a list of strings as the '
             'command.'),
-    DartdocOptionArgOnly<String>("templatesDir", null, isDir: true, mustExist: true, hide: true,
-        help: 'Path to a directory containing templates to use instead of the default ones. '
+    DartdocOptionArgOnly<String>("templatesDir", null,
+        isDir: true,
+        mustExist: true,
+        hide: true,
+        help:
+            'Path to a directory containing templates to use instead of the default ones. '
             'Directory must contain an html file for each of the following: 404error, category, '
             'class, constant, constructor, enum, function, index, library, method, mixin, '
             'property, top_level_constant, top_level_property, typedef. Partial templates are '

lib/src/element_type.dart

@@ -329,7 +329,8 @@ abstract class DefinedElementType extends ElementType {
   DartType get instantiatedType {
     if (_instantiatedType == null) {
       if (!interfaceType.typeArguments.every((t) => t is InterfaceType)) {
-        _instantiatedType = packageGraph.typeSystem.instantiateToBounds(interfaceType);
+        _instantiatedType =
+            packageGraph.typeSystem.instantiateToBounds(interfaceType);
       } else {
         _instantiatedType = interfaceType;
       }

lib/src/html/html_generator.dart

@@ -59,8 +59,7 @@ class HtmlGenerator extends Generator {
     String dirname = options?.templatesDir;
     if (dirname != null) {
       Directory templateDir = Directory(dirname);
-      templates = await Templates.fromDirectory(
-          templateDir,
+      templates = await Templates.fromDirectory(templateDir,
           headerPaths: headers,
           footerPaths: footers,
           footerTextPaths: footerTexts);

lib/src/html/template_data.dart

@@ -295,7 +295,8 @@ class MethodTemplateData extends TemplateData<Method> {
   @override
   Method get self => method;
   @override
-  String get title => '${method.name} method - ${container.name} ${containerDesc} - '
+  String get title =>
+      '${method.name} method - ${container.name} ${containerDesc} - '
       '${library.name} library - Dart API';
   @override
   String get layoutTitle => _layoutTitle(
@@ -328,7 +329,8 @@ class PropertyTemplateData extends TemplateData<Field> {
   Field get self => property;
 
   @override
-  String get title => '${property.name} $type - ${container.name} ${containerDesc} - '
+  String get title =>
+      '${property.name} $type - ${container.name} ${containerDesc} - '
       '${library.name} library - Dart API';
   @override
   String get layoutTitle =>

lib/src/model/categorization.dart

@@ -2,7 +2,6 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
-
 import 'package:dartdoc/src/model/model.dart';
 
 final categoryRegexp = RegExp(
@@ -115,7 +114,7 @@ abstract class Categorization implements ModelElement {
           .map((n) => package.nameToCategory[n])
           .where((c) => c != null)
           .toList()
-        ..sort();
+            ..sort();
     }
     return _categories;
   }

lib/src/model/category.dart

@@ -10,7 +10,6 @@ import 'package:dartdoc/src/model/model.dart';
 import 'package:dartdoc/src/package_meta.dart';
 import 'package:dartdoc/src/warnings.dart';
 
-
 /// A category is a subcategory of a package, containing libraries tagged
 /// with a @category identifier.
 class Category extends Nameable

tool/grind.dart

@@ -224,12 +224,42 @@ void analyze() async {
   );
 }
 
-@Task('analyze, test, and self-test dartdoc')
-@Depends(analyze, checkBuild, test, testDartdoc)
-void buildbotNoPublish() => null;
+@Task('Check for dartfmt cleanliness')
+void dartfmt() async {
+  if (Platform.version.contains('dev')) {
+    List<String> filesToFix = [];
+    // Filter out test packages as they always have strange formatting.
+    // Passing parameters to dartfmt for directories to search results in
+    // filenames being stripped of the dirname so we have to filter here.
+    void addFileToFix(String fileName) {
+      if (path.split(fileName).first == 'testing') return;
+      filesToFix.add(fileName);
+    }
 
-@Task('analyze, test, and self-test dartdoc')
-@Depends(analyze, checkBuild, test, testDartdoc, tryPublish)
+    log('Validating dartfmt with version ${Platform.version}');
+    await SubprocessLauncher('dartfmt').runStreamed(
+        sdkBin('dartfmt'),
+        [
+          '-n',
+          '.',
+        ],
+        perLine: addFileToFix);
+    if (filesToFix.isNotEmpty) {
+      fail(
+          'dartfmt found files needing reformatting. Use this command to reformat:\n'
+          'dartfmt -w ${filesToFix.map((f) => "\'$f\'").join(' ')}');
+    }
+  } else {
+    log('Skipping dartfmt check, requires latest dev version of SDK');
+  }
+}
+
+@Task('Run quick presubmit checks.')
+@Depends(analyze, checkBuild, smokeTest, dartfmt, tryPublish)
+void presubmit() => null;
+
+@Task('Run long tests, self-test dartdoc, and run the publish test')
+@Depends(presubmit, test, testDartdoc)
 void buildbot() => null;
 
 @Task('Generate docs for the Dart SDK')
@@ -846,26 +876,49 @@ Future<void> checkBuild() async {
 @Task('Dry run of publish to pub.dartlang')
 @Depends(checkChangelogHasVersion)
 Future<void> tryPublish() async {
-  var launcher = SubprocessLauncher('try-publish');
-  await launcher.runStreamed(sdkBin('pub'), ['publish', '-n']);
+  if (Platform.version.contains('dev')) {
+    log('Skipping publish check -- requires a stable version of the SDK');
+  } else {
+    var launcher = SubprocessLauncher('try-publish');
+    await launcher.runStreamed(sdkBin('pub'), ['publish', '-n']);
+  }
+}
+
+@Task('Run a smoke test, only')
+Future<void> smokeTest() async {
+  await testDart2(smokeTestFiles);
+  await testFutures.wait();
+}
+
+@Task('Run non-smoke tests, only')
+Future<void> longTest() async {
+  await testDart2(testFiles);
+  await testFutures.wait();
 }
 
 @Task('Run all the tests.')
 Future<void> test() async {
-  await testDart2();
+  await testDart2(smokeTestFiles.followedBy(testFiles));
   await testFutures.wait();
 }
 
+List<File> get smokeTestFiles => Directory('test')
+    .listSync(recursive: true)
+    .whereType<File>()
+    .where((e) => path.basename(e.path) == 'model_test.dart')
+    .toList();
+
 List<File> get testFiles => Directory('test')
     .listSync(recursive: true)
-    .where((e) => e is File && e.path.endsWith('test.dart'))
-    .cast<File>()
+    .whereType<File>()
+    .where((e) => e.path.endsWith('test.dart'))
+    .where((e) => path.basename(e.path) != 'model_test.dart')
     .toList();
 
-Future<void> testDart2() async {
+Future<void> testDart2(Iterable<File> tests) async {
   List<String> parameters = ['--enable-asserts'];
 
-  for (File dartFile in testFiles) {
+  for (File dartFile in tests) {
     await testFutures.addFutureFromClosure(() =>
         CoverageSubprocessLauncher('dart2-${path.basename(dartFile.path)}')
             .runStreamed(

tool/travis.sh

@@ -37,16 +37,12 @@ elif [ "$DARTDOC_BOT" = "packages" ]; then
   PACKAGE_NAME=flutter_plugin_tools PACKAGE_VERSION=">=0.0.14+1" pub run grinder build-pub-package 2>&1 | grep "dartdoc failed: dartdoc could not find any libraries to document.$"
   PACKAGE_NAME=shelf_exception_handler PACKAGE_VERSION=">=0.2.0" pub run grinder build-pub-package
 elif [ "$DARTDOC_BOT" = "sdk-analyzer" ]; then
-  echo "Running main dartdoc bot against the SDK analyzer"
+  echo "Running all tests against the SDK analyzer"
   unset COVERAGE_TOKEN
-  DARTDOC_GRIND_STEP=buildbot-no-publish pub run grinder test-with-analyzer-sdk
+  pub run grinder test-with-analyzer-sdk
 else
   echo "Running main dartdoc bot"
-  if echo "${DART_VERSION}" | grep -q dev ; then
-    pub run grinder buildbot-no-publish
-  else
-    pub run grinder buildbot
-  fi
+  pub run grinder buildbot
   if [ -n "$COVERAGE_TOKEN" ] ; then
     coveralls-lcov --repo-token="${COVERAGE_TOKEN}" lcov.info
   fi