Skip to content

Commit

Permalink
Add --builtin-only flag to module migrator
Browse files Browse the repository at this point in the history
Also fixes some built-in functions that weren't being migrated.
  • Loading branch information
jathak committed Jul 12, 2024
1 parent dce67db commit faea6f5
Show file tree
Hide file tree
Showing 10 changed files with 96 additions and 36 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
## 2.1.0

### Module Migrator

* Add a new `--builtin-only` flag, which migrates global functions to their
`sass:` module equivalents, while leaving `@import` rules unchanged.

* Fix bug where some functions (`opacity`, `is-bracketed`, and
`selector-extend`) would not be migrated.

## 2.0.3

### Module Migrator
Expand Down
2 changes: 1 addition & 1 deletion lib/src/migration_visitor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ abstract class MigrationVisitor extends ScopedAstVisitor {
/// syntax, in which case this returns an empty string.
String get semicolon => currentUrl.path.endsWith('.sass') ? "" : ";";

MigrationVisitor(this.importCache, this.migrateDependencies);
MigrationVisitor(this.importCache, {required this.migrateDependencies});

/// Runs a new migration on [stylesheet] (and its dependencies, if
/// [migrateDependencies] is true) and returns a map of migrated contents.
Expand Down
9 changes: 4 additions & 5 deletions lib/src/migrators/calc_interpolation.dart
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,17 @@ class CalculationInterpolationMigrator extends Migrator {
@override
Map<Uri, String> migrateFile(
ImportCache importCache, Stylesheet stylesheet, Importer importer) {
var visitor =
_CalculationInterpolationVisitor(importCache, migrateDependencies);
var visitor = _CalculationInterpolationVisitor(importCache,
migrateDependencies: migrateDependencies);
var result = visitor.run(stylesheet, importer);
missingDependencies.addAll(visitor.missingDependencies);
return result;
}
}

class _CalculationInterpolationVisitor extends MigrationVisitor {
_CalculationInterpolationVisitor(
ImportCache importCache, bool migrateDependencies)
: super(importCache, migrateDependencies);
_CalculationInterpolationVisitor(super.importCache,
{required super.migrateDependencies});

@override
void visitFunctionExpression(FunctionExpression node) {
Expand Down
9 changes: 5 additions & 4 deletions lib/src/migrators/division.dart
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ More info: https://sass-lang.com/d/slash-div""";
Map<Uri, String> migrateFile(
ImportCache importCache, Stylesheet stylesheet, Importer importer) {
var visitor = _DivisionMigrationVisitor(
importCache, isPessimistic, useMultiplication, migrateDependencies);
importCache, isPessimistic, useMultiplication,
migrateDependencies: migrateDependencies);
var result = visitor.run(stylesheet, importer);
missingDependencies.addAll(visitor.missingDependencies);
return result;
Expand All @@ -80,9 +81,9 @@ class _DivisionMigrationVisitor extends MigrationVisitor {
final bool isPessimistic;
final bool useMultiplication;

_DivisionMigrationVisitor(ImportCache importCache, this.isPessimistic,
this.useMultiplication, bool migrateDependencies)
: super(importCache, migrateDependencies);
_DivisionMigrationVisitor(
super.importCache, this.isPessimistic, this.useMultiplication,
{required super.migrateDependencies});

/// True when division is allowed by the context the current node is in.
var _isDivisionAllowed = false;
Expand Down
58 changes: 42 additions & 16 deletions lib/src/migrators/module.dart
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ class ModuleMigrator extends Migrator {

@override
final argParser = ArgParser()
..addFlag('builtin-only',
help: 'Migrates global functions without migrating @import.')
..addMultiOption('remove-prefix',
abbr: 'p',
help: 'Removes PREFIX from all migrated member names.\n'
Expand Down Expand Up @@ -77,6 +79,13 @@ class ModuleMigrator extends Migrator {
Map<Uri, String> migrateFile(
ImportCache importCache, Stylesheet stylesheet, Importer importer) {
var forwards = {for (var arg in argResults!['forward']) ForwardType(arg)};
var builtInOnly = argResults!['builtin-only'] as bool;
if (builtInOnly &&
(argResults!.wasParsed('forward') ||
argResults!.wasParsed('remove-prefix'))) {
throw MigrationException('--forward and --remove-prefix may not be '
'passed with --builtin-only.');
}
if (forwards.contains(ForwardType.prefixed) &&
!argResults!.wasParsed('remove-prefix')) {
throw MigrationException(
Expand All @@ -85,8 +94,10 @@ class ModuleMigrator extends Migrator {
}

var references = References(importCache, stylesheet, importer);
var visitor = _ModuleMigrationVisitor(importCache, references,
globalResults!['load-path'] as List<String>, migrateDependencies,
var visitor = _ModuleMigrationVisitor(
importCache, references, globalResults!['load-path'] as List<String>,
migrateDependencies: migrateDependencies,
builtInOnly: builtInOnly,
prefixesToRemove: (argResults!['remove-prefix'] as List<String>)
.map((prefix) => prefix.replaceAll('_', '-')),
forwards: forwards);
Expand Down Expand Up @@ -186,9 +197,6 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
/// main migration pass is used.
final References references;

/// Cache used to load stylesheets.
final ImportCache importCache;

/// List of paths that stylesheets can be loaded from.
final List<String> loadPaths;

Expand All @@ -199,6 +207,9 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
/// The values of the --forward flag.
final Set<ForwardType> forwards;

/// Whether to migrate only global functions, leaving `@import` rules as-is.
final bool builtInOnly;

/// Constructs a new module migration visitor.
///
/// [importCache] must be the same one used by [references].
Expand All @@ -210,22 +221,25 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
/// the module migrator will filter out the dependencies' migration results.
///
/// This converts the OS-specific relative [loadPaths] to absolute URL paths.
_ModuleMigrationVisitor(this.importCache, this.references,
List<String> loadPaths, bool migrateDependencies,
{Iterable<String> prefixesToRemove = const [], this.forwards = const {}})
_ModuleMigrationVisitor(
super.importCache, this.references, List<String> loadPaths,
{required super.migrateDependencies,
required this.builtInOnly,
Iterable<String> prefixesToRemove = const [],
this.forwards = const {}})
: loadPaths = List.unmodifiable(
loadPaths.map((path) => p.toUri(p.absolute(path)).path)),
prefixesToRemove = UnmodifiableSetView(prefixesToRemove.toSet()),
super(importCache, migrateDependencies);
prefixesToRemove = UnmodifiableSetView(prefixesToRemove.toSet());

/// Checks which global declarations need to be renamed, then runs the
/// migrator.
@override
Map<Uri, String> run(Stylesheet stylesheet, Importer importer) {
references.globalDeclarations.forEach(_renameDeclaration);
if (!builtInOnly) references.globalDeclarations.forEach(_renameDeclaration);
var migrated = super.run(stylesheet, importer);

if (forwards.contains(ForwardType.importOnly) || _needsImportOnly) {
if (!builtInOnly &&
(forwards.contains(ForwardType.importOnly) || _needsImportOnly)) {
var url = stylesheet.span.sourceUrl!;
var importOnlyUrl = getImportOnlyUrl(url);
var results = _generateImportOnly(url, importOnlyUrl);
Expand Down Expand Up @@ -597,7 +611,8 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
/// Adds a namespace to any function call that requires it.
@override
void visitFunctionExpression(FunctionExpression node) {
if (node.namespace != null) {
if (node.namespace != null ||
(builtInOnly && references.sources[node] is! BuiltInSource)) {
super.visitFunctionExpression(node);
return;
}
Expand All @@ -613,7 +628,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
});
}

if (node.name == "get-function") {
if (node.name == "get-function" && !builtInOnly) {
var declaration = references.getFunctionReferences[node];
if (declaration != null) {
_unreferencable.check(declaration, node);
Expand Down Expand Up @@ -776,7 +791,12 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
}
}
if (ruleUrl != null) {
if (_useAllowed) {
if (builtInOnly) {
_upstreamStylesheets.add(currentUrl);
if (migrateDependencies) visitDependency(ruleUrl, import.span);
_upstreamStylesheets.remove(currentUrl);
return;
} else if (_useAllowed) {
migratedRules.addAll(_migrateImportToRules(ruleUrl, import.span));
} else {
migratedRules.add(_migrateImportToLoadCss(ruleUrl, import.span)
Expand Down Expand Up @@ -1071,6 +1091,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
_useAllowed = false;
super.visitIncludeRule(node);
if (node.namespace != null) return;
if (builtInOnly && references.sources[node] is! BuiltInSource) return;

var declaration = references.mixins[node];
if (declaration == null) {
Expand All @@ -1089,7 +1110,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
@override
void visitMixinRule(MixinRule node) {
_useAllowed = false;
_renameReference(nameSpan(node), MemberDeclaration(node));
if (!builtInOnly) _renameReference(nameSpan(node), MemberDeclaration(node));
super.visitMixinRule(node);
}

Expand Down Expand Up @@ -1119,6 +1140,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
@override
void visitVariableExpression(VariableExpression node) {
if (node.namespace != null) return;
if (builtInOnly && references.sources[node] is! BuiltInSource) return;
var declaration = references.variables[node];
if (declaration == null) {
// TODO(jathak): Error here as part of fixing #182.
Expand All @@ -1145,6 +1167,10 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
/// renaming or namespacing if necessary.
@override
void visitVariableDeclaration(VariableDeclaration node) {
if (builtInOnly) {
super.visitVariableDeclaration(node);
return;
}
var declaration = MemberDeclaration(node);
var defaultDeclaration =
references.defaultVariableDeclarations[declaration];
Expand Down
4 changes: 4 additions & 0 deletions lib/src/migrators/module/built_in_functions.dart
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const builtInFunctionModules = {
"complement": "color",
"invert": "color",
"alpha": "color",
"opacity": "color",
"opacify": "color",
"fade-in": "color",
"transparentize": "color",
Expand All @@ -43,6 +44,7 @@ const builtInFunctionModules = {
"is-superselector": "selector",
"simple-selectors": "selector",
"selector-parse": "selector",
"selector-extend": "selector",
"percentage": "math",
"round": "math",
"ceil": "math",
Expand All @@ -62,6 +64,7 @@ const builtInFunctionModules = {
"zip": "list",
"index": "list",
"list-separator": "list",
"is-bracketed": "list",
"feature-exists": "meta",
"variable-exists": "meta",
"global-variable-exists": "meta",
Expand Down Expand Up @@ -100,6 +103,7 @@ const builtInFunctionNameChanges = {
"selector-replace": "replace",
"selector-unify": "unify",
"selector-parse": "parse",
"selector-extend": "extend",
"unitless": "is-unitless",
"comparable": "compatible",
"list-separator": "separator",
Expand Down
10 changes: 5 additions & 5 deletions lib/src/migrators/namespace.dart
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,9 @@ class NamespaceMigrator extends Migrator {
var renamer = Renamer<UseRule>(argResults!['rename'].join('\n'),
{'': ((rule) => rule.namespace!), 'url': (rule) => rule.url.toString()},
sourceUrl: '--rename');
var visitor = _NamespaceMigrationVisitor(renamer,
argResults!['force'] as bool, importCache, migrateDependencies);
var visitor = _NamespaceMigrationVisitor(
renamer, argResults!['force'] as bool, importCache,
migrateDependencies: migrateDependencies);
var result = visitor.run(stylesheet, importer);
missingDependencies.addAll(visitor.missingDependencies);
return result;
Expand All @@ -62,9 +63,8 @@ class _NamespaceMigrationVisitor extends MigrationVisitor {
assertInStylesheet(__usedNamespaces, '_usedNamespaces');
Set<String>? __usedNamespaces;

_NamespaceMigrationVisitor(this.renamer, this.forceRename,
ImportCache importCache, bool migrateDependencies)
: super(importCache, migrateDependencies);
_NamespaceMigrationVisitor(this.renamer, this.forceRename, super.importCache,
{required super.migrateDependencies});

@override
void visitStylesheet(Stylesheet node) {
Expand Down
7 changes: 4 additions & 3 deletions lib/src/migrators/strict_unary.dart
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,17 @@ class StrictUnaryMigrator extends Migrator {
@override
Map<Uri, String> migrateFile(
ImportCache importCache, Stylesheet stylesheet, Importer importer) {
var visitor = _UnaryMigrationVisitor(importCache, migrateDependencies);
var visitor = _UnaryMigrationVisitor(importCache,
migrateDependencies: migrateDependencies);
var result = visitor.run(stylesheet, importer);
missingDependencies.addAll(visitor.missingDependencies);
return result;
}
}

class _UnaryMigrationVisitor extends MigrationVisitor {
_UnaryMigrationVisitor(ImportCache importCache, bool migrateDependencies)
: super(importCache, migrateDependencies);
_UnaryMigrationVisitor(super.importCache,
{required super.migrateDependencies});

@override
void visitBinaryOperationExpression(BinaryOperationExpression node) {
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: ^10.4.8
source_span: ^1.8.1
stack_trace: ^1.10.0
string_scanner: ^1.1.0
Expand Down
19 changes: 19 additions & 0 deletions test/migrators/module/built_in_functions/builtin_only.hrx
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<==> arguments
--builtin-only

<==> input/entrypoint.scss
@import "library";
a {
color: mix($red, $blue);
}

<==> input/_library.scss
$red: red;
$blue: blue;

<==> output/entrypoint.scss
@use "sass:color";
@import "library";
a {
color: color.mix($red, $blue);
}

0 comments on commit faea6f5

Please sign in to comment.