Re-enable new calculation functions (#2080)

Author: nex3

CHANGELOG.md

@@ -1,5 +1,31 @@
 ## 1.67.0
 
+* All functions defined in CSS Values and Units 4 are now once again parsed as
+  calculation objects: `round()`, `mod()`, `rem()`, `sin()`, `cos()`, `tan()`,
+  `asin()`, `acos()`, `atan()`, `atan2()`, `pow()`, `sqrt()`, `hypot()`,
+  `log()`, `exp()`, `abs()`, and `sign()`.
+
+  Unlike in 1.65.0, function calls are _not_ locked into being parsed as
+  calculations or plain Sass functions at parse-time. This means that
+  user-defined functions will take precedence over CSS calculations of the same
+  name. Although the function names `calc()` and `clamp()` are still forbidden,
+  users may continue to freely define functions whose names overlap with other
+  CSS calculations (including `abs()`, `min()`, `max()`, and `round()` whose
+  names overlap with global Sass functions).
+
+* As a consequence of the change in calculation parsing described above,
+  calculation functions containing interpolation are now parsed more strictly
+  than before. However, all interpolations that would have produced valid CSS
+  will continue to work, so this is not considered a breaking change.
+
+* Interpolations in calculation functions that aren't used in a position that
+  could also have a normal calculation value are now deprecated. For example,
+  `calc(1px #{"+ 2px"})` is deprecated, but `calc(1px + #{"2px"})` is still
+  allowed. This deprecation is named `calc-interp`. See [the Sass website] for
+  more information.
+
+  [the Sass website]: https://sass-lang.com/d/calc-interp
+
 * **Potentially breaking bug fix**: The importer used to load a given file is no
   longer used to load absolute URLs that appear in that file. This was
   unintented behavior that contradicted the Sass specification. Absolute URLs

lib/src/ast/sass.dart

@@ -13,7 +13,6 @@ export 'sass/dependency.dart';
 export 'sass/expression.dart';
 export 'sass/expression/binary_operation.dart';
 export 'sass/expression/boolean.dart';
-export 'sass/expression/calculation.dart';
 export 'sass/expression/color.dart';
 export 'sass/expression/function.dart';
 export 'sass/expression/if.dart';

lib/src/ast/sass/expression.dart

@@ -2,13 +2,16 @@
 // MIT-style license that can be found in the LICENSE file or at
 // https://opensource.org/licenses/MIT.
 
+import 'package:charcode/charcode.dart';
 import 'package:meta/meta.dart';
 
 import '../../exception.dart';
 import '../../logger.dart';
 import '../../parse/scss.dart';
+import '../../util/nullable.dart';
+import '../../value.dart';
 import '../../visitor/interface/expression.dart';
-import 'node.dart';
+import '../sass.dart';
 
 /// A SassScript expression in a Sass syntax tree.
 ///
@@ -27,3 +30,85 @@ abstract interface class Expression implements SassNode {
   factory Expression.parse(String contents, {Object? url, Logger? logger}) =>
       ScssParser(contents, url: url, logger: logger).parseExpression();
 }
+
+// Use an extension class rather than a method so we don't have to make
+// [Expression] a concrete base class for something we'll get rid of anyway once
+// we remove the global math functions that make this necessary.
+extension ExpressionExtensions on Expression {
+  /// Whether this expression can be used in a calculation context.
+  ///
+  /// @nodoc
+  @internal
+  bool get isCalculationSafe => accept(_IsCalculationSafeVisitor());
+}
+
+// We could use [AstSearchVisitor] to implement this more tersely, but that
+// would default to returning `true` if we added a new expression type and
+// forgot to update this class.
+class _IsCalculationSafeVisitor implements ExpressionVisitor<bool> {
+  const _IsCalculationSafeVisitor();
+
+  bool visitBinaryOperationExpression(BinaryOperationExpression node) =>
+      (const {
+        BinaryOperator.times,
+        BinaryOperator.dividedBy,
+        BinaryOperator.plus,
+        BinaryOperator.minus
+      }).contains(node.operator) &&
+      (node.left.accept(this) || node.right.accept(this));
+
+  bool visitBooleanExpression(BooleanExpression node) => false;
+
+  bool visitColorExpression(ColorExpression node) => false;
+
+  bool visitFunctionExpression(FunctionExpression node) => true;
+
+  bool visitInterpolatedFunctionExpression(
+          InterpolatedFunctionExpression node) =>
+      true;
+
+  bool visitIfExpression(IfExpression node) => true;
+
+  bool visitListExpression(ListExpression node) =>
+      node.separator == ListSeparator.space &&
+      !node.hasBrackets &&
+      node.contents.length > 1 &&
+      node.contents.every((expression) => expression.accept(this));
+
+  bool visitMapExpression(MapExpression node) => false;
+
+  bool visitNullExpression(NullExpression node) => false;
+
+  bool visitNumberExpression(NumberExpression node) => true;
+
+  bool visitParenthesizedExpression(ParenthesizedExpression node) =>
+      node.expression.accept(this);
+
+  bool visitSelectorExpression(SelectorExpression node) => false;
+
+  bool visitStringExpression(StringExpression node) {
+    if (node.hasQuotes) return false;
+
+    // Exclude non-identifier constructs that are parsed as [StringExpression]s.
+    // We could just check if they parse as valid identifiers, but this is
+    // cheaper.
+    var text = node.text.initialPlain;
+    return
+        // !important
+        !text.startsWith("!") &&
+            // ID-style identifiers
+            !text.startsWith("#") &&
+            // Unicode ranges
+            text.codeUnitAtOrNull(1) != $plus &&
+            // url()
+            text.codeUnitAtOrNull(3) != $lparen;
+  }
+
+  bool visitSupportsExpression(SupportsExpression node) => false;
+
+  bool visitUnaryOperationExpression(UnaryOperationExpression node) => false;
+
+  bool visitValueExpression(ValueExpression node) => false;
+
+  bool visitVariableExpression(VariableExpression node) => true;
+}

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

@@ -6,6 +6,7 @@ import 'package:charcode/charcode.dart';
 import 'package:meta/meta.dart';
 import 'package:source_span/source_span.dart';
 
+import '../../../util/span.dart';
 import '../../../visitor/interface/expression.dart';
 import '../expression.dart';
 import 'list.dart';
@@ -45,6 +46,17 @@ final class BinaryOperationExpression implements Expression {
     return left.span.expand(right.span);
   }
 
+  /// Returns the span that covers only [operator].
+  ///
+  /// @nodoc
+  @internal
+  FileSpan get operatorSpan => left.span.file == right.span.file &&
+          left.span.end.offset < right.span.start.offset
+      ? left.span.file
+          .span(left.span.end.offset, right.span.start.offset)
+          .trim()
+      : span;
+
   BinaryOperationExpression(this.operator, this.left, this.right)
       : allowsSlash = false;
 

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

@@ -21,6 +21,9 @@ final class Interpolation implements SassNode {
 
   final FileSpan span;
 
+  /// Returns whether this contains no interpolated expressions.
+  bool get isPlain => asPlain != null;
+
   /// If this contains no interpolated expressions, returns its text contents.
   ///
   /// Otherwise, returns `null`.

lib/src/ast/sass/interpolation.dart

@@ -69,6 +69,11 @@ enum Deprecation {
       deprecatedIn: '1.62.3',
       description: 'Passing null as alpha in the ${isJS ? 'JS' : 'Dart'} API.'),
 
+  calcInterp('calc-interp',
+      deprecatedIn: '1.67.0',
+      description: 'Using interpolation in a calculation outside a value '
+          'position.'),
+
   /// Deprecation for `@import` rules.
   import.future('import', description: '@import rules.'),
 

lib/src/deprecation.dart

@@ -134,8 +134,6 @@ final class Protofier {
           ..operator = _protofyCalculationOperator(value.operator)
           ..left = _protofyCalculationValue(value.left)
           ..right = _protofyCalculationValue(value.right);
-      case CalculationInterpolation():
-        result.interpolation = value.value;
       case _:
         throw "Unknown calculation value $value";
     }
@@ -352,7 +350,7 @@ final class Protofier {
               _deprotofyCalculationValue(value.operation.left),
               _deprotofyCalculationValue(value.operation.right)),
         Value_Calculation_CalculationValue_Value.interpolation =>
-          CalculationInterpolation(value.interpolation),
+          SassString('(${value.interpolation})', quotes: false),
         Value_Calculation_CalculationValue_Value.notSet =>
           throw mandatoryError("Value.Calculation.value")
       };