Migrate to null safety (#183)

Author: jathak

.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

CHANGELOG.md

@@ -1,3 +1,7 @@
+## 1.3.8
+
+* No user-visible changes.
+
 ## 1.3.7
 
 ### Module Migrator

bin/sass_migrator.dart

@@ -12,6 +12,7 @@ import 'package:sass_migrator/src/runner.dart';
 // We can't declare args as a List<String> or Iterable<String> 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<String>());
 }

lib/src/migration_visitor.dart

@@ -51,20 +51,21 @@ abstract class MigrationVisitor extends RecursiveAstVisitor {
   /// The patches to be applied to the stylesheet being migrated.
   @protected
   List<Patch> get patches => UnmodifiableListView(_patches);
-  List<Patch> _patches;
+  List<Patch> get _patches => assertInStylesheet(__patches, 'patches');
+  List<Patch>? __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.
   ///
   /// If this is `null`, relative imports aren't supported in the current
   /// 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>(T? value, String name) {
+    if (value != null) return value;
+    throw StateError("Can't access $name when not visiting a stylesheet.");
+  }
 }

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<Map<Uri, String>> {
       "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<Uri, String>> {
   Map<Uri, String> run() {
     var allMigrated = <Uri, String>{};
     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<Map<Uri, String>> {
       }
 
       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<Map<Uri, String>> {
   /// 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<Map<Uri, String>> {
       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}');
-      }
+      });
     }
   }
 }

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<Uri, String> 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;