Skip to content

Commit 74bfa89

Browse files
nex3Tofandeljathak
authored
Fix the conversion of private names to public names (#275)
Co-authored-by: Adrien Foulon <6115458+Tofandel@users.noreply.github.com> Co-authored-by: Jennifer Thakar <jathak@gmail.com>
1 parent 4cc80c4 commit 74bfa89

File tree

14 files changed

+315
-46
lines changed

14 files changed

+315
-46
lines changed

‎CHANGELOG.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,11 @@
1+
## 2.3.3
2+
3+
### Module System Migration
4+
5+
* Fix some bugs in the conversion of private names that are referenced across
6+
files to public names, especially when `--remove-prefix` and/or multiple
7+
leading dashes/underscores are involved.
8+
19
## 2.3.2
210

311
* Update to be compatible with the latest version of the Dart Sass AST.

‎lib/src/migrators/module.dart‎

Lines changed: 60 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import 'package:args/args.dart';
88
import 'package:collection/collection.dart';
9+
import 'package:charcode/charcode.dart';
910
import 'package:path/path.dart' as p;
1011
import 'package:sass_api/sass_api.dart';
1112
import 'package:source_span/source_span.dart';
@@ -263,7 +264,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
263264
var forwardsByUrl = <Uri, Map<String, Set<MemberDeclaration>>>{};
264265
var hiddenByUrl = <Uri, Set<MemberDeclaration>>{};
265266
for (var declaration in references.globalDeclarations) {
266-
var private = declaration.name.startsWith('-');
267+
var private = _isPrivate(declaration.name);
267268

268269
// Whether this member will be exposed by the regular entrypoint.
269270
var visibleAtEntrypoint = !private &&
@@ -352,19 +353,38 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
352353
if (declaration.isForwarded) return;
353354

354355
var name = declaration.name;
355-
if (name.startsWith('-') &&
356-
references.referencedOutsideDeclaringStylesheet(declaration)) {
357-
// Remove leading `-` since private members can't be accessed outside
358-
// the module they're declared in.
359-
name = name.substring(1);
360-
}
361-
name = _unprefix(name);
356+
name = _unprefix(name,
357+
// Private members can't be accessed outside the module they're
358+
// declared in.
359+
forcePublic:
360+
references.referencedOutsideDeclaringStylesheet(declaration));
362361
if (name != declaration.name) {
363362
renamedMembers[declaration] = name;
364363
if (_upstreamStylesheets.isEmpty) _needsImportOnly = true;
365364
}
366365
}
367366

367+
/// Returns whether [identifier] is a private member name.
368+
///
369+
/// Assumes [identifier] is a valid CSS identifier.
370+
bool _isPrivate(String identifier) {
371+
return identifier.startsWith('-') || identifier.startsWith('_');
372+
}
373+
374+
/// Converts a private identifier to a public one.
375+
String _privateToPublic(String identifier) {
376+
if (!_isPrivate(identifier)) return identifier;
377+
378+
for (var i = 0; i < identifier.length; i++) {
379+
var char = identifier.codeUnitAt(i);
380+
if (char != $dash && char != $underscore) {
381+
return identifier.substring(i);
382+
}
383+
}
384+
385+
return 'private$identifier';
386+
}
387+
368388
/// Returns whether the member named [name] should be forwarded in the
369389
/// entrypoint.
370390
///
@@ -1025,8 +1045,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
10251045
newName = declaration.name.substring(importOnlyPrefix.length);
10261046
}
10271047

