Code review

Author: nex3

lib/src/migrators/module.dart

@@ -356,13 +356,11 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
     if (declaration.isForwarded) return;
 
     var name = declaration.name;
-    if (_isPrivate(name) &&
-        references.referencedOutsideDeclaringStylesheet(declaration)) {
-      // Private members can't be accessed outside the module they're declared
-      // in.
-      name = _privateToPublic(name);
-    }
-    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;
@@ -1100,8 +1098,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
         if (declaration is ImportOnlyMemberDeclaration) {
           name = name.substring(declaration.importOnlyPrefix.length);
         }
-        if (_isPrivate(name)) name = _privateToPublic(name);
-        name = _unprefix(name);
+        name = _unprefix(name, forcePublic: true);
         if (subprefix.isNotEmpty) name = '$subprefix$name';
         if (declaration.member is VariableDeclaration) name = '\$$name';
         allHidden.add(name);
@@ -1251,17 +1248,33 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
   /// longest matching prefix removed.
   ///
   /// Otherwise, returns [name] unaltered.
-  String _unprefix(String name) {
-    var isPrivate = _isPrivate(name);
-    var unprivateName = isPrivate ? _privateToPublic(name) : name;
-    var prefix = _prefixFor(unprivateName);
-    if (prefix == null) return name;
+  ///
+  /// 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);
+    }
 
-    var withoutPrefix = unprivateName.substring(prefix.length);
     if (_isPrivate(withoutPrefix)) {
       withoutPrefix = _privateToPublic(withoutPrefix);
     }
-    return (isPrivate ? '-' : '') + withoutPrefix;
+    return (isPrivate && !forcePublic ? '-' : '') + withoutPrefix;
   }
 
   /// Returns the namespace that built-in module [module] is loaded under.

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;

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;
+}

test/migrators/module/scope/pseudoprivate/double_dash.hrx

@@ -7,7 +7,8 @@ a {
 }
 
 <==> input/_library.scss
-$--var: red;
+$--red: red;
+$--var: $--red;
 
 <==> output/entrypoint.scss
 @use "library";
@@ -16,4 +17,5 @@ a {
 }
 
 <==> output/_library.scss
-$var: red;
+$--red: red;
+$var: $--red;

test/migrators/module/scope/pseudoprivate/underscore.hrx

@@ -7,7 +7,8 @@ a {
 }
 
 <==> input/_library.scss
-$_var: red;
+$_red: red;
+$_var: $_red;
 
 <==> output/entrypoint.scss
 @use "library";
@@ -16,4 +17,5 @@ a {
 }
 
 <==> output/_library.scss
-$var: red;
+$_red: red;
+$var: $_red;