Improve inspect() output for complex units (#2138)

Closes #2070

CHANGELOG.md

@@ -1,5 +1,8 @@
 ## 1.69.6
 
+* Produce better output for numbers with complex units in `meta.inspect()` and
+  debugging messages.
+
 ### JS API
 
 * Fix a bug where certain exceptions could produce `SourceSpan`s that didn't

lib/src/visitor/serialize.dart

@@ -6,7 +6,6 @@ import 'dart:math' as math;
 import 'dart:typed_data';
 
 import 'package:charcode/charcode.dart';
-import 'package:collection/collection.dart';
 import 'package:source_maps/source_maps.dart';
 import 'package:string_scanner/string_scanner.dart';
 
@@ -489,13 +488,8 @@ final class _SerializeVisitor
 
   void _writeCalculationValue(Object value) {
     switch (value) {
-      case SassNumber(value: double(isFinite: false), hasComplexUnits: true):
-        if (!_inspect) {
-          throw SassScriptException("$value isn't a valid CSS value.");
-        }
-
-        _writeNumber(value.value);
-        _buffer.write(value.unitString);
+      case SassNumber(hasComplexUnits: true) when !_inspect:
+        throw SassScriptException("$value isn't a valid CSS value.");
 
       case SassNumber(value: double(isFinite: false)):
         switch (value.value) {
@@ -507,12 +501,15 @@ final class _SerializeVisitor
             _buffer.write('NaN');
         }
 
-        if (value.numeratorUnits.firstOrNull case var unit?) {
-          _writeOptionalSpace();
-          _buffer.writeCharCode($asterisk);
-          _writeOptionalSpace();
-          _buffer.writeCharCode($1);
-          _buffer.write(unit);
+        _writeCalculationUnits(value.numeratorUnits, value.denominatorUnits);
+
+      case SassNumber(hasComplexUnits: true):
+        _writeNumber(value.value);
+        if (value.numeratorUnits case [var first, ...var rest]) {
+          _buffer.write(first);
+          _writeCalculationUnits(rest, value.denominatorUnits);
+        } else {
+          _writeCalculationUnits([], value.denominatorUnits);
         }
 
       case Value():
@@ -534,14 +531,36 @@ final class _SerializeVisitor
                 _parenthesizeCalculationRhs(operator, right.operator)) ||
             (operator == CalculationOperator.dividedBy &&
                 right is SassNumber &&
-                !right.value.isFinite &&
-                right.hasUnits);
+                (right.value.isFinite
+                    ? right.hasComplexUnits
+                    : right.hasUnits));
         if (parenthesizeRight) _buffer.writeCharCode($lparen);
         _writeCalculationValue(right);
         if (parenthesizeRight) _buffer.writeCharCode($rparen);
     }
   }
 
+  /// Writes the complex numerator and denominator units beyond the first
+  /// numerator unit for a number as they appear in a calculation.
+  void _writeCalculationUnits(
+      List<String> numeratorUnits, List<String> denominatorUnits) {
+    for (var unit in numeratorUnits) {
+      _writeOptionalSpace();
+      _buffer.writeCharCode($asterisk);
+      _writeOptionalSpace();
+      _buffer.writeCharCode($1);
+      _buffer.write(unit);
+    }
+
+    for (var unit in denominatorUnits) {
+      _writeOptionalSpace();
+      _buffer.writeCharCode($slash);
+      _writeOptionalSpace();
+      _buffer.writeCharCode($1);
+      _buffer.write(unit);
+    }
+  }
+
   /// Returns whether the right-hand operation of a calculation should be
   /// parenthesized.
   ///
@@ -787,16 +806,15 @@ final class _SerializeVisitor
       return;
     }
 
-    _writeNumber(value.value);
-
-    if (!_inspect) {
-      if (value.hasComplexUnits) {
+    if (value.hasComplexUnits) {
+      if (!_inspect) {
         throw SassScriptException("$value isn't a valid CSS value.");
       }
 
-      if (value.numeratorUnits case [var first]) _buffer.write(first);
+      visitCalculation(SassCalculation.unsimplified('calc', [value]));
     } else {
-      _buffer.write(value.unitString);
+      _writeNumber(value.value);
+      if (value.numeratorUnits case [var first]) _buffer.write(first);
     }
   }
 

test/embedded/function_test.dart

@@ -912,7 +912,7 @@ void main() {
                     ..value = 1
                     ..numerators.addAll(["em", "px", "foo"])),
                 inspect: true),
-            "1em*px*foo");
+            "calc(1em * 1px * 1foo)");
       });
 
       test("with one denominator", () async {
@@ -923,7 +923,7 @@ void main() {
                     ..value = 1
                     ..denominators.add("em")),
                 inspect: true),
-            "1em^-1");
+            "calc(1 / 1em)");
       });
 
       test("with multiple denominators", () async {
@@ -934,7 +934,7 @@ void main() {
                     ..value = 1
                     ..denominators.addAll(["em", "px", "foo"])),
                 inspect: true),
-            "1(em*px*foo)^-1");
+            "calc(1 / 1em / 1px / 1foo)");
       });
 
       test("with numerators and denominators", () async {
@@ -946,7 +946,7 @@ void main() {
                     ..numerators.addAll(["em", "px"])
                     ..denominators.addAll(["s", "foo"])),
                 inspect: true),
-            "1em*px/s*foo");
+            "calc(1em * 1px / 1s / 1foo)");
       });
     });
 

test/output_test.dart

@@ -113,10 +113,55 @@ void main() {
     });
   });
 
-  // Tests for sass/dart-sass#417.
+  // Tests for sass/dart-sass#2070.
   //
-  // Note there's no need for "in Sass" cases as it's not possible to have
-  // trailing loud comments in the Sass syntax.
+  // These aren't covered by sass-spec because the inspect format for
+  // non-literal values isn't covered by the spec.
+  group("uses a nice format to inspect numbers with complex units", () {
+    group("finite", () {
+      test("top-level", () {
+        expect(compileString("""
+          @use 'sass:meta';
+          a {b: meta.inspect(1px * 1em)};
+        """), equalsIgnoringWhitespace('a { b: calc(1px * 1em); }'));
+      });
+
+      test("in calc", () {
+        expect(compileString("""
+          @use 'sass:meta';
+          a {b: meta.inspect(calc(1px * 1em))};
+        """), equalsIgnoringWhitespace('a { b: calc(1px * 1em); }'));
+      });
+
+      test("nested in calc", () {
+        expect(compileString("""
+          @use 'sass:meta';
+          a {b: meta.inspect(calc(c / (1px * 1em)))};
+        """), equalsIgnoringWhitespace('a { b: calc(c / (1px * 1em)); }'));
+      });
+
+      test("numerator and denominator", () {
+        expect(compileString("""
+          @use 'sass:math';
+          @use 'sass:meta';
+          a {b: meta.inspect(1px * math.div(math.div(1em, 1s), 1x))};
+        """), equalsIgnoringWhitespace('a { b: calc(1px * 1em / 1s / 1x); }'));
+      });
+
+      test("denominator only", () {
+        expect(compileString("""
+          @use 'sass:math';
+          @use 'sass:meta';
+          a {b: meta.inspect(math.div(math.div(1, 1s), 1x))};
+        """), equalsIgnoringWhitespace('a { b: calc(1 / 1s / 1x); }'));
+      });
+    });
+  });
+
+  // Tests for sass/dart-sass#417.
+  //
+  // Note there's no need for "in Sass" cases as it's not possible to have
+  // trailing loud comments in the Sass syntax.
   group("preserve trailing loud comments in SCSS", () {
     test("after open block", () {
       expect(compileString("""