Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Contract for TypeConverter.ConvertFrom(object) #236

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Nathan-022
Copy link

As far as I can tell this is a valid contract, it doesn't appear that ConvertFrom should ever return null - it either successfully returns the conversion or throws a NotSupportedException

@ndykman
Copy link
Contributor

ndykman commented Sep 4, 2015

Should doesn't mean can never. The following is an example of getting the method to return null. Pathological, but possible.

  public class NullTypeConverter : TypeConverter
  {
    public override object ConvertFrom(ITypeDescriptorContext context, CultureInfo culture, object value)
    {
        return null;
    }
  }

  [TypeConverter(typeof(NullTypeConverter))]
  public class MyNullClass
  {
  }

  public static class Program
  {
    public static void Main()
    {
      /// Should print true 
      Console.WriteLine(TypeDescriptor.GetConverter(typeof(MyNullClass)).ConvertFrom("foo") == null);
    }
  }

@brettshearer
Copy link

Is there any return point from this sort of problem to what was probably intended in the framework originally?
I know it would be a breaking change – could we somehow make contracts breaking changes at runtime by a ‘strict’ flag in application manifest?

From: Nathan Dykman [mailto:[email protected]]
Sent: Saturday, 5 September 2015 9:16 AM
To: Microsoft/CodeContracts [email protected]
Subject: Re: [CodeContracts] Contract for TypeConverter.ConvertFrom(object) (#236)

Should doesn't mean can never. The following is an example of getting the method to return null. Pathological, but possible.

public class NullTypeConverter : TypeConverter

{

public override object ConvertFrom(ITypeDescriptorContext context, CultureInfo culture, object value)

{

    return null;

}

}

[TypeConverter(typeof(NullTypeConverter))]

public class MyNullClass

{

}

public static class Program

{

public static void Main()

{

  /// Should print true

  Console.WriteLine(TypeDescriptor.GetConverter(typeof(MyNullClass)).ConvertFrom("foo") == null);

}

}


Reply to this email directly or view it on GitHubhttps://github.com//pull/236#issuecomment-137876550.

@yaakov-h
Copy link
Contributor

yaakov-h commented Sep 5, 2015

According to the Microsoft Reference Source:

TypeConverter:

        public object ConvertFrom(object value) {
            return ConvertFrom(null, CultureInfo.CurrentCulture, value);
        }

one two, skip a few...

        public virtual object ConvertFrom(ITypeDescriptorContext context, CultureInfo culture, object value) {
            InstanceDescriptor id = value as InstanceDescriptor;
            if (id != null) {
                return id.Invoke();
            }
            throw GetConvertFromException(value);
        }

InstanceDescriptor:

        public object Invoke() {
            object[] translatedArguments = new object[arguments.Count];
            arguments.CopyTo(translatedArguments, 0);

            // Instance descriptors can contain other instance
            // descriptors.  Translate them if necessary.
            //
            for(int i = 0; i < translatedArguments.Length; i++) {
                if (translatedArguments[i] is InstanceDescriptor) {
                    translatedArguments[i] = ((InstanceDescriptor)translatedArguments[i]).Invoke();
                }
            }

            if (member is ConstructorInfo) {
                return ((ConstructorInfo)member).Invoke(translatedArguments);
            }
            else if (member is MethodInfo) {
                return ((MethodInfo)member).Invoke(null, translatedArguments);
            }
            else if (member is PropertyInfo) {
                return ((PropertyInfo)member).GetValue(null, translatedArguments);
            }
            else if (member is FieldInfo) {
                return ((FieldInfo)member).GetValue(null);
            }
            else {
                Debug.Fail("Unrecognized reflection type in instance descriptor: " + member.GetType().Name);
            }

            return null;
        }

Thus in the following code, convertedValue will be null:

using System.ComponentModel;
using System.ComponentModel.Design.Serialization;

namespace TypeConverterExaggeration
{
    class Program
    {
        static void Main(string[] args)
        {
            var descriptor = new InstanceDescriptor(null, null);
            var converter = new TypeConverter();
            var convertedValue = converter.ConvertFrom(descriptor);
        }
    }
}

So even if the framework intended ConvertFrom to never return null, the framework's own implementation can do so. I don't see any logical way to apply non-null as a postcondition, even if you want your own code to be stricter than the framework.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants