Enforce valid package names on flutter create (#9854)

* Enforce valid package names on flutter create

Fixes #9564

* refactor

* fix other tests

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 @@ class CreateCommand extends FlutterCommand {
     // 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 @@ Host platform code is in the android/ and ios/ directories under $relativePlugin
   }
 }
 
-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 @@ final Set<String> _packageDependencies = new Set<String>.from(<String>[
 /// 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.";

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 @@ void main() {
       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>{

packages/flutter_tools/test/analyze_once_test.dart

@@ -22,12 +22,14 @@ void main() {
 
   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 @@ void main() {
     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 @@ void main() {
     // 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 @@ void main() {
 
       // 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 @@ void main() {
 
       // 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 @@ void main() {
 
       // 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',

packages/flutter_tools/test/create_test.dart

@@ -6,7 +6,6 @@ import 'dart:async';
 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 @@ import 'src/context.dart';
 void main() {
   group('create', () {
     Directory temp;
+    Directory projectDir;
 
     setUpAll(() {
       Cache.disableLocking();
@@ -27,6 +27,7 @@ void main() {
 
     setUp(() {
       temp = fs.systemTempDirectory.createTempSync('flutter_tools');
+      projectDir = temp.childDirectory('flutter_project');
     });
 
     tearDown(() {
@@ -36,25 +37,25 @@ void main() {
     // 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 @@ void main() {
       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 @@ void main() {
           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 @@ void main() {
       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 @@ void main() {
       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 @@ void main() {
       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.')
+      );
     });
   });
 }

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 @@ void main() {
       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(  )'));
 

packages/flutter_tools/test/packages_test.dart

@@ -7,7 +7,6 @@ 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/packages.dart';
 import 'package:test/test.dart';
 
@@ -30,15 +29,8 @@ void main() {
       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 @@ void main() {
       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');
     });
   });
 }

packages/flutter_tools/test/src/base/common_test.dart

@@ -14,7 +14,15 @@ void main() {
     });
 
     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'));
     });
   });
 }

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/common.dart';
 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 @@ void updateFileModificationTime(String path,
 }
 
 /// 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 throwsProcessExit([dynamic exitCode]) {
 
 /// 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;
+}

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 @@ void main() {
         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 @@ void main() {
       });
 
       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);
       });
     });