Automatic fix for IDE0072 is changing code behaviour
- Dominant language
- No language data
- Stars
- 1.9k
- Forks
- 173
- Avg merge
- 10d 13h
- Merged PRs (30d)
- 1
Description
I might be in a minority for that, but i think `dotnet format` should not populate missing switch branches with `throw new NotImplementedException()`. Scenario:
I had this code in codebase:
```csharp
query = tasksSortBy switch
{
TasksSortBy.Name => query.OrderBy(e => e.Name),
TasksSortBy.Completed => query.OrderBy(e => e.Completed),
_ => query.OrderBy(e => e.CreatedAt)
};
```
Even though there is TasksSortBy.Unspecified - this code is just fine, since we have the wildcard `_` in the place to handle it. But i still want to keep the diagnostic to avoid missing enum values that are "real". So the diagnostic is fine, maybe Unspecified is too much (but it's a good practise from gRPC protocol, so we are using it there). My issue is with the automatic fix. After running the `dotnet format` i ended up with:
```csharp
query = tasksSortBy switch
{
TasksSortBy.Name => query.OrderBy(e => e.Name),
TasksSortBy.Completed => query.OrderBy(e => e.Completed),
TasksSortBy.Unspecified => throw new NotImplementedExcetion(),
_ => query.OrderBy(e => e.CreatedAt)
};
```
which for me it is outright dangerous. Fortunately for us - the integration test suite quickly caught the issue in CI pipeline, but the developer did not check the code after running the `dotnet format` (which is understandable, it should be just format). In this case `format` command changed the code behaviour. I'm opening this thread to figure out if more people think like me here :).
To be clear - the "solution" isn't that clear either, but from my perspective it would be better if `dotnet format` left the code "uncompilable" at this point. So:
```csharp
query = tasksSortBy switch
{
TasksSortBy.Name => query.OrderBy(e => e.Name),
TasksSortBy.Completed => query.OrderBy(e => e.Completed),
TasksSortBy.Unspecified => // TODO : implement that
_ => query.OrderBy(e => e.CreatedAt)
};
```
is much better, since it's forcing the developer to implement the missing branch. What do you all think about that?
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.