HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Inspector Rubberduck

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
rubberduckinspectorstackoverflow

Problem

Our Rubberduck open-source VBE add-in project is coming along nicely. One of the main features we're going to be implementing in the next release, is code inspections.

I started by defining an abstraction:

namespace Rubberduck.Inspections
{
    [ComVisible(false)]
    public enum CodeInspectionSeverity
    {
        DoNotShow,
        Hint,
        Suggestion,
        Warning,
        Error
    }

    [ComVisible(false)]
    public enum CodeInspectionType
    {
        MaintainabilityAndReadabilityIssues,
        CodeQualityIssues
    }

    /// 
    /// An interface that abstracts a code inspection.
    /// 
    [ComVisible(false)]
    public interface IInspection
    {
        /// 
        /// Gets a short description for the code inspection.
        /// 
        string Name { get; }

        /// 
        /// Gets a short message that describes how a code issue can be fixed.
        /// 
        string QuickFixMessage { 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; }

        /// 
        /// Gets/sets a valud indicating whether the inspection is enabled or not.
        /// 
        bool IsEnabled { get; set; }

        /// 
        /// Runs code inspection on specified tree node (and child nodes).
        /// 
        /// The  to analyze.
        /// Returns inspection results, if any.
        IEnumerable Inspect(SyntaxTreeNode node);
    }
}


Out of necessity came a CodeInspection base class to implement it:

```
namespace Rubberduck.Inspections
{
[ComVisible(false)]
public abstract class CodeInspection : IInspection
{
protected CodeInspection(string name, string message, CodeInspectionType type, CodeInspectionSeverity severity)
{
_name = name;

Solution

Having worked on a similar system, I came up with a very similar design. So either we're both doing something right, or we're both doing it wrong :)

Here are some things I would change, but obviously different requirements call for different decisions and may not all be applicable here.

Remove CodeInspection class

I don't think you gain much from having a base class here. Now your inspections look more like this

public class ObsoleteCommentSyntaxInspection : IInspection
{
    public string Name
    {
        get { return Properties.Resources.ObsoleteCommentSyntaxInspectionName; }
    }

    public CodeInspectionType InspectionType
    {
        get { return CodeInspectionType.MaintainabilityAndReadabilityIssues; }
    }

    public CodeInspectionSeverity Severity
    {
        get { return CodeInspectionSeverity.Suggestion; }
    }

    ...


Remove IsEnabled property

I think enabling/disabling inspections should be handled at a different level. Especially if you want something more complex like a project-specific settings file overriding a global one, something like an InspectionSettings class would be good here.

Inspections might have more than one fix (or none)

Consider returning an IEnumerable for quick fixes.

Rename Inspect

The return type is IEnumerable, so I would recommend something like GetInspectionResults.

Code Snippets

public class ObsoleteCommentSyntaxInspection : IInspection
{
    public string Name
    {
        get { return Properties.Resources.ObsoleteCommentSyntaxInspectionName; }
    }

    public CodeInspectionType InspectionType
    {
        get { return CodeInspectionType.MaintainabilityAndReadabilityIssues; }
    }

    public CodeInspectionSeverity Severity
    {
        get { return CodeInspectionSeverity.Suggestion; }
    }

    ...

Context

StackExchange Code Review Q#71200, answer score: 12

Revisions (0)

No revisions yet.