FxCop rule to access properties in a specific way

Question

I've got an Item class (to which I have no access), which has two different ways to access Fields of the Item. I'd like to have a custom FxCop rule that prefers one over the other.

I can get the value from a specific field by having an item and directly accessing item["fieldname"], which will return a string.
The code for this:

public string this[string fieldName]
{
  get
  {
    Assert.ArgumentNotNullOrEmpty(fieldName, "fieldName");
    ID result;
    if (ID.TryParse(fieldName, out result))
      return this[result];
    if ((int) fieldName[0] == 64)
      return this.Fields.GetSpecialField(fieldName);
    Field field = this.Fields[fieldName];
    if (field == null)
      return "";
    else
      return field.Value;
  }
  set
  {
    Assert.ArgumentNotNullOrEmpty(fieldName, "fieldName");
    ID result;
    if (ID.TryParse(fieldName, out result))
    {
      this[result] = value;
    }
    else
    {
      Field field = this.Fields[fieldName];
      if (field == null)
        return;
      field.Value = value;
    }
  }
}

public string this[int index]
{
  get
  {
    Field field = this.Fields[index];
    if (field == null)
      return "";
    else
      return field.Value;
  }
  set
  {
    Field field = this.Fields[index];
    if (field == null)
      return;
    field.Value = value;
  }
}

public string this[ID fieldID]
{
  get
  {
    Assert.ArgumentNotNull((object) fieldID, "fieldID");
    return this.Fields[fieldID].Value;
  }
  set
  {
    Assert.ArgumentNotNull((object) fieldID, "fieldID");
    this.Fields[fieldID].Value = value;
  }
}

Alternatively, I can access the Field property like so: item.Fields["Fieldname"].Value.
The code for the alternative:

public Field this[ID fieldID]
{
  get
  {
    return new Field(fieldID, this._ownerItem);
  }
}

public Field this[string fieldName]
{
  get
  {
    ID fieldId = TemplateManager.GetFieldId(fieldName, this.TemplateId, this.Database);
    if (fieldId == null)
      return (Field) null;
    else
      return this[fieldId];
  }
}

public Field this[int index]
{
  get
  {
    Field[] fields = this.GetFields();
    if (index < fields.Length && index >= 0)
      return fields[index];
    else
      return (Field) null;
  }
}

This approach can throw a NullReferenceException, for instance if the fieldname is misspelled. The Field object will then be null, so the Value can't be retrieved.

Now, as the first way is safer, I'd like to give a warning when the second method is used. I've found that I'll need to use the

public override ProblemCollection Check(Member member)
{
} 

method, and then cast the member to a Microsoft.FxCop.Sdk.PropertyNode. From there however, I don't know how to go about it, especially because they use the same code at the end (the first option calls the second option, but if the fieldname is misspelled and the field is null, the first option will just return an empty string).

Alternatively, if that's not possible, I'd like to give an FxCop error if the second option is used without checking for the null value of the Field.

Some administrative details: I'm using Visual Studio 2012 with FxCop 10.

Edit
Just to clarify further, this rule shouldn't check the properties themselves(i.e. something like the Naming.IdentifiersShouldBeCasedCorrectly), but how they are called from elsewhere in the code.

Answer 1

Since you're looking at how the properties are used elsewhere in the code, you'll need to use BaseIntrospectionRule's visitor pattern support to look for references to the problematic property.

Only the contents of analyzed assemblies will be passed to Check, so if the assembly containing Item and FieldsCollection is merely referenced rather than analyzed, the Item indexer shouldn't cause an issue.

Without delving into dataflow analysis (which is largely beyond Introspector's scope), you can take a very rough approximation by running a visitor through all analyzed method bodies and flagging calls of the Field.Value getter:

public override ProblemCollection Check(Member member)
{
    var method = member as Method;
    if (method != null)
    {
        VisitStatements(method.Body.Statements);
    }
    return Problems;
}

public override void VisitMethodCall(MethodCall methodCall)
{
    var memberBinding = methodCall.Callee as MemberBinding;
    if (memberBinding != null)
    {
        var methodCalled = memberBinding.BoundMember as Method;
        if (methodCalled != null)
        {
            if (methodCalled.FullName == "Some.Namespace.Field.get_Value")
                Problems.Add(new Problem(GetResolution(), methodCall));
        }
    }
    base.VisitMethodCall(methodCall);
}

A better approximation would also traverse into the zeroth Operand of the MethodCall, only raising an issue if that Operand is a call to the FieldsCollection indexer.

Answer 2

Why not use de ObsoleteAttribute?