dotnet / dotnet/runtime

TypeConverter.GetConvertFromException and TypeConverter.GetConvertToException methods should not throw

Open
#122,790 6 comments 0 reactions 0 assignees View on GitHub
area-System.ComponentModel help wanted
Dominant language
C#
Stars
18.3k
Forks
5.6k
PR merge metrics
PR metrics pending

Description

The [TypeConverter.GetConvertFromException](https://github.com/dotnet/runtime/blob/3c040478f19e0f317790acab05dbe3ada9f52dc4/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeConverter.cs#L192) and [TypeConverter.GetConvertToException](https://github.com/dotnet/runtime/blob/3c040478f19e0f317790acab05dbe3ada9f52dc4/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeConverter.cs#L202) methods are throwing exceptions instead of returning the exception instances.

Take a look at the usage:

https://github.com/dotnet/runtime/blob/3c040478f19e0f317790acab05dbe3ada9f52dc4/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeConverter.cs#L51-L61

If change the code to look like this:
``` c#
///
/// Converts the given object to the converter's native type.
///
public virtual object? ConvertFrom(ITypeDescriptorContext? context, CultureInfo? culture, object value)
{
if (value is InstanceDescriptor instanceDescriptor)
{
return instanceDescriptor.Invoke();
}
var exception = GetConvertFromException(value);

throw exception;
}
```
the `throw exception;` line will never be reached, because exception is thrown from within the `GetConvertFromException`. Same applies to to `GetConvertToException`.

Impact?
1. Inaccurate exception stack trace.
2. Issues with code coverage, the line which does `throw GetConvertFromException(value);` will always be partially covered.

Even the methods semantics and documentation states that the exception instance is returned and not thrown:

https://github.com/dotnet/runtime/blob/3c040478f19e0f317790acab05dbe3ada9f52dc4/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeConverter.cs#L189-L206

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.