1028-
if (_shouldForward(declaration.name) &&
1029-
!declaration.name.startsWith('-')) {
1048+
if (_shouldForward(declaration.name) && !_isPrivate(declaration.name)) {
10301049
var subprefix = "";
10311050
if (importOnlyPrefix != null) {
10321051
var prefix = _prefixFor(declaration.name);
@@ -1036,7 +1055,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
10361055
}
10371056
if (declaration.name != newName) _needsImportOnly = true;
10381057
shownByPrefix.putIfAbsent(subprefix, () => {}).add(declaration);
1039-
} else if (!newName.startsWith('-')) {
1058+
} else if (!_isPrivate(newName)) {
10401059
hidden.add(declaration);
10411060
}
10421061
}
@@ -1076,8 +1095,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
10761095
if (declaration is ImportOnlyMemberDeclaration) {
10771096
name = name.substring(declaration.importOnlyPrefix.length);
10781097
}
1079-
if (name.startsWith('-')) name = name.substring(1);
1080-
name = _unprefix(name);
1098+
name = _unprefix(name, forcePublic: true);
10811099
if (subprefix.isNotEmpty) name = '$subprefix$name';
10821100
if (declaration.member is VariableDeclaration) name = '\$$name';
10831101
allHidden.add(name);
@@ -1205,7 +1223,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
12051223
void _renameReference(FileSpan span, MemberDeclaration declaration) {
12061224
var newName = renamedMembers[declaration];
12071225
if (newName != null) {
1208-
if (newName.startsWith('-') &&
1226+
if (_isPrivate(newName) &&
12091227
declaration.name.endsWith(newName.substring(1))) {
12101228
addPatch(patchDelete(span,
12111229
start: 1, end: declaration.name.length - newName.length + 1));
@@ -1225,14 +1243,36 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
12251243

12261244
/// If [name] starts with any of [prefixesToRemove], returns it with the
12271245
/// longest matching prefix removed.
1246+
/// If forcePublic is true, any leading dash or underscore will be removed.
12281247
///
12291248
/// Otherwise, returns [name] unaltered.
1230-
String _unprefix(String name) {
1231-
var isPrivate = name.startsWith('-');
1232-
var unprivateName = isPrivate ? name.substring(1) : name;
1233-
var prefix = _prefixFor(unprivateName);
1234-
if (prefix == null) return name;
1235-
return (isPrivate ? '-' : '') + unprivateName.substring(prefix.length);
1249+
///
1250+
/// If [forcePublic] is true, this will ensure that the name is always a
1251+
/// public identifier.
1252+
String _unprefix(String name, {bool forcePublic = false}) {
1253+
bool isPrivate;
1254+
String withoutPrefix;
1255+
1256+
// Allow users to pass prefixes that either do or do not include leading
1257+
// dashes/underscores. As usual, the longest prefix wins, so we check for
1258+
// ones that do include them first.
1259+
var prefix = _prefixFor(name);
1260+
if (prefix != null) {
1261+
isPrivate = false;
1262+
withoutPrefix = name.substring(prefix.length);
1263+
} else {
1264+
isPrivate = _isPrivate(name);
1265+
var unprivateName = isPrivate ? _privateToPublic(name) : name;
1266+
prefix = _prefixFor(unprivateName);
1267+
if (prefix == null)
1268+
return isPrivate && forcePublic ? unprivateName : name;
1269+
withoutPrefix = unprivateName.substring(prefix.length);
1270+
}
1271+
1272+
if (_isPrivate(withoutPrefix)) {
1273+
withoutPrefix = _privateToPublic(withoutPrefix);
1274+
}
1275+
return (isPrivate && !forcePublic ? '-' : '') + withoutPrefix;
12361276
}
12371277

12381278
/// Returns the namespace that built-in module [module] is loaded under.

‎pubspec.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
name: sass_migrator
2-
version: 2.3.2
2+
version: 2.3.3
33
description: A tool for running migrations on Sass files
44
homepage: https://github.com/sass/migrator
55

‎test/migrators/module/remove_prefix_flag/pseudoprivate_multi_dash.hrx‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<==> arguments
2+
--migrate-deps --remove-prefix=lib-
3+
4+
<==> input/entrypoint.scss
5+
@import "dependency";
6+
7+
a {
8+
background: $--lib-pseudoprivate;
9+
}
10+
11+
<==> input/_dependency.scss
12+
$--lib-pseudoprivate: blue;
13+
14+
<==> output/entrypoint.scss
15+
@use "dependency";
16+
17+
a {
18+
background: dependency.$pseudoprivate;
19+
}
20+
21+
<==> output/_dependency.scss
22+
$pseudoprivate: blue;

‎test/migrators/module/remove_prefix_flag/with_dash.hrx‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<==> arguments
2+
--migrate-deps --remove-prefix=--app-
3+
<==> input/entrypoint.scss
4+
@import "library";
5+
a {
6+
color: $--app-var;
7+
background: $__app-var2;
8+
}
9+
10+
<==> input/_library.scss
11+
$--app-red: red;
12+
$--app-var: $--app-red;
13+
$__app-var2: blue;
14+
15+
<==> output/entrypoint.scss
16+
@use "library";
17+
a {
18+
color: library.$var;
19+
background: library.$var2;
20+
}
21+
22+
<==> output/_library.scss
23+
$red: red;
24+
$var: $red;
25+
$var2: blue;

‎test/migrators/module/remove_prefix_flag/with_leading_dash.hrx‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<==> arguments
2+
--remove-prefix=--lib-
3+
4+
<==> input/entrypoint.scss
5+
$--lib-a: 5;
6+
$--lib_b: $--lib-a + 2;
7+
8+
@function --lib-fn($--lib-local) {
9+
@return $--lib-local;
10+
}
11+
12+
@mixin --lib-mixin {
13+
--lib-property: --lib-value;
14+
}
15+
16+
--lib-selector {
17+
property: --lib-fn(0);
18+
@include --lib-mixin;
19+
}
20+
21+
<==> output/entrypoint.scss
22+
$a: 5;
23+
$b: $a + 2;
24+
25+
@function fn($--lib-local) {
26+
@return $--lib-local;
27+
}
28+
29+
@mixin mixin {
30+
--lib-property: --lib-value;
31+
}
32+
33+
--lib-selector {
34+
property: fn(0);
35+
@include mixin;
36+
}

‎test/migrators/module/remove_prefix_flag/without_dash.hrx‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<==> arguments
2+
--remove-prefix=lib
3+
4+
<==> input/entrypoint.scss
5+
$lib-a: 5;
6+
$lib_b: $lib-a + 2;
7+
8+
@function lib-fn($lib-local) {
9+
@return $lib-local;
10+
}
11+
12+
@mixin lib-mixin {
13+
lib-property: lib-value;
14+
}
15+
16+
lib-selector {
17+
lib-property: lib-fn(0);
18+
@include lib-mixin;
19+
}
20+
21+
<==> output/entrypoint.scss
22+
$a: 5;
23+
$b: $a + 2;
24+
25+
@function fn($lib-local) {
26+
@return $lib-local;
27+
}
28+
29+
@mixin mixin {
30+
lib-property: lib-value;
31+
}
32+
33+
lib-selector {
34+
lib-property: fn(0);
35+
@include mixin;
36+
}
37+
38+
<==> output/entrypoint.import.scss
39+
@forward "entrypoint" as lib-*;

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

Lines changed: 0 additions & 25 deletions
This file was deleted.

‎test/migrators/module/scope/pseudoprivate/dash.hrx‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<==> arguments
2+
--migrate-deps
3+
<==> input/entrypoint.scss
4+
@import "library";
5+
a {
6+
color: $-var;
7+
}
8+
9+
<==> input/_library.scss
10+
$-red: red;
11+
$-var: $-red;
12+
13+
<==> output/entrypoint.scss
14+
@use "library";
15+
a {
16+
color: library.$var;
17+
}
18+
19+
<==> output/_library.scss
20+
$-red: red;
21+
$var: $-red;

‎test/migrators/module/scope/pseudoprivate/dashes_only.hrx‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<==> arguments
2+
--migrate-deps
3+
<==> input/entrypoint.scss
4+
@import "library";
5+
a {
6+
color: $---;
7+
}
8+
9+
<==> input/_library.scss
10+
$---: red;
11+
12+
<==> output/entrypoint.scss
13+
@use "library";
14+
a {
15+
color: library.$private---;
16+
}
17+
18+
<==> output/_library.scss
19+
$private---: red;

0 commit comments

Comments
 (0)