Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FileInfoFactory and DirectoryInfoFactory Wrap implementations don't match the interface for nullability #975

Closed
mikeeheler opened this issue Apr 21, 2023 · 2 comments · Fixed by #976
Labels
state: needs discussion Issues that need further discussion state: released Issues that are released type: bug Issues that describe misbehaving functionality

Comments

@mikeeheler
Copy link

mikeeheler commented Apr 21, 2023

Describe the bug
The implementations for FileInfoFactory.Wrap and DirectoryInfoFactory.Wrap require non-null references, while the interfaces are defined as [return: NotNullIfNotNull("fileInfo")] IFileInfoFactory.Wrap(FileInfo? fileInfo) and likewise for IDirectoryInfoFactory.Wrap.

To Reproduce
Steps to reproduce the behavior:

  1. new FileSystem().FileInfo.Wrap(null);
  2. The analyzer and compiler accept the syntax since IFileInfoFactory.Wrap accepts FileInfo? and returns IFileInfo?
  3. An ArgumentNullException is thrown

Expected behavior
I expect to get a null IFileInfo? back if the input is null, and no exception thrown.

Additional context
Fixing this will help compatibility with System.CommandLine where defining options as FileInfo? is a common practice, i.e.:

var inputFileOption = new Option<FileInfo?>("--input");
var command = new Command("test") { inputFileOption };
command.SetHandler(
    (FileInfo? inputFile) => {
        var fileSystem = new FileSystem();
        var concreteCommand = new ConcreteCommand(fileSystem:fileSystem) {
            InputFile = fileSystem.FileInfo.Wrap(inputFile)
        }
        concreteCommand.Run();
    },
    inputFileOption
);
@mikeeheler mikeeheler added state: needs discussion Issues that need further discussion type: bug Issues that describe misbehaving functionality labels Apr 21, 2023
@vbreuss
Copy link
Member

vbreuss commented Apr 22, 2023

Thanks for creating this issue, @mikeeheler.
I already created a pull request (#976) to fix this bug!

@mergify mergify bot closed this as completed in #976 Apr 23, 2023
mergify bot pushed a commit that referenced this issue Apr 23, 2023
…r `IFileInfo` (#976)

Fixes #975:
When providing `null` as parameter to the implementations of `IFileInfoFactory` or `IDirectoryInfoFactory` return `null` instead of throwing an exception.
mergify bot pushed a commit that referenced this issue Apr 24, 2023
…FileSystemWatcher` (#977)

Building on #976 also fix null-handling for DriveInfo and FileSystemWatcher factories:
When providing null as parameter to the implementations of IDriveInfoFactory or IFileSystemWatcherFactory return null instead of throwing an exception.

This extends the issue reported in #975 to other factories used in System.IO.Abstractions.
@github-actions
Copy link

This is addressed in release v19.2.17.

@github-actions github-actions bot added the state: released Issues that are released label Apr 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state: needs discussion Issues that need further discussion state: released Issues that are released type: bug Issues that describe misbehaving functionality
Projects
None yet
2 participants