Adding animation directive for injecting animated diagrams (#1698)

* Adding animation directive for injecting animated diagrams

* Updated Testing Goldens

* Adding two more tests, updating goldens again

Author: gspencergoog

lib/resources/play_button.svg

@@ -2878,6 +2878,7 @@ abstract class ModelElement extends Canonicalization
       _rawDocs = computeDocumentationComment ?? '';
       _rawDocs = stripComments(_rawDocs) ?? '';
       _rawDocs = _injectExamples(_rawDocs);
+      _rawDocs = _injectAnimations(_rawDocs);
       _rawDocs = _stripMacroTemplatesAndAddToIndex(_rawDocs);
     }
     return _rawDocs;
@@ -3527,24 +3528,156 @@ abstract class ModelElement extends Canonicalization
     });
   }
 
-  /// Replace {@macro ...} in API comments with the contents of the macro
+  /// Replace {@animation ...} in API comments with some HTML to manage an
+  /// MPEG 4 video as an animation.
   ///
   /// Syntax:
   ///
-  ///     {@macro NAME}
+  ///     {@animation NAME WIDTH HEIGHT URL}
+  ///
+  /// Example:
+  ///
+  ///     {@animation my_video 300 300 https://example.com/path/to/video.mp4}
+  ///
+  /// Which will render the HTML necessary for embedding a simple click-to-play
+  /// HTML5 video player with no controls.
+  ///
+  /// The NAME should be a unique name that is a valid javascript identifier,
+  /// and will be used as the id for the video tag.
+  ///
+  /// The width and height must be integers specifying the dimensions of the
+  /// video file in pixels.
+  String _injectAnimations(String rawDocs) {
+    // Matches all animation directives (even some invalid ones). This is so
+    // we can give good error messages if the directive is malformed, instead of
+    // just silently emitting it as-is.
+    final RegExp basicAnimationRegExp =
+        new RegExp(r'''{@animation\s+([^}]+)}''');
+
+    // Animations have four parameters, and the last one can be surrounded by
+    // quotes (which are ignored). This RegExp is used to validate the directive
+    // for the correct number of parameters.
+    final RegExp animationRegExp =
+        new RegExp(r'''{@animation\s+([^}\s]+)\s+([^}\s]+)\s+([^}\s]+)'''
+            r'''\s+['"]?([^}]+)['"]?}''');
+
+    // Matches valid javascript identifiers.
+    final RegExp validNameRegExp = new RegExp(r'^[a-zA-Z_][a-zA-Z0-9_]*$');
+
+    // Keeps names unique.
+    final Set<String> uniqueNames = new Set<String>();
+
+    return rawDocs.replaceAllMapped(basicAnimationRegExp, (basicMatch) {
+      final Match match = animationRegExp.firstMatch(basicMatch[0]);
+      if (match == null) {
+        warn(PackageWarning.invalidParameter,
+            message: 'Invalid @animation directive: ${basicMatch[0]}\n'
+                'Animation directives must be of the form: {@animation NAME '
+                'WIDTH HEIGHT URL}');
+        return '';
+      }
+      String name = match[1];
+      if (!validNameRegExp.hasMatch(name)) {
+        warn(PackageWarning.invalidParameter,
+            message: 'An animation has an invalid name: $name. The name can '
+                'only contain letters, numbers and underscores.');
+        return '';
+      } else {
+        if (uniqueNames.contains(name)) {
+          warn(PackageWarning.invalidParameter,
+              message:
+                  'An animation has a non-unique name: $name. Animation names '
+                  'must be unique.');
+          return '';
+        }
+        uniqueNames.add(name);
+      }
+      int width;
+      try {
+        width = int.parse(match[2]);
+      } on FormatException {
+        warn(PackageWarning.invalidParameter,
+            message: 'An animation has an invalid width ($name): ${match[2]}. The '
+                'width must be an integer.');
+        return '';
+      }
+      int height;
+      try {
+        height = int.parse(match[3]);
+      } on FormatException {
+        warn(PackageWarning.invalidParameter,
+            message: 'An animation has an invalid height ($name): ${match[3]}. The '
+                'height must be an integer.');
+        return '';
+      }
+      Uri movieUrl;
+      try {
+        movieUrl = Uri.parse(match[4]);
+      } on FormatException catch (e) {
+        warn(PackageWarning.invalidParameter,
+            message: 'An animation URL could not be parsed ($name): ${match[4]}\n$e');
+        return '';
+      }
+      final String overlayName = '${name}_play_button_';
+
+      // Blank lines before and after, and no indenting at the beginning and end
+      // is needed so that Markdown doesn't confuse this with code, so be
+      // careful of whitespace here.
+      return '''
+
+<div style="position: relative;">
+  <div id="${overlayName}"
+       onclick="if ($name.paused) {
+                  $name.play();
+                  this.style.display = 'none';
+                } else {
+                  $name.pause();
+                  this.style.display = 'block';
+                }"
+       style="position:absolute;
+              width:${width}px;
+              height:${height}px;
+              z-index:100000;
+              background-position: center;
+              background-repeat: no-repeat;
+              background-image: url(static-assets/play_button.svg);">
+  </div>
+  <video id="$name"
+         style="width:${width}px; height:${height}px;"
+         onclick="if (this.paused) {
+                    this.play();
+                    $overlayName.style.display = 'none';
+                  } else {
+                    this.pause();
+                    $overlayName.style.display = 'block';
+                  }" loop>
+    <source src="$movieUrl" type="video/mp4"/>
+  </video>
+</div>
+
+''';  // String must end at beginning of line, or following inline text will be
+      // indented.
+    });
+  }
+
+  /// Replace &#123;@macro ...&#125; in API comments with the contents of the macro
+  ///
+  /// Syntax:
+  ///
+  ///     &#123;@macro NAME&#125;
   ///
   /// Example:
   ///
   /// You define the template in any comment for a documentable entity like:
   ///
