[analyzer] Provide a `ResourceProvider` that takes a `FileSystem` from the file package

[gspencergoog]

trafficstars

Description

In the Dart Analyzer's API, the parseFile method takes a ResourceProvider object. In the rest of my app, I'm using the Dart file package to be able to replace the LocalFileSystem with a MemoryFileSystem during testing.

The ResourceProvider API has a similar mechanism that lets me use a MemoryResourceProvider, but what I really want is to just have a regular ResourceProvider that takes a FileSystem so I can just spoof the filesystem in one place, because using two means they are independent memory file systems that I have to keep in sync in my tests.

There doesn't appear to be a version of ResourceProvider that takes a FileSystem, but it doesn't look too hard to make one.

Could we provide one? Or, at least, would a PR that created one be considered, seeing as how it would probably add a dependency on the file package? Alternatively, the file package could provide a ResourceProvider, but it's much more widely used than the analyzer package, and would add a dependency on the analyzer.

[dart-github-bot]

Summary: The user wants to use the file package's FileSystem with the Dart Analyzer's ResourceProvider to simplify testing. Currently, the ResourceProvider doesn't accept a FileSystem, requiring separate memory file systems that need to be kept in sync. The user proposes adding a ResourceProvider that takes a FileSystem or having the file package provide a ResourceProvider.

[bwilkerson]

I wouldn't have any problem adding a dependency on the file package (it's already vended into the SDK), nor do I have any problem, in theory, with supporting a resource provider subclass that wraps the abstractions in that package. I am a little concerned about taking on the maintenance burden, though it probably wouldn't be high.

But a more interesting question might be whether we could replace our use of ResourceProvider with the abstractions in the file package. That would reduce the amount of code in the analyzer package and negate the need for a wrapper. To answer that question we'd need to compare the two systems to see how different they are (beyond just the names that were chosen). And, even if there were no blockers, moving all our code to the file package would be a bigger task than just writing a wrapper.

[gspencergoog]

I like the idea of eliminating ResourceProvider: the file package is pretty widely used, and standardizing on it seems like a good idea. The only complication is that it would be a big breaking change, but it would be easy to provide a deprecation period where they both work.

It seems like https://github.com/dart-lang/sdk/issues/56404 might also eventually provide a good replacement, but maybe that replacement is the file package?

In the meantime (the low level I/O issue is likely to take a while to resolve), I'll see what it would take to eliminate ResourceProvider and use the file package instead.

[bwilkerson]

Thanks for the link to #56404. I find this comment to be a bit concerning: https://github.com/dart-lang/sdk/issues/56404#issuecomment-2275930275. I think I'd want it to be fully supported before switching to it.

@devoncarew Can you comment on the status of the file package?

[gspencergoog]

OK, I took a look. Yeah, from an architectural standpoint, it might be worthwhile to remove ResourceProvider, I think: the ResourceProvider API duplicates a lot of the functionality that's in the file package, so from a functionality perspective it would work. However, it's pervasive in the analyzer, and it would be a lot of work to replace, both for us and for customers of the package.

Each of the functions on ResourceProvider is used 100-300 times in the analyzer package itself, and potentially in a lot of places by the users of the analyzer package and analyzer plugin writers. Replacing the resource provider with FileSystem would require implementing an OverlayFileSystem to replace OverlayResourceProvider (totally doable), but would also probably require replacing the File and Resource classes with file's File and FileSystemEntity, which is also a large change with lots of usage in the code.

So, overall, if you have a lot of developer cycles going to waste somewhere, they would be well used in the conversion, but since that is highly unlikely, I think the small effort required to write a ResourceProvider wrapper to FileSystem is a lot more economical.

[devoncarew]

@devoncarew Can you comment on the status of the file package?

It's not maintained by the Dart team, though members have updated it past breaking dart:io changes; the last word on it was "only supporting this for the extent needed for the flutter_tool itself; anything else is best effort for now".

[gspencergoog]

It's not maintained by the Dart team, though members have updated it past breaking dart:io changes; the last word on it was "only supporting this for the extent needed for the flutter_tool itself; anything else is best effort for now".

Given that we authored file, and the flutter package (not just flutter_tool) has a dev dependency on it, it seems like the Dash team should be supporting it in some fashion. The flutter repo has 596 files that import it, and the Dart SDK has 122 third party files that import it. I'd say we're already pretty dependent upon it.

[gspencergoog]

Here's a PR with a proposed wrapper: https://dart-review.googlesource.com/c/sdk/+/382243