diff --git a/CHANGELOG.md b/CHANGELOG.md index 3170da8b..fc56c7d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +## 2.3.3 + +### Module System Migration + +* Fix some bugs in the conversion of private names that are referenced across + files to public names, especially when `--remove-prefix` and/or multiple + leading dashes/underscores are involved. + ## 2.3.2 * Update to be compatible with the latest version of the Dart Sass AST. diff --git a/lib/src/migrators/module.dart b/lib/src/migrators/module.dart index ba623ece..6188bd22 100644 --- a/lib/src/migrators/module.dart +++ b/lib/src/migrators/module.dart @@ -6,6 +6,7 @@ import 'package:args/args.dart'; import 'package:collection/collection.dart'; +import 'package:charcode/charcode.dart'; import 'package:path/path.dart' as p; import 'package:sass_api/sass_api.dart'; import 'package:source_span/source_span.dart'; @@ -263,7 +264,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor { var forwardsByUrl = >>{}; var hiddenByUrl = >{}; for (var declaration in references.globalDeclarations) { - var private = declaration.name.startsWith('-'); + var private = _isPrivate(declaration.name); // Whether this member will be exposed by the regular entrypoint. var visibleAtEntrypoint = !private && @@ -352,19 +353,38 @@ class _ModuleMigrationVisitor extends MigrationVisitor { if (declaration.isForwarded) return; var name = declaration.name; - if (name.startsWith('-') && - references.referencedOutsideDeclaringStylesheet(declaration)) { - // Remove leading `-` since private members can't be accessed outside - // the module they're declared in. - name = name.substring(1); - } - name = _unprefix(name); + name = _unprefix(name, + // Private members can't be accessed outside the module they're + // declared in. + forcePublic: + references.referencedOutsideDeclaringStylesheet(declaration)); if (name != declaration.name) { renamedMembers[declaration] = name; if (_upstreamStylesheets.isEmpty) _needsImportOnly = true; } } + /// Returns whether [identifier] is a private member name. + /// + /// Assumes [identifier] is a valid CSS identifier. + bool _isPrivate(String identifier) { + return identifier.startsWith('-') || identifier.startsWith('_'); + } + + /// Converts a private identifier to a public one. + String _privateToPublic(String identifier) { + if (!_isPrivate(identifier)) return identifier; + + for (var i = 0; i < identifier.length; i++) { + var char = identifier.codeUnitAt(i); + if (char != $dash && char != $underscore) { + return identifier.substring(i); + } + } + + return 'private$identifier'; + } + /// Returns whether the member named [name] should be forwarded in the /// entrypoint. /// @@ -1025,8 +1045,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor { newName = declaration.name.substring(importOnlyPrefix.length); } - if (_shouldForward(declaration.name) && - !declaration.name.startsWith('-')) { + if (_shouldForward(declaration.name) && !_isPrivate(declaration.name)) { var subprefix = ""; if (importOnlyPrefix != null) { var prefix = _prefixFor(declaration.name); @@ -1036,7 +1055,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor { } if (declaration.name != newName) _needsImportOnly = true; shownByPrefix.putIfAbsent(subprefix, () => {}).add(declaration); - } else if (!newName.startsWith('-')) { + } else if (!_isPrivate(newName)) { hidden.add(declaration); } } @@ -1076,8 +1095,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor { if (declaration is ImportOnlyMemberDeclaration) { name = name.substring(declaration.importOnlyPrefix.length); } - if (name.startsWith('-')) name = name.substring(1); - name = _unprefix(name); + name = _unprefix(name, forcePublic: true); if (subprefix.isNotEmpty) name = '$subprefix$name'; if (declaration.member is VariableDeclaration) name = '\$$name'; allHidden.add(name); @@ -1205,7 +1223,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor { void _renameReference(FileSpan span, MemberDeclaration declaration) { var newName = renamedMembers[declaration]; if (newName != null) { - if (newName.startsWith('-') && + if (_isPrivate(newName) && declaration.name.endsWith(newName.substring(1))) { addPatch(patchDelete(span, start: 1, end: declaration.name.length - newName.length + 1)); @@ -1225,14 +1243,36 @@ class _ModuleMigrationVisitor extends MigrationVisitor { /// If [name] starts with any of [prefixesToRemove], returns it with the /// longest matching prefix removed. + /// If forcePublic is true, any leading dash or underscore will be removed. /// /// Otherwise, returns [name] unaltered. - String _unprefix(String name) { - var isPrivate = name.startsWith('-'); - var unprivateName = isPrivate ? name.substring(1) : name; - var prefix = _prefixFor(unprivateName); - if (prefix == null) return name; - return (isPrivate ? '-' : '') + unprivateName.substring(prefix.length); + /// + /// If [forcePublic] is true, this will ensure that the name is always a + /// public identifier. + String _unprefix(String name, {bool forcePublic = false}) { + bool isPrivate; + String withoutPrefix; + + // Allow users to pass prefixes that either do or do not include leading + // dashes/underscores. As usual, the longest prefix wins, so we check for + // ones that do include them first. + var prefix = _prefixFor(name); + if (prefix != null) { + isPrivate = false; + withoutPrefix = name.substring(prefix.length); + } else { + isPrivate = _isPrivate(name); + var unprivateName = isPrivate ? _privateToPublic(name) : name; + prefix = _prefixFor(unprivateName); + if (prefix == null) + return isPrivate && forcePublic ? unprivateName : name; + withoutPrefix = unprivateName.substring(prefix.length); + } + + if (_isPrivate(withoutPrefix)) { + withoutPrefix = _privateToPublic(withoutPrefix); + } + return (isPrivate && !forcePublic ? '-' : '') + withoutPrefix; } /// Returns the namespace that built-in module [module] is loaded under. diff --git a/pubspec.yaml b/pubspec.yaml index 46f4be54..9ac43a40 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: sass_migrator -version: 2.3.2 +version: 2.3.3 description: A tool for running migrations on Sass files homepage: https://github.com/sass/migrator diff --git a/test/migrators/module/remove_prefix_flag/pseudoprivate_multi_dash.hrx b/test/migrators/module/remove_prefix_flag/pseudoprivate_multi_dash.hrx new file mode 100644 index 00000000..586757a5 --- /dev/null +++ b/test/migrators/module/remove_prefix_flag/pseudoprivate_multi_dash.hrx @@ -0,0 +1,22 @@ +<==> arguments +--migrate-deps --remove-prefix=lib- + +<==> input/entrypoint.scss +@import "dependency"; + +a { + background: $--lib-pseudoprivate; +} + +<==> input/_dependency.scss +$--lib-pseudoprivate: blue; + +<==> output/entrypoint.scss +@use "dependency"; + +a { + background: dependency.$pseudoprivate; +} + +<==> output/_dependency.scss +$pseudoprivate: blue; diff --git a/test/migrators/module/remove_prefix_flag/with_dash.hrx b/test/migrators/module/remove_prefix_flag/with_dash.hrx new file mode 100644 index 00000000..98f193b0 --- /dev/null +++ b/test/migrators/module/remove_prefix_flag/with_dash.hrx @@ -0,0 +1,25 @@ +<==> arguments +--migrate-deps --remove-prefix=--app- +<==> input/entrypoint.scss +@import "library"; +a { + color: $--app-var; + background: $__app-var2; +} + +<==> input/_library.scss +$--app-red: red; +$--app-var: $--app-red; +$__app-var2: blue; + +<==> output/entrypoint.scss +@use "library"; +a { + color: library.$var; + background: library.$var2; +} + +<==> output/_library.scss +$red: red; +$var: $red; +$var2: blue; diff --git a/test/migrators/module/remove_prefix_flag/with_leading_dash.hrx b/test/migrators/module/remove_prefix_flag/with_leading_dash.hrx new file mode 100644 index 00000000..b50190bb --- /dev/null +++ b/test/migrators/module/remove_prefix_flag/with_leading_dash.hrx @@ -0,0 +1,36 @@ +<==> arguments +--remove-prefix=--lib- + +<==> input/entrypoint.scss +$--lib-a: 5; +$--lib_b: $--lib-a + 2; + +@function --lib-fn($--lib-local) { + @return $--lib-local; +} + +@mixin --lib-mixin { + --lib-property: --lib-value; +} + +--lib-selector { + property: --lib-fn(0); + @include --lib-mixin; +} + +<==> output/entrypoint.scss +$a: 5; +$b: $a + 2; + +@function fn($--lib-local) { + @return $--lib-local; +} + +@mixin mixin { + --lib-property: --lib-value; +} + +--lib-selector { + property: fn(0); + @include mixin; +} diff --git a/test/migrators/module/remove_prefix_flag/without_dash.hrx b/test/migrators/module/remove_prefix_flag/without_dash.hrx new file mode 100644 index 00000000..8b91a787 --- /dev/null +++ b/test/migrators/module/remove_prefix_flag/without_dash.hrx @@ -0,0 +1,39 @@ +<==> arguments +--remove-prefix=lib + +<==> input/entrypoint.scss +$lib-a: 5; +$lib_b: $lib-a + 2; + +@function lib-fn($lib-local) { + @return $lib-local; +} + +@mixin lib-mixin { + lib-property: lib-value; +} + +lib-selector { + lib-property: lib-fn(0); + @include lib-mixin; +} + +<==> output/entrypoint.scss +$a: 5; +$b: $a + 2; + +@function fn($lib-local) { + @return $lib-local; +} + +@mixin mixin { + lib-property: lib-value; +} + +lib-selector { + lib-property: fn(0); + @include mixin; +} + +<==> output/entrypoint.import.scss +@forward "entrypoint" as lib-*; diff --git a/test/migrators/module/scope/pseudoprivate.hrx b/test/migrators/module/scope/pseudoprivate.hrx deleted file mode 100644 index a6ba89bb..00000000 --- a/test/migrators/module/scope/pseudoprivate.hrx +++ /dev/null @@ -1,25 +0,0 @@ -<==> arguments ---migrate-deps -<==> input/entrypoint.scss -@import "library"; -a { - color: $-dash; - background: $_underscore; -} - -<==> input/_library.scss -$-dash: red; -$_underscore: blue; -$_unreferenced: green; - -<==> output/entrypoint.scss -@use "library"; -a { - color: library.$dash; - background: library.$underscore; -} - -<==> output/_library.scss -$dash: red; -$underscore: blue; -$_unreferenced: green; diff --git a/test/migrators/module/scope/pseudoprivate/dash.hrx b/test/migrators/module/scope/pseudoprivate/dash.hrx new file mode 100644 index 00000000..2d8d0208 --- /dev/null +++ b/test/migrators/module/scope/pseudoprivate/dash.hrx @@ -0,0 +1,21 @@ +<==> arguments +--migrate-deps +<==> input/entrypoint.scss +@import "library"; +a { + color: $-var; +} + +<==> input/_library.scss +$-red: red; +$-var: $-red; + +<==> output/entrypoint.scss +@use "library"; +a { + color: library.$var; +} + +<==> output/_library.scss +$-red: red; +$var: $-red; diff --git a/test/migrators/module/scope/pseudoprivate/dashes_only.hrx b/test/migrators/module/scope/pseudoprivate/dashes_only.hrx new file mode 100644 index 00000000..b7a98db3 --- /dev/null +++ b/test/migrators/module/scope/pseudoprivate/dashes_only.hrx @@ -0,0 +1,19 @@ +<==> arguments +--migrate-deps +<==> input/entrypoint.scss +@import "library"; +a { + color: $---; +} + +<==> input/_library.scss +$---: red; + +<==> output/entrypoint.scss +@use "library"; +a { + color: library.$private---; +} + +<==> output/_library.scss +$private---: red; diff --git a/test/migrators/module/scope/pseudoprivate/double_dash.hrx b/test/migrators/module/scope/pseudoprivate/double_dash.hrx new file mode 100644 index 00000000..ac6fbb97 --- /dev/null +++ b/test/migrators/module/scope/pseudoprivate/double_dash.hrx @@ -0,0 +1,21 @@ +<==> arguments +--migrate-deps +<==> input/entrypoint.scss +@import "library"; +a { + color: $--var; +} + +<==> input/_library.scss +$--red: red; +$--var: $--red; + +<==> output/entrypoint.scss +@use "library"; +a { + color: library.$var; +} + +<==> output/_library.scss +$--red: red; +$var: $--red; diff --git a/test/migrators/module/scope/pseudoprivate/indirect.hrx b/test/migrators/module/scope/pseudoprivate/indirect.hrx new file mode 100644 index 00000000..0ad6ff75 --- /dev/null +++ b/test/migrators/module/scope/pseudoprivate/indirect.hrx @@ -0,0 +1,21 @@ +<==> arguments +--migrate-deps +<==> input/entrypoint.scss +@import "library"; +a { + color: $middle; +} + +<==> input/_library.scss +$-upstream: red; +$middle: $-upstream; + +<==> output/entrypoint.scss +@use "library"; +a { + color: library.$middle; +} + +<==> output/_library.scss +$-upstream: red; +$middle: $-upstream; diff --git a/test/migrators/module/scope/pseudoprivate/underscore.hrx b/test/migrators/module/scope/pseudoprivate/underscore.hrx new file mode 100644 index 00000000..ce1e13cb --- /dev/null +++ b/test/migrators/module/scope/pseudoprivate/underscore.hrx @@ -0,0 +1,21 @@ +<==> arguments +--migrate-deps +<==> input/entrypoint.scss +@import "library"; +a { + color: $_var; +} + +<==> input/_library.scss +$_red: red; +$_var: $_red; + +<==> output/entrypoint.scss +@use "library"; +a { + color: library.$var; +} + +<==> output/_library.scss +$_red: red; +$var: $_red; diff --git a/test/migrators/module/scope/pseudoprivate/with_prefix.hrx b/test/migrators/module/scope/pseudoprivate/with_prefix.hrx new file mode 100644 index 00000000..99d2af88 --- /dev/null +++ b/test/migrators/module/scope/pseudoprivate/with_prefix.hrx @@ -0,0 +1,21 @@ +<==> arguments +--migrate-deps --remove-prefix=app +<==> input/entrypoint.scss +@import "library"; +a { + color: $app-var; +} + +<==> input/_library.scss +$_app-red: red; +$app-var: $_app-red; + +<==> output/entrypoint.scss +@use "library"; +a { + color: library.$var; +} + +<==> output/_library.scss +$_red: red; +$var: $_red;