-  ///     {@template foo}
+  ///     &#123;@template foo&#125;
   ///     Foo contents!
-  ///     {@endtemplate}
+  ///     &#123;@endtemplate&#125;
   ///
   /// and them somewhere use it like this:
   ///
   ///     Some comments
-  ///     {@macro foo}
+  ///     &#123;@macro foo&#125;
   ///     More comments
   ///
   /// Which will render
@@ -3564,13 +3697,13 @@ abstract class ModelElement extends Canonicalization
     });
   }
 
-  /// Parse {@template ...} in API comments and store them in the index on the package.
+  /// Parse &#123;@template ...&#125; in API comments and store them in the index on the package.
   ///
   /// Syntax:
   ///
-  ///     {@template NAME}
+  ///     &#123;@template NAME&#125;
   ///     The contents of the macro
-  ///     {@endtemplate}
+  ///     &#123;@endtemplate&#125;
   ///
   String _stripMacroTemplatesAndAddToIndex(String rawDocs) {
     final templateRegExp = new RegExp(
@@ -4133,6 +4266,9 @@ class PackageGraph extends Canonicalization
         // so bracket with a triple quote for defense.
         warningMessage = 'generic type handled as HTML: """${message}"""';
         break;
+      case PackageWarning.invalidParameter:
+        warningMessage = 'invalid parameter to dartdoc directive: ${message}';
+        break;
     }
 
     List<String> messageParts = [warningMessage];
@@ -5434,7 +5570,14 @@ class PackageBuilder {
     // TODO(jcollins-g): explode this into detailed command line options.
     if (config.showWarnings) {
       for (PackageWarning kind in PackageWarning.values) {
-        warningOptions.warn(kind);
+        switch (kind) {
+          case PackageWarning.invalidParameter:
+            warningOptions.error(kind);
+            break;
+          default:
+            warningOptions.warn(kind);
+            break;
+        }
       }
     }
     return warningOptions;

lib/src/model.dart

@@ -86,6 +86,10 @@ final Map<PackageWarning, PackageWarningHelpText> packageWarningText = const {
       PackageWarning.typeAsHtml,
       "typeAsHtml",
       "Use of <> in a comment for type parameters is being treated as HTML by markdown"),
+  PackageWarning.invalidParameter: const PackageWarningHelpText(
+      PackageWarning.invalidParameter,
+      "invalidParameter",
+      "A parameter given to a dartdoc directive was invalid."),
 };
 
 /// Something that package warnings can be called on.  Optionally associated
@@ -125,6 +129,7 @@ enum PackageWarning {
   unknownFile,
   missingFromSearchIndex,
   typeAsHtml,
+  invalidParameter,
 }
 
 /// Warnings it is OK to skip if we can determine the warnable isn't documented.

lib/src/warnings.dart

@@ -394,4 +394,4 @@ packages:
     source: hosted
     version: "2.1.13"
 sdks:
-  dart: ">=2.0.0-dev.54.0 <=2.0.0-dev.55.0"
+  dart: ">=2.0.0-dev.54.0 <=2.0.0-dev.58.0"

pubspec.lock

@@ -496,6 +496,86 @@ void main() {
     });
   });
 
