diff --git a/CHANGELOG.md b/CHANGELOG.md index cc87d15..88d72af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,21 +1,40 @@ -## 1.8.2 +## 2.0.0 + +* **Breaking change**: The `media-logic` migrator has been removed as the + [corresponding breaking change][media logic] has been completed in Dart Sass. + If you still need to migrate legacy code, use migrator version 1.8.1. + + [media logic]: https://sass-lang.com/documentation/breaking-changes/media-logic/ + +* Update to be compatible with the latest version of the Dart Sass AST. ### Calc Functions Interpolation Migrator -* Add parentheses in place of interpolation when necessary to preserve the evaluation order. +* Add parentheses in place of interpolation when necessary to preserve the + evaluation order. + +### Division Migrator + +* `/` division should now be left untouched in all CSS calculation functions. + This was already the case for `calc`, `clamp`, `min`, and `max`, but it now + applies to the new functions that Dart Sass 1.67.0 added support for. ## 1.8.1 ### Calc Functions Interpolation Migrator -* Migration for more than one interpolation or expressions in a calc function parameter. +* Migration for more than one interpolation or expressions in a calc function + parameter. ## 1.8.0 ### Calc Functions Interpolation Migrator -* Removes interpolation in calculation functions `calc()`, `clamp()`, `min()`, and `max()`. - See the [scss/function-calculation-no-interpolation](https://github.com/stylelint-scss/stylelint-scss/tree/master/src/rules/function-calculation-no-interpolation) rule for more information. +* Removes interpolation in calculation functions `calc()`, `clamp()`, `min()`, + and `max()`. See the [scss/function-calculation-no-interpolation] rule for + more information. + + [scss/function-calculation-no-interpolation]: https://github.com/stylelint-scss/stylelint-scss/tree/master/src/rules/function-calculation-no-interpolation ## 1.7.3 diff --git a/lib/src/migration_visitor.dart b/lib/src/migration_visitor.dart index c90f6e2..d13de19 100644 --- a/lib/src/migration_visitor.dart +++ b/lib/src/migration_visitor.dart @@ -13,6 +13,8 @@ import 'package:source_span/source_span.dart'; import 'exception.dart'; import 'patch.dart'; +import 'util/scope.dart'; +import 'util/scoped_ast_visitor.dart'; /// A visitor that migrates a stylesheet. /// @@ -24,8 +26,7 @@ import 'patch.dart'; /// If [migrateDependencies] is enabled, this visitor will construct and run a /// new instance of itself (using [newInstance]) each time it encounters an /// `@import` or `@use` rule. -abstract class MigrationVisitor - with RecursiveStatementVisitor, RecursiveAstVisitor { +abstract class MigrationVisitor extends ScopedAstVisitor { /// A mapping from URLs to migrated contents for stylesheets already migrated. final _migrated = {}; @@ -117,26 +118,31 @@ abstract class MigrationVisitor /// Visits the stylesheet at [dependency], resolved based on the current /// stylesheet's URL and importer. + /// + /// When [forImport] is true, this preserves the [currentScope]. Otherwise, + /// the dependency is visited with a new global scope for the new module. @protected void visitDependency(Uri dependency, FileSpan context, {bool forImport = false}) { if (dependency.scheme == 'sass') return; var result = importCache.import(dependency, baseImporter: _importer, baseUrl: _currentUrl, forImport: forImport); - if (result != null) { + if (result case (var newImporter, var stylesheet)) { // If [dependency] comes from a non-relative import, don't migrate it, // because it's likely to be outside the user's repository and may even be // authored by a different person. // // TODO(nweiz): Add a flag to override this behavior for load paths // (#104). - if (result.item1 != _importer) return; + if (newImporter != _importer) return; var oldImporter = _importer; - _importer = result.item1; - var stylesheet = result.item2; + var oldScope = currentScope; + _importer = newImporter; + if (!forImport) currentScope = Scope(); visitStylesheet(stylesheet); _importer = oldImporter; + currentScope = oldScope; } else { _missingDependencies.putIfAbsent( context.sourceUrl!.resolveUri(dependency), () => context); diff --git a/lib/src/migrator.dart b/lib/src/migrator.dart index 83092d2..d46addc 100644 --- a/lib/src/migrator.dart +++ b/lib/src/migrator.dart @@ -93,7 +93,7 @@ abstract class Migrator extends Command> { throw MigrationException("Could not find Sass file at '$entrypoint'."); } - var migrated = migrateFile(importCache, tuple.item2, tuple.item1); + var migrated = migrateFile(importCache, tuple.$2, tuple.$1); migrated.forEach((file, contents) { if (allMigrated.containsKey(file) && contents != allMigrated[file]) { throw MigrationException( diff --git a/lib/src/migrators/calc_interpolation.dart b/lib/src/migrators/calc_interpolation.dart index db8d991..b570343 100644 --- a/lib/src/migrators/calc_interpolation.dart +++ b/lib/src/migrators/calc_interpolation.dart @@ -33,12 +33,12 @@ class _CalculationInterpolationVisitor extends MigrationVisitor { : super(importCache, migrateDependencies); @override - void visitCalculationExpression(CalculationExpression node) { + void visitFunctionExpression(FunctionExpression node) { const calcFunctions = ['calc', 'clamp', 'min', 'max']; final interpolation = RegExp(r'\#{\s*[^}]+\s*}'); final hasOperation = RegExp(r'[-+*/]+'); if (calcFunctions.contains(node.name)) { - for (var arg in node.arguments) { + for (var arg in node.arguments.positional) { var newArg = arg.toString(); for (var match in interpolation.allMatches(arg.toString())) { var noInterpolation = @@ -58,6 +58,6 @@ class _CalculationInterpolationVisitor extends MigrationVisitor { } } } - super.visitCalculationExpression(node); + super.visitFunctionExpression(node); } } diff --git a/lib/src/migrators/division.dart b/lib/src/migrators/division.dart index abedbca..39827e8 100644 --- a/lib/src/migrators/division.dart +++ b/lib/src/migrators/division.dart @@ -10,8 +10,35 @@ import 'package:sass_api/sass_api.dart'; import '../migration_visitor.dart'; import '../migrator.dart'; import '../patch.dart'; +import '../util/scope.dart'; import '../utils.dart'; +/// The CSS calculation functions where slash-division is allowed (unless +/// there's a user-defined function with the same name). +const _calcFunctions = { + 'calc', + 'clamp', + 'min', + 'max', + 'round', + 'mod', + 'rem', + 'sin', + 'cos', + 'tan', + 'asin', + 'acos', + 'atan', + 'atan2', + 'pow', + 'sqrt', + 'hypot', + 'log', + 'exp', + 'abs', + 'sign', +}; + /// Migrates stylesheets that use the `/` operator for division to use the /// `divide` function instead. class DivisionMigrator extends Migrator { @@ -81,13 +108,16 @@ class _DivisionMigrationVisitor extends MigrationVisitor { void visitStylesheet(Stylesheet node) { var oldNamespaces = __existingNamespaces; var oldUseRules = __useRulesToInsert; + var oldScope = currentScope; __existingNamespaces = { for (var rule in node.uses) rule.url: rule.namespace }; __useRulesToInsert = []; + currentScope = Scope(); super.visitStylesheet(node); __existingNamespaces = oldNamespaces; __useRulesToInsert = oldUseRules; + currentScope = oldScope; } /// Inserts [_useRulesToInsert] before the first existing dependency (or at @@ -136,9 +166,10 @@ class _DivisionMigrationVisitor extends MigrationVisitor { /// Allows division within this argument invocation. @override - void visitArgumentInvocation(ArgumentInvocation invocation) { + void visitArgumentInvocation(ArgumentInvocation invocation, + {bool inCalcContext = false}) { _withContext(() => super.visitArgumentInvocation(invocation), - isDivisionAllowed: true, inCalcContext: false); + isDivisionAllowed: true, inCalcContext: inCalcContext); } /// If this is a division operation, migrates it. @@ -157,20 +188,15 @@ 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. + /// for calc functions and new-syntax color functions. @override void visitFunctionExpression(FunctionExpression node) { if (_tryColorFunction(node)) return; - visitArgumentInvocation(node.arguments); + var validCalcs = _calcFunctions.difference(currentScope.allFunctionNames); + visitArgumentInvocation(node.arguments, + inCalcContext: + node.namespace == null && validCalcs.contains(node.name)); } /// Visits interpolation with [_isDivisionAllowed] and [_inCalcContext] set @@ -198,9 +224,9 @@ class _DivisionMigrationVisitor extends MigrationVisitor { void visitParenthesizedExpression(ParenthesizedExpression node, {bool negated = false}) { _withContext(() { - var expression = node.expression; - if (expression is BinaryOperationExpression && - expression.operator == BinaryOperator.dividedBy) { + if (node.expression + case BinaryOperationExpression(operator: BinaryOperator.dividedBy) && + var expression) { if (_visitSlashOperation(expression) && !negated) { addPatch(patchDelete(node.span, end: 1)); addPatch(patchDelete(node.span, start: node.span.length - 1)); @@ -215,9 +241,11 @@ class _DivisionMigrationVisitor extends MigrationVisitor { /// parenthesized expression. @override void visitUnaryOperationExpression(UnaryOperationExpression node) { - var operand = node.operand; - if (node.operator == UnaryOperator.minus && - operand is ParenthesizedExpression) { + if (node + case UnaryOperationExpression( + operator: UnaryOperator.minus, + :ParenthesizedExpression operand + )) { visitParenthesizedExpression(operand, negated: true); return; } @@ -244,40 +272,48 @@ class _DivisionMigrationVisitor extends MigrationVisitor { return false; } - ListExpression? channels; - if (node.arguments.positional.length == 1 && - node.arguments.named.isEmpty && - node.arguments.positional.first is ListExpression) { - channels = node.arguments.positional.first as ListExpression?; - } else if (node.arguments.positional.isEmpty && - node.arguments.named.containsKey(r'$channels') && - node.arguments.named.length == 1 && - node.arguments.named[r'$channels'] is ListExpression) { - channels = node.arguments.named[r'$channels'] as ListExpression?; - } - if (channels == null || - channels.hasBrackets || - channels.separator != ListSeparator.space || - channels.contents.length != 3 || - channels.contents.last is! BinaryOperationExpression) { - return false; - } + var channels = switch (node.arguments) { + ArgumentInvocation( + positional: [ListExpression arg], + named: Map(isEmpty: true) + ) => + arg, + ArgumentInvocation( + positional: [], + named: {r'$channels': ListExpression arg} + ) => + arg, + _ => null, + }; - var last = channels.contents.last as BinaryOperationExpression; - if (last.left is! NumberExpression || last.right is! NumberExpression) { - // Handles cases like `rgb(10 20 30/2 / 0.5)`, since converting `30/2` to - // `divide(30, 20)` would cause `/ 0.5` to be interpreted as division. - _patchSpacesToCommas(channels); - _patchOperatorToComma(last); + if (channels + case ListExpression( + hasBrackets: false, + separator: ListSeparator.space, + contents: [_, _, BinaryOperationExpression last] + )) { + // Handles cases like `rgb(10 20 30/2 / 0.5)`, since converting `30/2` + // to `div(30, 20)` would cause `/ 0.5` to be interpreted as division. + if (last + case BinaryOperationExpression( + left: NumberExpression(), + right: NumberExpression() + )) { + // Nothing to patch + } else { + _patchSpacesToCommas(channels); + _patchOperatorToComma(last); + } + _withContext(() { + // Non-null assertion is required because of dart-lang/language#1536. + channels.contents[0].accept(this); + channels.contents[1].accept(this); + last.left.accept(this); + }, isDivisionAllowed: true); + last.right.accept(this); + return true; } - _withContext(() { - // Non-null assertion is required because of dart-lang/language#1536. - channels!.contents[0].accept(this); - channels.contents[1].accept(this); - last.left.accept(this); - }, isDivisionAllowed: true); - last.right.accept(this); - return true; + return false; } /// Visits a `/` operation [node] and migrates it to either the `division` @@ -291,9 +327,10 @@ class _DivisionMigrationVisitor extends MigrationVisitor { node.right.accept(this); return false; } + var status = _NumberStatus.of(node); if ((!_isDivisionAllowed && _onlySlash(node)) || - _isDefinitelyNotNumber(node)) { + status == _NumberStatus.no) { // Definitely not division if (_isDivisionAllowed || _containsInterpolation(node)) { // We only want to convert a non-division slash operation to a @@ -305,7 +342,9 @@ class _DivisionMigrationVisitor extends MigrationVisitor { } return true; } - if (_expectsNumericResult || _isDefinitelyNumber(node) || !isPessimistic) { + if (_expectsNumericResult || + status == _NumberStatus.yes || + !isPessimistic) { // Definitely division _withContext(() => super.visitBinaryOperationExpression(node), expectsNumericResult: true); @@ -330,111 +369,74 @@ class _DivisionMigrationVisitor extends MigrationVisitor { /// 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; + if (node.right case NumberExpression(unit: null, value: var divisor) + when _allowedDivisors.contains(divisor)) { + var operatorSpan = node.left.span + .extendThroughWhitespace() + .end + .pointSpan() + .extendIfMatches('/'); + addPatch(Patch(operatorSpan, '*')); + addPatch(Patch(node.right.span, '${1 / divisor}')); + return true; + } + return false; } /// Visits the arguments of a `/` operation that is being converted into a /// call to `slash-list`, converting slashes to commas and removing /// unnecessary interpolation. void _visitSlashListArguments(Expression node) { - if (node is BinaryOperationExpression && - node.operator == BinaryOperator.dividedBy) { - _visitSlashListArguments(node.left); - _patchOperatorToComma(node); - _visitSlashListArguments(node.right); - } else if (node is StringExpression && - node.text.contents.length == 1 && - node.text.contents.first is Expression) { - // Remove `#{` and `}` - addPatch(patchDelete(node.span, end: 2)); - addPatch(patchDelete(node.span, start: node.span.length - 1)); - (node.text.contents.first as Expression).accept(this); - } else { - node.accept(this); + switch (node) { + case BinaryOperationExpression(operator: BinaryOperator.dividedBy): + _visitSlashListArguments(node.left); + _patchOperatorToComma(node); + _visitSlashListArguments(node.right); + case StringExpression(text: Interpolation(contents: [Expression item])): + // Remove `#{` and `}` + addPatch(patchDelete(node.span, end: 2)); + addPatch(patchDelete(node.span, start: node.span.length - 1)); + item.accept(this); + default: + node.accept(this); } } - /// Returns true if we assume that [operator] always returns a number. - /// - /// This is true for `*` and `%`. - bool _returnsNumbers(BinaryOperator operator) => - operator == BinaryOperator.times || operator == BinaryOperator.modulo; - /// Returns true if we assume that [operator] always operators on numbers. /// /// This is true for `*`, `%`, `<`, `<=`, `>`, and `>=`. - bool _operatesOnNumbers(BinaryOperator operator) => - _returnsNumbers(operator) || - operator == BinaryOperator.lessThan || - operator == BinaryOperator.lessThanOrEquals || - operator == BinaryOperator.greaterThan || - operator == BinaryOperator.greaterThanOrEquals; + bool _operatesOnNumbers(BinaryOperator operator) => { + BinaryOperator.times, + BinaryOperator.modulo, + BinaryOperator.lessThan, + BinaryOperator.lessThanOrEquals, + BinaryOperator.greaterThan, + BinaryOperator.greaterThanOrEquals + }.contains(operator); /// Returns true if [node] is entirely composed of number literals and slash /// operations. - bool _onlySlash(Expression node) { - if (node is NumberExpression) return true; - if (node is BinaryOperationExpression) { - return node.operator == BinaryOperator.dividedBy && - _onlySlash(node.left) && - _onlySlash(node.right); - } - return false; - } - - /// Returns true if [node] is known to always evaluate to a number. - bool _isDefinitelyNumber(Expression node) { - if (node is NumberExpression) return true; - if (node is ParenthesizedExpression) { - return _isDefinitelyNumber(node.expression); - } else if (node is UnaryOperationExpression) { - return _isDefinitelyNumber(node.operand); - } else if (node is BinaryOperationExpression) { - return _returnsNumbers(node.operator) || - (_isDefinitelyNumber(node.left) && _isDefinitelyNumber(node.right)); - } - return false; - } - - /// Returns true if [node] contains a subexpression known to not be a number. - bool _isDefinitelyNotNumber(Expression node) { - if (node is ParenthesizedExpression) { - return _isDefinitelyNotNumber(node.expression); - } - if (node is BinaryOperationExpression) { - return _isDefinitelyNotNumber(node.left) || - _isDefinitelyNotNumber(node.right); - } - return node is BooleanExpression || - node is ColorExpression || - node is ListExpression || - node is MapExpression || - node is NullExpression || - node is StringExpression; - } + bool _onlySlash(Expression node) => switch (node) { + NumberExpression() => true, + BinaryOperationExpression( + operator: BinaryOperator.dividedBy, + :var left, + :var right + ) => + _onlySlash(left) && _onlySlash(right), + _ => false + }; /// Returns true if [node] contains an interpolation. - bool _containsInterpolation(Expression node) { - if (node is ParenthesizedExpression) { - return _containsInterpolation(node.expression); - } - if (node is BinaryOperationExpression) { - return _containsInterpolation(node.left) || - _containsInterpolation(node.right); - } - return node is StringExpression && node.text.asPlain == null; - } + bool _containsInterpolation(Expression node) => switch (node) { + ParenthesizedExpression(:var expression) || + UnaryOperationExpression(operand: var expression) => + _containsInterpolation(expression), + BinaryOperationExpression(:var left, :var right) => + _containsInterpolation(left) || _containsInterpolation(right), + StringExpression(text: Interpolation(asPlain: null)) => true, + _ => false + }; /// Converts a space-separated list [node] to a comma-separated list. void _patchSpacesToCommas(ListExpression node) { @@ -455,14 +457,17 @@ class _DivisionMigrationVisitor extends MigrationVisitor { /// Adds patches removing unnecessary parentheses around [node] if it is a /// ParenthesizedExpression. void _patchParensIfAny(SassNode node) { - if (node is! ParenthesizedExpression) return; - var expression = node.expression; - if (expression is BinaryOperationExpression && - expression.operator == BinaryOperator.dividedBy) { - return; + switch (node) { + case ParenthesizedExpression( + expression: BinaryOperationExpression( + operator: BinaryOperator.dividedBy + ) + ): + return; + case ParenthesizedExpression(): + addPatch(patchDelete(node.span, end: 1)); + addPatch(patchDelete(node.span, start: node.span.length - 1)); } - addPatch(patchDelete(node.span, end: 1)); - addPatch(patchDelete(node.span, start: node.span.length - 1)); } /// Runs [operation] with the given context. @@ -482,3 +487,40 @@ class _DivisionMigrationVisitor extends MigrationVisitor { _inCalcContext = previousCalcContext; } } + +/// Represents whether an expression is definitely a number, definitely not a +/// number, or unknown. +enum _NumberStatus { + yes, + no, + maybe; + + /// Returns [yes] if [node] is definitely a number, [no] if [node] is + /// definitely not a number, and [maybe] otherwise. + static _NumberStatus of(Expression node) => switch (node) { + NumberExpression() || + BinaryOperationExpression( + operator: BinaryOperator.times || BinaryOperator.modulo + ) => + yes, + BooleanExpression() || + ColorExpression() || + ListExpression() || + MapExpression() || + NullExpression() || + StringExpression() => + no, + ParenthesizedExpression(:var expression) || + UnaryOperationExpression(operand: var expression) => + of(expression), + BinaryOperationExpression(:var left, :var right) => switch (( + of(left), + of(right) + )) { + (yes, yes) => yes, + (no, _) || (_, no) => no, + _ => maybe + }, + _ => maybe, + }; +} diff --git a/lib/src/migrators/media_logic.dart b/lib/src/migrators/media_logic.dart deleted file mode 100644 index d072713..0000000 --- a/lib/src/migrators/media_logic.dart +++ /dev/null @@ -1,57 +0,0 @@ -// Copyright 2022 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:source_span/source_span.dart'; - -import '../migration_visitor.dart'; -import '../migrator.dart'; -import '../patch.dart'; - -/// Migrates deprecated `@media` query syntax to use interpolation. -class MediaLogicMigrator extends Migrator { - final name = 'media-logic'; - final description = r'Migrates deprecated `@media` query syntax.\n' - 'See https://sass-lang.com/d/media-logic.'; - - /// For each stylesheet URL, the set of relevant spans that require migration. - final _expressionsToMigrate = >{}; - - @override - void handleDeprecation(String message, FileSpan? span) { - if (span == null) return; - if (!message.startsWith('Starting a @media query with ')) return; - _expressionsToMigrate.putIfAbsent(span.sourceUrl!, () => {}).add(span); - } - - @override - Map migrateFile( - ImportCache importCache, Stylesheet stylesheet, Importer importer) { - var visitor = _MediaLogicVisitor( - importCache, migrateDependencies, _expressionsToMigrate); - var result = visitor.run(stylesheet, importer); - missingDependencies.addAll(visitor.missingDependencies); - return result; - } -} - -class _MediaLogicVisitor extends MigrationVisitor { - /// For each stylesheet URL, the set of relevant spans that require migration. - final Map> _expressionsToMigrate; - - _MediaLogicVisitor(ImportCache importCache, bool migrateDependencies, - this._expressionsToMigrate) - : super(importCache, migrateDependencies); - - @override - void beforePatch(Stylesheet node) { - var expressions = _expressionsToMigrate[node.span.sourceUrl] ?? {}; - for (var expression in expressions) { - addPatch(Patch.insert(expression.start, '#{')); - addPatch(Patch.insert(expression.end, '}')); - } - } -} diff --git a/lib/src/migrators/module.dart b/lib/src/migrators/module.dart index 76d1513..e1afbf8 100644 --- a/lib/src/migrators/module.dart +++ b/lib/src/migrators/module.dart @@ -9,18 +9,17 @@ import 'package:collection/collection.dart'; import 'package:path/path.dart' as p; import 'package:sass_api/sass_api.dart'; import 'package:source_span/source_span.dart'; -import 'package:tuple/tuple.dart'; import '../exception.dart'; import '../migration_visitor.dart'; import '../migrator.dart'; import '../patch.dart'; import '../utils.dart'; +import '../util/member_declaration.dart'; import '../util/node_modules_importer.dart'; import 'module/built_in_functions.dart'; import 'module/forward_type.dart'; -import 'module/member_declaration.dart'; import 'module/reference_source.dart'; import 'module/references.dart'; import 'module/unreferencable_members.dart'; @@ -117,7 +116,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor { /// Maps canonical URLs to the original URL and importer from the `@import` /// rule that last imported that URL. - final _originalImports = >{}; + final _originalImports = {}; /// Tracks members that are unreferencable in the current scope. var _unreferencable = UnreferencableMembers(); @@ -310,24 +309,20 @@ class _ModuleMigrationVisitor extends MigrationVisitor { // If entrypoint exposes no members, it should still be forwarded to ensure // that the import-only file still includes its CSS. - var dependency = - _absoluteUrlToDependency(entrypoint, relativeTo: importOnlyUrl).item1; + var (dependency, _) = + _absoluteUrlToDependency(entrypoint, relativeTo: importOnlyUrl); var forwards = forwardsByUrl.remove(entrypoint); var entrypointForwards = forwards != null ? _forwardRulesForShown(entrypoint, '"$dependency"', forwards, {}) : ['@forward "$dependency"']; - var tuples = [ - for (var entry in forwardsByUrl.entries) - Tuple3( - entry.key, - _absoluteUrlToDependency(entry.key, relativeTo: importOnlyUrl) - .item1, - entry.value) - ]; var forwardLines = [ - for (var tuple in tuples) - ..._forwardRulesForShown(tuple.item1, '"${tuple.item2}"', tuple.item3, - hiddenByUrl[tuple.item1] ?? {}), + for (var MapEntry(key: url, value: shownByPrefix) + in forwardsByUrl.entries) + ..._forwardRulesForShown( + url, + '"${_absoluteUrlToDependency(url, relativeTo: importOnlyUrl).$1}"', + shownByPrefix, + hiddenByUrl[url] ?? {}), ...entrypointForwards ]; var semicolon = entrypoint.path.endsWith('.sass') ? '' : ';'; @@ -378,10 +373,9 @@ class _ModuleMigrationVisitor extends MigrationVisitor { .map((declaration) => declaration.sourceUrl) .toSet()) { if (url == currentUrl || _forwardedUrls.contains(url)) continue; - var forwards = - _makeForwardRules(url, '"${_absoluteUrlToDependency(url).item1}"'); + var (ruleUrl, isRelative) = _absoluteUrlToDependency(url); + var forwards = _makeForwardRules(url, '"$ruleUrl"'); if (forwards == null) continue; - var isRelative = _absoluteUrlToDependency(url).item2; (isRelative ? relativeForwards : loadPathForwards) .addAll([for (var rule in forwards) '$rule$semicolon\n']); } @@ -407,7 +401,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor { importCache .canonicalize(rule.url, baseImporter: importer, baseUrl: node.span.sourceUrl) - ?.item2 ?? + ?.$2 ?? rule.url: rule.namespace }; _determineNamespaces(node.span.sourceUrl!, _namespaces); @@ -511,7 +505,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor { var ruleUrlsForSources = { for (var source in sources.whereType()) source: source.originalRuleUrl ?? - _absoluteUrlToDependency(source.url, relativeTo: currentUrl).item1 + _absoluteUrlToDependency(source.url, relativeTo: currentUrl).$1 }; // Then handle `@import` rules, in order of path segment count. for (var sources in _orderSources(ruleUrlsForSources)) { @@ -593,10 +587,10 @@ class _ModuleMigrationVisitor extends MigrationVisitor { /// Visits [children] with a new scope for tracking unreferencable members. @override - void visitChildren(List children) { + void visitChildren(List children, {bool withScope = true}) { var oldUnreferencable = _unreferencable; _unreferencable = UnreferencableMembers(_unreferencable); - super.visitChildren(children); + super.visitChildren(children, withScope: withScope); _unreferencable = oldUnreferencable; } @@ -753,10 +747,8 @@ class _ModuleMigrationVisitor extends MigrationVisitor { /// `meta.load-css`, or static `@import` rules @override void visitImportRule(ImportRule node) { - var imports = + var (staticImports, dynamicImports) = partitionOnType(node.imports); - var staticImports = imports.item1; - var dynamicImports = imports.item2; if (dynamicImports.isEmpty) { _useAllowed = false; return; @@ -769,22 +761,20 @@ class _ModuleMigrationVisitor extends MigrationVisitor { for (var import in dynamicImports) { Uri? ruleUrl = import.url; - var tuple = importCache.canonicalize(ruleUrl, - baseImporter: importer, baseUrl: currentUrl, forImport: true); - var canonicalImport = tuple?.item2; - if (canonicalImport != null && - references.orphanImportOnlyFiles.containsKey(canonicalImport)) { + if (importCache.canonicalize(ruleUrl, + baseImporter: importer, baseUrl: currentUrl, forImport: true) + case (var newImporter, var canonicalImport, originalUrl: _)? + when references.orphanImportOnlyFiles.containsKey(canonicalImport)) { ruleUrl = null; - var url = references.orphanImportOnlyFiles[canonicalImport]?.url; - if (url != null && tuple != null) { - var canonicalRedirect = importCache - .canonicalize(url, - baseImporter: tuple.item1, baseUrl: canonicalImport)! - .item2; - ruleUrl = _absoluteUrlToDependency(canonicalRedirect).item1; + if (references.orphanImportOnlyFiles[canonicalImport] + case ForwardRule(:var url)) { + if (importCache.canonicalize(url, + baseImporter: newImporter, baseUrl: canonicalImport) + case (_, var canonicalRedirect, originalUrl: _)?) { + (ruleUrl, _) = _absoluteUrlToDependency(canonicalRedirect); + } } } - if (ruleUrl != null) { if (_useAllowed) { migratedRules.addAll(_migrateImportToRules(ruleUrl, import.span)); @@ -829,10 +819,8 @@ class _ModuleMigrationVisitor extends MigrationVisitor { /// corresponding regular file. This allows imports of import-only files that /// redirect to a different path to be migrated in-place. List _migrateImportToRules(Uri ruleUrl, FileSpan context) { - var tuple = _migrateImportCommon(ruleUrl, context); - var canonicalUrl = tuple.item1; - var config = tuple.item2; - var forwardForConfig = tuple.item3; + var (canonicalUrl, config, forwardForConfig) = + _migrateImportCommon(ruleUrl, context); var asClause = ''; var defaultNamespace = namespaceForPath(ruleUrl.path); @@ -879,10 +867,9 @@ class _ModuleMigrationVisitor extends MigrationVisitor { _unreferencable.add(declaration, UnreferencableType.fromImporter); } - var tuple = _migrateImportCommon(ruleUrl, context); - var canonicalUrl = tuple.item1; - var config = tuple.item2; - if (tuple.item3 != null) { + var (canonicalUrl, config, forwardForConfig) = + _migrateImportCommon(ruleUrl, context); + if (forwardForConfig != null) { throw MigrationSourceSpanException( "This declaration attempts to override a default value in an " "indirect, nested import of ${p.prettyUri(canonicalUrl)}, which is " @@ -912,8 +899,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor { /// `meta.load-css`. The third is a string of variables that should be added /// to a `show` clause of a `@forward` rule so that they can be configured by /// an upstream file. - Tuple3 _migrateImportCommon( - Uri ruleUrl, FileSpan context) { + (Uri, String?, String?) _migrateImportCommon(Uri ruleUrl, FileSpan context) { var oldConfiguredVariables = __configuredVariables; __configuredVariables = {}; _upstreamStylesheets.add(currentUrl); @@ -930,9 +916,8 @@ class _ModuleMigrationVisitor extends MigrationVisitor { // Associate the importer for this URL with the resolved URL so that we can // re-use this import URL later on. - var canonicalUrl = tuple.item2; - _originalImports.putIfAbsent( - canonicalUrl, () => Tuple2(ruleUrl, tuple.item1)); + var canonicalUrl = tuple.$2; + _originalImports.putIfAbsent(canonicalUrl, () => (ruleUrl, tuple.$1)); // Pass the variables that were configured by the importing file to `with`, // and forward the rest and add them to `oldConfiguredVariables` because @@ -990,7 +975,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor { } else if (configured.isNotEmpty) { normalConfig = "(\n " + configured.join(',\n ') + "\n)"; } - return Tuple3(canonicalUrl, normalConfig, extraForward); + return (canonicalUrl, normalConfig, extraForward); } /// If [url] contains any member declarations that should be forwarded from @@ -1115,7 +1100,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor { void visitUseRule(UseRule node) { _usedUrls.add(importCache .canonicalize(node.url, baseImporter: importer, baseUrl: currentUrl) - ?.item2 ?? + ?.$2 ?? node.url); } @@ -1126,7 +1111,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor { void visitForwardRule(ForwardRule node) { _forwardedUrls.add(importCache .canonicalize(node.url, baseImporter: importer, baseUrl: currentUrl) - ?.item2 ?? + ?.$2 ?? node.url); } @@ -1249,8 +1234,8 @@ class _ModuleMigrationVisitor extends MigrationVisitor { } if (!_usedUrls.contains(url)) { // Add new `@use` rule for indirect dependency - var tuple = _absoluteUrlToDependency(url); - var defaultNamespace = namespaceForPath(tuple.item1.path); + var (dependency, isRelative) = _absoluteUrlToDependency(url); + var defaultNamespace = namespaceForPath(dependency.path); // There are a few edge cases where the reference in [declaration] wasn't // tracked by [references.sources], so we add a namespace with simple // conflict resolution if one for this URL doesn't already exist. @@ -1259,8 +1244,8 @@ class _ModuleMigrationVisitor extends MigrationVisitor { var namespace = _namespaces[url]; var asClause = defaultNamespace == namespace ? '' : ' as $namespace'; _usedUrls.add(url); - (tuple.item2 ? _additionalRelativeUseRules : _additionalLoadPathUseRules) - .add('@use "${tuple.item1}"$asClause'); + (isRelative ? _additionalRelativeUseRules : _additionalLoadPathUseRules) + .add('@use "$dependency"$asClause'); } return _namespaces[url]; } @@ -1272,11 +1257,10 @@ class _ModuleMigrationVisitor extends MigrationVisitor { /// The first item of the returned tuple is the dependency, the second item /// is true when this dependency is resolved relative to the current URL and /// false when it's resolved relative to a load path. - Tuple2 _absoluteUrlToDependency(Uri url, {Uri? relativeTo}) { + (Uri, bool) _absoluteUrlToDependency(Uri url, {Uri? relativeTo}) { relativeTo ??= currentUrl; - var tuple = _originalImports[url]; - if (tuple != null && tuple.item2 is NodeModulesImporter) { - return Tuple2(tuple.item1, false); + if (_originalImports[url] case (var url, NodeModulesImporter _)) { + return (url, false); } var basename = p.url.basenameWithoutExtension(url.path); @@ -1298,11 +1282,12 @@ class _ModuleMigrationVisitor extends MigrationVisitor { ]; var relativePath = minBy(potentialUrls, (url) => url.length)!; var isRelative = relativePath == potentialUrls.first; - return Tuple2( - Uri( - path: p.url - .relative(p.url.join(p.url.dirname(relativePath), basename))), - isRelative); + return ( + Uri( + path: p.url + .relative(p.url.join(p.url.dirname(relativePath), basename))), + isRelative + ); } /// Returns the longest prefix in [prefixesToRemove] such that [identifier] diff --git a/lib/src/migrators/module/references.dart b/lib/src/migrators/module/references.dart index 584b90f..328180e 100644 --- a/lib/src/migrators/module/references.dart +++ b/lib/src/migrators/module/references.dart @@ -10,12 +10,13 @@ import 'package:sass_api/sass_api.dart'; import '../../exception.dart'; import '../../util/bidirectional_map.dart'; +import '../../util/member_declaration.dart'; +import '../../util/scope.dart'; +import '../../util/scoped_ast_visitor.dart'; import '../../util/unmodifiable_bidirectional_map_view.dart'; import '../../utils.dart'; import 'built_in_functions.dart'; -import 'member_declaration.dart'; import 'reference_source.dart'; -import 'scope.dart'; /// A bidirectional mapping between member declarations and references to those /// members. @@ -167,7 +168,7 @@ class References { } /// A visitor that builds a References object. -class _ReferenceVisitor with RecursiveStatementVisitor, RecursiveAstVisitor { +class _ReferenceVisitor extends ScopedAstVisitor { final _variables = BidirectionalMap(); final _variableReassignments = BidirectionalMap< MemberDeclaration, MemberDeclaration>(); @@ -183,11 +184,6 @@ class _ReferenceVisitor with RecursiveStatementVisitor, RecursiveAstVisitor { final _sources = {}; final _orphanImportOnlyFiles = {}; - /// The current global scope. - /// - /// This persists across imports, but not across module loads. - late Scope _scope; - /// Mapping from canonical stylesheet URLs to the global scope of the module /// contained within it. /// @@ -251,22 +247,22 @@ class _ReferenceVisitor with RecursiveStatementVisitor, RecursiveAstVisitor { /// [importer]) and its dependencies. References build(Stylesheet stylesheet, Importer importer) { _importer = importer; - _scope = Scope(); + currentScope = Scope(); _currentUrl = stylesheet.span.sourceUrl!; _isOrphanImportOnly = isImportOnlyFile(_currentUrl); - _moduleScopes[_currentUrl] = _scope; + _moduleScopes[_currentUrl] = currentScope; _declarationSources = {}; _moduleSources[_currentUrl] = _declarationSources; visitStylesheet(stylesheet); - for (var variable in _scope.variables.values) { + for (var variable in currentScope.variables.values) { var original = _variableReassignments[variable] ?? variable; _globalDeclarations.add(original); _globalDeclarations.addAll(_variableReassignments.keysForValue(original)); } - _globalDeclarations.addAll(_scope.mixins.values); - _globalDeclarations.addAll(_scope.functions.values); - _checkUnresolvedReferences(_scope); + _globalDeclarations.addAll(currentScope.mixins.values); + _globalDeclarations.addAll(currentScope.functions.values); + _checkUnresolvedReferences(currentScope); _resolveBuiltInFunctionReferences(); return References._( _variables, @@ -325,7 +321,7 @@ class _ReferenceVisitor with RecursiveStatementVisitor, RecursiveAstVisitor { _namespaces = {}; _currentUrl = node.span.sourceUrl!; _isOrphanImportOnly = isImportOnlyFile(_currentUrl); - super.visitChildren(node.children); + super.visitChildren(node.children, withScope: false); if (_isOrphanImportOnly) { _orphanImportOnlyFiles[_currentUrl] = _lastRegularForward?.span.sourceUrl == _currentUrl @@ -350,17 +346,18 @@ class _ReferenceVisitor with RecursiveStatementVisitor, RecursiveAstVisitor { "Could not find Sass file at '${p.prettyUri(import.url)}'.", import.span); } + var (newImporter, stylesheet) = result; var oldImporter = _importer; - _importer = result.item1; + _importer = newImporter; var oldLibraryUrl = _libraryUrl; - var url = result.item2.span.sourceUrl!; + var url = stylesheet.span.sourceUrl!; if (_importer != oldImporter && !isImportOnlyFile(url)) { _libraryUrl ??= url; } var oldRuleUrl = _currentRuleUrl; _currentRuleUrl = import.url; - visitStylesheet(result.item2); + visitStylesheet(stylesheet); var importSource = ImportSource(url, import); for (var entry in _declarationSources.entries.toList()) { var declaration = entry.key; @@ -415,28 +412,28 @@ class _ReferenceVisitor with RecursiveStatementVisitor, RecursiveAstVisitor { "Could not find Sass file at '${p.prettyUri(ruleUrl)}'.", nodeForSpan.span); } + var (newImporter, stylesheet) = result; - var stylesheet = result.item2; var canonicalUrl = stylesheet.span.sourceUrl!; if (_moduleScopes.containsKey(canonicalUrl)) return canonicalUrl; - var oldScope = _scope; - _scope = Scope(); - _moduleScopes[canonicalUrl] = _scope; + var oldScope = currentScope; + currentScope = Scope(); + _moduleScopes[canonicalUrl] = currentScope; var oldSources = _declarationSources; _declarationSources = {}; _moduleSources[canonicalUrl] = _declarationSources; var oldImporter = _importer; - _importer = result.item1; + _importer = newImporter; var oldLibraryUrl = _libraryUrl; _libraryUrl = null; var oldRuleUrl = _currentRuleUrl; _currentRuleUrl = ruleUrl; visitStylesheet(stylesheet); - _checkUnresolvedReferences(_scope); + onScopeClose(); _libraryUrl = oldLibraryUrl; _importer = oldImporter; - _scope = oldScope; + currentScope = oldScope; _declarationSources = oldSources; _currentRuleUrl = oldRuleUrl; return canonicalUrl; @@ -462,19 +459,19 @@ class _ReferenceVisitor with RecursiveStatementVisitor, RecursiveAstVisitor { } if (_visibleThroughForward(declaration.name, node.prefix, node.shownVariables, node.hiddenVariables)) { - _forwardMember(declaration, node, canonicalUrl, _scope.variables); + _forwardMember(declaration, node, canonicalUrl, currentScope.variables); } } for (var declaration in moduleScope.mixins.values) { if (_visibleThroughForward(declaration.name, node.prefix, node.shownMixinsAndFunctions, node.hiddenMixinsAndFunctions)) { - _forwardMember(declaration, node, canonicalUrl, _scope.mixins); + _forwardMember(declaration, node, canonicalUrl, currentScope.mixins); } } for (var declaration in moduleScope.functions.values) { if (_visibleThroughForward(declaration.name, node.prefix, node.shownMixinsAndFunctions, node.hiddenMixinsAndFunctions)) { - _forwardMember(declaration, node, canonicalUrl, _scope.functions); + _forwardMember(declaration, node, canonicalUrl, currentScope.functions); } } } @@ -513,31 +510,10 @@ class _ReferenceVisitor with RecursiveStatementVisitor, RecursiveAstVisitor { } } - /// Visits each of [node]'s expressions and children. - /// - /// All of [node]'s arguments are declared as local variables in a new scope. - @override - void visitCallableDeclaration(CallableDeclaration node) { - var oldScope = _scope; - _scope = Scope(_scope); - for (var argument in node.arguments.arguments) { - _scope.variables[argument.name] = MemberDeclaration(argument); - var defaultValue = argument.defaultValue; - if (defaultValue != null) visitExpression(defaultValue); - } - super.visitChildren(node.children); - _checkUnresolvedReferences(_scope); - _scope = oldScope; - } - - /// Visits [children] with a local scope. + /// Check any unresolved references each time a scope is closed. @override - void visitChildren(List children) { - var oldScope = _scope; - _scope = Scope(_scope); - super.visitChildren(children); - _checkUnresolvedReferences(_scope); - _scope = oldScope; + void onScopeClose() { + _checkUnresolvedReferences(currentScope); } /// Finds any declarations in [scope] that match one of the references in @@ -593,12 +569,14 @@ class _ReferenceVisitor with RecursiveStatementVisitor, RecursiveAstVisitor { /// If [namespace] is null or does not exist within this stylesheet, this /// returns the current stylesheet's scope. Scope _scopeForNamespace(String? namespace) => - _moduleScopes[_namespaces[namespace]] ?? _scope; + _moduleScopes[_namespaces[namespace]] ?? currentScope; /// Declares a variable in the current scope. @override void visitVariableDeclaration(VariableDeclaration node) { - super.visitVariableDeclaration(node); + // Visit expression directly so we can bypass ScopedAstVisitor adding this + // declaration and handle it ourselves. + visitExpression(node.expression); var member = MemberDeclaration(node); _declarationSources[member] = CurrentSource(_currentUrl); _registerLibraryUrl(member); @@ -641,7 +619,7 @@ class _ReferenceVisitor with RecursiveStatementVisitor, RecursiveAstVisitor { var source = _declarationSources[declaration]; if (source != null) _sources[node] = source; } else if (namespace == null) { - _unresolvedReferences[node] = _scope; + _unresolvedReferences[node] = currentScope; } } @@ -652,7 +630,6 @@ class _ReferenceVisitor with RecursiveStatementVisitor, RecursiveAstVisitor { var member = MemberDeclaration(node); _declarationSources[member] = CurrentSource(_currentUrl); _registerLibraryUrl(member); - _scope.mixins[node.name] = member; } /// Visits an `@include` rule, storing the mixin reference. @@ -670,7 +647,7 @@ class _ReferenceVisitor with RecursiveStatementVisitor, RecursiveAstVisitor { _mixins[node] = declaration; _sources[node] = _declarationSources[declaration]!; } else if (namespace == null) { - _unresolvedReferences[node] = _scope; + _unresolvedReferences[node] = currentScope; } } @@ -681,7 +658,6 @@ class _ReferenceVisitor with RecursiveStatementVisitor, RecursiveAstVisitor { var member = MemberDeclaration(node); _declarationSources[member] = CurrentSource(_currentUrl); _registerLibraryUrl(member); - _scope.functions[node.name] = member; } /// Visits a function call, storing it if it is a user-defined function. @@ -703,7 +679,7 @@ class _ReferenceVisitor with RecursiveStatementVisitor, RecursiveAstVisitor { if (node.name == 'get-function') { _sources[node] = BuiltInSource("meta"); } else { - _unresolvedReferences[node] = _scope; + _unresolvedReferences[node] = currentScope; return; } } @@ -718,7 +694,7 @@ class _ReferenceVisitor with RecursiveStatementVisitor, RecursiveAstVisitor { if (declaration != null && !_fromForwardRuleInCurrent(declaration)) { _getFunctionReferences[node] = declaration; } else if (namespace == null) { - _unresolvedReferences[node] = _scope; + _unresolvedReferences[node] = currentScope; } } diff --git a/lib/src/migrators/module/unreferencable_members.dart b/lib/src/migrators/module/unreferencable_members.dart index 2c0d190..6a915bd 100644 --- a/lib/src/migrators/module/unreferencable_members.dart +++ b/lib/src/migrators/module/unreferencable_members.dart @@ -6,7 +6,7 @@ import 'package:sass_api/sass_api.dart'; -import 'member_declaration.dart'; +import '../../util/member_declaration.dart'; import 'unreferencable_type.dart'; /// Tracks members that are unreferencable in the current scope. diff --git a/lib/src/runner.dart b/lib/src/runner.dart index e58631a..e5fd227 100644 --- a/lib/src/runner.dart +++ b/lib/src/runner.dart @@ -15,7 +15,6 @@ import 'package:term_glyph/term_glyph.dart' as glyph; import 'io.dart'; import 'migrators/calc_interpolation.dart'; import 'migrators/division.dart'; -import 'migrators/media_logic.dart'; import 'migrators/module.dart'; import 'migrators/namespace.dart'; import 'migrators/strict_unary.dart'; @@ -57,7 +56,6 @@ class MigratorRunner extends CommandRunner> { help: 'Print the version of the Sass migrator.', negatable: false); addCommand(CalculationInterpolationMigrator()); addCommand(DivisionMigrator()); - addCommand(MediaLogicMigrator()); addCommand(ModuleMigrator()); addCommand(NamespaceMigrator()); addCommand(StrictUnaryMigrator()); diff --git a/lib/src/migrators/module/member_declaration.dart b/lib/src/util/member_declaration.dart similarity index 99% rename from lib/src/migrators/module/member_declaration.dart rename to lib/src/util/member_declaration.dart index e744794..22bce60 100644 --- a/lib/src/migrators/module/member_declaration.dart +++ b/lib/src/util/member_declaration.dart @@ -7,7 +7,7 @@ import 'package:path/path.dart' as p; import 'package:sass_api/sass_api.dart'; -import '../../utils.dart'; +import '../utils.dart'; /// A wrapper class for nodes that declare a variable, function, or mixin. class MemberDeclaration { diff --git a/lib/src/migrators/module/scope.dart b/lib/src/util/scope.dart similarity index 77% rename from lib/src/migrators/module/scope.dart rename to lib/src/util/scope.dart index 4f89e7f..7a0b50b 100644 --- a/lib/src/migrators/module/scope.dart +++ b/lib/src/util/scope.dart @@ -34,6 +34,17 @@ class Scope { /// Returns true if this scope is global, and false otherwise. bool get isGlobal => parent == null; + /// The set of all variable names defined in this scope or its ancestors. + Set get allVariableNames => + {...variables.keys, ...?parent?.allVariableNames}; + + /// The set of all mixin names defined in this scope or its ancestors. + Set get allMixinNames => {...mixins.keys, ...?parent?.allMixinNames}; + + /// The set of all function names defined in this scope or its ancestors. + Set get allFunctionNames => + {...functions.keys, ...?parent?.allFunctionNames}; + /// Returns true if this scope is [ancestor] or one of its descendents. bool isDescendentOf(Scope ancestor) => this == ancestor || (parent?.isDescendentOf(ancestor) ?? false); @@ -52,4 +63,6 @@ class Scope { /// if it does not. MemberDeclaration? findFunction(String name) => functions[name] ?? parent?.findFunction(name); + + String toString() => '${functions.keys}->$parent'; } diff --git a/lib/src/util/scoped_ast_visitor.dart b/lib/src/util/scoped_ast_visitor.dart new file mode 100644 index 0000000..a0c2449 --- /dev/null +++ b/lib/src/util/scoped_ast_visitor.dart @@ -0,0 +1,102 @@ +// 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:meta/meta.dart'; +import 'package:sass_api/sass_api.dart'; + +import 'member_declaration.dart'; +import 'scope.dart'; + +/// A recursive AST visitor that tracks the Sass members declared in the +/// current scope. +abstract class ScopedAstVisitor + with RecursiveStatementVisitor, RecursiveAstVisitor { + /// The current scope, containing any visible Sass members without a namespace + /// to the current point in the AST. + /// + /// Subclasses that visit multiple modules should update this when changing + /// the module being visited. + @protected + var currentScope = Scope(); + + /// A callback called when the visitor closes a local scope. + /// + /// Subclasses should override this if they need to do something with the + /// current local scope before it is closed. + @protected + void onScopeClose() {} + + /// Evaluates [inScope] in a new child scope of the current scope. + @protected + void scoped(void Function() inScope) { + var oldScope = currentScope; + currentScope = Scope(currentScope); + inScope(); + onScopeClose(); + currentScope = oldScope; + } + + @override + void visitStylesheet(Stylesheet node) { + visitChildren(node.children, withScope: false); + } + + /// Visits each child in [children] sequentially. + /// + /// When [withScope] is true, the children will be visited with a shared + /// local scope. + @override + void visitChildren(List children, {bool withScope = true}) { + visit() { + super.visitChildren(children); + } + + return withScope ? scoped(visit) : visit(); + } + + /// Creates a new child scope, declares [node]'s arguments within it and + /// then visits [node]'s children. + @override + void visitCallableDeclaration(CallableDeclaration node) { + scoped(() { + for (var argument in node.arguments.arguments) { + currentScope.variables[argument.name] = MemberDeclaration(argument); + if (argument.defaultValue case var defaultValue?) { + visitExpression(defaultValue); + } + } + visitChildren(node.children, withScope: false); + }); + } + + /// Adds [node] to the current scope and then visits it. + @override + void visitFunctionRule(FunctionRule node) { + currentScope.functions[node.name] = MemberDeclaration(node); + super.visitFunctionRule(node); + } + + /// Adds [node] to the current scope and then visits it. + @override + void visitMixinRule(MixinRule node) { + currentScope.mixins[node.name] = MemberDeclaration(node); + super.visitMixinRule(node); + } + + /// Visits [node] and then adds it to the current or the global scope. + /// + /// If [node] is namespaced, no scope will be updated. + @override + void visitVariableDeclaration(VariableDeclaration node) { + super.visitVariableDeclaration(node); + var scope = switch (node) { + VariableDeclaration(isGlobal: true) => currentScope.global, + VariableDeclaration(namespace: null) => currentScope, + _ => null + }; + scope?.variables[node.name] = MemberDeclaration(node); + } +} diff --git a/lib/src/utils.dart b/lib/src/utils.dart index feba634..1c9e9aa 100644 --- a/lib/src/utils.dart +++ b/lib/src/utils.dart @@ -7,7 +7,6 @@ import 'package:charcode/charcode.dart'; import 'package:sass_api/sass_api.dart'; import 'package:source_span/source_span.dart'; -import 'package:tuple/tuple.dart'; import 'io.dart'; import 'patch.dart'; @@ -220,7 +219,7 @@ bool isImportOnlyFile(Uri url) => /// /// This asserts that every element in [iterable] is either an `F` or a `G`, and /// returns one list containing all the `F`s and one containing all the `G`s. -Tuple2, List> partitionOnType( +(List, List) partitionOnType( Iterable iterable) { var fs = []; var gs = []; @@ -233,5 +232,5 @@ Tuple2, List> partitionOnType( } } - return Tuple2(fs, gs); + return (fs, gs); } diff --git a/pubspec.yaml b/pubspec.yaml index bed5f79..1b919d4 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,10 +1,10 @@ name: sass_migrator -version: 1.8.2-dev +version: 2.0.0 description: A tool for running migrations on Sass files homepage: https://github.com/sass/migrator environment: - sdk: '>=2.12.0 <3.0.0' + sdk: '>=3.0.0 <4.0.0' dependencies: args: ^2.1.0 @@ -17,25 +17,24 @@ dependencies: node_interop: ^2.0.2 node_io: ^2.2.0 path: ^1.8.0 - sass_api: ^3.0.4 + sass_api: ^9.2.7 source_span: ^1.8.1 stack_trace: ^1.10.0 string_scanner: ^1.1.0 term_glyph: ^1.2.0 - tuple: ^2.0.0 dev_dependencies: archive: ^3.1.2 cli_pkg: ^2.1.2 crypto: ^3.0.1 grinder: ^0.9.0 - http: ^0.13.1 + http: ^1.1.2 node_preamble: ^2.0.0 pub_semver: ^2.0.0 test: ^1.17.1 test_descriptor: ^2.0.0 test_process: ^2.0.0 - xml: ^5.1.0 + xml: ^6.5.0 yaml: ^3.1.0 executables: diff --git a/test/migrators/division/calculations.hrx b/test/migrators/division/calculations.hrx index 6b8c857..7594bd8 100644 --- a/test/migrators/division/calculations.hrx +++ b/test/migrators/division/calculations.hrx @@ -1,23 +1,37 @@ <==> input/entrypoint.scss +@function sqrt($a) { + @return $a; +} + a { $x: 300px; $y: 100%; + $z: 200; 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)); + h: sqrt($z / 2); + i: log($z / 2); } <==> output/entrypoint.scss +@function sqrt($a) { + @return $a; +} + a { $x: 300px; $y: 100%; + $z: 200; 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)); + h: sqrt($z * 0.5); + i: log($z / 2); } diff --git a/test/migrators/media_logic/add_interpolation.hrx b/test/migrators/media_logic/add_interpolation.hrx deleted file mode 100644 index 0375b89..0000000 --- a/test/migrators/media_logic/add_interpolation.hrx +++ /dev/null @@ -1,25 +0,0 @@ -<==> input/entrypoint.scss -@media (not (foo)) { - a {b: c} -} - -@media ((foo) and (bar)) { - d {e: f} -} - -@media ((foo) or (bar)) { - g {h: i} -} - -<==> output/entrypoint.scss -@media (#{not (foo)}) { - a {b: c} -} - -@media (#{(foo) and (bar)}) { - d {e: f} -} - -@media (#{(foo) or (bar)}) { - g {h: i} -} diff --git a/test/migrators/migrator_dart_test.dart b/test/migrators/migrator_dart_test.dart index 84fe0e5..8d75fb4 100644 --- a/test/migrators/migrator_dart_test.dart +++ b/test/migrators/migrator_dart_test.dart @@ -9,7 +9,6 @@ import '../utils.dart'; main() { testMigrator("calc_interpolation"); testMigrator("division"); - testMigrator("media_logic"); testMigrator("module"); testMigrator("namespace"); testMigrator("strict_unary");