Unverified Commit 68ec71f4 authored by Andrew Kolos's avatar Andrew Kolos Committed by GitHub

[flutter_tools] Remove `Version.unknown` (#124771)

Fixes #124756 by removing the concept of `Version.unknown`.

`Version` fields that needed the ability to represent an unknown version have been made nullable. Assigning `null` to them represents an unknown version.
parent 4543766a
......@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
import '../base/common.dart';
import '../base/file_system.dart';
import '../base/io.dart';
import '../base/process.dart';
......@@ -39,15 +40,16 @@ final RegExp _dotHomeStudioVersionMatcher =
// See https://github.com/flutter/flutter/issues/124252.
String? get javaPath => globals.androidStudio?.javaPath;
class AndroidStudio implements Comparable<AndroidStudio> {
class AndroidStudio {
/// A [version] value of null represents an unknown version.
AndroidStudio(
this.directory, {
Version? version,
this.configured,
this.version,
this.configuredPath,
this.studioAppName = 'AndroidStudio',
this.presetPluginsPath,
}) : version = version ?? Version.unknown {
_init(version: version);
}) {
_initAndValidate();
}
static AndroidStudio? fromMacOSBundle(String bundlePath) {
......@@ -147,8 +149,13 @@ class AndroidStudio implements Comparable<AndroidStudio> {
final String directory;
final String studioAppName;
final Version version;
final String? configured;
/// The version of Android Studio.
///
/// A null value represents an unknown version.
final Version? version;
final String? configuredPath;
final String? presetPluginsPath;
String? _javaPath;
......@@ -163,8 +170,12 @@ class AndroidStudio implements Comparable<AndroidStudio> {
if (presetPluginsPath != null) {
return presetPluginsPath!;
}
final int major = version.major;
final int minor = version.minor;
// TODO(andrewkolos): This is a bug. We shouldn't treat an unknown
// version as equivalent to 0.0.
// See https://github.com/flutter/flutter/issues/121468.
final int major = version?.major ?? 0;
final int minor = version?.minor ?? 0;
final String? homeDirPath = globals.fsUtils.homeDirPath;
if (homeDirPath == null) {
return null;
......@@ -217,25 +228,31 @@ class AndroidStudio implements Comparable<AndroidStudio> {
List<String> get validationMessages => _validationMessages;
@override
int compareTo(AndroidStudio other) {
final int result = version.compareTo(other.version);
if (result == 0) {
return directory.compareTo(other.directory);
}
return result;
}
/// Locates the newest, valid version of Android Studio.
///
/// In the case that `--android-studio-dir` is configured, the version of
/// Android Studio found at that location is always returned, even if it is
/// invalid.
static AndroidStudio? latestValid() {
final String? configuredStudio = globals.config.getValue('android-studio-dir') as String?;
if (configuredStudio != null) {
String configuredStudioPath = configuredStudio;
if (globals.platform.isMacOS && !configuredStudioPath.endsWith('Contents')) {
configuredStudioPath = globals.fs.path.join(configuredStudioPath, 'Contents');
final String? configuredStudioPath = globals.config.getValue('android-studio-dir') as String?;
if (configuredStudioPath != null) {
String correctedConfiguredStudioPath = configuredStudioPath;
if (globals.platform.isMacOS && !correctedConfiguredStudioPath.endsWith('Contents')) {
correctedConfiguredStudioPath = globals.fs.path.join(correctedConfiguredStudioPath, 'Contents');
}
if (!globals.fs.directory(correctedConfiguredStudioPath).existsSync()) {
throwToolExit('''
Could not find the Android Studio installation at the manually configured path "$configuredStudioPath".
Please verify that the path is correct and update it by running this command: flutter config --android-studio-dir '<path>'
To have flutter search for Android Studio installations automatically, remove
the configured path by running this command: flutter config --android-studio-dir ''
''');
}
return AndroidStudio(configuredStudioPath,
configured: configuredStudio);
return AndroidStudio(correctedConfiguredStudioPath,
configuredPath: configuredStudioPath);
}
// Find all available Studio installations.
......@@ -245,7 +262,19 @@ class AndroidStudio implements Comparable<AndroidStudio> {
}
AndroidStudio? newest;
for (final AndroidStudio studio in studios.where((AndroidStudio s) => s.isValid)) {
if (newest == null || studio.compareTo(newest) > 0) {
if (newest == null) {
newest = studio;
continue;
}
// We prefer installs with known versions.
if (studio.version != null && newest.version == null) {
newest = studio;
} else if (studio.version != null && newest.version != null &&
studio.version! > newest.version!) {
newest = studio;
} else if (studio.version == null && newest.version == null &&
studio.directory.compareTo(newest.directory) > 0) {
newest = studio;
}
}
......@@ -331,13 +360,16 @@ class AndroidStudio implements Comparable<AndroidStudio> {
static List<AndroidStudio> _allLinuxOrWindows() {
final List<AndroidStudio> studios = <AndroidStudio>[];
bool hasStudioAt(String path, { Version? newerThan }) {
bool alreadyFoundStudioAt(String path, { Version? newerThan }) {
return studios.any((AndroidStudio studio) {
if (studio.directory != path) {
return false;
}
if (newerThan != null) {
return studio.version.compareTo(newerThan) >= 0;
if (studio.version == null) {
return false;
}
return studio.version!.compareTo(newerThan) >= 0;
}
return true;
});
......@@ -373,7 +405,7 @@ class AndroidStudio implements Comparable<AndroidStudio> {
for (final Directory entity in entities) {
final AndroidStudio? studio = AndroidStudio.fromHomeDot(entity);
if (studio != null && !hasStudioAt(studio.directory, newerThan: studio.version)) {
if (studio != null && !alreadyFoundStudioAt(studio.directory, newerThan: studio.version)) {
studios.removeWhere((AndroidStudio other) => other.directory == studio.directory);
studios.add(studio);
}
......@@ -404,7 +436,7 @@ class AndroidStudio implements Comparable<AndroidStudio> {
version: Version.parse(version),
studioAppName: title,
);
if (!hasStudioAt(studio.directory, newerThan: studio.version)) {
if (!alreadyFoundStudioAt(studio.directory, newerThan: studio.version)) {
studios.removeWhere((AndroidStudio other) => other.directory == studio.directory);
studios.add(studio);
}
......@@ -415,14 +447,14 @@ class AndroidStudio implements Comparable<AndroidStudio> {
}
final String? configuredStudioDir = globals.config.getValue('android-studio-dir') as String?;
if (configuredStudioDir != null && !hasStudioAt(configuredStudioDir)) {
if (configuredStudioDir != null && !alreadyFoundStudioAt(configuredStudioDir)) {
studios.add(AndroidStudio(configuredStudioDir,
configured: configuredStudioDir));
configuredPath: configuredStudioDir));
}
if (globals.platform.isLinux) {
void checkWellKnownPath(String path) {
if (globals.fs.isDirectorySync(path) && !hasStudioAt(path)) {
if (globals.fs.isDirectorySync(path) && !alreadyFoundStudioAt(path)) {
studios.add(AndroidStudio(path));
}
}
......@@ -438,12 +470,12 @@ class AndroidStudio implements Comparable<AndroidStudio> {
return keyMatcher.stringMatch(plistValue)?.split('=').last.trim().replaceAll('"', '');
}
void _init({Version? version}) {
void _initAndValidate() {
_isValid = false;
_validationMessages.clear();
if (configured != null) {
_validationMessages.add('android-studio-dir = $configured');
if (configuredPath != null) {
_validationMessages.add('android-studio-dir = $configuredPath');
}
if (!globals.fs.isDirectorySync(directory)) {
......@@ -453,15 +485,15 @@ class AndroidStudio implements Comparable<AndroidStudio> {
final String javaPath;
if (globals.platform.isMacOS) {
if (version != null && version.major < 2020) {
if (version != null && version!.major < 2020) {
javaPath = globals.fs.path.join(directory, 'jre', 'jdk', 'Contents', 'Home');
} else if (version != null && version.major == 2022) {
} else if (version != null && version!.major == 2022) {
javaPath = globals.fs.path.join(directory, 'jbr', 'Contents', 'Home');
} else {
javaPath = globals.fs.path.join(directory, 'jre', 'Contents', 'Home');
}
} else {
if (version != null && version.major == 2022) {
if (version != null && version!.major == 2022) {
javaPath = globals.fs.path.join(directory, 'jbr');
} else {
javaPath = globals.fs.path.join(directory, 'jre');
......
......@@ -6,7 +6,6 @@ import '../base/config.dart';
import '../base/file_system.dart';
import '../base/platform.dart';
import '../base/user_messages.dart';
import '../base/version.dart';
import '../doctor_validator.dart';
import '../intellij/intellij.dart';
import 'android_studio.dart';
......@@ -46,7 +45,7 @@ class AndroidStudioValidator extends DoctorValidator {
final List<ValidationMessage> messages = <ValidationMessage>[];
ValidationType type = ValidationType.missing;
final String? studioVersionText = _studio.version == Version.unknown
final String? studioVersionText = _studio.version == null
? null
: userMessages.androidStudioVersion(_studio.version.toString());
messages.add(ValidationMessage(
......@@ -83,7 +82,7 @@ class AndroidStudioValidator extends DoctorValidator {
(String m) => ValidationMessage.error(m),
));
messages.add(ValidationMessage(userMessages.androidStudioNeedsUpdate));
if (_studio.configured != null) {
if (_studio.configuredPath != null) {
messages.add(ValidationMessage(userMessages.androidStudioResetDir));
}
}
......
......@@ -4,13 +4,20 @@
import 'package:meta/meta.dart';
// TODO(reidbaker): Investigate using pub_semver instead of this class.
/// Represents the version of some piece of software.
///
/// While a [Version] object has fields resembling semver, it does not
/// necessarily represent a semver version.
@immutable
class Version implements Comparable<Version> {
/// Creates a new [Version] object.
factory Version(int? major, int? minor, int? patch, {String? text}) {
///
/// A null [minor] or [patch] version is logically equivalent to 0. Using null
/// for these parameters only affects the generation of [text], if no value
/// for it is provided.
factory Version(int major, int? minor, int? patch, {String? text}) {
if (text == null) {
text = major == null ? '0' : '$major';
text = '$major';
if (minor != null) {
text = '$text.$minor';
}
......@@ -19,7 +26,7 @@ class Version implements Comparable<Version> {
}
}
return Version._(major ?? 0, minor ?? 0, patch ?? 0, text);
return Version._(major, minor ?? 0, patch ?? 0, text);
}
/// Public constant constructor when all fields are non-null, without default value fallbacks.
......@@ -67,9 +74,6 @@ class Version implements Comparable<Version> {
return primary;
}
static Version get unknown => Version(0, 0, 0, text: 'unknown');
/// The major version number: "1" in "1.2.3".
final int major;
......
......@@ -19,7 +19,7 @@ class ConfigCommand extends FlutterCommand {
negatable: false,
help: 'Clear the saved development certificate choice used to sign apps for iOS device deployment.');
argParser.addOption('android-sdk', help: 'The Android SDK directory.');
argParser.addOption('android-studio-dir', help: 'The Android Studio install directory.');
argParser.addOption('android-studio-dir', help: 'The Android Studio install directory. If unset, flutter will search for valid installs at well-known locations.');
argParser.addOption('build-dir', help: 'The relative path to override a projects build directory.',
valueHelp: 'out/');
argParser.addFlag('machine',
......
......@@ -125,11 +125,6 @@ abstract class IntelliJValidator extends DoctorValidator {
}
void _validateIntelliJVersion(List<ValidationMessage> messages, Version minVersion) {
// Ignore unknown versions.
if (minVersion == Version.unknown) {
return;
}
final Version? installedVersion = Version.parse(version);
if (installedVersion == null) {
return;
......
......@@ -18,8 +18,7 @@ const String extensionMarketplaceUrl =
'https://marketplace.visualstudio.com/items?itemName=$extensionIdentifier';
class VsCode {
VsCode._(this.directory, this.extensionDirectory, { Version? version, this.edition, required FileSystem fileSystem})
: version = version ?? Version.unknown {
VsCode._(this.directory, this.extensionDirectory, { this.version, this.edition, required FileSystem fileSystem}) {
if (!fileSystem.isDirectorySync(directory)) {
_validationMessages.add(ValidationMessage.error('VS Code not found at $directory'));
......@@ -76,7 +75,7 @@ class VsCode {
final String directory;
final String extensionDirectory;
final Version version;
final Version? version;
final String? edition;
Version? _extensionVersion;
......@@ -284,7 +283,7 @@ class VsCode {
@override
String toString() =>
'VS Code ($version)${_extensionVersion != Version.unknown ? ', Flutter ($_extensionVersion)' : ''}';
'VS Code ($version)${_extensionVersion != null ? ', Flutter ($_extensionVersion)' : ''}';
static String? _getVersionFromPackageJson(String packageJsonPath, FileSystem fileSystem) {
if (!fileSystem.isFileSync(packageJsonPath)) {
......
......@@ -7,7 +7,6 @@ import 'package:process/process.dart';
import '../base/file_system.dart';
import '../base/platform.dart';
import '../base/user_messages.dart';
import '../base/version.dart';
import '../doctor_validator.dart';
import 'vscode.dart';
......@@ -24,7 +23,7 @@ class VsCodeValidator extends DoctorValidator {
@override
Future<ValidationResult> validate() async {
final String? vsCodeVersionText = _vsCode.version == Version.unknown
final String? vsCodeVersionText = _vsCode.version == null
? null
: userMessages.vsCodeVersion(_vsCode.version.toString());
......
......@@ -4,6 +4,8 @@
import 'package:file/memory.dart';
import 'package:flutter_tools/src/android/android_studio.dart';
import 'package:flutter_tools/src/base/common.dart';
import 'package:flutter_tools/src/base/config.dart';
import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/platform.dart';
import 'package:flutter_tools/src/base/version.dart';
......@@ -849,6 +851,177 @@ void main() {
),
});
});
group('latestValid', () {
late Platform platform;
late FileSystem fileSystem;
setUp(() {
platform = FakePlatform(
operatingSystem: 'windows',
environment: <String, String>{
'LOCALAPPDATA': r'C:\Users\Dash\AppData\Local',
}
);
fileSystem = MemoryFileSystem.test(style: FileSystemStyle.windows);
});
testUsingContext('choses the install with the latest version', () {
const List<String> versions = <String> [
'4.0',
'2022.0',
'3.1',
];
for (final String version in versions) {
fileSystem.file('C:\\Users\\Dash\\AppData\\Local\\Google\\AndroidStudio$version\\.home')
..createSync(recursive: true)
..writeAsStringSync('C:\\Program Files\\AndroidStudio$version');
fileSystem.directory('C:\\Program Files\\AndroidStudio$version')
.createSync(recursive: true);
}
expect(AndroidStudio.allInstalled().length, 3);
expect(AndroidStudio.latestValid()!.version, Version(2022, 0, 0));
}, overrides: <Type, Generator>{
FileSystem: () => fileSystem,
Platform: () => platform,
ProcessManager: () => FakeProcessManager.any(),
});
testUsingContext('prefers installs with known versions over installs with unknown versions', () {
const List<String> versions = <String> [
'3.0',
'unknown',
];
for (final String version in versions) {
fileSystem.file('C:\\Users\\Dash\\AppData\\Local\\Google\\AndroidStudio$version\\.home')
..createSync(recursive: true)
..writeAsStringSync('C:\\Program Files\\AndroidStudio$version');
fileSystem.directory('C:\\Program Files\\AndroidStudio$version')
.createSync(recursive: true);
}
expect(AndroidStudio.allInstalled().length, 2);
expect(AndroidStudio.latestValid()!.version, Version(3, 0, 0));
}, overrides: <Type, Generator>{
FileSystem: () => fileSystem,
Platform: () => platform,
ProcessManager: () => FakeProcessManager.any(),
});
testUsingContext('choses install with lexicographically greatest directory if no installs have known versions', () {
const List<String> versions = <String> [
'Apple',
'Zucchini',
'Banana',
];
for (final String version in versions) {
fileSystem.file('C:\\Users\\Dash\\AppData\\Local\\Google\\AndroidStudio$version\\.home')
..createSync(recursive: true)
..writeAsStringSync('C:\\Program Files\\AndroidStudio$version');
fileSystem.directory('C:\\Program Files\\AndroidStudio$version')
.createSync(recursive: true);
}
expect(AndroidStudio.allInstalled().length, 3);
expect(AndroidStudio.latestValid()!.directory, r'C:\Program Files\AndroidStudioZucchini');
}, overrides: <Type, Generator>{
FileSystem: () => fileSystem,
Platform: () => platform,
ProcessManager: () => FakeProcessManager.any(),
});
testUsingContext('choses install with lexicographically greatest directory if all installs have the same version', () {
fileSystem.file(r'C:\Users\Dash\AppData\Local\Google\AndroidStudioPreview4.0\.home')
..createSync(recursive: true)
..writeAsStringSync(r'C:\Program Files\AndroidStudioPreview4.0');
fileSystem.directory(r'C:\Program Files\AndroidStudioPreview4.0')
.createSync(recursive: true);
fileSystem.file(r'C:\Users\Dash\AppData\Local\Google\AndroidStudio4.0\.home')
..createSync(recursive: true)
..writeAsStringSync(r'C:\Program Files\AndroidStudio4.0');
fileSystem.directory(r'C:\Program Files\AndroidStudio4.0')
.createSync(recursive: true);
expect(AndroidStudio.allInstalled().length, 2);
expect(AndroidStudio.latestValid()!.directory, contains('Preview'));
}, overrides: <Type, Generator>{
FileSystem: () => fileSystem,
Platform: () => platform,
ProcessManager: () => FakeProcessManager.any(),
});
testUsingContext('always chooses the install configured by --android-studio-dir, even if the install is invalid', () {
const String configuredAndroidStudioDir = r'C:\Users\Dash\Desktop\android-studio';
globals.config.setValue('android-studio-dir', configuredAndroidStudioDir);
// The directory exists, but nothing is inside.
fileSystem.directory(configuredAndroidStudioDir).createSync(recursive: true);
(globals.processManager as FakeProcessManager).excludedExecutables.add(
fileSystem.path.join(configuredAndroidStudioDir, 'jre', 'bin', 'java'),
);
const List<String> validVersions = <String> [
'4.0',
'2.0',
'3.1',
];
for (final String version in validVersions) {
fileSystem.file('C:\\Users\\Dash\\AppData\\Local\\Google\\AndroidStudio$version\\.home')
..createSync(recursive: true)
..writeAsStringSync('C:\\Program Files\\AndroidStudio$version');
fileSystem.directory('C:\\Program Files\\AndroidStudio$version')
.createSync(recursive: true);
}
const List<String> validJavaPaths = <String>[
r'C:\Program Files\AndroidStudio4.0\jre\bin\java',
r'C:\Program Files\AndroidStudio2.0\jre\bin\java',
r'C:\Program Files\AndroidStudio3.1\jre\bin\java',
];
for (final String javaPath in validJavaPaths) {
(globals.processManager as FakeProcessManager).addCommand(FakeCommand(
command: <String>[
globals.fs.path.join(javaPath),
'-version',
],
));
}
expect(AndroidStudio.allInstalled().length, 4);
final AndroidStudio chosenInstall = AndroidStudio.latestValid()!;
expect(chosenInstall.directory, configuredAndroidStudioDir);
expect(chosenInstall.isValid, false);
}, overrides: <Type, Generator>{
Config: () => Config.test(),
FileSystem: () => fileSystem,
Platform: () => platform,
ProcessManager: () => FakeProcessManager.empty(),
});
testUsingContext('throws a ToolExit if --android-studio-dir is configured but the directory does not exist', () async {
const String configuredAndroidStudioDir = r'C:\Users\Dash\Desktop\android-studio';
globals.config.setValue('android-studio-dir', configuredAndroidStudioDir);
expect(fileSystem.directory(configuredAndroidStudioDir).existsSync(), false);
expect(() => AndroidStudio.latestValid(), throwsA(
(dynamic e) => e is ToolExit &&
e.message!.startsWith('Could not find the Android Studio installation at the manually configured path')
)
);
}, overrides: <Type, Generator>{
Config: () => Config.test(),
FileSystem: () => fileSystem,
Platform: () => platform,
ProcessManager: () => FakeProcessManager.any(),
});
});
}
class FakePlistUtils extends Fake implements PlistParser {
......
......@@ -1064,7 +1064,7 @@ void main() {
class FakeXcodeProjectInterpreter extends Fake implements XcodeProjectInterpreter {
@override
Version version = Version.unknown;
Version version = Version(0, 0, 0);
@override
bool isInstalled = false;
......
......@@ -24,8 +24,7 @@ baz=qux
group('Version', () {
testWithoutContext('can parse and compare', () {
expect(Version.unknown.toString(), equals('unknown'));
expect(Version(null, null, null).toString(), equals('0'));
expect(Version(0, null, null).toString(), equals('0'));
expect(const Version.withText(1, 2, 3, 'versionText').toString(), 'versionText');
final Version v1 = Version.parse('1')!;
......@@ -33,7 +32,7 @@ baz=qux
expect(v1.minor, equals(0));
expect(v1.patch, equals(0));
expect(v1, greaterThan(Version.unknown));
expect(v1, greaterThan(Version(0, 0, 0)));
final Version v2 = Version.parse('1.2')!;
expect(v2.major, equals(1));
......
......@@ -5,7 +5,6 @@
import 'package:file/memory.dart';
import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/platform.dart';
import 'package:flutter_tools/src/base/version.dart';
import 'package:flutter_tools/src/vscode/vscode.dart';
import '../../src/common.dart';
......@@ -38,7 +37,7 @@ void main() {
final VsCode vsCode = VsCode.fromDirectory('', '', fileSystem: fileSystem);
expect(vsCode.version, Version.unknown);
expect(vsCode.version, null);
});
testWithoutContext('can locate VS Code installed via Snap', () {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment