Enforce valid package names on flutter create (#9854) * Enforce valid package names on flutter create Fixes #9564 * refactor * fix other tests
diff --git a/packages/flutter_tools/lib/src/commands/create.dart b/packages/flutter_tools/lib/src/commands/create.dart index a54ef14..fe1383a 100644 --- a/packages/flutter_tools/lib/src/commands/create.dart +++ b/packages/flutter_tools/lib/src/commands/create.dart
@@ -4,6 +4,8 @@ import 'dart:async'; +import 'package:linter/src/rules/pub/package_names.dart' as package_names; // ignore: implementation_imports + import '../android/android.dart' as android; import '../android/android_sdk.dart' as android_sdk; import '../android/gradle.dart' as gradle; @@ -96,7 +98,7 @@ // TODO(goderbauer): Work-around for: https://github.com/dart-lang/path/issues/24 if (fs.path.basename(dirPath) == '.') dirPath = fs.path.dirname(dirPath); - final String projectName = _normalizeProjectName(fs.path.basename(dirPath)); + final String projectName = fs.path.basename(dirPath); String error =_validateProjectDir(dirPath, flutterRoot: flutterRoot); if (error != null) @@ -235,14 +237,6 @@ } } -String _normalizeProjectName(String name) { - name = name.replaceAll('-', '_').replaceAll(' ', '_'); - // Strip any extension (like .dart). - if (name.contains('.')) - name = name.substring(0, name.indexOf('.')); - return name; -} - String _createAndroidIdentifier(String name) { return 'com.yourcompany.$name'; } @@ -283,6 +277,9 @@ /// Return `null` if the project name is legal. Return a validation message if /// we should disallow the project name. String _validateProjectName(String projectName) { + if (!package_names.isValidPackageName(projectName)) + return '"$projectName" is not a valid Dart package name.\n\n${package_names.details}'; + if (_packageDependencies.contains(projectName)) { return "Invalid project name: '$projectName' - this will conflict with Flutter " "package dependencies.";
diff --git a/packages/flutter_tools/test/analytics_test.dart b/packages/flutter_tools/test/analytics_test.dart index 2a6c9d0..d3f9ded 100644 --- a/packages/flutter_tools/test/analytics_test.dart +++ b/packages/flutter_tools/test/analytics_test.dart
@@ -5,7 +5,6 @@ import 'package:args/command_runner.dart'; import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/cache.dart'; -import 'package:flutter_tools/src/commands/create.dart'; import 'package:flutter_tools/src/commands/config.dart'; import 'package:flutter_tools/src/commands/doctor.dart'; import 'package:flutter_tools/src/doctor.dart'; @@ -40,19 +39,17 @@ flutterUsage.onSend.listen((Map<String, dynamic> data) => count++); flutterUsage.enabled = false; - final CreateCommand command = new CreateCommand(); - CommandRunner<Null> runner = createTestCommandRunner(command); - await runner.run(<String>['create', '--no-pub', temp.path]); + await createProject(temp); expect(count, 0); flutterUsage.enabled = true; - await runner.run(<String>['create', '--no-pub', temp.path]); + await createProject(temp); expect(count, flutterUsage.isFirstRun ? 0 : 2); count = 0; flutterUsage.enabled = false; final DoctorCommand doctorCommand = new DoctorCommand(); - runner = createTestCommandRunner(doctorCommand); + final CommandRunner<Null>runner = createTestCommandRunner(doctorCommand); await runner.run(<String>['doctor']); expect(count, 0); }, overrides: <Type, Generator>{
diff --git a/packages/flutter_tools/test/analyze_once_test.dart b/packages/flutter_tools/test/analyze_once_test.dart index c4dc986..5cc5f97 100644 --- a/packages/flutter_tools/test/analyze_once_test.dart +++ b/packages/flutter_tools/test/analyze_once_test.dart
@@ -22,12 +22,14 @@ group('analyze once', () { Directory tempDir; + String projectPath; File libMain; setUpAll(() { Cache.disableLocking(); tempDir = fs.systemTempDirectory.createTempSync('analyze_once_test_').absolute; - libMain = fs.file(fs.path.join(tempDir.path, 'lib', 'main.dart')); + projectPath = fs.path.join(tempDir.path, 'flutter_project'); + libMain = fs.file(fs.path.join(projectPath, 'lib', 'main.dart')); }); tearDownAll(() { @@ -38,7 +40,7 @@ testUsingContext('flutter create', () async { await runCommand( command: new CreateCommand(), - arguments: <String>['create', tempDir.path], + arguments: <String>['create', projectPath], statusTextContains: <String>[ 'All done!', 'Your main program file is lib/main.dart', @@ -50,7 +52,7 @@ // Analyze in the current directory - no arguments testUsingContext('flutter analyze working directory', () async { await runCommand( - command: new AnalyzeCommand(workingDirectory: tempDir), + command: new AnalyzeCommand(workingDirectory: fs.directory(projectPath)), arguments: <String>['analyze'], statusTextContains: <String>['No issues found!'], ); @@ -86,7 +88,7 @@ // Analyze in the current directory - no arguments await runCommand( - command: new AnalyzeCommand(workingDirectory: tempDir), + command: new AnalyzeCommand(workingDirectory: fs.directory(projectPath)), arguments: <String>['analyze'], statusTextContains: <String>[ 'Analyzing', @@ -116,7 +118,7 @@ // Insert an analysis_options.yaml file in the project // which will trigger a lint for broken code that was inserted earlier - final File optionsFile = fs.file(fs.path.join(tempDir.path, 'analysis_options.yaml')); + final File optionsFile = fs.file(fs.path.join(projectPath, 'analysis_options.yaml')); await optionsFile.writeAsString(''' include: package:flutter/analysis_options_user.yaml linter: @@ -126,7 +128,7 @@ // Analyze in the current directory - no arguments await runCommand( - command: new AnalyzeCommand(workingDirectory: tempDir), + command: new AnalyzeCommand(workingDirectory: fs.directory(projectPath)), arguments: <String>['analyze'], statusTextContains: <String>[ 'Analyzing',
diff --git a/packages/flutter_tools/test/create_test.dart b/packages/flutter_tools/test/create_test.dart index 1a3bfd5..3d26334 100644 --- a/packages/flutter_tools/test/create_test.dart +++ b/packages/flutter_tools/test/create_test.dart
@@ -6,7 +6,6 @@ import 'dart:convert'; import 'package:args/command_runner.dart'; -import 'package:flutter_tools/src/base/common.dart'; import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/io.dart'; import 'package:flutter_tools/src/cache.dart'; @@ -20,6 +19,7 @@ void main() { group('create', () { Directory temp; + Directory projectDir; setUpAll(() { Cache.disableLocking(); @@ -27,6 +27,7 @@ setUp(() { temp = fs.systemTempDirectory.createTempSync('flutter_tools'); + projectDir = temp.childDirectory('flutter_project'); }); tearDown(() { @@ -36,25 +37,25 @@ // Verify that we create a project that is well-formed. testUsingContext('project', () async { return _createAndAnalyzeProject( - temp, + projectDir, <String>[], - fs.path.join(temp.path, 'lib', 'main.dart'), + fs.path.join(projectDir.path, 'lib', 'main.dart'), ); }); testUsingContext('project with-driver-test', () async { return _createAndAnalyzeProject( - temp, + projectDir, <String>['--with-driver-test'], - fs.path.join(temp.path, 'lib', 'main.dart'), + fs.path.join(projectDir.path, 'lib', 'main.dart'), ); }); testUsingContext('plugin project', () async { return _createAndAnalyzeProject( - temp, + projectDir, <String>['--plugin'], - fs.path.join(temp.path, 'example', 'lib', 'main.dart'), + fs.path.join(projectDir.path, 'example', 'lib', 'main.dart'), ); }); @@ -65,21 +66,21 @@ final CreateCommand command = new CreateCommand(); final CommandRunner<Null> runner = createTestCommandRunner(command); - await runner.run(<String>['create', '--no-pub', temp.path]); + await runner.run(<String>['create', '--no-pub', projectDir.path]); void expectExists(String relPath) { - expect(fs.isFileSync('${temp.path}/$relPath'), true); + expect(fs.isFileSync('${projectDir.path}/$relPath'), true); } expectExists('lib/main.dart'); - for (FileSystemEntity file in temp.listSync(recursive: true)) { + for (FileSystemEntity file in projectDir.listSync(recursive: true)) { if (file is File && file.path.endsWith('.dart')) { final String original = file.readAsStringSync(); final Process process = await Process.start( sdkBinaryName('dartfmt'), <String>[file.path], - workingDirectory: temp.path, + workingDirectory: projectDir.path, ); final String formatted = await process.stdout.transform(UTF8.decoder).join(); @@ -93,7 +94,7 @@ fs.path.join('ios', 'Flutter', 'Generated.xcconfig'); expectExists(xcodeConfigPath); final File xcodeConfigFile = - fs.file(fs.path.join(temp.path, xcodeConfigPath)); + fs.file(fs.path.join(projectDir.path, xcodeConfigPath)); final String xcodeConfig = xcodeConfigFile.readAsStringSync(); expect(xcodeConfig, contains('FLUTTER_ROOT=')); expect(xcodeConfig, contains('FLUTTER_APPLICATION_PATH=')); @@ -107,9 +108,9 @@ final CreateCommand command = new CreateCommand(); final CommandRunner<Null> runner = createTestCommandRunner(command); - await runner.run(<String>['create', '--no-pub', temp.path]); + await runner.run(<String>['create', '--no-pub', projectDir.path]); - await runner.run(<String>['create', '--no-pub', temp.path]); + await runner.run(<String>['create', '--no-pub', projectDir.path]); }); // Verify that we help the user correct an option ordering issue @@ -119,13 +120,10 @@ final CreateCommand command = new CreateCommand(); final CommandRunner<Null> runner = createTestCommandRunner(command); - try { - await runner.run(<String>['create', temp.path, '--pub']); - fail('expected ToolExit exception'); - } on ToolExit catch (e) { - expect(e.exitCode, 2); - expect(e.message, contains('Try moving --pub')); - } + expect( + runner.run(<String>['create', projectDir.path, '--pub']), + throwsToolExit(exitCode: 2, message: 'Try moving --pub') + ); }); // Verify that we fail with an error code when the file exists. @@ -133,15 +131,23 @@ Cache.flutterRoot = '../..'; final CreateCommand command = new CreateCommand(); final CommandRunner<Null> runner = createTestCommandRunner(command); - final File existingFile = fs.file("${temp.path.toString()}/bad"); + final File existingFile = fs.file("${projectDir.path.toString()}/bad"); if (!existingFile.existsSync()) - existingFile.createSync(); - try { - await runner.run(<String>['create', existingFile.path]); - fail('expected ToolExit exception'); - } on ToolExit catch (e) { - expect(e.message, contains('file exists')); - } + existingFile.createSync(recursive: true); + expect( + runner.run(<String>['create', existingFile.path]), + throwsToolExit(message: 'file exists') + ); + }); + + testUsingContext('fails when invalid package name', () async { + Cache.flutterRoot = '../..'; + final CreateCommand command = new CreateCommand(); + final CommandRunner<Null> runner = createTestCommandRunner(command); + expect( + runner.run(<String>['create', fs.path.join(projectDir.path, 'invalidName')]), + throwsToolExit(message: '"invalidName" is not a valid Dart package name.') + ); }); }); }
diff --git a/packages/flutter_tools/test/format_test.dart b/packages/flutter_tools/test/format_test.dart index 43c610d..563d4b6 100644 --- a/packages/flutter_tools/test/format_test.dart +++ b/packages/flutter_tools/test/format_test.dart
@@ -2,12 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'dart:async'; - import 'package:args/command_runner.dart'; import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/cache.dart'; -import 'package:flutter_tools/src/commands/create.dart'; import 'package:flutter_tools/src/commands/format.dart'; import 'package:test/test.dart'; @@ -27,17 +24,10 @@ temp.deleteSync(recursive: true); }); - Future<Null> createProject() async { - final CreateCommand command = new CreateCommand(); - final CommandRunner<Null> runner = createTestCommandRunner(command); - - await runner.run(<String>['create', '--no-pub', temp.path]); - } - testUsingContext('a file', () async { - await createProject(); + final String projectPath = await createProject(temp); - final File srcFile = fs.file(fs.path.join(temp.path, 'lib', 'main.dart')); + final File srcFile = fs.file(fs.path.join(projectPath, 'lib', 'main.dart')); final String original = srcFile.readAsStringSync(); srcFile.writeAsStringSync(original.replaceFirst('main()', 'main( )'));
diff --git a/packages/flutter_tools/test/packages_test.dart b/packages/flutter_tools/test/packages_test.dart index 805a54b..3e8ebb1 100644 --- a/packages/flutter_tools/test/packages_test.dart +++ b/packages/flutter_tools/test/packages_test.dart
@@ -7,7 +7,6 @@ import 'package:args/command_runner.dart'; import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/cache.dart'; -import 'package:flutter_tools/src/commands/create.dart'; import 'package:flutter_tools/src/commands/packages.dart'; import 'package:test/test.dart'; @@ -30,15 +29,8 @@ temp.deleteSync(recursive: true); }); - Future<Null> createProject() async { - final CreateCommand command = new CreateCommand(); - final CommandRunner<Null> runner = createTestCommandRunner(command); - - await runner.run(<String>['create', '--no-pub', temp.path]); - } - - Future<Null> runCommand(String verb, { List<String> args }) async { - await createProject(); + Future<String> runCommand(String verb, { List<String> args }) async { + final String projectPath = await createProject(temp); final PackagesCommand command = new PackagesCommand(); final CommandRunner<Null> runner = createTestCommandRunner(command); @@ -46,32 +38,34 @@ final List<String> commandArgs = <String>['packages', verb]; if (args != null) commandArgs.addAll(args); - commandArgs.add(temp.path); + commandArgs.add(projectPath); await runner.run(commandArgs); + + return projectPath; } - void expectExists(String relPath) { - expect(fs.isFileSync('${temp.path}/$relPath'), true); + void expectExists(String projectPath, String relPath) { + expect(fs.isFileSync(fs.path.join(projectPath, relPath)), true); } // Verify that we create a project that is well-formed. testUsingContext('get', () async { - await runCommand('get'); - expectExists('lib/main.dart'); - expectExists('.packages'); + final String projectPath = await runCommand('get'); + expectExists(projectPath, 'lib/main.dart'); + expectExists(projectPath, '.packages'); }); testUsingContext('get --offline', () async { - await runCommand('get', args: <String>['--offline']); - expectExists('lib/main.dart'); - expectExists('.packages'); + final String projectPath = await runCommand('get', args: <String>['--offline']); + expectExists(projectPath, 'lib/main.dart'); + expectExists(projectPath, '.packages'); }); testUsingContext('upgrade', () async { - await runCommand('upgrade'); - expectExists('lib/main.dart'); - expectExists('.packages'); + final String projectPath = await runCommand('upgrade'); + expectExists(projectPath, 'lib/main.dart'); + expectExists(projectPath, '.packages'); }); }); }
diff --git a/packages/flutter_tools/test/src/base/common_test.dart b/packages/flutter_tools/test/src/base/common_test.dart index 7b1420f..35aa577 100644 --- a/packages/flutter_tools/test/src/base/common_test.dart +++ b/packages/flutter_tools/test/src/base/common_test.dart
@@ -14,7 +14,15 @@ }); test('throws ToolExit with exitCode', () { - expect(() => throwToolExit('message', exitCode: 42), throwsToolExit(42)); + expect(() => throwToolExit('message', exitCode: 42), throwsToolExit(exitCode: 42)); + }); + + test('throws ToolExit with message', () { + expect(() => throwToolExit('message'), throwsToolExit(message: 'message')); + }); + + test('throws ToolExit with message and exit code', () { + expect(() => throwToolExit('message', exitCode: 42), throwsToolExit(exitCode: 42, message: 'message')); }); }); }
diff --git a/packages/flutter_tools/test/src/common.dart b/packages/flutter_tools/test/src/common.dart index 38c95f3..dbec977 100644 --- a/packages/flutter_tools/test/src/common.dart +++ b/packages/flutter_tools/test/src/common.dart
@@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:async'; + import 'package:args/command_runner.dart'; import 'package:test/test.dart'; @@ -9,6 +11,7 @@ import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/platform.dart'; import 'package:flutter_tools/src/base/process.dart'; +import 'package:flutter_tools/src/commands/create.dart'; import 'package:flutter_tools/src/runner/flutter_command.dart'; import 'package:flutter_tools/src/runner/flutter_command_runner.dart'; @@ -63,10 +66,13 @@ } /// Matcher for functions that throw [ToolExit]. -Matcher throwsToolExit([int exitCode]) { - return exitCode == null - ? throwsA(isToolExit) - : throwsA(allOf(isToolExit, (ToolExit e) => e.exitCode == exitCode)); +Matcher throwsToolExit({int exitCode, String message}) { + Matcher matcher = isToolExit; + if (exitCode != null) + matcher = allOf(matcher, (ToolExit e) => e.exitCode == exitCode); + if (message != null) + matcher = allOf(matcher, (ToolExit e) => e.message.contains(message)); + return throwsA(matcher); } /// Matcher for [ToolExit]s. @@ -81,3 +87,13 @@ /// Matcher for [ProcessExit]s. const Matcher isProcessExit = const isInstanceOf<ProcessExit>(); + +/// Creates a flutter project in the [temp] directory. +/// Returns the path to the flutter project. +Future<String> createProject(Directory temp) async { + final String projectPath = fs.path.join(temp.path, 'flutter_project'); + final CreateCommand command = new CreateCommand(); + final CommandRunner<Null> runner = createTestCommandRunner(command); + await runner.run(<String>['create', '--no-pub', projectPath]); + return projectPath; +} \ No newline at end of file
diff --git a/packages/flutter_tools/test/upgrade_test.dart b/packages/flutter_tools/test/upgrade_test.dart index f192be0..c5e7d39 100644 --- a/packages/flutter_tools/test/upgrade_test.dart +++ b/packages/flutter_tools/test/upgrade_test.dart
@@ -2,13 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'dart:async'; - -import 'package:args/command_runner.dart'; import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/os.dart'; import 'package:flutter_tools/src/cache.dart'; -import 'package:flutter_tools/src/commands/create.dart'; import 'package:flutter_tools/src/commands/upgrade.dart'; import 'package:test/test.dart'; @@ -50,18 +46,11 @@ temp.deleteSync(recursive: true); }); - Future<Null> createProject() async { - final CreateCommand command = new CreateCommand(); - final CommandRunner<Null> runner = createTestCommandRunner(command); - await runner.run(<String>['create', '--no-pub', temp.path]); - } testUsingContext('in project', () async { - await createProject(); - - final String proj = temp.path; - expect(findProjectRoot(proj), proj); - expect(findProjectRoot(fs.path.join(proj, 'lib')), proj); + final String projectPath = await createProject(temp); + expect(findProjectRoot(projectPath), projectPath); + expect(findProjectRoot(fs.path.join(projectPath, 'lib')), projectPath); final String hello = fs.path.join(Cache.flutterRoot, 'examples', 'hello_world'); expect(findProjectRoot(hello), hello); @@ -69,8 +58,8 @@ }); testUsingContext('outside project', () async { - await createProject(); - expect(findProjectRoot(temp.parent.path), null); + final String projectPath = await createProject(temp); + expect(findProjectRoot(fs.directory(projectPath).parent.path), null); expect(findProjectRoot(Cache.flutterRoot), null); }); });