Skip to content

Commit

Permalink
Add parens around negated variable when namespaced (#115)
Browse files Browse the repository at this point in the history
Fixes #114.

In fixing this, I also found a fixed a bug when migrating removed color
functions, as a patch to namespace the argument would be applied before
the patch adding the new argument name. This was fixed by ensuring the
arguments of a function expression are visited after the function itself
is patched.
  • Loading branch information
jathak authored Oct 10, 2019
1 parent 08fb490 commit fceb900
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 4 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@
only be considered configured when the configuring declaration is upstream of
the `!default` declaration.

* When namespacing a negated variable, adds parentheses around it to prevent the
`-` from being parsed as part of the namespace.

* Fix a bug in the migrating of removed color functions when the amount is a
variable being namespaced.

## 1.0.0

* Initial release.
21 changes: 17 additions & 4 deletions lib/src/migrators/module.dart
Original file line number Diff line number Diff line change
Expand Up @@ -522,8 +522,10 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
/// Adds a namespace to any function call that requires it.
@override
void visitFunctionExpression(FunctionExpression node) {
super.visitFunctionExpression(node);
if (node.namespace != null) return;
if (node.namespace != null) {
super.visitFunctionExpression(node);
return;
}
if (references.sources.containsKey(node)) {
var declaration = references.functions[node];
_unreferencable.check(declaration, node);
Expand Down Expand Up @@ -561,6 +563,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
', \$module: "$namespace"'));
}, getFunctionCall: true);
}
super.visitFunctionExpression(node);
}

/// Calls [patchNamespace] when the function [node] requires a namespace.
Expand Down Expand Up @@ -635,8 +638,13 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
void _patchRemovedColorFunction(String name, Expression arg,
{FileSpan existingArgName}) {
var parameter = removedColorFunctions[name];
var needsParens =
parameter.endsWith('-') && arg is BinaryOperationExpression;
// Surround the argument in parens if negated to avoid `-` being parsed
// as part of the namespace.
var needsParens = parameter.endsWith('-') &&
(arg is BinaryOperationExpression ||
arg is FunctionExpression ||
(arg is VariableExpression &&
references.variables[arg]?.sourceUrl != currentUrl));
var leftParen = needsParens ? '(' : '';
if (existingArgName == null) {
addPatch(patchBefore(arg, '$parameter$leftParen'));
Expand Down Expand Up @@ -927,7 +935,12 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
_renameReference(nameSpan(node), declaration);
var namespace = _namespaceForDeclaration(declaration);
if (namespace != null) {
// Surround the variable in parens if negated to avoid `-` being parsed
// as part of the namespace.
var negated = matchesBeforeSpan(node.span, '-');
if (negated) addPatch(patchBefore(node, '('));
addPatch(patchBefore(node, '$namespace.'));
if (negated) addPatch(patchAfter(node, ')'));
}
}

Expand Down
7 changes: 7 additions & 0 deletions lib/src/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,13 @@ FileSpan extendBackward(FileSpan span, String text) {
return span.file.span(start - text.length, span.end.offset);
}

/// Returns true if [span] is preceded by exactly [text].
bool matchesBeforeSpan(FileSpan span, String text) {
var start = span.start.offset;
if (start - text.length < 0) return false;
return span.file.getText(start - text.length, start) == text;
}

/// Returns whether [character] is whitespace, according to Sass's definition.
bool isWhitespace(int character) =>
character == $space ||
Expand Down
23 changes: 23 additions & 0 deletions test/migrators/module/namespace_negation.hrx
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<==> arguments
--migrate-deps

<==> input/entrypoint.scss
@import "library";
a {
b: -$amount;
color: darken($color, $amount);
background: darken($color, fn());
}

<==> input/_library.scss
$color: blue;
$amount: 10%;

<==> output/entrypoint.scss
@use "sass:color";
@use "library";
a {
b: -(library.$amount);
color: color.adjust(library.$color, $lightness: -(library.$amount));
background: color.adjust(library.$color, $lightness: -(fn()));
}

0 comments on commit fceb900

Please sign in to comment.