Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate to null safety #183

Merged
merged 7 commits into from
Apr 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 1.3.8

* No user-visible changes.

## 1.3.7

### Module Migrator
Expand Down
3 changes: 2 additions & 1 deletion bin/sass_migrator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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>());
}
32 changes: 22 additions & 10 deletions lib/src/migration_visitor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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
Expand All @@ -102,10 +103,10 @@ abstract class MigrationVisitor extends RecursiveAstVisitor {
"it's loaded.");
}

_migrated[_currentUrl] = results;
_migrated[currentUrl] = results;
}

_patches = oldPatches;
__patches = oldPatches;
_currentUrl = oldUrl;
}

Expand Down Expand Up @@ -139,7 +140,7 @@ abstract class MigrationVisitor extends RecursiveAstVisitor {
_importer = oldImporter;
} else {
_missingDependencies.putIfAbsent(
context.sourceUrl.resolveUri(dependency), () => context);
context.sourceUrl!.resolveUri(dependency), () => context);
}
}

Expand Down Expand Up @@ -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.");
}
}
28 changes: 14 additions & 14 deletions lib/src/migrator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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.
///
Expand Down Expand Up @@ -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
];
Expand All @@ -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();
Expand All @@ -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]);
Expand All @@ -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}');
}
});
}
}
}
15 changes: 8 additions & 7 deletions lib/src/migrators/division.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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 ||
Expand All @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment referencing https://github.com/dart-lang/sdk/issues/45348 so it's clear why this non-null assertion is necessary.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

channels.contents[1].accept(this);
last.left.accept(this);
}, isDivisionAllowed: true);
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down
Loading