Preserve rgb/rgba/hsl/hsla formats in expanded mode (#1651)

This also fixes a bug where four- and eight-digit hex numbers weren't
being translated to more compatible formats.

Closes #1634

CHANGELOG.md

@@ -4,6 +4,21 @@
   are defined in dependencies even if they're invoked from application
   stylesheets.
 
+* In expanded mode, Sass will now emit colors using `rgb()`, `rbga()`, `hsl()`,
+  and `hsla()` function notation if they were defined using the corresponding
+  notation. As per our browser support policy, this change was only done once
+  95% of browsers were confirmed to support this output format, and so is not
+  considered a breaking change.
+
+  Note that this output format is intended for human readability and not for
+  interoperability with other tools. As always, Sass targets the CSS
+  specification, and any tool that consumes Sass's output should parse all
+  colors that are supported by the CSS spec.
+
+* Fix a bug in which a color written using the four- or eight-digit hex format
+  could be emitted as a hex color rather than a format with higher browser
+  compatibility.
+
 ## 1.49.9
 
 ### Embedded Sass

lib/sass.dart

@@ -26,7 +26,7 @@ export 'src/exception.dart' show SassException;
 export 'src/importer.dart';
 export 'src/logger.dart';
 export 'src/syntax.dart';
-export 'src/value.dart' hide SassApiColor;
+export 'src/value.dart' hide ColorFormat, SassApiColor, SpanColorFormat;
 export 'src/visitor/serialize.dart' show OutputStyle;
 export 'src/evaluation_context.dart' show warn;
 

lib/src/ast/sass/expression/color.dart

@@ -19,7 +19,7 @@ class ColorExpression implements Expression {
 
   final FileSpan span;
 
-  ColorExpression(this.value) : span = value.originalSpan!;
+  ColorExpression(this.value, this.span);
 
   T accept<T>(ExpressionVisitor<T> visitor) =>
       visitor.visitColorExpression(this);

lib/src/functions/color.dart

@@ -571,12 +571,13 @@ Value _rgb(String name, List<Value> arguments) {
   var green = arguments[1].assertNumber("green");
   var blue = arguments[2].assertNumber("blue");
 
-  return SassColor.rgb(
+  return SassColor.rgbInternal(
       fuzzyRound(_percentageOrUnitless(red, 255, "red")),
       fuzzyRound(_percentageOrUnitless(green, 255, "green")),
       fuzzyRound(_percentageOrUnitless(blue, 255, "blue")),
       alpha.andThen((alpha) =>
-          _percentageOrUnitless(alpha.assertNumber("alpha"), 1, "alpha")));
+          _percentageOrUnitless(alpha.assertNumber("alpha"), 1, "alpha")),
+      ColorFormat.rgbFunction);
 }
 
 Value _rgbTwoArg(String name, List<Value> arguments) {
@@ -624,12 +625,13 @@ Value _hsl(String name, List<Value> arguments) {
   _checkPercent(saturation, "saturation");
   _checkPercent(lightness, "lightness");
 
-  return SassColor.hsl(
+  return SassColor.hslInternal(
       hue.value,
       saturation.value.clamp(0, 100),
       lightness.value.clamp(0, 100),
       alpha.andThen((alpha) =>
-          _percentageOrUnitless(alpha.assertNumber("alpha"), 1, "alpha")));
+          _percentageOrUnitless(alpha.assertNumber("alpha"), 1, "alpha")),
+      ColorFormat.hslFunction);
 }
 
 /// Prints a deprecation warning if [hue] has a unit other than `deg`.

lib/src/parse/stylesheet.dart

@@ -2301,14 +2301,14 @@ abstract class StylesheetParser extends Parser {
 
     var first = scanner.peekChar();
     if (first != null && isDigit(first)) {
-      return ColorExpression(_hexColorContents(start));
+      return ColorExpression(_hexColorContents(start), scanner.spanFrom(start));
     }
 
     var afterHash = scanner.state;
     var identifier = interpolatedIdentifier();
     if (_isHexColor(identifier)) {
       scanner.state = afterHash;
-      return ColorExpression(_hexColorContents(start));
+      return ColorExpression(_hexColorContents(start), scanner.spanFrom(start));
     }
 
     var buffer = InterpolationBuffer();
@@ -2326,7 +2326,7 @@ abstract class StylesheetParser extends Parser {
     int red;
     int green;
     int blue;
-    num alpha = 1;
+    num? alpha;
     if (!isHex(scanner.peekChar())) {
       // #abc
       red = (digit1 << 4) + digit1;
@@ -2351,7 +2351,14 @@ abstract class StylesheetParser extends Parser {
       }
     }
 
-    return SassColor.rgb(red, green, blue, alpha, scanner.spanFrom(start));
+    return SassColor.rgbInternal(
+        red,
+        green,
+        blue,
+        alpha,
+        // Don't emit four- or eight-digit hex colors as hex, since that's not
+        // yet well-supported in browsers.
+        alpha == null ? SpanColorFormat(scanner.spanFrom(start)) : null);
   }
 
   /// Returns whether [interpolation] is a plain string that can be parsed as a
@@ -2664,9 +2671,9 @@ abstract class StylesheetParser extends Parser {
 
         var color = colorsByName[lower];
         if (color != null) {
-          color = SassColor.rgb(
-              color.red, color.green, color.blue, color.alpha, identifier.span);
-          return ColorExpression(color);
+          color = SassColor.rgbInternal(color.red, color.green, color.blue,
+              color.alpha, SpanColorFormat(identifier.span));
+          return ColorExpression(color, identifier.span);
         }
       }
 

lib/src/value/color.dart

@@ -88,27 +88,27 @@ class SassColor extends Value {
   num get alpha => _alpha;
   final num _alpha;
 
-  /// The original string representation of this color, or `null` if one is
-  /// unavailable.
+  /// The format in which this color was originally written and should be
+  /// serialized in expanded mode, or `null` if the color wasn't written in a
+  /// supported format.
   ///
   /// @nodoc
   @internal
-  String? get original => originalSpan?.text;
-
-  /// The span tracking the location in which this color was originally defined.
-  ///
-  /// This is tracked as a span to avoid extra substring allocations.
-  ///
-  /// @nodoc
-  @internal
-  final FileSpan? originalSpan;
+  final ColorFormat? format;
 
   /// Creates an RGB color.
   ///
   /// Throws a [RangeError] if [red], [green], and [blue] aren't between `0` and
   /// `255`, or if [alpha] isn't between `0` and `1`.
-  SassColor.rgb(this._red, this._green, this._blue,
-      [num? alpha, this.originalSpan])
+  SassColor.rgb(int red, int green, int blue, [num? alpha])
+      : this.rgbInternal(red, green, blue, alpha);
+
+  /// Like [SassColor.rgb], but also takes a [format] parameter.
+  ///
+  /// @nodoc
+  @internal
+  SassColor.rgbInternal(this._red, this._green, this._blue,
+      [num? alpha, this.format])
       : _alpha = alpha == null ? 1 : fuzzyAssertRange(alpha, 0, 1, "alpha") {
     RangeError.checkValueInInterval(red, 0, 255, "red");
     RangeError.checkValueInInterval(green, 0, 255, "green");
@@ -120,11 +120,18 @@ class SassColor extends Value {
   /// Throws a [RangeError] if [saturation] or [lightness] aren't between `0`
   /// and `100`, or if [alpha] isn't between `0` and `1`.
   SassColor.hsl(num hue, num saturation, num lightness, [num? alpha])
+      : this.hslInternal(hue, saturation, lightness, alpha);
+
+  /// Like [SassColor.hsl], but also takes a [format] parameter.
+  ///
+  /// @nodoc
+  @internal
+  SassColor.hslInternal(num hue, num saturation, num lightness,
+      [num? alpha, this.format])
       : _hue = hue % 360,
         _saturation = fuzzyAssertRange(saturation, 0, 100, "saturation"),
         _lightness = fuzzyAssertRange(lightness, 0, 100, "lightness"),
-        _alpha = alpha == null ? 1 : fuzzyAssertRange(alpha, 0, 1, "alpha"),
-        originalSpan = null;
+        _alpha = alpha == null ? 1 : fuzzyAssertRange(alpha, 0, 1, "alpha");
 
   /// Creates an HWB color.
   ///
@@ -160,7 +167,7 @@ class SassColor extends Value {
 
   SassColor._(this._red, this._green, this._blue, this._hue, this._saturation,
       this._lightness, this._alpha)
-      : originalSpan = null;
+      : format = null;
 
   /// @nodoc
   @internal
@@ -333,3 +340,44 @@ extension SassApiColor on SassColor {
   /// representation is readily available.
   bool get hasCalculatedHsl => _saturation != null;
 }
+
+/// A union interface of possible formats in which a Sass color could be
+/// defined.
+///
+/// When a color is serialized in expanded mode, it should preserve its original
+/// format.
+@internal
+abstract class ColorFormat {
+  /// A color defined using the `rgb()` or `rgba()` functions.
+  static const rgbFunction = _ColorFormatEnum("rgbFunction");
+
+  /// A color defined using the `hsl()` or `hsla()` functions.
+  static const hslFunction = _ColorFormatEnum("hslFunction");
+}
+
+/// The class for enum values of the [ColorFormat] type.
+@sealed
+class _ColorFormatEnum implements ColorFormat {
+  final String _name;
+
+  const _ColorFormatEnum(this._name);
+
+  String toString() => _name;
+}
+
+/// A [ColorFormat] where the color is serialized as the exact same text that
+/// was used to specify it originally.
+///
+/// This is tracked as a span rather than a string to avoid extra substring
+/// allocations.
+@internal
+@sealed
+class SpanColorFormat implements ColorFormat {
+  /// The span tracking the location in which this color was originally defined.
+  final FileSpan _span;
+
+  /// The original string that was used to define this color in the Sass source.
+  String get original => _span.text;
+
+  SpanColorFormat(this._span);
+}

lib/src/visitor/serialize.dart

@@ -522,48 +522,90 @@ class _SerializeVisitor
 
   void visitColor(SassColor value) {
     // In compressed mode, emit colors in the shortest representation possible.
-    if (_isCompressed && fuzzyEquals(value.alpha, 1)) {
-      var name = namesByColor[value];
-      var hexLength = _canUseShortHex(value) ? 4 : 7;
-      if (name != null && name.length <= hexLength) {
-        _buffer.write(name);
-      } else if (_canUseShortHex(value)) {
-        _buffer.writeCharCode($hash);
-        _buffer.writeCharCode(hexCharFor(value.red & 0xF));
-        _buffer.writeCharCode(hexCharFor(value.green & 0xF));
-        _buffer.writeCharCode(hexCharFor(value.blue & 0xF));
+    if (_isCompressed) {
+      if (!fuzzyEquals(value.alpha, 1)) {
+        _writeRgb(value);
       } else {
+        var name = namesByColor[value];
+        var hexLength = _canUseShortHex(value) ? 4 : 7;
+        if (name != null && name.length <= hexLength) {
+          _buffer.write(name);
+        } else if (_canUseShortHex(value)) {
+          _buffer.writeCharCode($hash);
+          _buffer.writeCharCode(hexCharFor(value.red & 0xF));
+          _buffer.writeCharCode(hexCharFor(value.green & 0xF));
+          _buffer.writeCharCode(hexCharFor(value.blue & 0xF));
+        } else {
+          _buffer.writeCharCode($hash);
+          _writeHexComponent(value.red);
+          _writeHexComponent(value.green);
+          _writeHexComponent(value.blue);
+        }
+      }
+    } else {
+      var format = value.format;
+      if (format != null) {
+        if (format == ColorFormat.rgbFunction) {
+          _writeRgb(value);
+        } else if (format == ColorFormat.hslFunction) {
+          _writeHsl(value);
+        } else {
+          _buffer.write((format as SpanColorFormat).original);
+        }
+      } else if (namesByColor.containsKey(value) &&
+          // Always emit generated transparent colors in rgba format. This works
+          // around an IE bug. See sass/sass#1782.
+          !fuzzyEquals(value.alpha, 0)) {
+        _buffer.write(namesByColor[value]);
+      } else if (fuzzyEquals(value.alpha, 1)) {
         _buffer.writeCharCode($hash);
         _writeHexComponent(value.red);
         _writeHexComponent(value.green);
         _writeHexComponent(value.blue);
+      } else {
+        _writeRgb(value);
       }
-      return;
     }
+  }
 
-    if (value.original != null) {
-      _buffer.write(value.original);
-    } else if (namesByColor.containsKey(value) &&
-        // Always emit generated transparent colors in rgba format. This works
-        // around an IE bug. See sass/sass#1782.
-        !fuzzyEquals(value.alpha, 0)) {
-      _buffer.write(namesByColor[value]);
-    } else if (fuzzyEquals(value.alpha, 1)) {
-      _buffer.writeCharCode($hash);
-      _writeHexComponent(value.red);
-      _writeHexComponent(value.green);
-      _writeHexComponent(value.blue);
-    } else {
-      _buffer
-        ..write("rgba(${value.red}")
-        ..write(_commaSeparator)
-        ..write(value.green)
-        ..write(_commaSeparator)
-        ..write(value.blue)
-        ..write(_commaSeparator);
+  /// Writes [value] as an `rgb()` or `rgba()` function.
+  void _writeRgb(SassColor value) {
+    var opaque = fuzzyEquals(value.alpha, 1);
+    _buffer
+      ..write(opaque ? "rgb(" : "rgba(")
+      ..write(value.red)
+      ..write(_commaSeparator)
+      ..write(value.green)
+      ..write(_commaSeparator)
+      ..write(value.blue);
+
+    if (!opaque) {
+      _buffer.write(_commaSeparator);
       _writeNumber(value.alpha);
-      _buffer.writeCharCode($rparen);
     }
+
+    _buffer.writeCharCode($rparen);
+  }
+
+  /// Writes [value] as an `hsl()` or `hsla()` function.
+  void _writeHsl(SassColor value) {
+    var opaque = fuzzyEquals(value.alpha, 1);
+    _buffer.write(opaque ? "hsl(" : "hsla(");
+    _writeNumber(value.hue);
+    _buffer.write("deg");
+    _buffer.write(_commaSeparator);
+    _writeNumber(value.saturation);
+    _buffer.writeCharCode($percent);
+    _buffer.write(_commaSeparator);
+    _writeNumber(value.lightness);
+    _buffer.writeCharCode($percent);
+
+    if (!opaque) {
+      _buffer.write(_commaSeparator);
+      _writeNumber(value.alpha);
+    }
+
+    _buffer.writeCharCode($rparen);
   }
 
   /// Returns whether [color]'s hex pair representation is symmetrical (e.g.

pkg/sass_api/lib/sass_api.dart

@@ -16,7 +16,7 @@ export 'package:sass/src/ast/sass.dart' hide AtRootQuery;
 export 'package:sass/src/async_import_cache.dart';
 export 'package:sass/src/exception.dart' show SassFormatException;
 export 'package:sass/src/import_cache.dart';
-export 'package:sass/src/value/color.dart';
+export 'package:sass/src/value/color.dart' hide ColorFormat, SpanColorFormat;
 export 'package:sass/src/visitor/find_dependencies.dart';
 export 'package:sass/src/visitor/interface/expression.dart';
 export 'package:sass/src/visitor/interface/statement.dart';