Unverified Commit 2221a9f5 authored by Jonah Williams's avatar Jonah Williams Committed by GitHub

[flutter_tool] Cache the path context in the flutter tool (#48250)

recomputing the path context in the getter violates flutter repo style guide and adds a small but measurable overhead to all path operations
parent 53f98c98
......@@ -7,6 +7,7 @@ import 'dart:io' as io show Directory, File, Link;
import 'package:file/file.dart';
import 'package:meta/meta.dart';
import 'package:path/path.dart' as p; // ignore: package_path_import
import '../globals.dart' as globals;
import 'common.dart' show throwToolExit;
......@@ -36,6 +37,21 @@ class ErrorHandlingFileSystem extends ForwardingFileSystem {
@override
File file(dynamic path) => ErrorHandlingFile(delegate, delegate.file(path));
// Caching the path context here and clearing when the currentDirectory setter
// is updated works since the flutter tool restricts usage of dart:io directly
// via the forbidden import tests. Otherwise, the path context's current
// working directory might get out of sync, leading to unexpected results from
// methods like `path.relative`.
@override
p.Context get path => _cachedPath ??= delegate.path;
p.Context _cachedPath;
@override
set currentDirectory(dynamic path) {
_cachedPath = null;
delegate.currentDirectory = path;
}
}
class ErrorHandlingFile
......
......@@ -6,6 +6,7 @@ import 'package:file/file.dart';
import 'package:flutter_tools/src/base/error_handling_file_system.dart';
import 'package:mockito/mockito.dart';
import 'package:platform/platform.dart';
import 'package:path/path.dart' as path; // ignore: package_path_import
import '../../src/common.dart';
import '../../src/context.dart';
......@@ -14,6 +15,7 @@ import '../../src/testbed.dart';
class MockFile extends Mock implements File {}
class MockFileSystem extends Mock implements FileSystem {}
class MockPlatform extends Mock implements Platform {}
class MockPathContext extends Mock implements path.Context {}
void main() {
group('throws ToolExit on Windows', () {
......@@ -32,6 +34,7 @@ void main() {
when(windowsPlatform.isWindows).thenReturn(true);
when(windowsPlatform.isLinux).thenReturn(false);
when(windowsPlatform.isMacOS).thenReturn(false);
when(mockFileSystem.path).thenReturn(MockPathContext());
testbed = Testbed(overrides: <Type, Generator>{
Platform: () => windowsPlatform,
});
......@@ -96,4 +99,23 @@ void main() {
expectedMessage: 'The file is being used by another program',
);
});
test('Caches path context correctly', () {
final MockFileSystem mockFileSystem = MockFileSystem();
final FileSystem fs = ErrorHandlingFileSystem(mockFileSystem);
expect(identical(fs.path, fs.path), true);
});
test('Clears cache when CWD changes', () {
final MockFileSystem mockFileSystem = MockFileSystem();
final FileSystem fs = ErrorHandlingFileSystem(mockFileSystem);
final Object firstPath = fs.path;
fs.currentDirectory = null;
when(mockFileSystem.path).thenReturn(MockPathContext());
expect(identical(firstPath, fs.path), false);
});
}
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