Runtime: FileInfo.Delete() does not update .Exists

[CodeAngry]

FileInfo.Delete() does not update .Exists

So I just noticed:

    // this file exists
    var fi = new FileInfo(@"S:\Ome\File.path");
    // here fi.Exists == true
    fi.Delete();
    // still fi.Exists == true
    fi.Refresh();
    // finally fi.Exists == false

The code I wrote assumed Delete() changes .Exists and it was failing very unexpectedly. Checked the actual source code and found Delete() is implemented as:

    public override void Delete() => FileSystem.DeleteFile(FullPath);

instead of:

    public override void Delete()
    {
        FileSystem.DeleteFile(FullPath);
        Invalidate();
    }

Why is this simple operation not updating .Exists if it succeeds? On the other hand MoveTo() changes the fi object completely updating to the new path. So if you do a Delete() you need to followup with a Refresh() while a MoveTo() is good (even if it would have been better if Move() returned a new FileInfo with the new details instead of changing the current one).

Is this inconsistency by design or was it just overlooked?

[stephentoub]

Is this inconsistency by design or was it just overlooked?

https://docs.microsoft.com/en-us/dotnet/api/system.io.fileinfo.exists?view=netframework-4.8#remarks

"When first called, FileInfo calls Refresh and caches information about the file. On subsequent calls, you must call Refresh to get the latest copy of the information."

[CodeAngry]

I understand that but shouldn't Delete invalidate things. This is not about .Exists but .Delete. Move invalidates the data.

[wfurt]

AFAIK Refresh is expected for compatibility reasons even if not really intuitive. Create() has similar behavior.

cc: @JeremyKuhne

[AntonLapounov]

Then we must revert dotnet/corefx#40677 as well. We should either preserve compatibility or fix the issue consistently, not just one method of many.

[JeremyKuhne]

I agree that updating the behavior consistently when users explicitly change state is the way to go. We need to document this breaking change, of course.

[Jlalond]

@JeremyKuhne you can assign me to this if it's still available

[Jlalond]

Hey @JeremyKuhne / @carlossanlop to clarify, we're just calling refresh when we create a FileInfo, or delete, and the same for DirectoryInfo? @wfurt in the referenced issue you mentioned you had a way to more performantly do this other than a full refresh, do you still advise going down that route?

[wfurt]

I would suggest to call Invalidate() as proposed above @Jlalond . If you do Refresh() we will be forced to do work immediately. With Invalidate() we would do work only if somebody asks again. I would expect that for example most developers assume file deleted after FileInfo.Delete() if it does not throw. I see no reason to do fstat just to verify that.

[Jlalond]

That's fair @wfurt, but should we call refresh upon create then?

[wfurt]

Yes, I think so. We should be consistent and as @JeremyKuhne suggested we should update state for explicit actions. It seems like https://github.com/dotnet/corefx/pull/40677 did it only when creating directories.
I'm not sure how should we handle documentation @carlossanlop

[carlossanlop]

I can take care of the documentation after this change is made. I also have to address dotnet/corefx#40677 as you requested.

[Jlalond]

Alright so refresh upon create and invalidate on delete. Thanks everyone

[wfurt]

dotnet/corefx#40677 use Invalidate() for Create(). I think same logic can apply for creation. One can assume the object exist if Create() does not throw. We can defer work unless somebody explicitly asks.

[andrew-boyarshin]

Is this already fixed in #34229? If so, milestone change is incorrect and this issue should be closed.

[danmosemsft]

Thank you yes @carlossanlop this it's fixed I think