Inspector Rubberduck and the abstract inspections

The Rubberduck code inspections have just seen yet another structural change, hopefully for the better.

The IInspectionModel interface was originally named IInspection; it only exposes the bare-bones inspection properties, those needed by the CodeInspectionSetting class:

public interface IInspectionModel
{
    /// 
    /// Gets the inspection type name.
    /// 
    string Name { get; }

    /// 
    /// Gets the name of the inspection, without the "Inspection" suffix.
    /// 
    string AnnotationName { get; }

    /// 
    /// Gets a short description for the code inspection.
    /// 
    string Description { get; }

    /// 
    /// Gets a value indicating the type of the code inspection.
    /// 
    CodeInspectionType InspectionType { get; }

    /// 
    /// Gets a value indicating the severity level of the code inspection.
    /// 
    CodeInspectionSeverity Severity { get; set; }
}

The CodeInspectionSetting type is XML-serialized into the Rubberduck settings file; this allows us to let the user determine an inspection's Severity level:

```
[XmlType(AnonymousType = true)]
public class CodeInspectionSetting : IInspectionModel
{
[XmlAttribute]
public string Name { get; set; }

[XmlIgnore]
public string Description { get; set; } // not serialized because culture-dependent

[XmlIgnore]
public string AnnotationName { get; set; }

[XmlAttribute]
public CodeInspectionSeverity Severity { get; set; }

[XmlIgnore]
public string SeverityLabel
{
get { return RubberduckUI.ResourceManager.GetString("CodeInspectionSeverity_" + Severity, RubberduckUI.Culture); }
set
{
foreach (var severity in Enum.GetValues(typeof (CodeInspectionSeverity)))
{
if (value == RubberduckUI.ResourceManager.GetString("CodeInspectionSeverity_" + severity, RubberduckUI.Culture))
{
Severity = (CodeInspectionSeverity)severity;

A couple minor suggestions...

I'd introduce some extension methods to give the common predicates on collections of declarations somewhere to live:

public static class DeclarationsPredicates
{
     public static IEnumerable WhichAreUserDeclarations(this IEnumerable source)
     {
         if (source == null)
         {
             // throw an error or sneakily coerce to Enumerable.Empty();
         }

         return source.Where(declaration => !declaration.IsBuiltIn);
     }

     public static IEnumerable WhereInspectionIsNotDisabledByAnnotation(
         this IEnumerable source,
         string annotationName)
     {
         if (source == null)
         {
             // throw an error or sneakily coerce to Enumerable.Empty();
         }

         return source.Where(declaration => !declaration.IsInspectionDisabled(annotationName));
     }
}

I'm sure there are others that you'd find useful! You could now rewrite your properties as:

protected virtual IEnumerable Declarations
{
    get { return State.AllDeclarations.WhereInspectionIsNotDisabledByAnnotation(AnnotationName); }
}

protected virtual IEnumerable UserDeclarations
{
    get { return State.AllUserDeclarations.WhereInspectionIsNotDisabledByAnnotation(AnnotationName); }
}

I think that's better because you've centralised your logic (I'm not overly fond of the name though). You could potentially remove the .IsInspectionDisabled() method from Declaration and put the logic in the extension method but that's up to you.

This is interesting ((dynamic)context.Context)... Why are you doing that?

I agree that SeverityLabel definitely doesn't fit ;) I'd expect it to be readonly anyway:

// C#6
public string SeverityLabel => RubberduckUI.ResourceManager.GetString("CodeInspectionSeverity_" + Severity, RubberduckUI.Culture);

If you want to change the label, you should have to change the severity!

You have a lot of magic strings around the place:

"Inspection", "Meta", "CodeInspectionSeverity_"... They should be well named constants.

The trailing underscore in this name looks odd to me:

RubberduckUI.ImplicitPublicMember_

I'm guessing that the string is "ImplicitPublicMember_"? Either way, chop the underscore off.

This method is named in camelCase but should be PascalCase ambiguousIdentifier()

This property name doesn't look quite right:

string Meta { get; }

I think it should be Metadata.

Your doc comment here isn't quite right:

/// 
/// Gets a value indicating the severity level of the code inspection.
/// 
CodeInspectionSeverity Severity { get; set; }

It actually gets or sets a value...

There's also the ` tag that you should add. I use the GhostDoc extension to help generate code documentation - that might help you out too.

I agree with your summary at the end - I'd drop back to just using an ICodeInspection interface. I changed the name because I don't like the double I in IInspection` and it's also a little more descriptive.

On the whole - very nice!

StackExchange Code Review Q#116600, answer score: 3