From 3d6b449372254e967f4f3f878b5e82a43088c463 Mon Sep 17 00:00:00 2001 From: Jennifer Thakar Date: Fri, 18 Jun 2021 16:39:39 -0700 Subject: [PATCH] Migrate to multiplication when possible (#198) --- CHANGELOG.md | 12 +++++ lib/src/migrators/division.dart | 53 ++++++++++++++----- pubspec.yaml | 2 +- test/cli_dart_test.dart | 22 +++----- test/migrators/division/always_division.hrx | 2 +- test/migrators/division/multiplication.hrx | 25 +++++++++ .../migrators/division/not_multiplication.hrx | 17 ++++++ 7 files changed, 104 insertions(+), 29 deletions(-) create mode 100644 test/migrators/division/multiplication.hrx create mode 100644 test/migrators/division/not_multiplication.hrx diff --git a/CHANGELOG.md b/CHANGELOG.md index 85c96bf..64f4249 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,15 @@ +## 1.5.0 + +### Division Migrator + +* When migrating division where the divisor is a common constant value, the + migrator will now convert it to multiplication, rather than `math.div`. + + For example: `$variable / 2` would be migrated to `$variable * 0.5`. + + To disable this and migrate all division to `math.div`, pass + `--no-multiplication`. + ## 1.4.5 * Glob syntax will no longer be resolved if a file with that literal name diff --git a/lib/src/migrators/division.dart b/lib/src/migrators/division.dart index a958f31..ab30c7b 100644 --- a/lib/src/migrators/division.dart +++ b/lib/src/migrators/division.dart @@ -32,26 +32,35 @@ More info: https://sass-lang.com/d/slash-div"""; ..addFlag('pessimistic', abbr: 'p', help: "Only migrate / expressions that are unambiguously division.", - negatable: false); + negatable: false) + ..addFlag('multiplication', + help: 'Migrate / expressions with certain constant divisors to use ' + 'multiplication instead.', + defaultsTo: true); bool get isPessimistic => argResults!['pessimistic'] as bool; + bool get useMultiplication => argResults!['multiplication'] as bool; @override Map migrateFile( ImportCache importCache, Stylesheet stylesheet, Importer importer) { var visitor = _DivisionMigrationVisitor( - importCache, this.isPessimistic, migrateDependencies); + importCache, isPessimistic, useMultiplication, migrateDependencies); var result = visitor.run(stylesheet, importer); missingDependencies.addAll(visitor.missingDependencies); return result; } } +/// The set of constant divisors that should be migrated to multiplication. +const _allowedDivisors = {2, 4, 5, 8, 10, 20, 40, 50, 80, 100, 1000}; + class _DivisionMigrationVisitor extends MigrationVisitor { final bool isPessimistic; + final bool useMultiplication; - _DivisionMigrationVisitor( - ImportCache importCache, this.isPessimistic, bool migrateDependencies) + _DivisionMigrationVisitor(ImportCache importCache, this.isPessimistic, + this.useMultiplication, bool migrateDependencies) : super(importCache, migrateDependencies); /// True when division is allowed by the context the current node is in. @@ -260,9 +269,8 @@ class _DivisionMigrationVisitor extends MigrationVisitor { /// Visits a `/` operation [node] and migrates it to either the `division` /// function or the `slash-list` function. /// - /// Returns true the `/` was migrated to either function call, and false if - /// the `/` is ambiguous and a warning was emitted instead (pessimistic mode - /// only). + /// Returns true the `/` was migrated to either function call (indicating that + /// parentheses surrounding this operation should be removed). bool _visitSlashOperation(BinaryOperationExpression node) { if ((!_isDivisionAllowed && _onlySlash(node)) || _isDefinitelyNotNumber(node)) { @@ -276,17 +284,17 @@ class _DivisionMigrationVisitor extends MigrationVisitor { _visitSlashListArguments(node); } return true; - } else if (_expectsNumericResult || - _isDefinitelyNumber(node) || - !isPessimistic) { + } + if (_expectsNumericResult || _isDefinitelyNumber(node) || !isPessimistic) { // Definitely division + _withContext(() => super.visitBinaryOperationExpression(node), + expectsNumericResult: true); + if (_tryMultiplication(node)) return false; addPatch(patchBefore(node, "${_builtInPrefix('math')}div(")); addPatch(patchAfter(node, ")")); _patchParensIfAny(node.left); _patchOperatorToComma(node); _patchParensIfAny(node.right); - _withContext(() => super.visitBinaryOperationExpression(node), - expectsNumericResult: true); return true; } else { emitWarning("Could not determine whether this is division", node.span); @@ -295,6 +303,27 @@ class _DivisionMigrationVisitor extends MigrationVisitor { } } + /// Given a division operation [node], patches it to use multiplication + /// instead if the reciprocal of the divisor can be accurately represented as + /// a decimal. + /// + /// Returns true if patched and false otherwise. + bool _tryMultiplication(BinaryOperationExpression node) { + if (!useMultiplication) return false; + var divisor = node.right; + if (divisor is! NumberExpression) return false; + if (divisor.unit != null) return false; + if (!_allowedDivisors.contains(divisor.value)) return false; + var operatorSpan = node.left.span + .extendThroughWhitespace() + .end + .pointSpan() + .extendIfMatches('/'); + addPatch(Patch(operatorSpan, '*')); + addPatch(Patch(node.right.span, '${1 / divisor.value}')); + return true; + } + /// Visits the arguments of a `/` operation that is being converted into a /// call to `slash-list`, converting slashes to commas and removing /// unnecessary interpolation. diff --git a/pubspec.yaml b/pubspec.yaml index 90a4f0d..6e4e4f5 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: sass_migrator -version: 1.4.5 +version: 1.5.0 description: A tool for running migrations on Sass files author: Jennifer Thakar homepage: https://github.com/sass/migrator diff --git a/test/cli_dart_test.dart b/test/cli_dart_test.dart index 1e65e01..0223216 100644 --- a/test/cli_dart_test.dart +++ b/test/cli_dart_test.dart @@ -37,15 +37,9 @@ void main() { await (await runMigrator(["division", "test-*.scss"])).shouldExit(0); - await d - .file("test-1.scss", '@use "sass:math";\n\na {b: math.div(1, 2)}') - .validate(); - await d - .file("test-2.scss", '@use "sass:math";\n\nc {d: math.div(1, 2)}') - .validate(); - await d - .file("test-3.scss", '@use "sass:math";\n\ne {f: math.div(1, 2)}') - .validate(); + await d.file("test-1.scss", 'a {b: (1 * 0.5)}').validate(); + await d.file("test-2.scss", 'c {d: (1 * 0.5)}').validate(); + await d.file("test-3.scss", 'e {f: (1 * 0.5)}').validate(); }); test("allows recursive glob arguments", () async { @@ -58,9 +52,9 @@ void main() { await (await runMigrator(["division", "**.scss"])).shouldExit(0); await d.dir('dir', [ - d.file("test-1.scss", '@use "sass:math";\n\na {b: math.div(1, 2)}'), - d.file("test-2.scss", '@use "sass:math";\n\nc {d: math.div(1, 2)}'), - d.file("test-3.scss", '@use "sass:math";\n\ne {f: math.div(1, 2)}') + d.file("test-1.scss", 'a {b: (1 * 0.5)}'), + d.file("test-2.scss", 'c {d: (1 * 0.5)}'), + d.file("test-3.scss", 'e {f: (1 * 0.5)}') ]).validate(); }); @@ -69,9 +63,7 @@ void main() { await (await runMigrator(["division", "[dir]/test.scss"])).shouldExit(0); - await d.dir('[dir]', [ - d.file("test.scss", '@use "sass:math";\n\na {b: math.div(1, 2)}') - ]).validate(); + await d.dir('[dir]', [d.file("test.scss", 'a {b: (1 * 0.5)}')]).validate(); }); group("with --dry-run", () { diff --git a/test/migrators/division/always_division.hrx b/test/migrators/division/always_division.hrx index dd9594c..e089afb 100644 --- a/test/migrators/division/always_division.hrx +++ b/test/migrators/division/always_division.hrx @@ -1,5 +1,5 @@ <==> arguments ---pessimistic +--pessimistic --no-multiplication <==> input/entrypoint.scss @function six-divided-by-three() { diff --git a/test/migrators/division/multiplication.hrx b/test/migrators/division/multiplication.hrx new file mode 100644 index 0000000..2fe9f27 --- /dev/null +++ b/test/migrators/division/multiplication.hrx @@ -0,0 +1,25 @@ +<==> input/entrypoint.scss +$a: 2000px / 2; +$b: $a / 4; +$c: $a / 5; +$d: ($a / 8); +$e: ($a / 10); +$f: ($a / 20); +$g: $a/40; +$h: ($a/50); +$i: ($a + $b)/80; +$j: ($a / 2)/100; +$k: $a/2/1000; + +<==> output/entrypoint.scss +$a: 2000px * 0.5; +$b: $a * 0.25; +$c: $a * 0.2; +$d: ($a * 0.125); +$e: ($a * 0.1); +$f: ($a * 0.05); +$g: $a*0.025; +$h: ($a*0.02); +$i: ($a + $b)*0.0125; +$j: ($a * 0.5)*0.01; +$k: $a*0.5*0.001; diff --git a/test/migrators/division/not_multiplication.hrx b/test/migrators/division/not_multiplication.hrx new file mode 100644 index 0000000..02e230c --- /dev/null +++ b/test/migrators/division/not_multiplication.hrx @@ -0,0 +1,17 @@ +<==> input/entrypoint.scss +$a: 2000px / 3; +$b: $a / 7; +$c: $a / 16; +$d: $a / 25; +$e: $a / 10000; +$f: $a / 2px; + +<==> output/entrypoint.scss +@use "sass:math"; + +$a: math.div(2000px, 3); +$b: math.div($a, 7); +$c: math.div($a, 16); +$d: math.div($a, 25); +$e: math.div($a, 10000); +$f: math.div($a, 2px);