From b4fc3d47df4d242f31162127dcee2f5832a5ccd6 Mon Sep 17 00:00:00 2001 From: Jennifer Thakar Date: Wed, 28 Apr 2021 16:57:39 -0700 Subject: [PATCH] Migrate to null safety (#183) --- .github/workflows/ci.yml | 2 + CHANGELOG.md | 4 + bin/sass_migrator.dart | 3 +- lib/src/migration_visitor.dart | 32 +- lib/src/migrator.dart | 28 +- lib/src/migrators/division.dart | 15 +- lib/src/migrators/module.dart | 290 ++++++++++-------- .../migrators/module/member_declaration.dart | 10 +- .../migrators/module/reference_source.dart | 14 +- lib/src/migrators/module/references.dart | 160 +++++----- lib/src/migrators/module/scope.dart | 8 +- .../module/unreferencable_members.dart | 8 +- lib/src/migrators/namespace.dart | 51 +-- lib/src/node_interop_stub.dart | 2 +- lib/src/patch.dart | 2 +- lib/src/renamer.dart | 25 +- lib/src/runner.dart | 18 +- lib/src/util/bidirectional_map.dart | 14 +- lib/src/util/node_modules_importer.dart | 6 +- .../unmodifiable_bidirectional_map_view.dart | 2 +- lib/src/utils.dart | 38 ++- pubspec.yaml | 56 ++-- ...se_and_import.hrx => mixed_use_import.hrx} | 0 .../mixed_use_import_no_namespace.hrx | 38 +++ test/utils.dart | 33 +- tool/grind.dart | 4 +- 26 files changed, 499 insertions(+), 364 deletions(-) rename test/migrators/module/partial_migration/{file_mixing_use_and_import.hrx => mixed_use_import.hrx} (100%) create mode 100644 test/migrators/module/partial_migration/mixed_use_import_no_namespace.hrx diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 052abe5e..67b28810 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -84,6 +84,8 @@ jobs: - run: dart pub get - name: Analyze dart run: dartanalyzer --fatal-warnings --fatal-infos lib tool test + - name: Check formatting + run: dartfmt -n --set-exit-if-changed . sanity_checks: name: Sanity checks diff --git a/CHANGELOG.md b/CHANGELOG.md index 4af75661..62ad5fbf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## 1.3.8 + +* No user-visible changes. + ## 1.3.7 ### Module Migrator diff --git a/bin/sass_migrator.dart b/bin/sass_migrator.dart index 11883bbb..cbe58833 100644 --- a/bin/sass_migrator.dart +++ b/bin/sass_migrator.dart @@ -12,6 +12,7 @@ import 'package:sass_migrator/src/runner.dart'; // We can't declare args as a List or Iterable beacause of // dart-lang/sdk#36627. main(Iterable args) { - if (process.argv != null) args = process.argv.skip(2); + var argv = process.argv; + if (argv != null) args = argv.skip(2); MigratorRunner().execute(args.cast()); } diff --git a/lib/src/migration_visitor.dart b/lib/src/migration_visitor.dart index 5b554c9d..5f589d67 100644 --- a/lib/src/migration_visitor.dart +++ b/lib/src/migration_visitor.dart @@ -51,12 +51,13 @@ abstract class MigrationVisitor extends RecursiveAstVisitor { /// The patches to be applied to the stylesheet being migrated. @protected List get patches => UnmodifiableListView(_patches); - List _patches; + List get _patches => assertInStylesheet(__patches, 'patches'); + List? __patches; /// URL of the stylesheet currently being migrated. @protected - Uri get currentUrl => _currentUrl; - Uri _currentUrl; + Uri get currentUrl => assertInStylesheet(_currentUrl, 'currentUrl'); + Uri? _currentUrl; /// The importer that's being used to resolve relative imports. /// @@ -64,7 +65,7 @@ abstract class MigrationVisitor extends RecursiveAstVisitor { /// stylesheet. @protected Importer get importer => _importer; - Importer _importer; + late Importer _importer; MigrationVisitor(this.importCache, this.migrateDependencies); @@ -84,10 +85,10 @@ abstract class MigrationVisitor extends RecursiveAstVisitor { /// file's state before calling the super method and restore it afterwards. @override void visitStylesheet(Stylesheet node) { - var oldPatches = _patches; + var oldPatches = __patches; var oldUrl = _currentUrl; - _patches = []; - _currentUrl = node.span.sourceUrl; + __patches = []; + _currentUrl = node.span.sourceUrl!; super.visitStylesheet(node); beforePatch(node); var results = patches.isNotEmpty @@ -102,10 +103,10 @@ abstract class MigrationVisitor extends RecursiveAstVisitor { "it's loaded."); } - _migrated[_currentUrl] = results; + _migrated[currentUrl] = results; } - _patches = oldPatches; + __patches = oldPatches; _currentUrl = oldUrl; } @@ -139,7 +140,7 @@ abstract class MigrationVisitor extends RecursiveAstVisitor { _importer = oldImporter; } else { _missingDependencies.putIfAbsent( - context.sourceUrl.resolveUri(dependency), () => context); + context.sourceUrl!.resolveUri(dependency), () => context); } } @@ -180,4 +181,15 @@ abstract class MigrationVisitor extends RecursiveAstVisitor { visitDependency(node.url, node.span); } } + + /// Asserts that [value] is not `null` and returns it. + /// + /// This is used for fields that are set whenever the migrator is visiting + /// a stylesheet, which means they should be non-null almost all the time + /// during a call to [run]. + @protected + T assertInStylesheet(T? value, String name) { + if (value != null) return value; + throw StateError("Can't access $name when not visiting a stylesheet."); + } } diff --git a/lib/src/migrator.dart b/lib/src/migrator.dart index 0383a9f6..62c3c20e 100644 --- a/lib/src/migrator.dart +++ b/lib/src/migrator.dart @@ -14,6 +14,7 @@ import 'package:sass/src/import_cache.dart'; import 'package:args/command_runner.dart'; import 'package:glob/glob.dart'; +import 'package:glob/list_local_fs.dart'; import 'package:meta/meta.dart'; import 'package:path/path.dart' as p; import 'package:sass_migrator/src/util/node_modules_importer.dart'; @@ -45,7 +46,7 @@ abstract class Migrator extends Command> { "See also https://sass-lang.com/documentation/cli/migrator#$name"; /// If true, dependencies will be migrated in addition to the entrypoints. - bool get migrateDependencies => globalResults['migrate-deps'] as bool; + bool get migrateDependencies => globalResults!['migrate-deps'] as bool; /// Map of missing dependency URLs to the spans that import/use them. /// @@ -75,11 +76,12 @@ abstract class Migrator extends Command> { Map run() { var allMigrated = {}; var importer = FilesystemImporter('.'); - var importCache = ImportCache([NodeModulesImporter()], - loadPaths: globalResults['load-path']); + var importCache = ImportCache( + importers: [NodeModulesImporter()], + loadPaths: globalResults!['load-path']); var entrypoints = [ - for (var argument in argResults.rest) + for (var argument in argResults!.rest) for (var entry in Glob(argument).listSync()) if (entry is File) entry.path ]; @@ -91,15 +93,14 @@ abstract class Migrator extends Command> { } var migrated = migrateFile(importCache, tuple.item2, tuple.item1); - for (var file in migrated.keys) { - if (allMigrated.containsKey(file) && - migrated[file] != allMigrated[file]) { + migrated.forEach((file, contents) { + if (allMigrated.containsKey(file) && contents != allMigrated[file]) { throw MigrationException( "The migrator has found multiple possible migrations for $file, " "depending on the context in which it's loaded."); } - allMigrated[file] = migrated[file]; - } + allMigrated[file] = contents; + }); } if (missingDependencies.isNotEmpty) _warnForMissingDependencies(); @@ -114,7 +115,7 @@ abstract class Migrator extends Command> { /// In verbose mode, this instead prints a full warning with the source span /// for each missing dependency. void _warnForMissingDependencies() { - if (globalResults['verbose'] as bool) { + if (globalResults!['verbose'] as bool) { for (var uri in missingDependencies.keys) { emitWarning("Could not find Sass file at '${p.prettyUri(uri)}'.", missingDependencies[uri]); @@ -123,11 +124,10 @@ abstract class Migrator extends Command> { var count = missingDependencies.length; emitWarning( "$count dependenc${count == 1 ? 'y' : 'ies'} could not be found."); - for (var uri in missingDependencies.keys) { - var context = missingDependencies[uri]; - printStderr(' ${p.prettyUri(uri)} ' + missingDependencies.forEach((url, context) { + printStderr(' ${p.prettyUri(url)} ' '@${p.prettyUri(context.sourceUrl)}:${context.start.line + 1}'); - } + }); } } } diff --git a/lib/src/migrators/division.dart b/lib/src/migrators/division.dart index 88bb7d16..a2e54c8a 100644 --- a/lib/src/migrators/division.dart +++ b/lib/src/migrators/division.dart @@ -38,7 +38,7 @@ More info: https://sass-lang.com/d/slash-div"""; help: "Only migrate / expressions that are unambiguously division.", negatable: false); - bool get isPessimistic => argResults['pessimistic'] as bool; + bool get isPessimistic => argResults!['pessimistic'] as bool; @override Map migrateFile( @@ -143,16 +143,16 @@ class _DivisionMigrationVisitor extends MigrationVisitor { return false; } - ListExpression channels; + ListExpression? channels; if (node.arguments.positional.length == 1 && node.arguments.named.isEmpty && node.arguments.positional.first is ListExpression) { - channels = node.arguments.positional.first; + channels = node.arguments.positional.first as ListExpression?; } else if (node.arguments.positional.isEmpty && node.arguments.named.containsKey(r'$channels') && node.arguments.named.length == 1 && node.arguments.named[r'$channels'] is ListExpression) { - channels = node.arguments.named[r'$channels']; + channels = node.arguments.named[r'$channels'] as ListExpression?; } if (channels == null || channels.hasBrackets || @@ -170,7 +170,8 @@ class _DivisionMigrationVisitor extends MigrationVisitor { _patchOperatorToComma(last); } _withContext(() { - channels.contents[0].accept(this); + // Non-null assertion is required because of dart-lang/language#1536. + channels!.contents[0].accept(this); channels.contents[1].accept(this); last.left.accept(this); }, isDivisionAllowed: true); @@ -326,7 +327,7 @@ class _DivisionMigrationVisitor extends MigrationVisitor { /// ParenthesizedExpression. void _patchParensIfAny(SassNode node) { if (node is! ParenthesizedExpression) return; - var expression = (node as ParenthesizedExpression).expression; + var expression = node.expression; if (expression is BinaryOperationExpression && expression.operator == BinaryOperator.dividedBy) { return; @@ -337,7 +338,7 @@ class _DivisionMigrationVisitor extends MigrationVisitor { /// Runs [operation] with the given context. void _withContext(void operation(), - {bool isDivisionAllowed, bool expectsNumericResult}) { + {bool? isDivisionAllowed, bool? expectsNumericResult}) { var previousDivisionAllowed = _isDivisionAllowed; var previousNumericResult = _expectsNumericResult; if (isDivisionAllowed != null) _isDivisionAllowed = isDivisionAllowed; diff --git a/lib/src/migrators/module.dart b/lib/src/migrators/module.dart index b038a02f..6373e9e6 100644 --- a/lib/src/migrators/module.dart +++ b/lib/src/migrators/module.dart @@ -84,9 +84,9 @@ class ModuleMigrator extends Migrator { /// a map of migrated contents. Map migrateFile( ImportCache importCache, Stylesheet stylesheet, Importer importer) { - var forwards = {for (var arg in argResults['forward']) ForwardType(arg)}; + var forwards = {for (var arg in argResults!['forward']) ForwardType(arg)}; if (forwards.contains(ForwardType.prefixed) && - !argResults.wasParsed('remove-prefix')) { + !argResults!.wasParsed('remove-prefix')) { throw MigrationException( 'You must provide --remove-prefix with --forward=prefixed so we know ' 'which prefixed members to forward.'); @@ -94,9 +94,9 @@ class ModuleMigrator extends Migrator { var references = References(importCache, stylesheet, importer); var visitor = _ModuleMigrationVisitor(importCache, references, - globalResults['load-path'] as List, migrateDependencies, - prefixesToRemove: (argResults['remove-prefix'] as List) - ?.map((prefix) => prefix.replaceAll('_', '-')), + globalResults!['load-path'] as List, migrateDependencies, + prefixesToRemove: (argResults!['remove-prefix'] as List) + .map((prefix) => prefix.replaceAll('_', '-')), forwards: forwards); var migrated = visitor.run(stylesheet, importer); _filesWithRenamedDeclarations.addAll( @@ -127,38 +127,49 @@ class _ModuleMigrationVisitor extends MigrationVisitor { final _originalImports = >{}; /// Tracks members that are unreferencable in the current scope. - UnreferencableMembers _unreferencable = UnreferencableMembers(); + var _unreferencable = UnreferencableMembers(); /// Namespaces of modules used in this stylesheet. - Map _namespaces; + Map get _namespaces => + assertInStylesheet(__namespaces, '_namespaces'); + Map? __namespaces; /// Set of canonical URLs that have a `@forward` rule in the current /// stylesheet. - Set _forwardedUrls; + Set get _forwardedUrls => + assertInStylesheet(__forwardedUrls, '_forwardedUrls'); + Set? __forwardedUrls; /// Set of canonical URLs that have a `@use` rule in the current stylesheet. /// /// This includes rules migrated from `@import` rules, additional rules in the /// sets below, and existing `@use` rules in the file prior to migration. - Set _usedUrls; + Set get _usedUrls => assertInStylesheet(__usedUrls, '_usedUrls'); + Set? __usedUrls; /// Set of additional `@use` rules for built-in modules. - Set _builtInUseRules; + Set get _builtInUseRules => + assertInStylesheet(__builtInUseRules, '_builtInUseRules'); + Set? __builtInUseRules; /// Set of additional `@use` rules for stylesheets at a load path. - Set _additionalLoadPathUseRules; + Set get _additionalLoadPathUseRules => assertInStylesheet( + __additionalLoadPathUseRules, '_additionalLoadPathUseRules'); + Set? __additionalLoadPathUseRules; /// Set of additional `@use` rules for stylesheets relative to the current /// one. - Set _additionalRelativeUseRules; + Set get _additionalRelativeUseRules => assertInStylesheet( + __additionalRelativeUseRules, '_additionalRelativeUseRules'); + Set? __additionalRelativeUseRules; /// The first `@import` rule in this stylesheet that was converted to a `@use` /// or `@forward` rule, or null if none has been visited yet. - FileLocation _beforeFirstImport; + FileLocation? _beforeFirstImport; /// The last `@import` rule in this stylesheet that was converted to a `@use` /// or `@forward` rule, or null if none has been visited yet. - FileLocation _afterLastImport; + FileLocation? _afterLastImport; /// Whether @use and @forward are allowed in the current context. var _useAllowed = true; @@ -171,7 +182,11 @@ class _ModuleMigrationVisitor extends MigrationVisitor { /// Set of variables declared outside the current stylesheet that overrode /// `!default` variables within the current stylesheet. - Set> _configuredVariables; + Set> get _configuredVariables => + __configuredVariables ?? + (throw StateError( + "Can't access _configuredVariables when not visiting a dependency.")); + Set>? __configuredVariables; /// A mapping between member declarations and references. /// @@ -205,12 +220,10 @@ class _ModuleMigrationVisitor extends MigrationVisitor { /// This converts the OS-specific relative [loadPaths] to absolute URL paths. _ModuleMigrationVisitor(this.importCache, this.references, List loadPaths, bool migrateDependencies, - {Iterable prefixesToRemove, this.forwards}) + {Iterable prefixesToRemove = const [], this.forwards = const {}}) : loadPaths = List.unmodifiable( loadPaths.map((path) => p.toUri(p.absolute(path)).path)), - prefixesToRemove = prefixesToRemove == null - ? const {} - : UnmodifiableSetView(prefixesToRemove.toSet()), + prefixesToRemove = UnmodifiableSetView(prefixesToRemove.toSet()), super(importCache, migrateDependencies); /// Checks which global declarations need to be renamed, then runs the @@ -221,7 +234,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor { var migrated = super.run(stylesheet, importer); if (forwards.contains(ForwardType.importOnly) || _needsImportOnly) { - var url = stylesheet.span.sourceUrl; + var url = stylesheet.span.sourceUrl!; var importOnlyUrl = getImportOnlyUrl(url); var results = _generateImportOnly(url, importOnlyUrl); if (results != null) migrated[importOnlyUrl] = results; @@ -238,7 +251,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor { /// /// If there are no previously-prefixed members or members from dependencies /// to forward, this returns null. - String _generateImportOnly(Uri entrypoint, Uri importOnlyUrl) { + String? _generateImportOnly(Uri entrypoint, Uri importOnlyUrl) { // Sort all members based on the URL they should be forwarded from and the // prefix they require (if any). var forwardsByUrl = >>{}; @@ -267,7 +280,6 @@ class _ModuleMigrationVisitor extends MigrationVisitor { var prefix = ''; if (declaration is ImportOnlyMemberDeclaration && - declaration.importOnlyPrefix != null && url == declaration.sourceUrl) { prefix = declaration.importOnlyPrefix; } else { @@ -299,7 +311,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor { // import-only file is necessary. if (forwardsByUrl.isEmpty || (forwardsByUrl.length == 1 && - forwardsByUrl[entrypoint]?.keys?.join() == '')) { + forwardsByUrl[entrypoint]?.keys.join() == '')) { return null; } @@ -307,9 +319,9 @@ class _ModuleMigrationVisitor extends MigrationVisitor { // that the import-only file still includes its CSS. var dependency = _absoluteUrlToDependency(entrypoint, relativeTo: importOnlyUrl).item1; - var entrypointForwards = forwardsByUrl.containsKey(entrypoint) - ? _forwardRulesForShown( - entrypoint, '"$dependency"', forwardsByUrl.remove(entrypoint), {}) + var forwards = forwardsByUrl.remove(entrypoint); + var entrypointForwards = forwards != null + ? _forwardRulesForShown(entrypoint, '"$dependency"', forwards, {}) : ['@forward "$dependency"']; var tuples = [ for (var entry in forwardsByUrl.entries) @@ -393,16 +405,16 @@ class _ModuleMigrationVisitor extends MigrationVisitor { /// visiting it and restores the per-file state afterwards. @override void visitStylesheet(Stylesheet node) { - var oldNamespaces = _namespaces; - var oldForwardedUrls = _forwardedUrls; - var oldUsedUrls = _usedUrls; - var oldBuiltInUseRules = _builtInUseRules; - var oldLoadPathUseRules = _additionalLoadPathUseRules; - var oldRelativeUseRules = _additionalRelativeUseRules; + var oldNamespaces = __namespaces; + var oldForwardedUrls = __forwardedUrls; + var oldUsedUrls = __usedUrls; + var oldBuiltInUseRules = __builtInUseRules; + var oldLoadPathUseRules = __additionalLoadPathUseRules; + var oldRelativeUseRules = __additionalRelativeUseRules; var oldBeforeFirstImport = _beforeFirstImport; var oldAfterLastImport = _afterLastImport; var oldUseAllowed = _useAllowed; - _namespaces = { + __namespaces = { for (var rule in node.uses) importCache .canonicalize(rule.url, @@ -410,22 +422,22 @@ class _ModuleMigrationVisitor extends MigrationVisitor { ?.item2 ?? rule.url: rule.namespace }; - _determineNamespaces(node.span.sourceUrl, _namespaces); - _usedUrls = {}; - _forwardedUrls = {}; - _builtInUseRules = {}; - _additionalLoadPathUseRules = {}; - _additionalRelativeUseRules = {}; + _determineNamespaces(node.span.sourceUrl!, _namespaces); + __usedUrls = {}; + __forwardedUrls = {}; + __builtInUseRules = {}; + __additionalLoadPathUseRules = {}; + __additionalRelativeUseRules = {}; _beforeFirstImport = null; _afterLastImport = null; _useAllowed = true; super.visitStylesheet(node); - _namespaces = oldNamespaces; - _forwardedUrls = oldForwardedUrls; - _usedUrls = oldUsedUrls; - _builtInUseRules = oldBuiltInUseRules; - _additionalLoadPathUseRules = oldLoadPathUseRules; - _additionalRelativeUseRules = oldRelativeUseRules; + __namespaces = oldNamespaces; + __forwardedUrls = oldForwardedUrls; + __usedUrls = oldUsedUrls; + __builtInUseRules = oldBuiltInUseRules; + __additionalLoadPathUseRules = oldLoadPathUseRules; + __additionalRelativeUseRules = oldRelativeUseRules; _beforeFirstImport = oldBeforeFirstImport; _afterLastImport = oldAfterLastImport; _useAllowed = oldUseAllowed; @@ -470,17 +482,16 @@ class _ModuleMigrationVisitor extends MigrationVisitor { /// /// [namespaces] should contain any existing namespaces prior to migration /// (i.e. from existing `@use` rules). - void _determineNamespaces(Uri url, Map namespaces) { + void _determineNamespaces(Uri url, Map namespaces) { var sourcesByNamespace = >{}; - for (var reference in references.sources.keys) { - if (reference.span.sourceUrl != url) continue; - var source = references.sources[reference]; - if (source is UseSource) continue; - if (namespaces.containsKey(source.url)) continue; + references.sources.forEach((reference, source) { + if (reference.span.sourceUrl != url) return; + if (source is UseSource) return; + if (namespaces.containsKey(source.url)) return; var namespace = source.preferredNamespace; - if (namespace == null) continue; + if (namespace == null) return; sourcesByNamespace.putIfAbsent(namespace, () => {}).add(source); - } + }); // First assign namespaces to module URLs without conflicts. var conflictingNamespaces = >{}; sourcesByNamespace.forEach((namespace, sources) { @@ -503,7 +514,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor { /// [currentUrl] is the canonical URL of the file that contains all of the /// references in [sources]. void _resolveNamespaceConflict(String namespace, Set sources, - Map namespaces, Uri currentUrl) { + Map namespaces, Uri currentUrl) { // Give first priority to a built-in module. var builtIns = sources.whereType(); if (builtIns.isNotEmpty) { @@ -522,7 +533,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor { // namespace. var paths = { for (var source in sources) - source: ruleUrlsForSources[source].split('/') + source: ruleUrlsForSources[source]!.split('/') ..removeLast() ..removeWhere((segment) => segment.contains('.')) }; @@ -545,7 +556,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor { } aliases = { for (var source in sources) - source: '${paths[source].removeLast()}-${aliases[source]}' + source: '${paths[source]!.removeLast()}-${aliases[source]}' }; } for (var source in sources) { @@ -558,7 +569,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor { /// If it is, but "sass-$module" is not, returns that. Otherwise, returns it /// with the lowest available number appended to the end. String _resolveBuiltInNamespace( - String module, Map existingNamespaces) { + String module, Map existingNamespaces) { return existingNamespaces.containsValue(module) && !existingNamespaces.containsValue('sass-$module') ? 'sass-$module' @@ -569,7 +580,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor { /// [existingNamespaces], returns it. Otherwise, returns it with the lowest /// available number appended to the end. String _incrementUntilAvailable( - String defaultNamespace, Map existingNamespaces) { + String defaultNamespace, Map existingNamespaces) { var count = 1; var namespace = defaultNamespace; while (existingNamespaces.containsValue(namespace)) { @@ -588,18 +599,18 @@ class _ModuleMigrationVisitor extends MigrationVisitor { byPathLength.putIfAbsent(pathSegments.length, () => {}).add(entry.key); } return [ - for (var length in byPathLength.keys.toList()..sort()) - byPathLength[length] + for (var entry in byPathLength.entries.sorted((a, b) => a.key - b.key)) + entry.value ]; } - /// Visits the children of [node] with a new scope for tracking unreferencable - /// members. + /// Visits [children] with a new scope for tracking unreferencable members. @override - void visitChildren(ParentStatement node) { + void visitChildren(List children) { + var oldUnreferencable = _unreferencable; _unreferencable = UnreferencableMembers(_unreferencable); - super.visitChildren(node); - _unreferencable = _unreferencable.parent; + super.visitChildren(children); + _unreferencable = oldUnreferencable; } /// Adds a namespace to any function call that requires it. @@ -611,8 +622,10 @@ class _ModuleMigrationVisitor extends MigrationVisitor { } if (references.sources.containsKey(node)) { var declaration = references.functions[node]; - _unreferencable.check(declaration, node); - _renameReference(nameSpan(node), declaration); + if (declaration != null) { + _unreferencable.check(declaration, node); + _renameReference(nameSpan(node), declaration); + } _patchNamespaceForFunction(node, declaration, (namespace) { addPatch(patchBefore(node.name, '$namespace.')); }); @@ -620,8 +633,11 @@ class _ModuleMigrationVisitor extends MigrationVisitor { if (node.name.asPlain == "get-function") { var declaration = references.getFunctionReferences[node]; - _unreferencable.check(declaration, node); - _renameReference(getStaticNameForGetFunctionCall(node), declaration); + if (declaration != null) { + _unreferencable.check(declaration, node); + var span = getStaticNameForGetFunctionCall(node); + if (span != null) _renameReference(span, declaration); + } // Ignore get-function calls that already have a module argument. var moduleArg = node.arguments.named['module']; @@ -633,8 +649,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor { // Warn for get-function calls without a static name. var nameArg = node.arguments.named['name'] ?? node.arguments.positional.first; - if (nameArg is! StringExpression || - (nameArg as StringExpression).text.asPlain == null) { + if (nameArg is! StringExpression || nameArg.text.asPlain == null) { emitWarning( "get-function call may require \$module parameter", nameArg.span); return; @@ -661,7 +676,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor { /// If [node] is a get-function call, [getFunctionCall] should be true. void _patchNamespaceForFunction( FunctionExpression node, - MemberDeclaration declaration, + MemberDeclaration? declaration, void patchNamespace(String namespace), {bool getFunctionCall = false}) { var span = getFunctionCall @@ -670,17 +685,19 @@ class _ModuleMigrationVisitor extends MigrationVisitor { if (span == null) return; var name = span.text.replaceAll('_', '-'); - var namespace = _namespaceForDeclaration(declaration); + var namespace = declaration.andThen(_namespaceForDeclaration); if (namespace != null) { patchNamespace(namespace); return; } - if (!builtInFunctionModules.containsKey(name)) return; - namespace = builtInFunctionModules[name]; + if (namespace == null) return; + name = builtInFunctionNameChanges[name] ?? name; - if (namespace == 'color' && removedColorFunctions.containsKey(name)) { + var parameter = removedColorFunctions[name]; + var amountArg = node.arguments.named['amount']; + if (namespace == 'color' && parameter != null) { if (getFunctionCall) { emitWarning( "$name is not available in the module system and should be " @@ -689,19 +706,19 @@ class _ModuleMigrationVisitor extends MigrationVisitor { return; } else if (node.arguments.positional.length == 2 && node.arguments.named.isEmpty) { - _patchRemovedColorFunction(name, node.arguments.positional.last); + _patchRemovedColorFunction(parameter, node.arguments.positional.last); name = 'adjust'; - } else if (node.arguments.named.containsKey('amount')) { - var arg = node.arguments.named['amount']; - _patchRemovedColorFunction(name, arg, - existingArgName: _findArgNameSpan(arg)); + } else if (amountArg != null) { + _patchRemovedColorFunction(parameter, amountArg, + existingArgName: _findArgNameSpan(amountArg)); name = 'adjust'; } else { emitWarning("Could not migrate malformed '$name' call", node.span); return; } } - patchNamespace(_findOrAddBuiltInNamespace(namespace)); + namespace = _findOrAddBuiltInNamespace(namespace); + if (namespace != null) patchNamespace(namespace); if (name != span.text.replaceAll('_', '-')) addPatch(Patch(span, name)); } @@ -718,9 +735,8 @@ class _ModuleMigrationVisitor extends MigrationVisitor { /// Patches the amount argument [arg] for a removed color function /// (e.g. `lighten`) to add the appropriate name (such as `$lightness`) and /// negate the argument if necessary. - void _patchRemovedColorFunction(String name, Expression arg, - {FileSpan existingArgName}) { - var parameter = removedColorFunctions[name]; + void _patchRemovedColorFunction(String parameter, Expression arg, + {FileSpan? existingArgName}) { // Surround the argument in parens if negated to avoid `-` being parsed // as part of the namespace. var needsParens = parameter.endsWith('-') && @@ -764,17 +780,18 @@ class _ModuleMigrationVisitor extends MigrationVisitor { var indent = ' ' * node.span.start.column; for (var import in dynamicImports) { - var ruleUrl = import.url; + String? ruleUrl = import.url; var tuple = importCache.canonicalize(Uri.parse(ruleUrl), baseImporter: importer, baseUrl: currentUrl, forImport: true); var canonicalImport = tuple?.item2; - if (references.orphanImportOnlyFiles.containsKey(canonicalImport)) { + if (canonicalImport != null && + references.orphanImportOnlyFiles.containsKey(canonicalImport)) { ruleUrl = null; var url = references.orphanImportOnlyFiles[canonicalImport]?.url; - if (url != null) { + if (url != null && tuple != null) { var canonicalRedirect = importCache .canonicalize(url, - baseImporter: tuple.item1, baseUrl: canonicalImport) + baseImporter: tuple.item1, baseUrl: canonicalImport)! .item2; ruleUrl = _absoluteUrlToDependency(canonicalRedirect).item1; } @@ -807,7 +824,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor { if (staticImports.isNotEmpty) { _useAllowed = false; addPatch(Patch.insert( - _afterLastImport, + _afterLastImport ?? node.span.file.location(0), '$indent@import ' + staticImports.map((import) => import.span.text).join(', ') + '$_semicolonIfNotIndented\n')); @@ -868,6 +885,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor { /// This is used for migrating `@import` rules that are nested or appear after /// some other rules. String _migrateImportToLoadCss(String ruleUrl, FileSpan context) { + var oldUnreferencable = _unreferencable; _unreferencable = UnreferencableMembers(_unreferencable); for (var declaration in references.allDeclarations) { if (declaration.sourceUrl != currentUrl) continue; @@ -885,7 +903,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor { _configuredVariables.last.member.span); } - _unreferencable = _unreferencable.parent; + _unreferencable = oldUnreferencable; for (var declaration in references.allDeclarations) { if (declaration.sourceUrl != canonicalUrl) continue; _unreferencable.add(declaration, UnreferencableType.fromNestedImport); @@ -907,10 +925,10 @@ class _ModuleMigrationVisitor extends MigrationVisitor { /// `meta.load-css`. The third is a string of variables that should be added /// to a `show` clause of a `@forward` rule so that they can be configured by /// an upstream file. - Tuple3 _migrateImportCommon( + Tuple3 _migrateImportCommon( String ruleUrl, FileSpan context) { - var oldConfiguredVariables = _configuredVariables; - _configuredVariables = {}; + var oldConfiguredVariables = __configuredVariables; + __configuredVariables = {}; _upstreamStylesheets.add(currentUrl); var parsedUrl = Uri.parse(ruleUrl); if (migrateDependencies) visitDependency(parsedUrl, context); @@ -919,6 +937,11 @@ class _ModuleMigrationVisitor extends MigrationVisitor { var tuple = importCache.canonicalize(parsedUrl, baseImporter: importer, baseUrl: currentUrl); + if (tuple == null) { + throw MigrationSourceSpanException( + "Could not find Sass file at '${p.prettyUri(parsedUrl)}'.", context); + } + // Associate the importer for this URL with the resolved URL so that we can // re-use this import URL later on. var canonicalUrl = tuple.item2; @@ -937,22 +960,23 @@ class _ModuleMigrationVisitor extends MigrationVisitor { locallyConfiguredVariables[variable.name] = variable; } else if (_upstreamStylesheets.contains(variable.sourceUrl)) { externallyConfiguredVariables[variable.name] = variable; - oldConfiguredVariables.add(variable); + // `oldConfiguredVariables` must be non-null here because + // `_upstreamStylesheets` is not empty. + oldConfiguredVariables!.add(variable); } } - _configuredVariables = oldConfiguredVariables; + __configuredVariables = oldConfiguredVariables; - String extraForward; + String? extraForward; if (externallyConfiguredVariables.isNotEmpty) { extraForward = externallyConfiguredVariables.keys .map((variable) => "\$$variable") .join(", "); } - String normalConfig; + String? normalConfig; var configured = []; - for (var name in locallyConfiguredVariables.keys) { - var variable = locallyConfiguredVariables[name]; + locallyConfiguredVariables.forEach((name, variable) { if (variable.member.isGuarded || references.variables.containsValue(variable)) { configured.add("\$$name: \$$name"); @@ -974,7 +998,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor { var nameFormat = _useAllowed ? '\$$name' : '"$name"'; configured.add("$nameFormat: ${variable.member.expression}"); } - } + }); if (configured.length == 1) { normalConfig = "(" + configured.first + ")"; } else if (configured.isNotEmpty) { @@ -987,7 +1011,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor { /// the entrypoint, returns a list of `@forward` rule(s) to do so. /// /// If nothing from [url] should be forwarded, returns null. - List _makeForwardRules(Uri url, String quotedUrl) { + List? _makeForwardRules(Uri url, String quotedUrl) { var shownByPrefix = >{}; var hidden = {}; @@ -997,9 +1021,8 @@ class _ModuleMigrationVisitor extends MigrationVisitor { if (declaration.sourceUrl != url) continue; var newName = renamedMembers[declaration] ?? declaration.name; - String importOnlyPrefix; - if (declaration is ImportOnlyMemberDeclaration && - declaration.importOnlyPrefix != null) { + String? importOnlyPrefix; + if (declaration is ImportOnlyMemberDeclaration) { importOnlyPrefix = declaration.importOnlyPrefix; newName = declaration.name.substring(importOnlyPrefix.length); } @@ -1046,14 +1069,13 @@ class _ModuleMigrationVisitor extends MigrationVisitor { for (var subprefix in shownByPrefix.keys.toList()..sort()) { var hiddenMembers = { ...hidden, - for (var other in shownByPrefix.keys) - if (other != subprefix) ...shownByPrefix[other] + for (var entry in shownByPrefix.entries) + if (entry.key != subprefix) ...entry.value }; var allHidden = {}; for (var declaration in hiddenMembers) { var name = declaration.name; - if (declaration is ImportOnlyMemberDeclaration && - declaration.importOnlyPrefix != null) { + if (declaration is ImportOnlyMemberDeclaration) { name = name.substring(declaration.importOnlyPrefix.length); } if (name.startsWith('-')) name = name.substring(1); @@ -1080,6 +1102,10 @@ class _ModuleMigrationVisitor extends MigrationVisitor { if (node.namespace != null) return; var declaration = references.mixins[node]; + if (declaration == null) { + // TODO(jathak): Error here as part of fixing #182. + return; + } _unreferencable.check(declaration, node); _renameReference(nameSpan(node), declaration); var namespace = _namespaceForDeclaration(declaration); @@ -1123,9 +1149,14 @@ class _ModuleMigrationVisitor extends MigrationVisitor { void visitVariableExpression(VariableExpression node) { if (node.namespace != null) return; var declaration = references.variables[node]; + if (declaration == null) { + // TODO(jathak): Error here as part of fixing #182. + return; + } _unreferencable.check(declaration, node); if (_reassignedVariables.contains(declaration)) { - declaration = references.variableReassignments[declaration]; + declaration = + references.variableReassignments[declaration] ?? declaration; } _renameReference(nameSpan(node), declaration); var namespace = _namespaceForDeclaration(declaration); @@ -1144,9 +1175,10 @@ class _ModuleMigrationVisitor extends MigrationVisitor { @override void visitVariableDeclaration(VariableDeclaration node) { var declaration = MemberDeclaration(node); - if (references.defaultVariableDeclarations.containsKey(declaration)) { - _configuredVariables - .add(references.defaultVariableDeclarations[declaration]); + var defaultDeclaration = + references.defaultVariableDeclarations[declaration]; + if (defaultDeclaration is MemberDeclaration) { + _configuredVariables.add(defaultDeclaration); } var existingNode = references.variableReassignments[declaration]; @@ -1167,9 +1199,8 @@ class _ModuleMigrationVisitor extends MigrationVisitor { /// If [declaration] was renamed, patches [span] to use the same name. void _renameReference(FileSpan span, MemberDeclaration declaration) { - if (declaration == null) return; - if (renamedMembers.containsKey(declaration)) { - var newName = renamedMembers[declaration]; + var newName = renamedMembers[declaration]; + if (newName != null) { if (newName.startsWith('-') && declaration.name.endsWith(newName.substring(1))) { addPatch(patchDelete(span, @@ -1183,8 +1214,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor { return; } - if (declaration is ImportOnlyMemberDeclaration && - declaration.importOnlyPrefix != null) { + if (declaration is ImportOnlyMemberDeclaration) { addPatch(patchDelete(span, end: declaration.importOnlyPrefix.length)); } } @@ -1204,7 +1234,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor { /// Returns the namespace that built-in module [module] is loaded under. /// /// This adds an additional `@use` rule if [module] has not been loaded yet. - String _findOrAddBuiltInNamespace(String module) { + String? _findOrAddBuiltInNamespace(String module) { var url = Uri.parse("sass:$module"); _namespaces.putIfAbsent( url, () => _resolveBuiltInNamespace(module, _namespaces)); @@ -1219,17 +1249,17 @@ class _ModuleMigrationVisitor extends MigrationVisitor { /// Finds the namespace for the stylesheet containing [declaration], adding a /// new `@use` rule if necessary. - String _namespaceForDeclaration(MemberDeclaration declaration) { - if (declaration == null) return null; - + String? _namespaceForDeclaration(MemberDeclaration declaration) { var url = declaration.sourceUrl; if (url == currentUrl) return null; // If we can load [declaration] from a library entrypoint URL, do so. Choose // the shortest one if there are multiple options. var libraryUrls = references.libraries[declaration]; - if (libraryUrls != null) { - url = minBy(libraryUrls, (url) => url.pathSegments.length); + if (libraryUrls != null && libraryUrls.isNotEmpty) { + var minUrl = + minBy(libraryUrls, (url) => url.pathSegments.length); + url = minUrl ?? url; } if (!_usedUrls.contains(url)) { // Add new `@use` rule for indirect dependency @@ -1256,10 +1286,12 @@ class _ModuleMigrationVisitor extends MigrationVisitor { /// The first item of the returned tuple is the dependency, the second item /// is true when this dependency is resolved relative to the current URL and /// false when it's resolved relative to a load path. - Tuple2 _absoluteUrlToDependency(Uri url, {Uri relativeTo}) { + Tuple2 _absoluteUrlToDependency(Uri url, {Uri? relativeTo}) { relativeTo ??= currentUrl; var tuple = _originalImports[url]; - if (tuple?.item2 is NodeModulesImporter) return Tuple2(tuple.item1, false); + if (tuple != null && tuple.item2 is NodeModulesImporter) { + return Tuple2(tuple.item1, false); + } var basename = p.url.basenameWithoutExtension(url.path); if ((basename == 'index' || basename == '_index') && @@ -1278,7 +1310,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor { if (p.url.isWithin(loadPath, url.path)) p.url.relative(url.path, from: loadPath) ]; - var relativePath = minBy(potentialUrls, (url) => url.length); + var relativePath = minBy(potentialUrls, (url) => url.length)!; var isRelative = relativePath == potentialUrls.first; return Tuple2( @@ -1290,12 +1322,12 @@ class _ModuleMigrationVisitor extends MigrationVisitor { /// starts with it and the remainder is a valid Sass identifier. /// /// If there is no such prefix, returns `null`. - String _prefixFor(String identifier) => maxBy( + String? _prefixFor(String identifier) => maxBy( prefixesToRemove.where((prefix) => prefix.length < identifier.length && identifier.startsWith(prefix) && Parser.isIdentifier(identifier.substring(prefix.length))), - (prefix) => prefix.length); + (prefix) => prefix!.length); /// Disallows `@use` after `@at-root` rules. @override diff --git a/lib/src/migrators/module/member_declaration.dart b/lib/src/migrators/module/member_declaration.dart index 36257e6b..ebed18df 100644 --- a/lib/src/migrators/module/member_declaration.dart +++ b/lib/src/migrators/module/member_declaration.dart @@ -58,7 +58,7 @@ class MemberDeclaration { throw ArgumentError( "MemberDefinition must contain a VariableDeclaration, Argument, " "MixinRule, or FunctionRule"); - }(), member.span.sourceUrl); + }(), member.span.sourceUrl!); /// Creates a MemberDefinition for a member that was forwarded through at /// least one non-import-only module. @@ -72,12 +72,12 @@ class MemberDeclaration { /// [ImportOnlyMemberDeclaration]. factory MemberDeclaration.forward( MemberDeclaration forwarded, ForwardRule forward) => - isImportOnlyFile(forward.span.sourceUrl) + isImportOnlyFile(forward.span.sourceUrl!) ? ImportOnlyMemberDeclaration._(forwarded, forward) : MemberDeclaration._( forwarded.member, '${forward.prefix ?? ""}${forwarded.name}', - forward.span.sourceUrl); + forward.span.sourceUrl!); MemberDeclaration._(this.member, this.name, this.sourceUrl); @@ -123,10 +123,10 @@ class ImportOnlyMemberDeclaration (forwarded is ImportOnlyMemberDeclaration ? forwarded.importOnlyPrefix : ""), - importOnlyUrl = forward.span.sourceUrl, + importOnlyUrl = forward.span.sourceUrl!, super._(forwarded.member, '${forward.prefix ?? ""}${forwarded.name}', forwarded.sourceUrl) { - assert(isImportOnlyFile(forward.span.sourceUrl)); + assert(isImportOnlyFile(forward.span.sourceUrl!)); } String toString() => diff --git a/lib/src/migrators/module/reference_source.dart b/lib/src/migrators/module/reference_source.dart index ef7187fd..a751827d 100644 --- a/lib/src/migrators/module/reference_source.dart +++ b/lib/src/migrators/module/reference_source.dart @@ -19,7 +19,7 @@ abstract class ReferenceSource { /// Returns the ideal namespace to use for this source, or null if the source /// doesn't have a namespace. - String get preferredNamespace; + String? get preferredNamespace; } /// A source for references that should be migrated like an `@import` rule. @@ -31,7 +31,7 @@ class ImportSource extends ReferenceSource { /// The URL of the `@import` rule that loaded this member, or null if this /// is for an indirect dependency forwarded in an import-only file. - final String originalRuleUrl; + final String? originalRuleUrl; /// Creates an [ImportSource] for [url] from [import]. /// @@ -80,7 +80,7 @@ class UseSource extends ReferenceSource { UseSource(this.url, this.use); - String get preferredNamespace => use.namespace; + String? get preferredNamespace => use.namespace; operator ==(other) => other is UseSource && url == other.url && use == other.use; @@ -106,7 +106,7 @@ class CurrentSource extends ReferenceSource { final Uri url; CurrentSource(this.url); - String get preferredNamespace => null; + String? get preferredNamespace => null; operator ==(other) => other is CurrentSource && url == other.url; int get hashCode => url.hashCode; @@ -133,7 +133,7 @@ class ForwardSource extends ReferenceSource { ForwardSource(this.url, this.forward); - String get preferredNamespace => null; + String? get preferredNamespace => null; operator ==(other) => other is ForwardSource && url == other.url && forward == other.forward; @@ -158,11 +158,11 @@ class ImportOnlySource extends ReferenceSource { /// the URL of the `@import` rule that loaded that import-only file. /// /// Otherwise, this will be null. - final String originalRuleUrl; + final String? originalRuleUrl; ImportOnlySource(this.url, this.realSourceUrl, this.originalRuleUrl); - String get preferredNamespace => null; + String? get preferredNamespace => null; operator ==(other) => other is ImportOnlySource && diff --git a/lib/src/migrators/module/references.dart b/lib/src/migrators/module/references.dart index 40fa6c3a..5e6b8667 100644 --- a/lib/src/migrators/module/references.dart +++ b/lib/src/migrators/module/references.dart @@ -93,7 +93,7 @@ class References { /// Map of import-only files that do not directly depend on their regular /// counterparts to the last forward appearing within it (or null, if no /// regular file is forwarded by the import-only file). - final Map orphanImportOnlyFiles; + final Map orphanImportOnlyFiles; /// An iterable of all member declarations. Iterable get allDeclarations => @@ -101,11 +101,11 @@ class References { /// Returns all references to [declaration]. Iterable referencesTo(MemberDeclaration declaration) { - if (declaration.member is FunctionRule) { + if (declaration is MemberDeclaration) { return functions .keysForValue(declaration) .followedBy(getFunctionReferences.keysForValue(declaration)); - } else if (declaration.member is MixinRule) { + } else if (declaration is MemberDeclaration) { return mixins.keysForValue(declaration); } return variables.keysForValue(declaration); @@ -132,7 +132,7 @@ class References { /// Finds the original declaration of the variable referenced in [reference]. /// /// The return value always wraps a [VariableDeclaration] or an [Argument]. - MemberDeclaration originalDeclaration(VariableExpression reference) { + MemberDeclaration? originalDeclaration(VariableExpression reference) { var declaration = variables[reference]; return variableReassignments[declaration] ?? declaration; } @@ -155,7 +155,7 @@ class References { Set globalDeclarations, Map> libraries, Map sources, - Map orphanImportOnlyFiles) + Map orphanImportOnlyFiles) : variables = UnmodifiableBidirectionalMapView(variables), variableReassignments = UnmodifiableBidirectionalMapView(variableReassignments), @@ -197,12 +197,12 @@ class _ReferenceVisitor extends RecursiveAstVisitor { final _globalDeclarations = {}; final _libraries = >{}; final _sources = {}; - final _orphanImportOnlyFiles = {}; + final _orphanImportOnlyFiles = {}; /// The current global scope. /// /// This persists across imports, but not across module loads. - Scope _scope; + late Scope _scope; /// Mapping from canonical stylesheet URLs to the global scope of the module /// contained within it. @@ -213,7 +213,7 @@ class _ReferenceVisitor extends RecursiveAstVisitor { final _moduleScopes = {}; /// Maps declarations to their source for the current stylesheet. - Map _declarationSources; + late Map _declarationSources; /// [_declarationSources] for each module. final _moduleSources = >{}; @@ -231,7 +231,7 @@ class _ReferenceVisitor extends RecursiveAstVisitor { /// Note: Unlike the similar property in _ModuleMigrationVisitor, this only /// includes namespaces for `@use` rules that already exist within the file. /// It doesn't include namespaces for to-be-migrated imports. - Map _namespaces; + var _namespaces = {}; /// The URL of the root of the current library being visited. /// @@ -239,31 +239,31 @@ class _ReferenceVisitor extends RecursiveAstVisitor { /// stylesheets loaded from a load path or from `node_modules`, this is the /// canonical URL of the last import that was handled with a different /// importer than the entrypoint's. - Uri _libraryUrl; + Uri? _libraryUrl; /// The canonical URL of the stylesheet currently being migrated. - Uri _currentUrl; + late Uri _currentUrl; /// The URL of the rule used to load the current stylesheet. - String _currentRuleUrl; + String? _currentRuleUrl; /// The importer that's currently being used to resolve relative imports. /// /// If this is `null`, relative imports aren't supported in the current /// stylesheet. - Importer _importer; + late Importer _importer; /// If the current stylesheet is an import-only file, this starts as true and /// is changed to false if it forwards its regular counterpart. /// /// This is always false for regular files. - bool _isOrphanImportOnly; + late bool _isOrphanImportOnly; /// Cache used to load stylesheets. - ImportCache importCache; + final ImportCache importCache; /// The last `@forward` rule to be visited that was not an import-only file. - ForwardRule _lastRegularForward; + ForwardRule? _lastRegularForward; _ReferenceVisitor(this.importCache); @@ -272,9 +272,11 @@ class _ReferenceVisitor extends RecursiveAstVisitor { References build(Stylesheet stylesheet, Importer importer) { _importer = importer; _scope = Scope(); - _moduleScopes[stylesheet.span.sourceUrl] = _scope; + _currentUrl = stylesheet.span.sourceUrl!; + _isOrphanImportOnly = isImportOnlyFile(_currentUrl); + _moduleScopes[_currentUrl] = _scope; _declarationSources = {}; - _moduleSources[stylesheet.span.sourceUrl] = _declarationSources; + _moduleSources[_currentUrl] = _declarationSources; visitStylesheet(stylesheet); for (var variable in _scope.variables.values) { @@ -306,7 +308,7 @@ class _ReferenceVisitor extends RecursiveAstVisitor { for (var function in functions) { if (_isCssCompatibilityOverload(function)) continue; if (function.name.asPlain == null) continue; - var name = function.name.asPlain.replaceAll('_', '-'); + var name = function.name.asPlain!.replaceAll('_', '-'); var module = builtInFunctionModules[name]; if (module != null) _sources[function] = BuiltInSource(module); } @@ -343,12 +345,12 @@ class _ReferenceVisitor extends RecursiveAstVisitor { var oldUrl = _currentUrl; var oldOrphaned = _isOrphanImportOnly; _namespaces = {}; - _currentUrl = node.span.sourceUrl; + _currentUrl = node.span.sourceUrl!; _isOrphanImportOnly = isImportOnlyFile(_currentUrl); - super.visitStylesheet(node); + super.visitChildren(node.children); if (_isOrphanImportOnly) { _orphanImportOnlyFiles[_currentUrl] = - _lastRegularForward.span.sourceUrl == _currentUrl + _lastRegularForward?.span.sourceUrl == _currentUrl ? _lastRegularForward : null; } @@ -374,7 +376,7 @@ class _ReferenceVisitor extends RecursiveAstVisitor { var oldImporter = _importer; _importer = result.item1; var oldLibraryUrl = _libraryUrl; - var url = result.item2.span.sourceUrl; + var url = result.item2.span.sourceUrl!; if (_importer != oldImporter && !isImportOnlyFile(url)) { _libraryUrl ??= url; } @@ -382,8 +384,9 @@ class _ReferenceVisitor extends RecursiveAstVisitor { _currentRuleUrl = import.url; visitStylesheet(result.item2); var importSource = ImportSource(url, import); - for (var declaration in _declarationSources.keys.toList()) { - var source = _declarationSources[declaration]; + for (var entry in _declarationSources.entries.toList()) { + var declaration = entry.key; + var source = entry.value; if (source.url != url) continue; if (source is CurrentSource || source is ForwardSource) { _declarationSources[declaration] = importSource; @@ -404,22 +407,24 @@ class _ReferenceVisitor extends RecursiveAstVisitor { @override void visitUseRule(UseRule node) { super.visitUseRule(node); + var namespace = node.namespace; + if (namespace == null) return; if (node.url.scheme == 'sass') { - _namespaces[node.namespace] = node.url; + _namespaces[namespace] = node.url; return; } var canonicalUrl = _loadUseOrForward(node.url, node); - _namespaces[node.namespace] = canonicalUrl; + _namespaces[namespace] = canonicalUrl; - var moduleSources = _moduleSources[canonicalUrl]; + // `_moduleSources[canonicalUrl]` is set in `_loadUseOrForward`. + var moduleSources = _moduleSources[canonicalUrl]!; var useSource = UseSource(canonicalUrl, node); - for (var declaration in moduleSources.keys) { - var source = moduleSources[declaration]; + moduleSources.forEach((declaration, source) { if (source.url == canonicalUrl && (source is CurrentSource || source is ForwardSource)) { _declarationSources[declaration] = useSource; } - } + }); } /// Given a URL from a `@use` or `@forward` rule, loads and visits the @@ -434,7 +439,7 @@ class _ReferenceVisitor extends RecursiveAstVisitor { } var stylesheet = result.item2; - var canonicalUrl = stylesheet.span.sourceUrl; + var canonicalUrl = stylesheet.span.sourceUrl!; if (_moduleScopes.containsKey(canonicalUrl)) return canonicalUrl; var oldScope = _scope; @@ -469,7 +474,9 @@ class _ReferenceVisitor extends RecursiveAstVisitor { _isOrphanImportOnly = false; } if (!isImportOnlyFile(canonicalUrl)) _lastRegularForward = node; - var moduleScope = _moduleScopes[canonicalUrl]; + + // `_moduleSources[canonicalUrl]` is set in `_loadUseOrForward`. + var moduleScope = _moduleScopes[canonicalUrl]!; for (var declaration in moduleScope.variables.values) { if (declaration.member is! VariableDeclaration) { throw StateError( @@ -497,7 +504,7 @@ class _ReferenceVisitor extends RecursiveAstVisitor { /// Returns true if [name] should be shown based on [prefix], [shown], and /// [hidden] from a `@forward` rule. bool _visibleThroughForward( - String name, String prefix, Set shown, Set hidden) { + String name, String? prefix, Set? shown, Set? hidden) { if (prefix != null) name = '$prefix$name'; return (shown?.contains(name) ?? true) && !(hidden?.contains(name) ?? false); @@ -524,7 +531,7 @@ class _ReferenceVisitor extends RecursiveAstVisitor { : null); } else { _declarationSources[declaration] = - ForwardSource(forward.span.sourceUrl, forward); + ForwardSource(forward.span.sourceUrl!, forward); } } @@ -533,30 +540,26 @@ class _ReferenceVisitor extends RecursiveAstVisitor { /// All of [node]'s arguments are declared as local variables in a new scope. @override void visitCallableDeclaration(CallableDeclaration node) { + var oldScope = _scope; _scope = Scope(_scope); for (var argument in node.arguments.arguments) { _scope.variables[argument.name] = MemberDeclaration(argument); - if (argument.defaultValue != null) visitExpression(argument.defaultValue); + var defaultValue = argument.defaultValue; + if (defaultValue != null) visitExpression(defaultValue); } - super.visitChildren(node); + super.visitChildren(node.children); _checkUnresolvedReferences(_scope); - _scope = _scope.parent; + _scope = oldScope; } - /// Visits the children of [node] with a local scope. - /// - /// Note: The children of a stylesheet are at the root, so we should not add - /// a local scope. + /// Visits [children] with a local scope. @override - void visitChildren(ParentStatement node) { - if (node is Stylesheet) { - super.visitChildren(node); - return; - } + void visitChildren(List children) { + var oldScope = _scope; _scope = Scope(_scope); - super.visitChildren(node); + super.visitChildren(children); _checkUnresolvedReferences(_scope); - _scope = _scope.parent; + _scope = oldScope; } /// Finds any declarations in [scope] that match one of the references in @@ -564,8 +567,9 @@ class _ReferenceVisitor extends RecursiveAstVisitor { /// /// This should be called on a scope immediately before it ends. void _checkUnresolvedReferences(Scope scope) { - for (var reference in _unresolvedReferences.keys.toList()) { - var refScope = _unresolvedReferences[reference]; + for (var entry in _unresolvedReferences.entries.toList()) { + var reference = entry.key; + var refScope = entry.value; if (!refScope.isDescendentOf(scope)) continue; if (reference is VariableExpression) { _linkUnresolvedReference( @@ -578,6 +582,7 @@ class _ReferenceVisitor extends RecursiveAstVisitor { if (name == null) continue; if (name == 'get-function') { var nameExpression = getStaticNameForGetFunctionCall(reference); + if (nameExpression == null) continue; var staticName = nameExpression.text.replaceAll('_', '-'); _linkUnresolvedReference( reference, staticName, scope.functions, _getFunctionReferences, @@ -603,7 +608,7 @@ class _ReferenceVisitor extends RecursiveAstVisitor { var declaration = declarations[name]; if (declaration == null) return; references[reference] = declaration; - if (trackSources) _sources[reference] = _declarationSources[declaration]; + if (trackSources) _sources[reference] = _declarationSources[declaration]!; _unresolvedReferences.remove(reference); } @@ -611,7 +616,7 @@ class _ReferenceVisitor extends RecursiveAstVisitor { /// /// If [namespace] is null or does not exist within this stylesheet, this /// returns the current stylesheet's scope. - Scope _scopeForNamespace(String namespace) => + Scope _scopeForNamespace(String? namespace) => _moduleScopes[_namespaces[namespace]] ?? _scope; /// Declares a variable in the current scope. @@ -648,14 +653,19 @@ class _ReferenceVisitor extends RecursiveAstVisitor { @override void visitVariableExpression(VariableExpression node) { super.visitVariableExpression(node); - var declaration = - _scopeForNamespace(node.namespace).findVariable(node.name); + var namespace = node.namespace; + var urlForNamespace = _namespaces[namespace]; + if (urlForNamespace != null && urlForNamespace.scheme == 'sass') { + _sources[node] = BuiltInSource(urlForNamespace.path); + return; + } + var declaration = _scopeForNamespace(namespace).findVariable(node.name); if (declaration != null && !_fromForwardRuleInCurrent(declaration)) { _variables[node] = declaration; if (declaration.member is VariableDeclaration) { - _sources[node] = _declarationSources[declaration]; + _sources[node] = _declarationSources[declaration]!; } - } else if (node.namespace == null) { + } else if (namespace == null) { _unresolvedReferences[node] = _scope; } } @@ -674,15 +684,17 @@ class _ReferenceVisitor extends RecursiveAstVisitor { @override void visitIncludeRule(IncludeRule node) { super.visitIncludeRule(node); - if (_namespaces[node.namespace]?.scheme == 'sass') { - _sources[node] = BuiltInSource(_namespaces[node.namespace].path); + var namespace = node.namespace; + var urlForNamespace = _namespaces[namespace]; + if (urlForNamespace != null && urlForNamespace.scheme == 'sass') { + _sources[node] = BuiltInSource(urlForNamespace.path); return; } - var declaration = _scopeForNamespace(node.namespace).findMixin(node.name); + var declaration = _scopeForNamespace(namespace).findMixin(node.name); if (declaration != null && !_fromForwardRuleInCurrent(declaration)) { _mixins[node] = declaration; - _sources[node] = _declarationSources[declaration]; - } else if (node.namespace == null) { + _sources[node] = _declarationSources[declaration]!; + } else if (namespace == null) { _unresolvedReferences[node] = _scope; } } @@ -701,19 +713,22 @@ class _ReferenceVisitor extends RecursiveAstVisitor { @override void visitFunctionExpression(FunctionExpression node) { super.visitFunctionExpression(node); - if (_namespaces[node.namespace]?.scheme == 'sass') { - _sources[node] = BuiltInSource(_namespaces[node.namespace].path); + var namespace = node.namespace; + var urlForNamespace = _namespaces[namespace]; + if (urlForNamespace != null && urlForNamespace.scheme == 'sass') { + _sources[node] = BuiltInSource(urlForNamespace.path); return; } - if (node.name.asPlain == null) return; - var name = node.name.asPlain.replaceAll('_', '-'); + var name = node.name.asPlain; + if (name == null) return; + name = name.replaceAll('_', '-'); - var declaration = _scopeForNamespace(node.namespace).findFunction(name); + var declaration = _scopeForNamespace(namespace).findFunction(name); if (declaration != null && !_fromForwardRuleInCurrent(declaration)) { _functions[node] = declaration; - _sources[node] = _declarationSources[declaration]; + _sources[node] = _declarationSources[declaration]!; return; - } else if (node.namespace == null) { + } else if (namespace == null) { if (name == 'get-function') { _sources[node] = BuiltInSource("meta"); } else { @@ -726,7 +741,7 @@ class _ReferenceVisitor extends RecursiveAstVisitor { var nameExpression = getStaticNameForGetFunctionCall(node); if (nameExpression == null) return; var moduleExpression = getStaticModuleForGetFunctionCall(node); - var namespace = moduleExpression?.text; + namespace = moduleExpression?.text; name = nameExpression.text.replaceAll('_', '-'); declaration = _scopeForNamespace(namespace).findFunction(name); if (declaration != null && !_fromForwardRuleInCurrent(declaration)) { @@ -739,8 +754,9 @@ class _ReferenceVisitor extends RecursiveAstVisitor { /// Registers the current library as a location from which [declaration] can /// be loaded. void _registerLibraryUrl(MemberDeclaration declaration) { - if (_libraryUrl == null) return; - _libraries.putIfAbsent(declaration, () => {}).add(_libraryUrl); + var libraryUrl = _libraryUrl; + if (libraryUrl == null) return; + _libraries.putIfAbsent(declaration, () => {}).add(libraryUrl); } /// Returns true if [declaration] is from a `@forward` rule in the current diff --git a/lib/src/migrators/module/scope.dart b/lib/src/migrators/module/scope.dart index 06c70789..33e8e898 100644 --- a/lib/src/migrators/module/scope.dart +++ b/lib/src/migrators/module/scope.dart @@ -15,7 +15,7 @@ import 'member_declaration.dart'; /// the stylesheet. class Scope { /// The parent of this scope, or null if this scope is global. - final Scope parent; + final Scope? parent; /// Variables defined in this scope. /// @@ -44,16 +44,16 @@ class Scope { /// Returns the declaration of a variable named [name] if it exists, or null /// if it does not. - MemberDeclaration findVariable(String name) => + MemberDeclaration? findVariable(String name) => variables[name] ?? parent?.findVariable(name); /// Returns the declaration of a mixin named [name] if it exists, or null if /// it does not. - MemberDeclaration findMixin(String name) => + MemberDeclaration? findMixin(String name) => mixins[name] ?? parent?.findMixin(name); /// Returns the declaration of a function named [name] if it exists, or null /// if it does not. - MemberDeclaration findFunction(String name) => + MemberDeclaration? findFunction(String name) => functions[name] ?? parent?.findFunction(name); } diff --git a/lib/src/migrators/module/unreferencable_members.dart b/lib/src/migrators/module/unreferencable_members.dart index 9dff56ea..27c814b9 100644 --- a/lib/src/migrators/module/unreferencable_members.dart +++ b/lib/src/migrators/module/unreferencable_members.dart @@ -15,7 +15,7 @@ import 'unreferencable_type.dart'; /// Tracks members that are unreferencable in the current scope. class UnreferencableMembers { /// The parent scope of this instance. - final UnreferencableMembers parent; + final UnreferencableMembers? parent; /// The members marked as unreferencable in this scope directly. final _unreferencable = {}; @@ -30,10 +30,8 @@ class UnreferencableMembers { /// Checks whether [declaration] is marked as unreferencable within this /// scope or any ancestor scope and throws an appropriate exception if it is. void check(MemberDeclaration declaration, SassNode reference) { - if (_unreferencable.containsKey(declaration)) { - throw _unreferencable[declaration] - .toException(reference, declaration.sourceUrl); - } + var type = _unreferencable[declaration]; + if (type != null) throw type.toException(reference, declaration.sourceUrl); parent?.check(declaration, reference); } } diff --git a/lib/src/migrators/namespace.dart b/lib/src/migrators/namespace.dart index d499491b..52e83434 100644 --- a/lib/src/migrators/namespace.dart +++ b/lib/src/migrators/namespace.dart @@ -42,11 +42,11 @@ class NamespaceMigrator extends Migrator { @override Map migrateFile( ImportCache importCache, Stylesheet stylesheet, Importer importer) { - var renamer = Renamer(argResults['rename'].join('\n'), - {'': (rule) => rule.namespace, 'url': (rule) => rule.url.toString()}, + var renamer = Renamer(argResults!['rename'].join('\n'), + {'': ((rule) => rule.namespace!), 'url': (rule) => rule.url.toString()}, sourceUrl: '--rename'); - var visitor = _NamespaceMigrationVisitor( - renamer, argResults['force'] as bool, importCache, migrateDependencies); + var visitor = _NamespaceMigrationVisitor(renamer, + argResults!['force'] as bool, importCache, migrateDependencies); var result = visitor.run(stylesheet, importer); missingDependencies.addAll(visitor.missingDependencies); return result; @@ -60,10 +60,14 @@ class _NamespaceMigrationVisitor extends MigrationVisitor { /// A set of spans for each *original* namespace in the current file. /// /// Each span covers just the namespace of a member reference. - Map> _spansByNamespace; + Map> get _spansByNamespace => + assertInStylesheet(__spansByNamespace, '_spansByNamespace'); + Map>? __spansByNamespace; /// The set of namespaces used in the current file *after* renaming. - Set _usedNamespaces; + Set get _usedNamespaces => + assertInStylesheet(__usedNamespaces, '_usedNamespaces'); + Set? __usedNamespaces; _NamespaceMigrationVisitor(this.renamer, this.forceRename, ImportCache importCache, bool migrateDependencies) @@ -71,13 +75,13 @@ class _NamespaceMigrationVisitor extends MigrationVisitor { @override void visitStylesheet(Stylesheet node) { - var oldSpansByNamespace = _spansByNamespace; - var oldUsedNamespaces = _usedNamespaces; - _spansByNamespace = {}; - _usedNamespaces = {}; + var oldSpansByNamespace = __spansByNamespace; + var oldUsedNamespaces = __usedNamespaces; + __spansByNamespace = {}; + __usedNamespaces = {}; super.visitStylesheet(node); - _spansByNamespace = oldSpansByNamespace; - _usedNamespaces = oldUsedNamespaces; + __spansByNamespace = oldSpansByNamespace; + __usedNamespaces = oldUsedNamespaces; } @override @@ -85,9 +89,10 @@ class _NamespaceMigrationVisitor extends MigrationVisitor { // Pass each `@use` rule through the renamer. var newNamespaces = >{}; for (var rule in node.children.whereType()) { - if (rule.namespace == null) continue; + var namespace = rule.namespace; + if (namespace == null) continue; newNamespaces - .putIfAbsent(renamer.rename(rule) ?? rule.namespace, () => {}) + .putIfAbsent(renamer.rename(rule) ?? namespace, () => {}) .add(rule); } @@ -127,13 +132,14 @@ class _NamespaceMigrationVisitor extends MigrationVisitor { /// Patch [rule] and all references to it with [newNamespace]. void _patchNamespace(UseRule rule, String newNamespace) { + var oldNamespace = rule.namespace!; _usedNamespaces.add(newNamespace); if (rule.namespace == newNamespace) return; var asClause = RegExp('\\s*as\\s+(${rule.namespace})').firstMatch(rule.span.text); if (asClause == null) { // Add an `as` clause to a rule that previously lacked one. - var end = RegExp(r"""@use\s("|').*?\1""").firstMatch(rule.span.text).end; + var end = RegExp(r"""@use\s("|').*?\1""").firstMatch(rule.span.text)!.end; addPatch( Patch.insert(rule.span.subspan(0, end).end, ' as $newNamespace')); } else if (namespaceForPath(rule.url.toString()) == newNamespace) { @@ -143,16 +149,16 @@ class _NamespaceMigrationVisitor extends MigrationVisitor { } else { // Change the namespace of an existing `as` clause. addPatch(Patch( - rule.span.subspan(asClause.end - rule.namespace.length, asClause.end), + rule.span.subspan(asClause.end - oldNamespace.length, asClause.end), newNamespace)); } - for (FileSpan span in _spansByNamespace[rule.namespace] ?? {}) { + for (FileSpan span in _spansByNamespace[oldNamespace] ?? {}) { addPatch(Patch(span, newNamespace)); } } /// If [namespace] is not null, add its span to [_spansByNamespace]. - void _addNamespaceSpan(String namespace, FileSpan span) { + void _addNamespaceSpan(String? namespace, FileSpan span) { if (namespace != null) { assert(span.text.startsWith(namespace)); _spansByNamespace @@ -185,10 +191,11 @@ class _NamespaceMigrationVisitor extends MigrationVisitor { @override void visitIncludeRule(IncludeRule node) { - if (node.namespace != null) { - var startNamespace = node.span.text.indexOf( - node.namespace, node.span.text[0] == '+' ? 1 : '@include'.length); - _addNamespaceSpan(node.namespace, node.span.subspan(startNamespace)); + var namespace = node.namespace; + if (namespace != null) { + var startNamespace = node.span.text + .indexOf(namespace, node.span.text[0] == '+' ? 1 : '@include'.length); + _addNamespaceSpan(namespace, node.span.subspan(startNamespace)); } super.visitIncludeRule(node); } diff --git a/lib/src/node_interop_stub.dart b/lib/src/node_interop_stub.dart index d53c5989..d376af0c 100644 --- a/lib/src/node_interop_stub.dart +++ b/lib/src/node_interop_stub.dart @@ -10,7 +10,7 @@ /// [`node_interop/node`]: https://pub.dartlang.org/documentation/node_interop/latest/node_interop.node/node_interop.node-library.html class Process { - List get argv => null; + List? get argv => null; } final process = Process(); diff --git a/lib/src/patch.dart b/lib/src/patch.dart index 1a989a32..13f8e142 100644 --- a/lib/src/patch.dart +++ b/lib/src/patch.dart @@ -31,7 +31,7 @@ class Patch implements Comparable { var buffer = StringBuffer(); int offset = 0; - Patch lastPatch; + Patch? lastPatch; for (var patch in sortedPatches) { // The module migrator generates duplicate patches when renaming two nodes // that share the same span (itself a workaround within the parser). diff --git a/lib/src/renamer.dart b/lib/src/renamer.dart index e2179ae3..ae9585be 100644 --- a/lib/src/renamer.dart +++ b/lib/src/renamer.dart @@ -67,7 +67,14 @@ class Renamer { /// a [String] or a [Uri]. static Renamer> map(String code, List keys, {dynamic sourceUrl}) { - return Renamer(code, {for (var key in keys) key: (input) => input[key]}, + return Renamer( + code, + { + for (var key in keys) + key: ((input) => + input[key] ?? + (throw ArgumentError.value(input, 'Missing key "$key".'))) + }, sourceUrl: sourceUrl); } @@ -101,7 +108,7 @@ class Renamer { static _Statement _readStatement( StringScanner scanner, Map keys) { var start = scanner.position; - FormatException lastException; + FormatException? lastException; // Tries each key in succession until one is successfully returned. for (var entry in keys.entries) { try { @@ -130,7 +137,7 @@ class Renamer { /// /// Otherwise, after consuming the key clause (if any), attempts to read /// the matcher and output clauses, throwing if it's unable to. - static _Statement _tryKey( + static _Statement? _tryKey( StringScanner scanner, String key, String Function(T input) keyFunction) { if (key.isNotEmpty && !scanner.scan('$key ')) return null; var matcher = _readMatcher(scanner); @@ -148,7 +155,7 @@ class Renamer { if (char == $semicolon || char == $lf) { scanner.error('statement ended unexpectedly'); } - scanner.readChar(); + char = scanner.readChar(); if (char == $backslash) { var next = scanner.readChar(); if (next == $semicolon || next == $space) { @@ -172,7 +179,7 @@ class Renamer { while (true) { var char = scanner.peekChar(); if ({null, $space, $semicolon, $lf}.contains(char)) break; - scanner.readChar(); + char = scanner.readChar(); if (char == $backslash) { var next = scanner.readChar(); if (next >= $0 && next <= $9) { @@ -194,7 +201,7 @@ class Renamer { } /// Runs this renamer based on [input]. - String rename(T input) { + String? rename(T input) { for (var statement in _statements) { var result = statement.rename(input); if (result != null) return result; @@ -223,7 +230,7 @@ class _Statement { _Statement(this.key, this.matcher, this.output); /// Return the output if this statement matches [input] or null otherwise. - String rename(T input) { + String? rename(T input) { var match = matcher.firstMatch(key(input)); if (match == null) return null; return output.map((item) => item.build(match)).join(); @@ -234,7 +241,7 @@ class _Statement { abstract class _OutputComponent { /// When constructing the output, this will be called with the match that /// the matcher clause found. - String build(RegExpMatch match); + String? build(RegExpMatch match); } /// Literal text that's part of a statement's output. @@ -252,5 +259,5 @@ class _Backreference extends _OutputComponent { _Backreference(this.number); /// Returns the captured group numbered [number] in [match]. - String build(RegExpMatch match) => match.group(number); + String? build(RegExpMatch match) => match.group(number); } diff --git a/lib/src/runner.dart b/lib/src/runner.dart index f3568433..8d0528b4 100644 --- a/lib/src/runner.dart +++ b/lib/src/runner.dart @@ -78,7 +78,7 @@ class MigratorRunner extends CommandRunner> { if (argResults.wasParsed('unicode')) { glyph.ascii = !(argResults['unicode'] as bool); } - Map migrated; + Map? migrated; try { migrated = await runCommand(argResults); } on UsageException catch (e) { @@ -109,24 +109,24 @@ class MigratorRunner extends CommandRunner> { if (argResults['dry-run']) { print('Dry run. Logging migrated files instead of overwriting...\n'); - for (var url in migrated.keys) { + migrated.forEach((url, contents) { if (argResults['verbose']) { // This isn't *strictly* HRX format, since it can produce absolute // URLs rather than those that are relative to the HRX root, but we // just need it to be readable, not to interoperate with other tools. print('<===> ${p.prettyUri(url)}'); - print(migrated[url]); + print(contents); } else { print(p.prettyUri(url)); } - } + }); } else { - for (var url in migrated.keys) { - assert(url.scheme == null || url.scheme == "file", + migrated.forEach((url, contents) { + assert(url.scheme.isEmpty || url.scheme == "file", "$url is not a file path."); if (argResults['verbose']) print("Migrating ${p.prettyUri(url)}"); - File(url.toFilePath()).writeAsStringSync(migrated[url]); - } + File(url.toFilePath()).writeAsStringSync(contents); + }); } } } @@ -138,7 +138,7 @@ Future _loadVersion() async { version += " compiled with dart2js " "${const String.fromEnvironment('dart-version')}"; } - if (version != null && version.isNotEmpty) return version; + if (version.isNotEmpty) return version; var libDir = p.fromUri( await Isolate.resolvePackageUri(Uri.parse('package:sass_migrator/'))); diff --git a/lib/src/util/bidirectional_map.dart b/lib/src/util/bidirectional_map.dart index 6d9f1f09..67c353f0 100644 --- a/lib/src/util/bidirectional_map.dart +++ b/lib/src/util/bidirectional_map.dart @@ -21,7 +21,7 @@ class BidirectionalMap extends MapBase { final _keysForValue = >{}; @override - V operator [](Object key) => _valueForKey[key]; + V? operator [](Object? key) => _valueForKey[key]; @override void operator []=(K key, V value) { @@ -43,16 +43,18 @@ class BidirectionalMap extends MapBase { Iterable get values => _keysForValue.keys; @override - V remove(Object key) { + V? remove(Object? key) { if (!_valueForKey.containsKey(key)) return null; - var value = _valueForKey.remove(key); - _keysForValue[value].remove(key); - if (_keysForValue[value].isEmpty) _keysForValue.remove(value); + V value = _valueForKey.remove(key)!; + var keys = _keysForValue[value]!; + keys.remove(key); + if (keys.isEmpty) _keysForValue.remove(value); return value; } /// Finds the keys associated with a given value. Iterable keysForValue(V value) sync* { - if (_keysForValue.containsKey(value)) yield* _keysForValue[value]; + var keys = _keysForValue[value]; + if (keys != null) yield* keys; } } diff --git a/lib/src/util/node_modules_importer.dart b/lib/src/util/node_modules_importer.dart index 6388a40a..694d2a88 100644 --- a/lib/src/util/node_modules_importer.dart +++ b/lib/src/util/node_modules_importer.dart @@ -23,7 +23,7 @@ class NodeModulesImporter extends Importer { /// [baseDirectory] and all of its ancestors. /// /// If not provided, [baseDirectory] defaults to the current directory. - NodeModulesImporter([String baseDirectory]) { + NodeModulesImporter([String? baseDirectory]) { var directory = baseDirectory ?? p.current; while (true) { var loadPath = p.join(directory, 'node_modules'); @@ -43,7 +43,7 @@ class NodeModulesImporter extends Importer { /// /// If no matching stylesheet can be found, or if [url] does not start with /// `~`, this returns null. - Uri canonicalize(Uri url) { + Uri? canonicalize(Uri url) { if (url.scheme == 'file') return _fsImporters.first.canonicalize(url); if (!url.path.startsWith('~')) return null; @@ -59,5 +59,5 @@ class NodeModulesImporter extends Importer { /// Loads [url] using a [FilesystemImporter]. /// /// [url] must be the canonical URL returned by [canonicalize]. - ImporterResult load(Uri url) => _fsImporters.first.load(url); + ImporterResult? load(Uri url) => _fsImporters.first.load(url); } diff --git a/lib/src/util/unmodifiable_bidirectional_map_view.dart b/lib/src/util/unmodifiable_bidirectional_map_view.dart index e679d0cf..9444e536 100644 --- a/lib/src/util/unmodifiable_bidirectional_map_view.dart +++ b/lib/src/util/unmodifiable_bidirectional_map_view.dart @@ -11,7 +11,7 @@ import 'bidirectional_map.dart'; /// A unmodifiable view of [BidirectionalMap]. class UnmodifiableBidirectionalMapView extends UnmodifiableMapView implements BidirectionalMap { - final BidirectionalMap _map; + final BidirectionalMap _map; UnmodifiableBidirectionalMapView(BidirectionalMap map) : _map = map, diff --git a/lib/src/utils.dart b/lib/src/utils.dart index 7d74c31c..988f14e6 100644 --- a/lib/src/utils.dart +++ b/lib/src/utils.dart @@ -54,6 +54,17 @@ extension ExtendSpan on FileSpan { } } +extension NullableExtension on T? { + /// If [this] is `null`, returns `null`. Otherwise, runs [fn] and returns its + /// result. + /// + /// Based on Rust's `Option.and_then`. + V? andThen(V Function(T value) fn) { + var self = this; // dart-lang/language#1520 + return self == null ? null : fn(self); + } +} + /// Returns the default namespace for a use rule with [path]. String namespaceForPath(String path) { // TODO(jathak): Confirm that this is a valid Sass identifier @@ -77,7 +88,7 @@ bool valuesAreUnique(Map map) => /// /// By default, this deletes the entire span. If [start] and/or [end] are /// provided, this deletes only the portion of the span within that range. -Patch patchDelete(FileSpan span, {int start = 0, int end}) => +Patch patchDelete(FileSpan span, {int start = 0, int? end}) => Patch(span.subspan(start, end), ""); /// Returns the next location after [import] that it would be safe to insert @@ -135,11 +146,12 @@ bool isWhitespace(int character) => /// `$` at the start of variable names. FileSpan nameSpan(SassNode node) { if (node is VariableDeclaration) { - var start = node.namespace == null ? 1 : node.namespace.length + 2; + var namespace = node.namespace; + var start = namespace == null ? 1 : namespace.length + 2; return node.span.subspan(start, start + node.name.length); } else if (node is VariableExpression) { - return node.span - .subspan(node.namespace == null ? 1 : node.namespace.length + 2); + var namespace = node.namespace; + return node.span.subspan(namespace == null ? 1 : namespace.length + 2); } else if (node is FunctionRule) { var startName = node.span.text .replaceAll('_', '-') @@ -164,7 +176,7 @@ FileSpan nameSpan(SassNode node) { } /// Emits a warning with [message] and optionally [context]; -void emitWarning(String message, [FileSpan context]) { +void emitWarning(String message, [FileSpan? context]) { if (context == null) { printStderr("WARNING: $message"); } else { @@ -174,7 +186,7 @@ void emitWarning(String message, [FileSpan context]) { /// Returns the only argument in [invocation], or null if [invocation] does not /// contain exactly one argument. -Expression getOnlyArgument(ArgumentInvocation invocation) { +Expression? getOnlyArgument(ArgumentInvocation invocation) { if (invocation.positional.length == 0 && invocation.named.length == 1) { return invocation.named.values.first; } else if (invocation.positional.length == 1 && invocation.named.isEmpty) { @@ -188,15 +200,14 @@ Expression getOnlyArgument(ArgumentInvocation invocation) { /// determined, this returns the span containing it. /// /// Otherwise, this returns null. -FileSpan getStaticNameForGetFunctionCall(FunctionExpression node) { +FileSpan? getStaticNameForGetFunctionCall(FunctionExpression node) { if (node.name.asPlain != 'get-function') return null; var nameArgument = node.arguments.named['name'] ?? node.arguments.positional.first; - if (nameArgument is! StringExpression || - (nameArgument as StringExpression).text.asPlain == null) { + if (nameArgument is! StringExpression || nameArgument.text.asPlain == null) { return null; } - return (nameArgument as StringExpression).hasQuotes + return nameArgument.hasQuotes ? nameArgument.span.subspan(1, nameArgument.span.length - 1) : nameArgument.span; } @@ -205,17 +216,16 @@ FileSpan getStaticNameForGetFunctionCall(FunctionExpression node) { /// determined, this returns the span containing it. /// /// Otherwise, this returns null. -FileSpan getStaticModuleForGetFunctionCall(FunctionExpression node) { +FileSpan? getStaticModuleForGetFunctionCall(FunctionExpression node) { if (node.name.asPlain != 'get-function') return null; var moduleArg = node.arguments.named['module']; if (moduleArg == null && node.arguments.positional.length > 2) { moduleArg = node.arguments.positional[2]; } - if (moduleArg is! StringExpression || - (moduleArg as StringExpression).text.asPlain == null) { + if (moduleArg is! StringExpression || moduleArg.text.asPlain == null) { return null; } - return (moduleArg as StringExpression).hasQuotes + return moduleArg.hasQuotes ? moduleArg.span.subspan(1, moduleArg.span.length - 2) : moduleArg.span; } diff --git a/pubspec.yaml b/pubspec.yaml index 1332c2c6..aa13f6ae 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,41 +1,41 @@ name: sass_migrator -version: 1.3.7 +version: 1.3.8 description: A tool for running migrations on Sass files author: Jennifer Thakar homepage: https://github.com/sass/migrator environment: - sdk: '>=2.6.0 <3.0.0' + sdk: '>=2.12.0 <3.0.0' dependencies: - args: "^1.5.1" - charcode: "^1.1.0" - collection: "^1.8.0" - glob: "^1.2.0" - js: "^0.6.0" - meta: ">=0.9.0 <2.0.0" - node_interop: "^1.0.0" - node_io: "^1.1.0" - path: "^1.6.0" - sass: "^1.24.3" - source_span: "^1.4.0" - string_scanner: "^1.0.5" - term_glyph: "^1.1.0" - tuple: "^1.0.2" + args: ^2.1.0 + charcode: ^1.2.0 + collection: ^1.15.0 + glob: ^2.0.1 + js: ^0.6.3 + meta: ^1.3.0 + node_interop: ^2.0.1 + node_io: ^2.0.0 + path: ^1.8.0 + sass: ^1.32.11 + source_span: ^1.8.1 + string_scanner: ^1.1.0 + term_glyph: ^1.2.0 + tuple: ^2.0.0 dev_dependencies: - archive: ">=1.0.0 <3.0.0" - cli_pkg: "^1.0.0-beta.10" - crypto: "^2.1.4" - grinder: "^0.8.0" - http: ">=0.11.0 <0.13.0" - node_preamble: "^1.3.0" - pub_semver: "^1.0.0" - test: ">=0.12.29 <2.0.0" - test_descriptor: "^1.1.1" - test_process: "^1.0.0" - xml: ">=2.4.0 <4.0.0" - yaml: "^2.0.0" + archive: ^3.1.2 + cli_pkg: ^1.3.0 + crypto: ^3.0.1 + grinder: ^0.9.0 + http: ^0.13.1 + node_preamble: ^2.0.0 + pub_semver: ^2.0.0 + test: ^1.17.1 + test_descriptor: ^2.0.0 + test_process: ^2.0.0 + xml: ^5.1.0 + yaml: ^3.1.0 executables: sass-migrator: sass_migrator diff --git a/test/migrators/module/partial_migration/file_mixing_use_and_import.hrx b/test/migrators/module/partial_migration/mixed_use_import.hrx similarity index 100% rename from test/migrators/module/partial_migration/file_mixing_use_and_import.hrx rename to test/migrators/module/partial_migration/mixed_use_import.hrx diff --git a/test/migrators/module/partial_migration/mixed_use_import_no_namespace.hrx b/test/migrators/module/partial_migration/mixed_use_import_no_namespace.hrx new file mode 100644 index 00000000..dc8eb37e --- /dev/null +++ b/test/migrators/module/partial_migration/mixed_use_import_no_namespace.hrx @@ -0,0 +1,38 @@ +<==> arguments +--migrate-deps + +<==> input/entrypoint.scss +@use "direct" as *; +@import "indirect"; + +a { + b: $existing; + c: $indirect-var; + d: $direct-var; +} + +<==> input/_direct.scss +$existing: red; +$var: blue; + +<==> input/_direct.import.scss +@forward "direct" as direct-*; + +<==> input/_indirect.scss +@use "direct"; + +$var: direct.$existing; + +<==> input/_indirect.import.scss +@forward "direct.import"; +@forward "indirect" as indirect-*; + +<==> output/entrypoint.scss +@use "direct" as *; +@use "indirect"; + +a { + b: $existing; + c: indirect.$var; + d: $var; +} diff --git a/test/utils.dart b/test/utils.dart index 774eb9d3..a84ca7eb 100644 --- a/test/utils.dart +++ b/test/utils.dart @@ -4,6 +4,7 @@ // license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. +import 'dart:convert'; import 'dart:io'; import 'package:cli_pkg/testing.dart' as pkg; @@ -24,7 +25,8 @@ Future runMigrator(List args) => pkg.start('sass-migrator', args, node: runNodeTests, workingDirectory: d.sandbox, - description: "migrator"); + description: "migrator", + encoding: utf8); /// Runs all tests for [migrator]. /// @@ -61,9 +63,9 @@ Future _testHrx(File hrxFile, String migrator) async { if (path.startsWith("entrypoint")) path ]); - if (files.expectedLog != null) { - expect(process.stdout, - emitsInOrder(files.expectedLog.trimRight().split("\n"))); + var expectedLog = files.expectedLog; + if (expectedLog != null) { + expect(process.stdout, emitsInOrder(expectedLog.trimRight().split("\n"))); } expect(process.stdout, emitsDone); @@ -87,17 +89,17 @@ Future _testHrx(File hrxFile, String migrator) async { } class _HrxTestFiles { - Map input = {}; - Map output = {}; + Map input = {}; + Map output = {}; List arguments = []; - String expectedLog; - String expectedError; - String expectedWarning; + String? expectedLog; + String? expectedError; + String? expectedWarning; _HrxTestFiles(String hrxText) { // TODO(jathak): Replace this with an actual HRX parser. - String filename; - String contents; + String? filename; + var contents = ""; for (var line in hrxText.substring(0, hrxText.length - 1).split("\n")) { if (line.startsWith("<==> ")) { if (filename != null) { @@ -112,7 +114,7 @@ class _HrxTestFiles { if (filename != null) _load(filename, contents); } - void _load(String filename, String contents) { + void _load(String filename, String? contents) { if (filename.startsWith("input/")) { input[filename.substring(6)] = contents; } else if (filename.startsWith("output/")) { @@ -133,8 +135,11 @@ class _HrxTestFiles { } } else if (filename == "arguments") { arguments = [ - for (var match in _argParseRegex.allMatches(contents)) - match.group(1) ?? match.group(2) ?? match.group(3) + for (var match in _argParseRegex.allMatches(contents!)) + match.group(1) ?? + match.group(2) ?? + match.group(3) ?? + (throw ArgumentError('Bad arguments for test')) ]; } } diff --git a/tool/grind.dart b/tool/grind.dart index 344ec00f..89cd120b 100644 --- a/tool/grind.dart +++ b/tool/grind.dart @@ -16,8 +16,8 @@ main(List args) { pkg.homebrewFormula.value = "migrator.rb"; pkg.jsRequires.value = {"fs": "fs", "os": "os", "path": "path"}; pkg.standaloneName.value = "sass-migrator"; - pkg.githubUser.value = Platform.environment["GH_USER"]; - pkg.githubPassword.value = Platform.environment["GH_TOKEN"]; + pkg.githubUser.fn = () => Platform.environment["GH_USER"]!; + pkg.githubPassword.fn = () => Platform.environment["GH_TOKEN"]!; pkg.addAllTasks(); grind(args);