Border.add, improve BorderSide.merge (#12327)

...and other minor Border improvements.
And tests.

This changes the merge logic I added yesterday to not support nulls
but instead support BorderSide.none and equivalents. This makes more
sense when dealing with actual Borders.

packages/flutter/lib/src/painting/borders.dart

@@ -88,13 +88,22 @@ class BorderSide {
   /// It is only valid to call this if [canMerge] returns true for the two
   /// sides.
   ///
-  /// If both sides are null, then this will return null. If one of the two
+  /// If one of the sides is zero-width with [BorderStyle.none], then the other
-  /// sides is null, then the other side is returned as-is.
+  /// side is return as-is. If both of the sides are zero-width with
+  /// [BorderStyle.none], then [BorderSide.zero] is returned.
+  ///
+  /// The arguments must not be null.
   static BorderSide merge(BorderSide a, BorderSide b) {
+    assert(a != null);
+    assert(b != null);
     assert(canMerge(a, b));
-    if (a == null)
+    final bool aIsNone = a.style == BorderStyle.none && a.width == 0.0;
-      return b; // might return null
+    final bool bIsNone = b.style == BorderStyle.none && b.width == 0.0;
-    if (b == null)
+    if (aIsNone && bIsNone)
+      return BorderSide.none;
+    if (aIsNone)
+      return b;
+    if (bIsNone)
       return a;
     assert(a.color == b.color);
     assert(a.style == b.style);
@@ -175,10 +184,15 @@ class BorderSide {
   /// Whether the two given [BorderSide]s can be merged using [new
   /// BorderSide.merge].
   ///
-  /// Two sides can be merged if one or both are null, or if they both have the
+  /// Two sides can be merged if one or both are zero-width with
-  /// same color and style.
+  /// [BorderStyle.none], or if they both have the same color and style.
+  ///
+  /// The arguments must not be null.
   static bool canMerge(BorderSide a, BorderSide b) {
-    if (a == null || b == null)
+    assert(a != null);
+    assert(b != null);
+    if ((a.style == BorderStyle.none && a.width == 0.0) ||
+        (b.style == BorderStyle.none && b.width == 0.0))
       return true;
     return a.style == b.style
         && a.color == b.color;
@@ -325,6 +339,28 @@ class Border {
     return new Border(top: side, right: side, bottom: side, left: side);
   }
 
+  /// Creates a [Border] that represents the addition of the two given
+  /// [Border]s.
+  ///
+  /// It is only valid to call this if [BorderSide.canMerge] returns true for
+  /// the pairwise combination of each side on both [Border]s.
+  ///
+  /// The arguments must not be null.
+  static Border merge(Border a, Border b) {
+    assert(a != null);
+    assert(b != null);
+    assert(BorderSide.canMerge(a.top, b.top));
+    assert(BorderSide.canMerge(a.right, b.right));
+    assert(BorderSide.canMerge(a.bottom, b.bottom));
+    assert(BorderSide.canMerge(a.left, b.left));
+    return new Border(
+      top: BorderSide.merge(a.top, b.top),
+      right: BorderSide.merge(a.right, b.right),
+      bottom: BorderSide.merge(a.bottom, b.bottom),
+      left: BorderSide.merge(a.left, b.left),
+    );
+  }
+
   /// The top side of this border.
   final BorderSide top;
 
@@ -341,7 +377,7 @@ class Border {
   ///
   /// This can be used, for example, with a [Padding] widget to inset a box by
   /// the size of these borders.
-  EdgeInsets get dimensions {
+  EdgeInsetsGeometry get dimensions {
     return new EdgeInsets.fromLTRB(left.width, top.width, right.width, bottom.width);
   }
 
@@ -374,13 +410,23 @@ class Border {
     return true;
   }
 
+  Border add(Border typedOther) {
+    if (BorderSide.canMerge(top, typedOther.top) &&
+        BorderSide.canMerge(right, typedOther.right) &&
+        BorderSide.canMerge(bottom, typedOther.bottom) &&
+        BorderSide.canMerge(left, typedOther.left)) {
+      return Border.merge(this, typedOther);
+    }
+    return null;
+  }
+
   /// Creates a new border with the widths of this border multiplied by `t`.
   Border scale(double t) {
     return new Border(
-      top: top.copyWith(width: t * top.width),
+      top: top.scale(t),
-      right: right.copyWith(width: t * right.width),
+      right: right.scale(t),
-      bottom: bottom.copyWith(width: t * bottom.width),
+      bottom: bottom.scale(t),
-      left: left.copyWith(width: t * left.width)
+      left: left.scale(t),
     );
   }
 
@@ -407,12 +453,12 @@ class Border {
   ///
   /// Uniform borders are more efficient to paint than more complex borders.
   ///
-  /// You can provide a [BoxShape] to draw the border on. If the shape in
+  /// You can provide a [BoxShape] to draw the border on. If the `shape` in
   /// [BoxShape.circle], there is the requirement that the border [isUniform].
   ///
-  /// If you specify a rectangular box shape (BoxShape.rectangle), then you may
+  /// If you specify a rectangular box shape ([BoxShape.rectangle]), then you
-  /// specify a [BorderRadius]. If a border radius is specified, there is the
+  /// may specify a [BorderRadius]. If a `borderRadius` is specified, there is
-  /// requirement that the border [isUniform].
+  /// the requirement that the border [isUniform].
   ///
   /// See also:
   ///
@@ -422,17 +468,22 @@ class Border {
     BorderRadius borderRadius,
   }) {
     if (isUniform) {
-      if (shape == BoxShape.circle) {
+      switch (top.style) {
-        assert(borderRadius == null, 'A borderRadius can only be given for rectangular boxes.');
+        case BorderStyle.none:
-        _paintUniformBorderWithCircle(canvas, rect);
+          return;
-        return;
+        case BorderStyle.solid:
+          if (shape == BoxShape.circle) {
+            assert(borderRadius == null, 'A borderRadius can only be given for rectangular boxes.');
+            _paintUniformBorderWithCircle(canvas, rect);
+            return;
+          }
+          if (borderRadius != null) {
+            _paintUniformBorderWithRadius(canvas, rect, borderRadius);
+            return;
+          }
+          _paintUniformBorderWithRectangle(canvas, rect);
+          return;
       }
-      if (borderRadius != null) {
-        _paintUniformBorderWithRadius(canvas, rect, borderRadius);
-        return;
-      }
-      _paintUniformBorderWithRectangle(canvas, rect);
-      return;
     }
 
     assert(borderRadius == null, 'A borderRadius can only be given for uniform borders.');
@@ -444,6 +495,7 @@ class Border {
   void _paintUniformBorderWithRadius(Canvas canvas, Rect rect,
                                      BorderRadius borderRadius) {
     assert(isUniform);
+    assert(top.style != BorderStyle.none);
     final Paint paint = new Paint()
       ..color = top.color;
     final RRect outer = borderRadius.toRRect(rect);
@@ -461,22 +513,18 @@ class Border {
 
   void _paintUniformBorderWithCircle(Canvas canvas, Rect rect) {
     assert(isUniform);
+    assert(top.style != BorderStyle.none);
     final double width = top.width;
-    final Paint paint = new Paint()
+    final Paint paint = top.toPaint();
-      ..color = top.color
-      ..strokeWidth = width
-      ..style = PaintingStyle.stroke;
     final double radius = (rect.shortestSide - width) / 2.0;
     canvas.drawCircle(rect.center, radius, paint);
   }
 
   void _paintUniformBorderWithRectangle(Canvas canvas, Rect rect) {
     assert(isUniform);
+    assert(top.style != BorderStyle.none);
     final double width = top.width;
-    final Paint paint = new Paint()
+    final Paint paint = top.toPaint();
-      ..color = top.color
-      ..strokeWidth = width
-      ..style = PaintingStyle.stroke;
     canvas.drawRect(rect.deflate(width / 2.0), paint);
   }
 

packages/flutter/test/painting/border_side_test.dart

@@ -32,20 +32,64 @@ void main() {
     );
   });
   test('BorderSide - merging', () {
+    final BorderSide blue = const BorderSide(color: const Color(0xFF0000FF));
+    final BorderSide blue2 = const BorderSide(color: const Color(0xFF0000FF), width: 2.0);
+    final BorderSide green = const BorderSide(color: const Color(0xFF00FF00));
+    final BorderSide green2 = const BorderSide(color: const Color(0xFF00FF00), width: 2.0);
+    final BorderSide green3 = const BorderSide(color: const Color(0xFF00FF00), width: 3.0);
+    final BorderSide green5 = const BorderSide(color: const Color(0xFF00FF00), width: 5.0);
+    final BorderSide none = const BorderSide(style: BorderStyle.none);
+    final BorderSide none2 = const BorderSide(color: const Color(0xFF0000FF), width: 2.0, style: BorderStyle.none);
+    final BorderSide none3 = const BorderSide(style: BorderStyle.none, width: 3.0);
     final BorderSide side2 = const BorderSide(width: 2.0);
     final BorderSide side3 = const BorderSide(width: 3.0);
     final BorderSide side5 = const BorderSide(width: 5.0);
-    final BorderSide green = const BorderSide(color: const Color(0xFF00FF00));
-    final BorderSide blue = const BorderSide(color: const Color(0xFF0000FF));
     final BorderSide solid = const BorderSide(style: BorderStyle.solid);
-    final BorderSide none = const BorderSide(style: BorderStyle.none);
+    final BorderSide yellowNone = const BorderSide(style: BorderStyle.none, color: const Color(0xFFFFFF00), width: 0.0);
-    expect(BorderSide.merge(null, null), isNull);
+    // canMerge
-    expect(BorderSide.merge(null, side2), side2);
+    expect(      BorderSide.canMerge(BorderSide.none, BorderSide.none), isTrue);
-    expect(BorderSide.merge(side2, null), side2);
+    expect(      BorderSide.canMerge(BorderSide.none, side2), isTrue);
+    expect(      BorderSide.canMerge(BorderSide.none, yellowNone), isTrue);
+    expect(      BorderSide.canMerge(green, blue), isFalse);
+    expect(      BorderSide.canMerge(green2, blue2), isFalse);
+    expect(      BorderSide.canMerge(green2, green3), isTrue);
+    expect(      BorderSide.canMerge(green2, none2), isFalse);
+    expect(      BorderSide.canMerge(none3, BorderSide.none), isTrue);
+    expect(      BorderSide.canMerge(none3, side2), isFalse);
+    expect(      BorderSide.canMerge(none3, yellowNone), isTrue);
+    expect(() => BorderSide.canMerge(null, null), throwsAssertionError);
+    expect(() => BorderSide.canMerge(null, side2), throwsAssertionError);
+    expect(      BorderSide.canMerge(side2, BorderSide.none), isTrue);
+    expect(      BorderSide.canMerge(side2, none3), isFalse);
+    expect(() => BorderSide.canMerge(side2, null), throwsAssertionError);
+    expect(      BorderSide.canMerge(side2, side3), isTrue);
+    expect(      BorderSide.canMerge(side2, yellowNone), isTrue);
+    expect(      BorderSide.canMerge(side3, side2), isTrue);
+    expect(      BorderSide.canMerge(solid, none), isFalse);
+    expect(      BorderSide.canMerge(yellowNone, side2), isTrue);
+    expect(      BorderSide.canMerge(yellowNone, yellowNone), isTrue);
+    // merge, for the same combinations
+    expect(      BorderSide.merge(BorderSide.none, BorderSide.none), BorderSide.none);
+    expect(      BorderSide.merge(BorderSide.none, side2), side2);
+    expect(      BorderSide.merge(BorderSide.none, yellowNone), BorderSide.none);
     expect(() => BorderSide.merge(green, blue), throwsAssertionError);
+    expect(() => BorderSide.merge(green2, blue2), throwsAssertionError);
+    expect(      BorderSide.merge(green2, green3), green5);
+    expect(() => BorderSide.merge(green2, none2), throwsAssertionError);
+    expect(      BorderSide.merge(none3, BorderSide.none), none3);
+    expect(() => BorderSide.merge(none3, side2), throwsAssertionError);
+    expect(      BorderSide.merge(none3, yellowNone), none3);
+    expect(() => BorderSide.merge(null, null), throwsAssertionError);
+    expect(() => BorderSide.merge(null, side2), throwsAssertionError);
+    expect(      BorderSide.merge(side2, BorderSide.none), side2);
+    expect(() => BorderSide.merge(side2, none3), throwsAssertionError);
+    expect(() => BorderSide.merge(side2, null), throwsAssertionError);
+    expect(      BorderSide.merge(side2, side3), side5);
+    expect(      BorderSide.merge(side2, yellowNone), side2);
+    expect(      BorderSide.merge(side3, side2), side5);
     expect(() => BorderSide.merge(solid, none), throwsAssertionError);
-    expect(BorderSide.merge(side2, side3), side5);
+    expect(      BorderSide.merge(yellowNone, side2), side2);
-    expect(BorderSide.merge(side3, side2), side5);
+    expect(      BorderSide.merge(yellowNone, yellowNone), BorderSide.none);
   });
   test('BorderSide - asserts when copied incorrectly', () {
     final BorderSide green2 = const BorderSide(color: const Color(0xFF00FF00), width: 2.0);
@@ -80,18 +124,6 @@ void main() {
     expect(paint2.color, const Color(0x00000000));
     expect(paint2.blendMode, BlendMode.srcOver);
   });
-  test('BorderSide - canMerge', () {
-    final BorderSide green2 = const BorderSide(color: const Color(0xFF00FF00), width: 2.0);
-    final BorderSide green3 = const BorderSide(color: const Color(0xFF00FF00), width: 3.0);
-    final BorderSide blue2 = const BorderSide(color: const Color(0xFF0000FF), width: 2.0);
-    final BorderSide none2 = const BorderSide(color: const Color(0xFF0000FF), width: 2.0, style: BorderStyle.none);
-    expect(BorderSide.canMerge(green2, green3), isTrue);
-    expect(BorderSide.canMerge(green2, blue2), isFalse);
-    expect(BorderSide.canMerge(green2, none2), isFalse);
-    expect(BorderSide.canMerge(blue2, null), isTrue);
-    expect(BorderSide.canMerge(null, null), isTrue);
-    expect(BorderSide.canMerge(null, blue2), isTrue);
-  });
   test('BorderSide - won\'t lerp into negative widths', () {
     final BorderSide side0 = const BorderSide(width: 0.0);
     final BorderSide side1 = const BorderSide(width: 1.0);

packages/flutter/test/painting/border_test.dart

@@ -0,0 +1,97 @@
+// Copyright 2015 The Chromium Authors. 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:flutter/painting.dart';
+import 'package:flutter_test/flutter_test.dart';
+
+void main() {
+  test('Border.merge', () {
+    final BorderSide magenta3 = const BorderSide(color: const Color(0xFFFF00FF), width: 3.0);
+    final BorderSide magenta6 = const BorderSide(color: const Color(0xFFFF00FF), width: 6.0);
+    final BorderSide yellow2 = const BorderSide(color: const Color(0xFFFFFF00), width: 2.0);
+    final BorderSide yellowNone0 = const BorderSide(color: const Color(0xFFFFFF00), width: 0.0, style: BorderStyle.none);
+    expect(
+      Border.merge(
+        new Border(top: yellow2),
+        new Border(right: magenta3),
+      ),
+      new Border(top: yellow2, right: magenta3),
+    );
+    expect(
+      Border.merge(
+        new Border(bottom: magenta3),
+        new Border(bottom: magenta3),
+      ),
+      new Border(bottom: magenta6),
+    );
+    expect(
+      Border.merge(
+        new Border(left: magenta3, right: yellowNone0),
+        new Border(right: yellow2),
+      ),
+      new Border(left: magenta3, right: yellow2),
+    );
+    expect(
+      Border.merge(const Border(), const Border()),
+      const Border(),
+    );
+    expect(
+      () => Border.merge(
+        new Border(left: magenta3),
+        new Border(left: yellow2),
+      ),
+      throwsAssertionError,
+    );
+  });
+
+  test('Border.add', () {
+    final BorderSide magenta3 = const BorderSide(color: const Color(0xFFFF00FF), width: 3.0);
+    final BorderSide magenta6 = const BorderSide(color: const Color(0xFFFF00FF), width: 6.0);
+    final BorderSide yellow2 = const BorderSide(color: const Color(0xFFFFFF00), width: 2.0);
+    final BorderSide yellowNone0 = const BorderSide(color: const Color(0xFFFFFF00), width: 0.0, style: BorderStyle.none);
+    expect(
+      new Border(top: yellow2).add(new Border(right: magenta3)),
+      new Border(top: yellow2, right: magenta3),
+    );
+    expect(
+      new Border(bottom: magenta3).add(new Border(bottom: magenta3)),
+      new Border(bottom: magenta6),
+    );
+    expect(
+      new Border(left: magenta3, right: yellowNone0).add(new Border(right: yellow2)),
+      new Border(left: magenta3, right: yellow2),
+    );
+    expect(
+      const Border().add(const Border()),
+      const Border(),
+    );
+    expect(
+      new Border(left: magenta3).add(new Border(left: yellow2)),
+      isNull,
+    );
+    final Border b3 = new Border(top: magenta3);
+    final Border b6 = new Border(top: magenta6);
+    expect(b3.add(b3), b6);
+    final Border b0 = new Border(top: yellowNone0);
+    final Border bZ = const Border();
+    expect(b0.add(b0), bZ);
+    expect(bZ.add(bZ), bZ);
+    expect(b0.add(bZ), bZ);
+    expect(bZ.add(b0), bZ);
+  });
+
+  test('Border.scale', () {
+    final BorderSide magenta3 = const BorderSide(color: const Color(0xFFFF00FF), width: 3.0);
+    final BorderSide magenta6 = const BorderSide(color: const Color(0xFFFF00FF), width: 6.0);
+    final BorderSide yellow2 = const BorderSide(color: const Color(0xFFFFFF00), width: 2.0);
+    final BorderSide yellowNone0 = const BorderSide(color: const Color(0xFFFFFF00), width: 0.0, style: BorderStyle.none);
+    final Border b3 = new Border(left: magenta3);
+    final Border b6 = new Border(left: magenta6);
+    expect(b3.scale(2.0), b6);
+    final Border bY0 = new Border(top: yellowNone0);
+    expect(bY0.scale(3.0), bY0);
+    final Border bY2 = new Border(top: yellow2);
+    expect(bY2.scale(0.0), bY0);
+  });
+}