+  group('Animation', () {
+    Class dog;
+    Method withAnimation;
+    Method withAnimationNonUnique;
+    Method withAnimationWrongParams;
+    Method withAnimationBadWidth;
+    Method withAnimationBadHeight;
+    Method withAnimationInOneLineDoc;
+    Method withAnimationInline;
+
+    setUp(() {
+      dog = exLibrary.classes.firstWhere((c) => c.name == 'Dog');
+      withAnimation =
+          dog.allInstanceMethods.firstWhere((m) => m.name == 'withAnimation');
+      withAnimationNonUnique = dog.allInstanceMethods
+          .firstWhere((m) => m.name == 'withAnimationNonUnique');
+      withAnimationWrongParams = dog.allInstanceMethods
+          .firstWhere((m) => m.name == 'withAnimationWrongParams');
+      withAnimationBadWidth = dog.allInstanceMethods
+          .firstWhere((m) => m.name == 'withAnimationBadWidth');
+      withAnimationBadHeight = dog.allInstanceMethods
+          .firstWhere((m) => m.name == 'withAnimationBadHeight');
+      withAnimationInOneLineDoc = dog.allInstanceMethods
+          .firstWhere((m) => m.name == 'withAnimationInOneLineDoc');
+      withAnimationInline = dog.allInstanceMethods
+          .firstWhere((m) => m.name == 'withAnimationInline');
+      packageGraph.allLocalModelElements.forEach((m) => m.documentation);
+    });
+
+    test("renders an animation within the method documentation", () {
+      expect(
+          withAnimation.documentation, contains('<video id="methodAnimation"'));
+    });
+    test("warns on a non-unique animation within a method", () {
+      expect(
+          packageGraph.packageWarningCounter.hasWarning(
+              withAnimationNonUnique,
+              PackageWarning.invalidParameter,
+              'An animation has a non-unique name: fooHerderAnimation. Animation names '
+              'must be unique.'),
+          isTrue);
+    });
+    test("warns on animation with missing parameters", () {
+      expect(
+          packageGraph.packageWarningCounter.hasWarning(
+              withAnimationWrongParams,
+              PackageWarning.invalidParameter,
+              'Invalid @animation directive: {@animation http://host/path/to/video.mp4}\n'
+              'Animation directives must be of the form: {@animation NAME '
+              'WIDTH HEIGHT URL}'),
+          isTrue);
+    });
+    test("warns on animation with non-integer width", () {
+      expect(
+          packageGraph.packageWarningCounter.hasWarning(
+              withAnimationBadWidth,
+              PackageWarning.invalidParameter,
+              'An animation has an invalid width (badWidthAnimation): 100px. The width must be an integer.'),
+          isTrue);
+    });
+    test("warns on animation with non-integer height", () {
+      expect(
+          packageGraph.packageWarningCounter.hasWarning(
+              withAnimationBadHeight,
+              PackageWarning.invalidParameter,
+              'An animation has an invalid height (badHeightAnimation): 100px. The height must be an integer.'),
+          isTrue);
+    });
+    test("Doesn't place animations in one line doc", () {
+      expect(
+          withAnimationInline.oneLineDoc, isNot(contains('<video')));
+      expect(
+          withAnimationInline.documentation, contains('<video'));
+    });
+    test("Handles animations inline properly", () {
+      expect(
+          withAnimationInline.documentation, isNot(contains('  works')));
+    });
+  });
+
   group('MultiplyInheritedExecutableElement handling', () {
     Class BaseThingy, BaseThingy2, ImplementingThingy2;
     Method aImplementingThingyMethod;
@@ -1019,7 +1099,7 @@ void main() {
     });
 
     test('get methods', () {
-      expect(Dog.publicInstanceMethods, hasLength(12));
+      expect(Dog.publicInstanceMethods, hasLength(19));
     });
 
     test('get operators', () {
@@ -1084,6 +1164,13 @@ void main() {
             'testGenericMethod',
             'testMethod',
             'toString',
+            'withAnimation',
+            'withAnimationBadHeight',
+            'withAnimationBadWidth',
+            'withAnimationInline',
+            'withAnimationInOneLineDoc',
+            'withAnimationNonUnique',
+            'withAnimationWrongParams',
             'withMacro',
             'withMacro2',
             'withPrivateMacro',

test/model_test.dart

@@ -355,6 +355,48 @@ class Dog implements Cat, E {
   /// Don't define this:  {@macro ThatDoesNotExist}
   void withUndefinedMacro() {}
 
+  /// Animation method
+  ///
+  /// {@animation methodAnimation 100 100 http://host/path/to/video.mp4}
+  /// More docs
+  void withAnimation() {}
+
+  /// Non-Unique Animation method (between methods)
+  ///
+  /// {@animation fooHerderAnimation 100 100 http://host/path/to/video.mp4}
+  /// {@animation fooHerderAnimation 100 100 http://host/path/to/video.mp4}
+  /// More docs
+  void withAnimationNonUnique() {}
+
+  /// Malformed Animation method with wrong parameters
+  ///
+  /// {@animation http://host/path/to/video.mp4}
+  /// More docs
+  void withAnimationWrongParams() {}
+
+  /// Malformed Animation method with non-integer width
+  ///
+  /// {@animation badWidthAnimation 100px 100 http://host/path/to/video.mp4}
+  /// More docs
+  void withAnimationBadWidth() {}
+
+  /// Malformed Animation method with non-integer height
+  ///
+  /// {@animation badHeightAnimation 100 100px http://host/path/to/video.mp4}
+  /// More docs
+  void withAnimationBadHeight() {}
+
+  /// Animation in one line doc {@animation oneLine 100 100 http://host/path/to/video.mp4}
+  ///
+  /// This tests to see that we do the right thing if the animation is in
+  /// the one line doc above.
+  void withAnimationInOneLineDoc() {}
+
+  /// Animation inline in text.
+  ///
+  /// Tests to see that an inline {@animation inline 100 100 http://host/path/to/video.mp4} works as expected.
+  void withAnimationInline() {}
+
   void testGeneric(Map<String, dynamic> args) {}
 
   void testMethod(Iterable it) {}