4

I have acquired an extension class which implements the following members with signatures that are violating the CA1006:DoNotNestGenericTypesInMemberSignatures rule.

The code to which the warning refers is included below.

How should I refactor the code to resolve the CA1006 warning?

Please keep in mind that I am not very familiar with expression trees, although I do have a pretty good grasp of anonymous methods, delegates, and lambdas.

Any help will be greatly appreciated.

    public static DataServiceQuery<TElement> Expand<TElement, TPropType>(this DataServiceQuery<TElement> source, Expression<Func<TElement, TPropType>> propertySelector) 
    {
        string includeString = BuildString(propertySelector);
        return source.Expand(includeString);
    }

    private static string BuildString(Expression propertySelector)
    {
        switch (propertySelector.NodeType)
        {
            case ExpressionType.Lambda:
                LambdaExpression lambdaExpression = (LambdaExpression)propertySelector;
                return BuildString(lambdaExpression.Body);

            case ExpressionType.Quote:
                UnaryExpression unaryExpression = (UnaryExpression)propertySelector;
                return BuildString(unaryExpression.Operand);

            case ExpressionType.MemberAccess:

                MemberExpression memberExpression = (MemberExpression)propertySelector;
                MemberInfo propertyInfo = memberExpression.Member;

                if (memberExpression.Expression is ParameterExpression)
                {
                    return propertyInfo.Name;
                }
                else
                {
                    // we've got a nested property (e.g. MyType.SomeProperty.SomeNestedProperty)
                    return BuildString(memberExpression.Expression) + "/" + propertyInfo.Name;
                }

            case ExpressionType.Call:
                MethodCallExpression methodCallExpression = (MethodCallExpression)propertySelector;
                if (IsSubInclude(methodCallExpression.Method)) // check that it's a SubInclude call
                {
                    // argument 0 is the expression to which the SubInclude is applied (this could be member access or another SubInclude)
                    // argument 1 is the expression to apply to get the included property
                    // Pass both to BuildString to get the full expression
                    return BuildString(methodCallExpression.Arguments[0]) + "/" +
                           BuildString(methodCallExpression.Arguments[1]);
                }
                // else drop out and throw
                break;
        }
        throw new InvalidOperationException("Expression must be a member expression or an SubInclude call: " + propertySelector.ToString());

    }

    private static readonly MethodInfo[] SubIncludeMethods;
    static MyExtensions()
    {
        Type type = typeof(MyExtensions);
        SubIncludeMethods = type.GetMethods().Where(mi => mi.Name == "SubExpand").ToArray();
    }

    private static bool IsSubInclude(MethodInfo methodInfo)
    {
        if (methodInfo.IsGenericMethod)
        {
            if (!methodInfo.IsGenericMethodDefinition)
            {
                methodInfo = methodInfo.GetGenericMethodDefinition();
            }
        }
        return SubIncludeMethods.Contains(methodInfo);
    }

    public static TPropType SubExpand<TSource, TPropType>(this Collection<TSource> source, Expression<Func<TSource, TPropType>> propertySelector)
        where TSource : class
        where TPropType : class
    {
        throw new InvalidOperationException("This method is only intended for use with DataServiceQueryExtensions.Expand to generate expressions trees"); // no actually using this - just want the expression! 
    }

    public static TPropType SubExpand<TSource, TPropType>(this TSource source, Expression<Func<TSource, TPropType>> propertySelector)
        where TSource : class
        where TPropType : class
    {
        throw new InvalidOperationException("This method is only intended for use with DataServiceQueryExtensions.Expand to generate expressions trees"); // no actually using this - just want the expression! 
    }
Vance Ceaser
  • 43
  • 1
  • 3
  • can you give an example of this code being used? It's quite complex and a little tricky to see where the fault is – DiskJunky Feb 18 '13 at 21:03
  • Is that referring to the propertySelector parameter of the public method? The warning is about making an interface to a method usable - but with an Expression, I don't see how it could be refactored unless you wrap the expression parameter in a class which provides it as a a return from a property or method call. – Jay Feb 18 '13 at 21:07
  • 1
    DiskJunky, thanks for your interest in helping to resolve this issue. Essentially, the code is being used to build lambda expressions as in the following. var scenarioGroups = from scenarioGroup in ctx.ScenarioGroups.Expand(scenarioGroup => scenarioGroup.Scenarios.SubExpand(sc => sc.XYLines.SubExpand(xy => xy.Points))) where ... – Vance Ceaser Feb 19 '13 at 14:05
  • And yes, Jay, it does. Furthermore, I think you're right in your assessment. However, I've been asked to explore possible alternatives to nesting rather than suppression of the warning. – Vance Ceaser Feb 19 '13 at 14:09

1 Answers1

9

The warning is a general warning supposed to help you design a better and simpler public interface. In this case you get a warning about having a Expression<Func<TElement, TPropType>> parameter in your method. However, for this method there is no point in simplifying the type and instead you should suppress the warning using an attribute or completely remove the rule from your ruleset.


A silly example where you probably should consider following the advice of the rule is a method like this:

public void F(Dictionary<String, List<Tuple<String, Int32>>> dictionary);
Martin Liversage
  • 104,481
  • 22
  • 209
  • 256
  • Thanks for the help Martin. I believe you are correct. It has become obvious that it is pointless to try and simplify the type. I'll just have to suppress it instead. – Vance Ceaser Feb 19 '13 at 14:13
  • 4
    After keeping this warning for a while I suppressed it as it was causing too much noise. Found out that 99% of the time the usage of a nested generic type with a "depth" of 1 (T>)was legitimate. Waiting for the rule to be refined to trigger only for a starting depth of 2 (T>>) which would make much more sense ;) – darkey Jun 04 '13 at 04:52