zio icon indicating copy to clipboard operation
zio copied to clipboard

Implicit string conversion to/from UPath is subtle & error prone

Open agocke opened this issue 2 years ago • 3 comments

Just hit this:

var fs = new SubFileSystem(new PhysicalFileSystem(), realPath));

I assume the proper way to do this is:

var physicalFs = new PhysicalFileSystem();
var fs = new SubFileSystem(physicalFs, physicalFs.ConvertPathFromInternal(realPath)));

If there weren't an implicit conversion, I probably would have guessed that. Overall, it seems like keeping UPaths and strings a bit separate is a good idea. Especially when working with subpaths like this, the chances that you'll confuse one for the other is pretty high.

agocke avatar Mar 08 '23 07:03 agocke

Agreed, that's one of the gray area of the API. Not allowing implicit conversion from string to UPath would have lead to annoying explicit cast in many places (more than the creation of filesystems but all methods provided by IFileSystem) so made this choice of simplicity vs correctness, but the downside is that there are a few cases like the one you point out that are not protected against a misusages (and the XML doc is not saying anything about this). Not sure I would want to change this after all these years this API has been released though...

xoofx avatar Mar 08 '23 07:03 xoofx

An analyzer might work here, along with an option to enable it. Not taking a breaking change makes sense.

In the meantime I might try an expedient route and see if the BannedApiAnalyzer could catch this.

agocke avatar Mar 08 '23 08:03 agocke

Aside: the / operator override is clever. 😄

agocke avatar Mar 08 '23 19:03 agocke