Stop migrating division inside calc expressions (#217)

Fixes #216.

Author: jathak

CHANGELOG.md

@@ -1,3 +1,9 @@
+## 1.5.3
+
+### Division Migrator
+
+* Fix a bug where division inside calc expressions was unnecessarily migrated.
+
 ## 1.5.2
 
 * No user-visible changes.

lib/src/migrators/division.dart

@@ -69,6 +69,10 @@ class _DivisionMigrationVisitor extends MigrationVisitor {
   /// True when the current node is expected to evaluate to a number.
   var _expectsNumericResult = false;
 
+  /// True when the current node is in calc context, so division should be
+  /// left unchanged.
+  var _inCalcContext = false;
+
   /// The namespaces that already exist in the current stylesheet.
   Map<Uri, String?> get _existingNamespaces =>
       assertInStylesheet(__existingNamespaces, '_existingNamespaces');
@@ -140,7 +144,7 @@ class _DivisionMigrationVisitor extends MigrationVisitor {
   @override
   void visitArgumentInvocation(ArgumentInvocation invocation) {
     _withContext(() => super.visitArgumentInvocation(invocation),
-        isDivisionAllowed: true);
+        isDivisionAllowed: true, inCalcContext: false);
   }
 
   /// If this is a division operation, migrates it.
@@ -159,6 +163,14 @@ class _DivisionMigrationVisitor extends MigrationVisitor {
     }
   }
 
+  /// Visits calculations with [_inCalcContext] set to true.
+  @override
+  void visitCalculationExpression(CalculationExpression node) {
+    _withContext(() {
+      super.visitCalculationExpression(node);
+    }, inCalcContext: true);
+  }
+
   /// Allows division within a function call's arguments, with special handling
   /// for new-syntax color functions.
   @override
@@ -167,6 +179,15 @@ class _DivisionMigrationVisitor extends MigrationVisitor {
     visitArgumentInvocation(node.arguments);
   }
 
+  /// Visits interpolation with [_isDivisionAllowed] and [_inCalcContext] set
+  /// to false.
+  @override
+  void visitInterpolation(Interpolation node) {
+    _withContext(() {
+      super.visitInterpolation(node);
+    }, isDivisionAllowed: false, inCalcContext: false);
+  }
+
   /// Disallows division within this list.
   @override
   void visitListExpression(ListExpression node) {
@@ -271,6 +292,12 @@ class _DivisionMigrationVisitor extends MigrationVisitor {
   /// Returns true the `/` was migrated to either function call (indicating that
   /// parentheses surrounding this operation should be removed).
   bool _visitSlashOperation(BinaryOperationExpression node) {
+    if (_inCalcContext) {
+      node.left.accept(this);
+      node.right.accept(this);
+      return false;
+    }
+
     if ((!_isDivisionAllowed && _onlySlash(node)) ||
         _isDefinitelyNotNumber(node)) {
       // Definitely not division
@@ -446,15 +473,18 @@ class _DivisionMigrationVisitor extends MigrationVisitor {
 
   /// Runs [operation] with the given context.
   void _withContext(void operation(),
-      {bool? isDivisionAllowed, bool? expectsNumericResult}) {
+      {bool? isDivisionAllowed,
+      bool? expectsNumericResult,
+      bool? inCalcContext}) {
     var previousDivisionAllowed = _isDivisionAllowed;
     var previousNumericResult = _expectsNumericResult;
-    if (isDivisionAllowed != null) _isDivisionAllowed = isDivisionAllowed;
-    if (expectsNumericResult != null) {
-      _expectsNumericResult = expectsNumericResult;
-    }
+    var previousCalcContext = _inCalcContext;
+    _isDivisionAllowed = isDivisionAllowed ?? _isDivisionAllowed;
+    _expectsNumericResult = expectsNumericResult ?? _expectsNumericResult;
+    _inCalcContext = inCalcContext ?? _inCalcContext;
     operation();
     _isDivisionAllowed = previousDivisionAllowed;
     _expectsNumericResult = previousNumericResult;
+    _inCalcContext = previousCalcContext;
   }
 }

pubspec.yaml

@@ -1,7 +1,6 @@
 name: sass_migrator
-version: 1.5.2
+version: 1.5.3
 description: A tool for running migrations on Sass files
-author: Jennifer Thakar <[email protected]>
 homepage: https://github.com/sass/migrator
 
 environment:
@@ -18,7 +17,7 @@ dependencies:
   node_interop: ^2.0.2
   node_io: ^2.1.0
   path: ^1.8.0
-  sass: ^1.38.1
+  sass: ^1.44.0
   source_span: ^1.8.1
   string_scanner: ^1.1.0
   term_glyph: ^1.2.0

test/migrators/division/calculations.hrx

@@ -0,0 +1,23 @@
+<==> input/entrypoint.scss
+a {
+  $x: 300px;
+  $y: 100%;
+  b: calc($x / 2);
+  c: clamp($x / 10, $y / 4, $x / 2);
+  d: min($x / 2, $y / 2);
+  e: calc(max($x / 2, $y / 2) / 2);
+  f: calc(#{$x / 2});
+  g: calc(fn($x / 2));
+}
+
+<==> output/entrypoint.scss
+a {
+  $x: 300px;
+  $y: 100%;
+  b: calc($x / 2);
+  c: clamp($x / 10, $y / 4, $x / 2);
+  d: min($x / 2, $y / 2);
+  e: calc(max($x / 2, $y / 2) / 2);
+  f: calc(#{$x * 0.5});
+  g: calc(fn($x * 0.5));
+}