First cut at a file system abstraction for the front end.

pkg/front_end/lib/file_system.dart

+// Copyright (c) 2016, 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.
+
+library front_end.file_system;
+
+import 'dart:async';
+
+import 'package:path/path.dart' as path;
+
+/// Abstract interface to file system operations.
+///
+/// All front end interaction with the filesystem goes through this interface;

[scheglov, 2016/10/18 17:10:32]

"filesystem" or "file system" here?

[Paul Berry, 2016/10/18 20:34:57]

I'm now using "file system" consistently.

+/// this makes it possible for clients to use the front end in a way that
+/// doesn't require file system access (e.g. to run unit tests, or to run
+/// inside a browser).
+///
+/// Not intended to be implemented or extended by clients.
+abstract class FileSystem {
+  /// Returns a [path.Context] suitable for use with this [FileSystem].

[Brian Wilkerson, 2016/10/18 17:36:59]

opinion: "[path.Context]" --> "context" | "path context". The type is on the
next line (for navigation purposes) and this reads much better.

[Paul Berry, 2016/10/18 20:34:57]

Done.

+  path.Context get context;
+
+  /// Returns a [FileSystemEntity] corresponding to the given [path].
+  ///
+  /// Uses of `..` and `.` in path are normalized before returning (so, for
+  /// example, `entityForPath('./foo') and entityForPath('foo') are equivalent).

[Brian Wilkerson, 2016/10/18 17:36:59]

nit: Missing or extra back-quote(s).

[Paul Berry, 2016/10/18 20:34:58]

Done.

+  ///
+  /// Does not check whether a file or folder exists at the given location.
+  FileSystemEntity entityForPath(String path);

[scheglov, 2016/10/18 17:10:32]

Maybe drop some extra strings and just name this File.

[Paul Berry, 2016/10/18 20:34:57]

I'm concerned that this would lead to confusion, because the thing pointed to by
a FileSystemEntity might not be a file; it might be a folder (or might not
exist).

+
+  /// Returns a [FileSystemEntity] corresponding to the given [uri].
+  ///
+  /// Uses of `..` and `.` in the URI are normalized before returning.
+  ///
+  /// If [uri] is not a `file:` URI, an [Error] will be thrown.
+  ///
+  /// Does not check whether a file or folder exists at the given location.
+  FileSystemEntity entityForUri(Uri uri);
+}
+
+/// Abstract representation of a file system entity that may or may not exist.
+///
+/// Not intended to be implemented or extended by clients.
+abstract class FileSystemEntity {

[Paul Berry, 2016/10/18 16:56:43]

I'm not sure how useful this class is.  An alternative implementation would be
to put all the functionality in FileSystem and just use paths, e.g.:

abstract class FileSystem {
  path.Context get context;
  Future<List<int>> readFileBytes(String path);
  Future<String> readFileString(String path);
}

(Clients that wanted to convert between URIs and paths could still do so using
FileSystem.context.fromUri() and FileSystem.context.toUri().)

Thoughts?

[scheglov, 2016/10/18 17:10:31]

I like having a more structured API with separate object for separate file.

[Brian Wilkerson, 2016/10/18 17:36:59]

I agree.

1. It's better to have objects with an API than to pass around primitive types
like strings.

2. If you fail when passed invalid file paths, then you have one point of
failure--when creating the instance--rather than a possible exception for every
method you support.

3. You can provide a single readFileAsString() method rather than a
readFileAsString(String) / readUriAsString(Uri) pair.

[Paul Berry, 2016/10/18 20:34:58]

Acknowledged.  Leaving as is.

+  /// Returns the absolute path represented by this file system entity.

[Brian Wilkerson, 2016/10/18 17:36:59]

Is it a normalized path or the path passed in to entityForPath?

[Paul Berry, 2016/10/18 20:34:57]

The normalized path.  I've added a clarifying comment.

+  String get path;
+
+  /// Returns a `file:` URI representing this file system entity.

[scheglov, 2016/10/18 17:10:31]

Is it a Uri wrapper for [path], or the original Uri from ""entityForUri?

[Paul Berry, 2016/10/18 20:34:58]

It's a Uri built from [path] (it has to be because the original Uri passed to
[entityForUri] might have contained `..` or `.` which got normalized away).

I've added a clarifying comment.

+  Uri get uri;
+
+  /// Attempts to access this file system entity as a file and read its contents
+  /// as raw bytes.
+  ///
+  /// If no such file exists, `null` is returned.  If an error occurs while
+  /// attempting to read the file, or the entity is a directory, the future is
+  /// completed with an [Exception].
+  Future<List<int>> readFileBytes();

[scheglov, 2016/10/18 17:10:32]

Maybe just readBytes().

I don't think we need to make a distinction between file existence and reading
error. Even if we perform a check (how - synchronously?), it is possible that
the file will be removed before we are able to read it.

[Brian Wilkerson, 2016/10/18 17:36:59]

Unless there's a strong argument to do otherwise I think we should match the
protocol from dart:io so that authors of clients of the front end don't have to
learn a new API.
I agree. Any failure to provide the content should throw an exception. I believe
that's the way the dart:io API works, and I think we should be consistent.

[Paul Berry, 2016/10/18 20:34:57]

Good points.  Fixed.

[Paul Berry, 2016/10/18 20:34:57]

Good point.  You are correct that this is how dart:io works.  I've modified the
comment to be consistent with this.

+
+  /// Attempts to access this file system entity as a file and read its contents
+  /// as a string.
+  ///
+  /// If no such file exists, `null` is returned.  If an error occurs while
+  /// attempting to read the file, or the entity is a directory, the future is
+  /// completed with an [Exception].
+  Future<String> readFileString();

[scheglov, 2016/10/18 17:10:32]

Again, maybe just readString().

We could specify that the file is read as UTF8.

The same comment about checking that file exists.

[Paul Berry, 2016/10/18 20:34:57]

Done.

+}