Add tests and fix behavior of remove prefix

lib/src/migrators/module.dart

@@ -267,7 +267,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
     var forwardsByUrl = <Uri, Map<String, Set<MemberDeclaration>>>{};
     var hiddenByUrl = <Uri, Set<MemberDeclaration>>{};
     for (var declaration in references.globalDeclarations) {
-      var private = _isPrivate(declaration.name);
+      var private = isPrivate(declaration.name);
 
       // Whether this member will be exposed by the regular entrypoint.
       var visibleAtEntrypoint = !private &&
@@ -356,38 +356,27 @@ 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, 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) {
-    assert(_isPrivate(identifier));
-    for (var i = 0; i < identifier.length; i++) {
-      var char = identifier.codeUnitAt(i);
-      if (char != $dash && char != $underscore) {
-        return identifier.substring(i);
+    if (isPrivate(identifier)) {
+      for (var i = 0; i < identifier.length; i++) {
+        var char = identifier.codeUnitAt(i);
+        if (char != $dash && char != $underscore) {
+          return identifier.substring(i);
+        }
       }
-    }
 
-    _generatedVariables++;
-    return 'var${_generatedVariables}';
+      _generatedVariables++;
+      return 'var${_generatedVariables}';
+    }
+    return identifier;
   }
 
   /// Returns whether the member named [name] should be forwarded in the
@@ -1050,7 +1039,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
         newName = declaration.name.substring(importOnlyPrefix.length);
       }
 
-      if (_shouldForward(declaration.name) && !_isPrivate(declaration.name)) {
+      if (_shouldForward(declaration.name) && !isPrivate(declaration.name)) {
         var subprefix = "";
         if (importOnlyPrefix != null) {
           var prefix = _prefixFor(declaration.name);
@@ -1060,7 +1049,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
         }
         if (declaration.name != newName) _needsImportOnly = true;
         shownByPrefix.putIfAbsent(subprefix, () => {}).add(declaration);
-      } else if (!_isPrivate(newName)) {
+      } else if (!isPrivate(newName)) {
         hidden.add(declaration);
       }
     }
@@ -1100,8 +1089,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);
@@ -1229,7 +1217,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
   void _renameReference(FileSpan span, MemberDeclaration declaration) {
     var newName = renamedMembers[declaration];
     if (newName != null) {
-      if (_isPrivate(newName) &&
+      if (isPrivate(newName) &&
           declaration.name.endsWith(newName.substring(1))) {
         addPatch(patchDelete(span,
             start: 1, end: declaration.name.length - newName.length + 1));
@@ -1249,19 +1237,32 @@ 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 = _isPrivate(name);
-    var unprivateName = isPrivate ? _privateToPublic(name) : name;
-    var prefix = _prefixFor(unprivateName);
-    if (prefix == null) return name;
-
-    var withoutPrefix = unprivateName.substring(prefix.length);
-    if (_isPrivate(withoutPrefix)) {
-      withoutPrefix = _privateToPublic(withoutPrefix);
+  String _unprefix(String name, {bool forcePublic = false}) {
+    var unprivateName = _privateToPublic(name);
+    var wasPrivate = unprivateName != name;
+    var prefix = _prefixFor(name);
+    var withoutPrefix = '';
+    var privatePrefix = '';
+    if (wasPrivate && prefix == null) {
+      prefix = _prefixFor(unprivateName);
+      if (prefix != null) {
+        privatePrefix = name.substring(0, name.length - unprivateName.length);
+        withoutPrefix = _privateToPublic(unprivateName.substring(prefix.length));
+      }
+    } else if (prefix != null) {
+      withoutPrefix = name.substring(prefix.length);
+      var withoutPrefixPublic = _privateToPublic(withoutPrefix);
+      if (wasPrivate && withoutPrefixPublic != withoutPrefix) {
+        privatePrefix = withoutPrefix.substring(0, withoutPrefix.length - withoutPrefixPublic.length);
+      }
+      withoutPrefix = withoutPrefixPublic;
     }
-    return (isPrivate ? '-' : '') + withoutPrefix;
+    if (prefix == null) return forcePublic ? unprivateName : name;
+
+    return forcePublic ? _privateToPublic(withoutPrefix) : privatePrefix + withoutPrefix;
   }
 
   /// Returns the namespace that built-in module [module] is loaded under.

lib/src/utils.dart

@@ -59,6 +59,13 @@ extension NullableExtension<T> on T? {
   }
 }
 
+/// Returns whether [identifier] is a private member name.
+///
+/// Assumes [identifier] is a valid CSS identifier.
+bool isPrivate(String identifier) {
+    return identifier.startsWith('-') || identifier.startsWith('_');
+}
+
 /// Returns the default namespace for a use rule with [path].
 String namespaceForPath(String path) {
   // TODO(jathak): Confirm that this is a valid Sass identifier

package.json

@@ -8,6 +8,9 @@
     "type": "git",
     "url": "https://github.com/sass/migrator"
   },
+  "scripts": {
+    "test": "dart run grinder pkg-npm-dev && dart pub run test -t node -r expanded"
+  },
   "author": {
     "name": "Jennifer Thakar",
     "email": "jathak@google.com",

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;

test/migrators/module/scope/pseudoprivate/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/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;

test/migrators/module/scope/pseudoprivate/indirect.hrx → test/migrators/module/scope/pseudoprivate/with_prefix.hrx

@@ -1,21 +1,21 @@
 <==> arguments
---migrate-deps
+--migrate-deps --remove-prefix=app
 <==> input/entrypoint.scss
 @import "library";
 a {
-  color: $middle;
+  color: $app-var;
 }
 
 <==> input/_library.scss
-$-upstream: red;
-$middle: $-upstream;
+$_app-red: red;
+$app-var: $_app-red;
 
 <==> output/entrypoint.scss
 @use "library";
 a {
-  color: library.$middle;
+  color: library.$var;
 }
 
 <==> output/_library.scss
-$-upstream: red;
-$middle: $-upstream;
+$_red: red;
+$var: $_red;