diff --git a/pkg/analysis_server/test/domain_edit_dartfix_test.dart b/pkg/analysis_server/test/domain_edit_dartfix_test.dart index c4f13642d65..c1b942b2496 100644 --- a/pkg/analysis_server/test/domain_edit_dartfix_test.dart +++ b/pkg/analysis_server/test/domain_edit_dartfix_test.dart @@ -171,17 +171,8 @@ void test() { } '''); createProject(); - EditDartfixResult result = - await performFix(includedFixes: ['non-nullable']); - expect(result.suggestions.length, greaterThanOrEqualTo(1)); - expect(result.hasErrors, isFalse); - expectEdits(result.edits, ''' -int f(int? i) => 0; -int g(int? i) => f(i); -void test() { - g(null); -} -'''); + expectLater(() async => await performFix(includedFixes: ['non-nullable']), + throwsA(TypeMatcher())); } Future test_nonNullable_analysisOptions_created() async { diff --git a/pkg/dartdev/test/commands/migrate_test.dart b/pkg/dartdev/test/commands/migrate_test.dart index 513ddc33019..a3b0c7923bd 100644 --- a/pkg/dartdev/test/commands/migrate_test.dart +++ b/pkg/dartdev/test/commands/migrate_test.dart @@ -7,6 +7,10 @@ import 'package:test/test.dart'; import '../utils.dart'; +// TODO(jcollins-g): Set to true and/or remove when when NNBD is enabled in the +// SDK running this test. +bool _nnbdIsEnabled = false; + void main() { group('migrate', defineMigrateTests); } @@ -30,24 +34,34 @@ void defineMigrateTests() { test('directory implicit', () { p = project(mainSrc: 'int get foo => 1;\n'); - var result = - p.runSync('migrate', ['--no-web-preview'], workingDir: p.dirPath); - expect(result.exitCode, 0); - expect(result.stderr, isEmpty); + var result = p.runSync( + 'migrate', + [ + '--no-web-preview', + '--server-path=${p.absolutePathToAnalysisServerFile}' + ], + workingDir: p.dirPath); + expect(result.exitCode, _nnbdIsEnabled ? 0 : 2); + expect(result.stderr, _nnbdIsEnabled ? isEmpty : isNotEmpty); expect(result.stdout, contains('Generating migration suggestions')); }); test('directory explicit', () { p = project(mainSrc: 'int get foo => 1;\n'); - var result = p.runSync('migrate', ['--no-web-preview', p.dirPath]); - expect(result.exitCode, 0); - expect(result.stderr, isEmpty); + var result = p.runSync('migrate', [ + '--no-web-preview', + '--server-path=${p.absolutePathToAnalysisServerFile}', + p.dirPath + ]); + expect(result.exitCode, _nnbdIsEnabled ? 0 : 2); + expect(result.stderr, _nnbdIsEnabled ? isEmpty : isNotEmpty); expect(result.stdout, contains('Generating migration suggestions')); }); test('bad directory', () { p = project(mainSrc: 'int get foo => 1;\n'); - var result = p.runSync('migrate', ['foo_bar_dir']); + var result = p.runSync('migrate', + ['--server-path=${p.absolutePathToAnalysisServerFile}', 'foo_bar_dir']); expect(result.exitCode, 64); expect(result.stderr, contains('not found; please provide a path to a package or directory')); diff --git a/pkg/dartdev/test/utils.dart b/pkg/dartdev/test/utils.dart index f852731cda1..2a354eaa915 100644 --- a/pkg/dartdev/test/utils.dart +++ b/pkg/dartdev/test/utils.dart @@ -63,18 +63,34 @@ class TestProject { ); } - /// The path relative from `Directory.current.path` to `dartdev.dart` is - /// different when executing these tests locally versus on the Dart - /// buildbots, this if-else captures this change and branches for each case. - String get absolutePathToDartdevFile { - var dartdevFilePathOnBots = path.absolute(path.join( - Directory.current.path, 'pkg', 'dartdev', 'bin', 'dartdev.dart')); - if (File(dartdevFilePathOnBots).existsSync()) { - return dartdevFilePathOnBots; - } else { - return path - .absolute(path.join(Directory.current.path, 'bin', 'dartdev.dart')); + String _sdkRootPath; + + /// Return the root of the SDK. + String get sdkRootPath { + if (_sdkRootPath == null) { + // Assumes the script importing this one is somewhere under the SDK. + String current = path.canonicalize(Platform.script.toFilePath()); + do { + String tryDir = path.dirname(current); + if (File(path.join(tryDir, 'pkg', 'dartdev', 'bin', 'dartdev.dart')) + .existsSync()) { + _sdkRootPath = tryDir; + return _sdkRootPath; + } + current = tryDir; + } while (path.dirname(current) != current); + throw StateError('can not find SDK repository root'); } + return _sdkRootPath; + } + + String get absolutePathToDartdevFile { + return path.join(sdkRootPath, 'pkg', 'dartdev', 'bin', 'dartdev.dart'); + } + + String get absolutePathToAnalysisServerFile { + return path.join( + sdkRootPath, 'pkg', 'analysis_server', 'bin', 'server.dart'); } File findFile(String name) { diff --git a/pkg/dartfix/lib/src/migrate/migrate.dart b/pkg/dartfix/lib/src/migrate/migrate.dart index 621a03e5468..f8339949ecb 100644 --- a/pkg/dartfix/lib/src/migrate/migrate.dart +++ b/pkg/dartfix/lib/src/migrate/migrate.dart @@ -12,6 +12,7 @@ import 'package:analysis_server_client/server.dart'; import 'package:args/command_runner.dart'; import 'package:cli_util/cli_logging.dart'; import 'package:path/path.dart' as path; +import 'package:nnbd_migration/src/messages.dart'; import '../util.dart'; import 'apply.dart'; @@ -430,6 +431,18 @@ class _ServerNotifications with NotificationHandler { void onAnalysisErrors(AnalysisErrorsParams event) { _analysisErrorsController.add(event); } + + @override + void onServerError(ServerErrorParams event) { + stderr.writeln('encountered error: ${event.message}'); + + for (String fatal in [migratedAlready, nnbdExperimentOff, sdkNnbdOff]) { + if (event.message.contains(fatal)) { + server.kill(); + exit(2); + } + } + } } class _ServerListener with ServerListener { diff --git a/pkg/nnbd_migration/lib/src/messages.dart b/pkg/nnbd_migration/lib/src/messages.dart new file mode 100644 index 00000000000..3a32d14f076 --- /dev/null +++ b/pkg/nnbd_migration/lib/src/messages.dart @@ -0,0 +1,9 @@ +// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +const String migratedAlready = + "Seem to be migrating code that's already migrated"; +const String nnbdExperimentOff = + 'Analyzer seems to need the nnbd experiment on in the SDK.'; +const String sdkNnbdOff = 'Analysis seems to have an SDK without NNBD enabled.'; diff --git a/pkg/nnbd_migration/lib/src/nullability_migration_impl.dart b/pkg/nnbd_migration/lib/src/nullability_migration_impl.dart index 0dda2f2fddd..48d2fbcce9d 100644 --- a/pkg/nnbd_migration/lib/src/nullability_migration_impl.dart +++ b/pkg/nnbd_migration/lib/src/nullability_migration_impl.dart @@ -3,6 +3,7 @@ // BSD-style license that can be found in the LICENSE file. import 'package:analysis_server/src/protocol_server.dart'; +import 'package:analyzer/dart/analysis/features.dart'; import 'package:analyzer/dart/analysis/results.dart'; import 'package:analyzer/file_system/physical_file_system.dart'; import 'package:analyzer/src/generated/resolver.dart'; @@ -15,6 +16,7 @@ import 'package:nnbd_migration/src/edge_builder.dart'; import 'package:nnbd_migration/src/edit_plan.dart'; import 'package:nnbd_migration/src/fix_aggregator.dart'; import 'package:nnbd_migration/src/fix_builder.dart'; +import 'package:nnbd_migration/src/messages.dart'; import 'package:nnbd_migration/src/node_builder.dart'; import 'package:nnbd_migration/src/nullability_node.dart'; import 'package:nnbd_migration/src/postmortem_file.dart'; @@ -79,6 +81,7 @@ class NullabilityMigrationImpl implements NullabilityMigration { @override void finalizeInput(ResolvedUnitResult result) { + _sanityCheck(result); if (!_propagated) { _propagated = true; _graph.propagate(_postmortemFileWriter); @@ -127,6 +130,7 @@ class NullabilityMigrationImpl implements NullabilityMigration { } void prepareInput(ResolvedUnitResult result) { + _sanityCheck(result); if (_variables == null) { _variables = Variables(_graph, result.typeProvider, instrumentation: _instrumentation, @@ -145,6 +149,7 @@ class NullabilityMigrationImpl implements NullabilityMigration { } void processInput(ResolvedUnitResult result) { + _sanityCheck(result); var unit = result.unit; try { DecoratedTypeParameterBounds.current = _decoratedTypeParameterBounds; @@ -167,6 +172,26 @@ class NullabilityMigrationImpl implements NullabilityMigration { _graph.update(_postmortemFileWriter); } + void _sanityCheck(ResolvedUnitResult result) { + final equalsParamType = result.typeProvider.objectType + .getMethod('==') + .parameters[0] + .type + .getDisplayString(withNullability: true); + if (equalsParamType == 'Object*') { + throw StateError(nnbdExperimentOff); + } + + if (equalsParamType != 'Object') { + throw StateError(sdkNnbdOff); + } + + if (result.unit.featureSet.isEnabled(Feature.non_nullable)) { + // TODO(jcollins-g): Allow for skipping already migrated compilation units. + throw StateError('$migratedAlready: ${result.path}'); + } + } + static Location _computeLocation( LineInfo lineInfo, SourceEdit edit, Source source) { final locationInfo = lineInfo.getLocation(edit.offset);