Skip to content

Commit

Permalink
Add --built-in-only flag to module migrator (#259)
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 authored Oct 11, 2024
1 parent c27792b commit c5f8ac8
Show file tree
Hide file tree
Showing 23 changed files with 340 additions and 42 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
## 2.2.0

### Module Migrator

* Add a new `--built-in-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.1.0

### Color Function 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
8 changes: 4 additions & 4 deletions lib/src/migrators/color.dart
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ class ColorMigrator extends Migrator {
Map<Uri, String> migrateFile(
ImportCache importCache, Stylesheet stylesheet, Importer importer) {
var references = References(importCache, stylesheet, importer);
var visitor =
_ColorMigrationVisitor(references, importCache, migrateDependencies);
var visitor = _ColorMigrationVisitor(references, importCache,
migrateDependencies: migrateDependencies);
var result = visitor.run(stylesheet, importer);
missingDependencies.addAll(visitor.missingDependencies);
return result;
Expand All @@ -37,8 +37,8 @@ final _colorUrl = Uri(scheme: 'sass', path: 'color');
class _ColorMigrationVisitor extends MigrationVisitor {
final References references;

_ColorMigrationVisitor(
this.references, super.importCache, super.migrateDependencies);
_ColorMigrationVisitor(this.references, super.importCache,
{required super.migrateDependencies});

/// The namespace of an existing `@use "sass:color"` rule in the current
/// file, if any.
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
65 changes: 49 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('built-in-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!['built-in-only'] as bool;
if (builtInOnly &&
(argResults!.wasParsed('forward') ||
argResults!.wasParsed('remove-prefix'))) {
throw MigrationException('--forward and --remove-prefix may not be '
'passed with --built-in-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,10 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
/// Adds a namespace to any function call that requires it.
@override
void visitFunctionExpression(FunctionExpression node) {
if (node.namespace != null) {
var source = references.sources[node];
if (node.namespace != null ||
source is UseSource ||
builtInOnly && source is! BuiltInSource) {
super.visitFunctionExpression(node);
return;
}
Expand Down Expand Up @@ -631,12 +648,15 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
// Warn for get-function calls without a static name.
var nameArg =
node.arguments.named['name'] ?? node.arguments.positional.first;
if (nameArg is! StringExpression || nameArg.text.asPlain == null) {
if ((nameArg is! StringExpression || nameArg.text.asPlain == null) &&
!builtInOnly) {
emitWarning(
"get-function call may require \$module parameter", nameArg.span);
return;
}

if (builtInOnly && declaration != null) return;

_patchNamespaceForFunction(node, declaration, (namespace) {
var beforeParen = node.span.end.offset - 1;
addPatch(Patch(node.span.file.span(beforeParen, beforeParen),
Expand Down Expand Up @@ -776,14 +796,21 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
}
}
if (ruleUrl != null) {
if (_useAllowed) {
if (builtInOnly) {
if (migrateDependencies) {
_upstreamStylesheets.add(currentUrl);
visitDependency(ruleUrl, import.span);
_upstreamStylesheets.remove(currentUrl);
}
} else if (_useAllowed) {
migratedRules.addAll(_migrateImportToRules(ruleUrl, import.span));
} else {
migratedRules.add(_migrateImportToLoadCss(ruleUrl, import.span)
.replaceAll('\n', '\n$indent'));
}
}
}
if (builtInOnly) return;

rulesText = migratedRules.join('$semicolon\n$indent');
if (rulesText.isEmpty) {
Expand Down Expand Up @@ -1071,6 +1098,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 +1117,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 +1147,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 +1174,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
12 changes: 9 additions & 3 deletions lib/src/migrators/module/references.dart
Original file line number Diff line number Diff line change
Expand Up @@ -383,13 +383,19 @@ class _ReferenceVisitor extends ScopedAstVisitor {
void visitUseRule(UseRule node) {
super.visitUseRule(node);
var namespace = node.namespace;
if (namespace == null) return;
if (node.url.scheme == 'sass') {
_namespaces[namespace] = node.url;
if (namespace != null) _namespaces[namespace] = node.url;
return;
}
var canonicalUrl = _loadUseOrForward(node.url, node);
_namespaces[namespace] = canonicalUrl;
if (namespace != null) {
_namespaces[namespace] = canonicalUrl;
} else {
var moduleScope = _moduleScopes[canonicalUrl]!;
currentScope.variables.addAll(moduleScope.variables);
currentScope.mixins.addAll(moduleScope.mixins);
currentScope.functions.addAll(moduleScope.functions);
}

// `_moduleSources[canonicalUrl]` is set in `_loadUseOrForward`.
var moduleSources = _moduleSources[canonicalUrl]!;
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
2 changes: 1 addition & 1 deletion pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: sass_migrator
version: 2.1.0
version: 2.2.0
description: A tool for running migrations on Sass files
homepage: https://github.com/sass/migrator

Expand Down
19 changes: 19 additions & 0 deletions test/migrators/module/built_in_functions/built_in_only/basic.hrx
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<==> arguments
--built-in-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);
}
Loading

0 comments on commit c5f8ac8

Please sign in to comment.