Add setting to blacklist types from nullable conversion

[eb-delska]

Adds new global setting: Make Reference Types Nullable Blacklist Closes #523 Customizable CustomTextWriter.InvalidStringsForPropertiesNeedingNullableTypes

After customizing code generation as part of migration from XrmToolkit it required ignoring more types from generated functions since conversion to nullable types happens after it. Instead of hardcoding these, I added new setting to set a list of additional ignored types to avoid breaking existing code. It could be extended to replace original list with default values and better handling of whitespace, but I left that as is for now.

[daryllabar]

Could I get a before and after for how this changes the generated code?

[daryllabar]

Also I'm struggling what the need for this particular feature is. You want nullable types, but not for all types, which I don't understand right now. @janis-veinbergs any details you'd like to share?

[eb-delska]

The simplest example why this would be needed are Guid types which are expected to be non nullable in our existing code, but it's probably only useful when using modified CustomizeCodeDomService. Examples taken from Account entity:

1. By default it generates System.Nullable without any changes

   [Microsoft.Xrm.Sdk.AttributeLogicalNameAttribute("processid")]
   public System.Nullable<System.Guid> ProcessId
   {
   get
   {
       return this.GetAttributeValue<System.Nullable<System.Guid>>("processid");
   }
   set
   {
       this.SetAttributeValue("processid", value);
   }
   }

2. We have a custom step that removes System.Nullable<> from it in extended CustomizeCodeDomService along with other changes resulting in the following before CustomTextWriter changes it:

   [Microsoft.Xrm.Sdk.AttributeLogicalNameAttribute("processid")]
   public System.Guid ProcessId
   {
   get
   {
       return this.GetPropertyValue<System.Guid>("processid");
   }
   set
   {
       this.SetPropertyValue<System.Guid>("processid", value, nameof(ProcessId));
   }
   }

3. Without these changes and adding Guid to blacklist CustomTextWriter adds nullable back and the function ends up like this:

   [Microsoft.Xrm.Sdk.AttributeLogicalNameAttribute("processid")]
   public System.Guid? ProcessId
   {
   get
   {
       return this.GetPropertyValue<System.Guid>("processid");
   }
   set
   {
       this.SetPropertyValue<System.Guid>("processid", value, nameof(ProcessId));
   }
   }

   We also generate additional methods to access OptionSetValue that's not expected to be nullable. Without blacklisting it also gets modified:

   [AttributeLogicalName("industrycode")]
   public OptionSetValue? IndustryCode_OptionSetValue
   {
   get
   {
       return this.GetPropertyValue<OptionSetValue>("industrycode");
   }
   set
   {
       this.SetPropertyValue<OptionSetValue>("industrycode", value, nameof(IndustryCode_OptionSetValue));
   }
   }

[janis-veinbergs]

@daryllabar it is to get away with less refactoring after xrmtoolkit migration

Xrmtoolit doesn't have nullable guids, OptionSetValues or EntityReferences. The default(T) is returned if attribute doesn't contain one rather than null.

However there are still properties that are nullable: bool?, DateTime? and some more.

[daryllabar]

Ok, so you still want nullable properties for some types, so you want to keep the nullable types still being generated, but there are just a few property types where you don't. I see this as being a rather unusual edge case that very few other situations will call for. If we have 5 or more other configurations that will be required for the migrating from the XrmToollKit that no one else would need, I think we should create an XrmToollKit flag, or at least hide the configs unless really needed.

Any idea how many of these configuration changes will be required, that would be rather specific to the XrmToolKit? My guess is that it's really hard to know, so maybe we should start adding some, and then we will reassess if we want to change how it is configured later.

So with that being said, this should be the actual list of values, not a black list. I might be able to find time this week to update it.

[janis-veinbergs]

There are lot of customizations, but all of them can be achieved by our own ICustomizeCodeDomService by implementing things listed in #492. Sneak peek:

            new Entity.ChangeEntityBaseClass().CustomizeCodeDom(codeUnit, services);
            new Entity.RenameEntityFieldClass().CustomizeCodeDom(codeUnit, services);
            new Entity.RenameEntityProperties().CustomizeCodeDom(codeUnit, services);
            new Entity.ChangeEntityConstructor().CustomizeCodeDom(codeUnit, services);

            new Entity.SetTypesNonNullable(this).CustomizeCodeDom(codeUnit, services);
            new Entity.SetStringPropertyValueBounds(this).CustomizeCodeDom(codeUnit, services);
            new Entity.SetNumberPropertyValueBounds(this).CustomizeCodeDom(codeUnit, services);
            new Entity.SetGenericPropertyValue(this).CustomizeCodeDom(codeUnit, services);

            new Entity.ImproveOptionSet(this).CustomizeCodeDom(codeUnit, services);
            new Entity.PrimaryIdAttribute(this).CustomizeCodeDom(codeUnit, services);
            new Entity.ChangeMoneyMethods(this).CustomizeCodeDom(codeUnit, services);
            new Entity.AddSetState(this).CustomizeCodeDom(codeUnit, services);

It could all be buried under a single flag. However this is not 1:1 migration path from XrmToolkit, because:

• XrmToolkit provides on itself options on how to configure generated code. So this is just for our configuration.
• Simple code refactoring is still required, mainly because of different enum names and the way id properties are "synchronized".

But the code that we have is an enabler for migration.

Regarding this pullrequest - this is a change that impacts custom text writer service and we cannot influence for our side without recreating

There are 2 paths forward:

• If this PR gets introduced and we will continue using our custom ICustomizeCodeDomService which inherits from yours to achieve our goals. We can also remove this configuration from UI not to overwhelm users as this is not very useful for greenfield projects.
• If you propose that we introduce a single flag "GenerateXrmToolkitStyleCode", a PR can be prepared which includes ALL The changes necessary then we can do that. Probably this flag doesn't even need to be exposed in UI or it can, depending on how you view it.