Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add legacy color function migrator #260

Merged
merged 2 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
## 2.1.0

### Color Function Migrator

* Adds a new `color` migrator that migrates off of legacy color functions that
were deprecated in Dart Sass 1.79.0.

## 2.0.3

### Module Migrator
Expand Down
192 changes: 192 additions & 0 deletions lib/src/migrators/color.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
// Copyright 2024 Google LLC
//
// Use of this source code is governed by an MIT-style
// license that can be found in the LICENSE file or at
// https://opensource.org/licenses/MIT.

import 'package:sass_api/sass_api.dart';
import 'package:sass_migrator/src/migrators/module/reference_source.dart';

import 'module/references.dart';
import '../migration_visitor.dart';
import '../migrator.dart';
import '../patch.dart';
import '../utils.dart';

/// Migrates off of legacy color functions.
class ColorMigrator extends Migrator {
final name = "color";
final description = "Migrates off of legacy color functions.";

@override
Map<Uri, String> migrateFile(
ImportCache importCache, Stylesheet stylesheet, Importer importer) {
var references = References(importCache, stylesheet, importer);
var visitor =
_ColorMigrationVisitor(references, importCache, migrateDependencies);
var result = visitor.run(stylesheet, importer);
missingDependencies.addAll(visitor.missingDependencies);
return result;
}
}

final _colorUrl = Uri(scheme: 'sass', path: 'color');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: document this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


class _ColorMigrationVisitor extends MigrationVisitor {
final References references;

_ColorMigrationVisitor(
this.references, super.importCache, super.migrateDependencies);

String? _colorModuleNamespace;
Set<String> _usedNamespaces = {};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: document these

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


@override
void visitStylesheet(Stylesheet node) {
var oldColorModuleNamespace = _colorModuleNamespace;
var oldUsedNamespaces = _usedNamespaces;
_colorModuleNamespace = null;
_usedNamespaces = {};
// Check all the namespaces used by this file before visiting the
// stylesheet, in case deprecated functions are called before all `@use`
// rules.
for (var useRule in node.uses) {
if (_colorModuleNamespace != null || useRule.namespace == null) continue;
if (useRule.url == _colorUrl) {
_colorModuleNamespace = useRule.namespace;
} else {
_usedNamespaces.add(useRule.namespace!);
}
}
super.visitStylesheet(node);
_colorModuleNamespace = oldColorModuleNamespace;
_usedNamespaces = oldUsedNamespaces;
}

@override
void visitFunctionExpression(FunctionExpression node) {
var source = references.sources[node];
if (source is! BuiltInSource || source.url != _colorUrl) return;
var isMigrating = true;
switch (node.name) {
case 'red' || 'green' || 'blue':
_patchChannel(node, 'rgb');
case 'hue' || 'saturation' || 'lightness':
_patchChannel(node, 'hsl');
case 'whiteness' || 'blackness':
_patchChannel(node, 'hwb');
case 'alpha':
_patchChannel(node);
case 'adjust-hue':
_patchAdjust(node, channel: 'hue', space: 'hsl');
case 'saturate'
when node.arguments.named.length + node.arguments.positional.length !=
1:
_patchAdjust(node, channel: 'saturation', space: 'hsl');
case 'desaturate':
_patchAdjust(node, channel: 'saturation', negate: true, space: 'hsl');
case 'transparentize' || 'fade-out':
_patchAdjust(node, channel: 'alpha', negate: true);
case 'opacify' || 'fade-in':
_patchAdjust(node, channel: 'alpha');
case 'lighten':
_patchAdjust(node, channel: 'lightness', space: 'hsl');
case 'darken':
_patchAdjust(node, channel: 'lightness', negate: true, space: 'hsl');
default:
isMigrating == false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this supposed to be

Suggested change
isMigrating == false;
isMigrating = false;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could also just return here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it was supposed to be assigning, but returning makes more sense

}
if (isMigrating && node.namespace == null) {
if (_colorModuleNamespace == null) {
_colorModuleNamespace = _findColorModuleNamespace();
var asClause = _colorModuleNamespace == 'color'
? ''
: ' as $_colorModuleNamespace';
addPatch(Patch.insert(
node.span.file.location(0), '@use "sass:color"$asClause;\n\n'));
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider pulling this into a _getOrAddColorModuleNamespace() function.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

addPatch(patchBefore(node, '$_colorModuleNamespace.'),
beforeExisting: true);
}
}

/// Find an unused namespace for the sass:color module.
String _findColorModuleNamespace() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider calling this _chooseColorModuleNamespace() to make it clearer that it's choosing a new namespace rather than finding one that's already loaded.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

if (!_usedNamespaces.contains('color')) return 'color';
if (!_usedNamespaces.contains('sass-color')) return 'sass-color';
var count = 2;
var namespace = 'color$count';
while (_usedNamespaces.contains(namespace)) {
namespace = 'color${++count}';
}
return namespace;
}

/// Patches a deprecated channel function to use `color.channel` instead.
void _patchChannel(FunctionExpression node, [String? colorSpace]) {
addPatch(Patch(node.nameSpan, 'channel'));

if (node.arguments.named.isEmpty) {
addPatch(patchAfter(node.arguments.positional.last,
", '${node.name}'${colorSpace == null ? '' : ', $colorSpace'}"));
} else {
addPatch(patchAfter(
[...node.arguments.positional, ...node.arguments.named.values].last,
", \$channel: '${node.name}'"
"${colorSpace == null ? '' : ', \$space: $colorSpace'}"));
}
}

/// Patches a deprecated adjustment function to use `color.adjust` instead.
void _patchAdjust(FunctionExpression node,
{required String channel, bool negate = false, String? space}) {
addPatch(Patch(node.nameSpan, 'adjust'));
switch (node.arguments) {
case ArgumentInvocation(positional: [_, var adjustment]):
addPatch(patchBefore(adjustment, '\$$channel: ${negate ? '-' : ''}'));
if (negate && adjustment.needsParens) {
addPatch(patchBefore(adjustment, '('));
addPatch(patchAfter(adjustment, ')'));
}
if (space != null) {
addPatch(patchAfter(adjustment, ', \$space: $space'));
}
case ArgumentInvocation(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style nit: empty lines between long cases.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

named: {'amount': var adjustment} || {'degrees': var adjustment}
):
var start = adjustment.span.start.offset - 1;
while (adjustment.span.file.getText(start, start + 1) != r'$') {
start--;
}
var argNameSpan = adjustment.span.file
.location(start + 1)
.pointSpan()
.extendIfMatches('amount')
.extendIfMatches('degrees');
addPatch(Patch(argNameSpan, channel));
if (negate) {
addPatch(patchBefore(adjustment, '-'));
if (adjustment.needsParens) {
addPatch(patchBefore(adjustment, '('));
addPatch(patchAfter(adjustment, ')'));
}
}
if (space != null) {
addPatch(patchAfter(adjustment, ', \$space: $space'));
}
default:
warn(node.span.message('Cannot migrate unexpected arguments.'));
}
}
}

extension _NeedsParens on Expression {
/// Returns true if this expression needs parentheses when it's negated.
bool get needsParens => switch (this) {
BinaryOperationExpression() ||
UnaryOperationExpression() ||
FunctionExpression() =>
true,
_ => false,
};
}
2 changes: 2 additions & 0 deletions lib/src/runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import 'dart:isolate';
import 'package:args/args.dart';
import 'package:args/command_runner.dart';
import 'package:path/path.dart' as p;
import 'package:sass_migrator/src/migrators/color.dart';
import 'package:source_span/source_span.dart';
import 'package:term_glyph/term_glyph.dart' as glyph;

Expand Down Expand Up @@ -55,6 +56,7 @@ class MigratorRunner extends CommandRunner<Map<Uri, String>> {
..addFlag('version',
help: 'Print the version of the Sass migrator.', negatable: false);
addCommand(CalculationInterpolationMigrator());
addCommand(ColorMigrator());
addCommand(DivisionMigrator());
addCommand(ModuleMigrator());
addCommand(NamespaceMigrator());
Expand Down
4 changes: 2 additions & 2 deletions pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: sass_migrator
version: 2.0.3
version: 2.1.0
description: A tool for running migrations on Sass files
homepage: https://github.com/sass/migrator

Expand All @@ -17,7 +17,7 @@ dependencies:
node_interop: ^2.0.2
node_io: ^2.3.0
path: ^1.8.0
sass_api: ^9.2.7
sass_api: ^12.0.0
source_span: ^1.8.1
stack_trace: ^1.10.0
string_scanner: ^1.1.0
Expand Down
10 changes: 5 additions & 5 deletions test/migrators/calc_interpolation/calc_remove_interpolation.hrx
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ $d: 5;
.a { .b: calc($b * #{$c + 1}); }

// More than one interpolations
.a {
.b: calc($b - #{$c + 1} + #{$d});
.a {
.b: calc($b - #{$c + 1} + #{$d});
.c: calc(100% - #{$TABLE_TITLE + 2px});
}

Expand All @@ -35,9 +35,9 @@ $d: 5;
.a { .b: calc($b * ($c + 1)); }

// More than one interpolations
.a {
.b: calc($b - ($c + 1) + $d);
.c: calc(100% - ($TABLE-TITLE + 2px));
.a {
.b: calc($b - ($c + 1) + $d);
.c: calc(100% - ($TABLE_TITLE + 2px));
}

// Nested
Expand Down
39 changes: 39 additions & 0 deletions test/migrators/color/global.hrx
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<==> input/entrypoint.scss
a {
b: red(gold);
c: green(gold);
d: blue(gold);
e: hue(gold);
f: saturation(gold);
g: lightness(gold);
h: adjust-hue(gold, 20deg);
i: saturate(gold, 10%);
j: desaturate(gold, 10%);
k: transparentize(gold, 0.1);
l: fade-out(gold, 0.1);
m: opacify(gold, 0.1);
n: fade-in(gold, 0.1);
o: lighten(gold, 10%);
p: darken(gold, 10%);
}

<==> output/entrypoint.scss
@use "sass:color";

a {
b: color.channel(gold, 'red', rgb);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have an explicit $space: on these as well?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

c: color.channel(gold, 'green', rgb);
d: color.channel(gold, 'blue', rgb);
e: color.channel(gold, 'hue', hsl);
f: color.channel(gold, 'saturation', hsl);
g: color.channel(gold, 'lightness', hsl);
h: color.adjust(gold, $hue: 20deg, $space: hsl);
i: color.adjust(gold, $saturation: 10%, $space: hsl);
j: color.adjust(gold, $saturation: -10%, $space: hsl);
k: color.adjust(gold, $alpha: -0.1);
l: color.adjust(gold, $alpha: -0.1);
m: color.adjust(gold, $alpha: 0.1);
n: color.adjust(gold, $alpha: 0.1);
o: color.adjust(gold, $lightness: 10%, $space: hsl);
p: color.adjust(gold, $lightness: -10%, $space: hsl);
}
29 changes: 29 additions & 0 deletions test/migrators/color/module.hrx
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<==> input/entrypoint.scss
@use "sass:color";

a {
b: color.red(gold);
c: color.green(gold);
d: color.blue(gold);
e: color.hue(gold);
f: color.saturation(gold);
g: color.lightness(gold);
h: color.whiteness(gold);
i: color.blackness(gold);
j: color.alpha(gold);
}

<==> output/entrypoint.scss
@use "sass:color";

a {
b: color.channel(gold, 'red', rgb);
c: color.channel(gold, 'green', rgb);
d: color.channel(gold, 'blue', rgb);
e: color.channel(gold, 'hue', hsl);
f: color.channel(gold, 'saturation', hsl);
g: color.channel(gold, 'lightness', hsl);
h: color.channel(gold, 'whiteness', hwb);
i: color.channel(gold, 'blackness', hwb);
j: color.channel(gold, 'alpha');
}
19 changes: 19 additions & 0 deletions test/migrators/color/named_arguments.hrx
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<==> input/entrypoint.scss
a {
b: red($color: gold);
c: adjust-hue(gold, $degrees: 20deg);
d: saturate(gold, $amount: 10%);
e: desaturate($color: gold, $amount: 10%);
f: lighten($amount: 10%, $color: gold);
}

<==> output/entrypoint.scss
@use "sass:color";

a {
b: color.channel($color: gold, $channel: 'red', $space: rgb);
c: color.adjust(gold, $hue: 20deg, $space: hsl);
d: color.adjust(gold, $saturation: 10%, $space: hsl);
e: color.adjust($color: gold, $saturation: -10%, $space: hsl);
f: color.adjust($lightness: 10%, $space: hsl, $color: gold);
}
19 changes: 19 additions & 0 deletions test/migrators/color/namespace.hrx
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<==> input/entrypoint.scss
@use "sass:math" as color;

a {
b: red(gold);
c: green(gold);
d: blue(gold);
}

<==> output/entrypoint.scss
@use "sass:color" as sass-color;

@use "sass:math" as color;

a {
b: sass-color.channel(gold, 'red', rgb);
c: sass-color.channel(gold, 'green', rgb);
d: sass-color.channel(gold, 'blue', rgb);
}
21 changes: 21 additions & 0 deletions test/migrators/color/overrides.hrx
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<==> input/entrypoint.scss
@use "color";
@import "color";

@function red($color) {
@return 1;
}

a {
b: red(gold);
c: color.blue(gold);
d: blue(gold);
}

<==> input/color.scss
@function blue($color) {
@return 1;
}

<==> log.txt
Nothing to migrate!
1 change: 1 addition & 0 deletions test/migrators/migrator_dart_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import '../utils.dart';

main() {
testMigrator("calc_interpolation");
testMigrator("color");
testMigrator("division");
testMigrator("module");
testMigrator("namespace");
Expand Down
Loading