proposal: Lint to forbid Uri.file(), Uri.toFilePath(), package:path/... without `windows` arguments

[DanTup]

I don't know what a good name for this lint would be, but here's the problem...

Using the Uri.file() constructor or Uri.toFilePath() instance method (and I suspect a number of package:path functions in code that is compiled for the web may behave unexpectedly on Windows.

For example, I'm on Windows and if I run the following code in DartPad it provides the output shown:

  final path = r"C:\foo\bar.txt";
  final uri = Uri.file(path);
  print('scheme: ${uri.scheme}');   // Outputs empty string (not "file")
  print(uri.toString());            // Outputs "C%3A%5Cfoo%5Cbar.txt" not "file:///C:/foo/bar.txt"

This is documented behaviour but it's easy to overlook (especially in Flutter code) and if your app interacts with the host OS in some way, will fail. For example DevTools had this bug and webdev has some workarounds too.

It would be nice if we could turn on a lint that forbids calling these functions that might behave incorrectly when compiled for the web and be forced to pass the windows parameter (and any equiv for package:path) to reduce the chance of these kinds of issues.

Kind

Guards against errors

Bad Examples

var clientPath = r"c:\foo\bar.dart";
var uri = Uri.file(filePath); // Wrong when running in the browser

Good Examples

var clientPath = r"c:\foo\bar.dart";
// caller needs to decide on a mechanism to determine whether the path is a Windows path (for example by examining the path or otherwise knowing the source of the paths)
var isWindows = ...;
var uri = Uri.file(filePath, windows: isWindows); // Good because it provides an argument for `windows` 

Discussion

Examples of issues/workarounds from differing path handling for web:

• https://github.com/flutter/devtools/issues/7115
• https://github.com/dart-lang/webdev/blob/29da6087bbed9b8f60b7d1d5d4e3f0bf052bb29e/test_common/lib/utilities.dart#L39

Discussion checklist

• [ ] List any existing rules this proposal modifies, complements, overlaps or conflicts with.
• [x] List any relevant issues (reported here, the SDK Tracker, or elsewhere).
• [ ] If there's any prior art (e.g., in other linters), please add references here.
• [ ] If this proposal corresponds to Effective Dart or Flutter Style Guide advice, please call it out. (If there isn't any corresponding advice, should there be?)
• [x] If this proposal is motivated by real-world examples, please provide as many details as you can. Demonstrating potential impact is especially valuable.

[lrhn]

I wonder if we should just always recognize \ as a path separator in Uri.file. I can't come up with any reasonable user-case where you want a literal \ in your /-separated file path. If that's actually needed, you can always use Uri(scheme: "file", path: r"set_operations/\/").

Having a windows flag in the Uri class is already questionable. A URI is intended to be platform independent, so even knowing that platforms exist is problematic. And having the parameter default depend on Platform.isWindows is a library dependency that we'd rather be without.

Best guess: If we change it to use both / and \ as path separators, and start ignoring the windows parameter, nobody will notice, except the people who would not get what they expect/want today.

The toFilePath operation is worse, because that should know the current platform. It should be on File, and it is: File.fromUri(uri).path. (I can see I've given it a "TODO: deprecate and move to File comment already. Still think that.)

[DanTup]

I would certainly prefer that! It's always felt odd to me that Uri.file() behaves differently across platforms (when it seems like the operation is really the same for both).

toFilePath() is trickier... but would it be awful if it assumed any path starting with a letter and then : (or %3a) was Windows? I know it's not technically sound, but I suspect the number of non-Windows paths that start /[a-z]: is probably a very small number.

Things are probably a bit trickier for package:path, but it'd be nice if that could also be smarter for cases like path.join(a, b) where a is absolute. I don't know if that'd be possible for all functions though, so it could end up being inconsistent.

Tbh I'd probably personally be happy if Uri/path and friends forced me to say if it's Windows.. the real issue here is this is a bit of a foot-gun, and it seems like there's a skew away from Windows on things like CI bots (and Googlers!), the issues might not be picked up as quickly as other kinds of issues.

[lrhn]

Trying to recognize a Windows path is easier when it starts with /C: or // (the / before C: is there because file: URIs always have an authority of //localhost). It's harder for a relative URI reference, which is where this functionality is probably most used, because Uri.path("foo/bar/baz").toFilePath() has no clue.

Again, I'd just deprecate the function and ask people to use File.fromUri(...).path, or a new static String pathFromUri(Uri uri) on File. It's OK that the File class knows the current platform, or that Windows exists, it's in dart:io next to Platform.isWindows.

The path package always uses the platform path separator.

[DanTup]

It's harder for a relative URI reference, which is where this functionality is probably most used, because Uri.path("foo/bar/baz").toFilePath() has no clue.

Oh, I didn't even realise you could do this. Uri.file with a relative path returns a non-file scheme URI, and I thought toFilePath() would throw with that (I just tested, it does not). I think in all cases I've ever used this it's been absolute paths (relative file paths in URIs feels kinda weird).

Again, I'd just deprecate the function and ask people to use File.fromUri(...).path, or a new static String pathFromUri(Uri uri) on File. It's OK that the File class knows the current platform, or that Windows exists, it's in dart:io next to Platform.isWindows.

Gets my vote. Putting the platform-specific parts closer to where it matters (and where they can't be accidentally used in web) could avoid some of these issues.

The path package always uses the platform path separator.

Yeah, and I think a lint could still be useful here because the current platform might still not match what the developer expects (particularly for something like DevTools where there's code compiled to web interacting with a backend). Being forced to do path.windows.xxx() and path.posix.xxx() or similar would avoid issues and force the application to explicitly decide (and hopefully think about) what they want.

[parlough]

The toFilePath operation is worse, because that should know the current platform. It should be on File, and it is: File.fromUri(uri).path. (I can see I've given it a "TODO: deprecate and move to File comment already. Still think that.)

Yes please!! I think Uri constructors and methods changing behavior depending on the platform (and also not on web) has caused confusion at many points and will continue to do so. Will likely do so even more often as Wasm starts to run in more places.

Yeah, and I think a lint could still be useful here because the current platform might still not match what the developer expects...

I think that sounds valuable :)