Skip to content

Architecture Overview

Clemens Lieb edited this page Aug 15, 2016 · 8 revisions

The entry point of the Rubberduck project is the COM-visible Extension class, which implements the IDTExtensibility2 COM interface. Code at the entry point is kept to a minimum:

private readonly IKernel _kernel = new StandardKernel(new FuncModule());

public void OnConnection(object Application, ext_ConnectMode ConnectMode, object AddInInst, ref Array custom)
{
    try
    {
        _kernel.Load(new RubberduckModule(_kernel, (VBE)Application, (AddIn)AddInInst));
        _kernel.Load(new UI.SourceControl.SourceControlBindings());
        _kernel.Load(new CommandBarsModule(_kernel));

        var app = _kernel.Get<App>();
        app.Startup();
    }
    catch (Exception exception)
    {
        System.Windows.Forms.MessageBox.Show(exception.ToString(), RubberduckUI.RubberduckLoadFailure, MessageBoxButtons.OK, MessageBoxIcon.Error);
    }
}

This is the application's composition root, where Ninject resolves the dependencies of every class involved in creating an instance of the App class. There shouldn't be much reasons to ever modify this code, unless new NinjectModule classes need to be loaded.

The RubberduckModule defines a number of conventions that automate a number of trivial things:

  • Interface ISomething will automatically bind to concrete type Something: no Ninject configuration code is needed for constructor-injecting types that follow this simple naming convention.
  • Simple abstract factories don't need a concrete type; simply name the interface ISomethingFactory and Ninject will generate a proxy concrete implementation, in singleton scope.
  • Code inspections must be derived from InspectionBase. These types are discovered using reflection, and automatically multi-bind to IInspection, in singleton scope.

##Commands and Menus

Every feature eventually requires some way for the user to get them to run. Sometimes a feature can be launched from the main "Rubberduck" menu, two or more context menus, and an inspection quick-fix. Our architecture solves this problem by implementing commands.

###Commands

Implementing a command is easy: derive a new class from CommandBase and override the Execute method. In its simplest form, a command could look like this:

public class AboutCommand : CommandBase
{
    public override void Execute(object parameter)
    {
        using (var window = new AboutWindow())
        {
            window.ShowDialog();
        }
    }
}

The base implementation for CanExecute simply returns true; override it to provide the logic that determines whether a command should be enabled or not - the WPF/XAML UI will use this logic to enable/disable the corresponding UI elements.

A command that has dependencies, should receive them as abstractions in its constructor - Ninject automatically takes care of injecting the concrete implementations.

###Refactoring commands

The refactorings have common behavior and dependencies that have been abstracted into a RefactorCommandBase base class that refactoring commands should derive from:

public abstract class RefactorCommandBase : CommandBase
{
    protected readonly IActiveCodePaneEditor Editor;
    protected readonly VBE Vbe;

    protected RefactorCommandBase(VBE vbe, IActiveCodePaneEditor editor)
    {
        Vbe = vbe;
        Editor = editor;
    }

    protected void HandleInvalidSelection(object sender, EventArgs e)
    {
        System.Windows.Forms.MessageBox.Show(RubberduckUI.ExtractMethod_InvalidSelectionMessage, RubberduckUI.ExtractMethod_Caption, MessageBoxButtons.OK, MessageBoxIcon.Exclamation);
    }
}

Hence, refactoring commands should take at least a VBE and IActiveCodePaneEditor dependencies:

public class RefactorExtractMethodCommand : RefactorCommandBase
{
    private readonly RubberduckParserState _state;

    public RefactorExtractMethodCommand(VBE vbe, RubberduckParserState state, IActiveCodePaneEditor editor)
        : base (vbe, editor)
    {
        _state = state;
    }

    public override void Execute(object parameter)
    {
        var factory = new ExtractMethodPresenterFactory(Editor, _state.AllDeclarations);
        var refactoring = new ExtractMethodRefactoring(factory, Editor);
        refactoring.InvalidSelection += HandleInvalidSelection;
        refactoring.Refactor();
    }
}

Parser state is also a common dependency, since it exposes the processed VBA code. See the RubberduckParserState page for more information about this specific class.


##Menus

One way of executing commands, is to associate them with menu items. The easiest way to implement this, is to derive a new class from the CommandMenuItemBase abstract class, and pass an ICommand to the base constructor - here's the simple AboutCommandMenuItem implementation:

public class AboutCommandMenuItem : CommandMenuItemBase
{
    public AboutCommandMenuItem(ICommand command) : base(command)
    {
    }

    public override string Key { get { return "RubberduckMenu_About"; } }
    public override bool BeginGroup { get { return true; } }
    public override int DisplayOrder { get { return (int)RubberduckMenuItemDisplayOrder.About; } }
}

The name of the type isn't a coincidence that it looks very much like the name of the corresponding command class.

###Naming Convention for CommandMenuItemBase implementations

The ICommand binding is automatically created using reflection, so as long as the naming convention is preserved, there is no additional Ninject configuration required to make it work.

The convention is, formally, as follows:

 [CommandClassName]MenuItem

Classes derived from CommandMenuItemBase simply need to override base members to alter behavior.

  • Key property must return a string representing the resource key that contains the localized caption to use. This property is abstract and must therefore be overridden in all derived types.
  • BeginGroup is virtual and only needs to be overridden when you want the menu item to begin a group; when this property returns true, the menu item is rendered with a separator line immediately above it.
  • DisplayOrder is also virtual and should be overridden to control the display order of menu items.
  • Image and Mask are virtual and return null by default; they should be overridden when a menu item should be rendered with an icon. The Mask is a black & white version of the Image bitmap, where everything that should be transparent is white, and everything that should be in color, is black.

###Controlling display order

Rather than hard-coding "magic" int values into each implementation, use an enum type: the order of the enum members will determine the order of the menu items. For example, the main RubberduckMenu uses this RubberduckMenuItemDisplayOrder enum:

public enum RubberduckMenuItemDisplayOrder
{
    UnitTesting,
    Refactorings,
    Navigate,
    CodeInspections,
    SourceControl,
    Options,
    About
}

This makes the code much cleaner, and makes it much easier to change how menus look like.

###Parent menus

Menus can have sub-items, which can have sub-items themselves: the "parent" items have no command associated to them, so they're not derived from CommandMenuItemBase. Instead, they are subclasses of the ParentMenuItemBase abstract class.

Here's the RefactoringsParentMenu implementation:

public class RefactoringsParentMenu : ParentMenuItemBase
{
    public RefactoringsParentMenu(IEnumerable<IMenuItem> items)
        : base("RubberduckMenu_Refactor", items)
    {
    }

    public override int DisplayOrder { get { return (int)RubberduckMenuItemDisplayOrder.Refactorings; } }
}

There's pretty much no code, in every single implementation: only the DisplayOrder and BeginGroup properties can be overridden, and the Key is passed to the base constructor along with the child items.

Every parent menu must receive an IEnumerable<IMenuItem> constructor parameter that contains its child items: Ninject needs to be told exactly what items to inject in each menu - for now, this is done in the CommandBarsModule class:

    private IEnumerable<IMenuItem> GetRubberduckMenuItems()
    {
        return new IMenuItem[]
        {
            _kernel.Get<AboutCommandMenuItem>(),
            _kernel.Get<OptionsCommandMenuItem>(),
            _kernel.Get<RunCodeInspectionsCommandMenuItem>(),
            _kernel.Get<ShowSourceControlPanelCommandMenuItem>(),
            GetUnitTestingParentMenu(),
            GetSmartIndenterParentMenu(),
            GetRefactoringsParentMenu(),
            GetNavigateParentMenu(),
        };
    }

    private IMenuItem GetUnitTestingParentMenu()
    {
        var items = new IMenuItem[]
        {
            _kernel.Get<RunAllTestsCommandMenuItem>(),
            _kernel.Get<TestExplorerCommandMenuItem>(),
            _kernel.Get<AddTestModuleCommandMenuItem>(),
            _kernel.Get<AddTestMethodCommandMenuItem>(),
            _kernel.Get<AddTestMethodExpectedErrorCommandMenuItem>(),
        };
        return new UnitTestingParentMenu(items);
    }

These methods determine what goes where. The order of the items in this code does not impact the actual order of menu items in the rendered menus - that's controlled by the DisplayOrder value of each item.


#Inspections

All code inspections derive from the InspectionBase class. By convention, their names end with Inspection, and their GetInspectionResults implementation return objects whose name is the same as the inspection class, ending with InspectionResult.

Here's the VariableNotUsedInspection implementation:

public sealed class VariableNotUsedInspection : InspectionBase
{
    public VariableNotUsedInspection(RubberduckParserState state)
        : base(state)
    {
        Severity = CodeInspectionSeverity.Warning;
    }

    public override string Description { get { return RubberduckUI.VariableNotUsed_; } }
    public override CodeInspectionType InspectionType { get { return CodeInspectionType.CodeQualityIssues; } }

    public override IEnumerable<CodeInspectionResultBase> GetInspectionResults()
    {
        var declarations = UserDeclarations.Where(declaration =>
            !declaration.IsWithEvents
            && declaration.DeclarationType == DeclarationType.Variable
            && declaration.References.All(reference => reference.IsAssignment));

        return declarations.Select(issue => 
            new IdentifierNotUsedInspectionResult(this, issue, ((dynamic)issue.Context).ambiguousIdentifier(), issue.QualifiedName.QualifiedModuleName));
    }
}

All inspections should be sealed classes derived from InspectionBase, passing the RubberduckParserState into the base constructor.

A noteworthy exception are Inspections that have to work on the IParseTree produced by the ANTLR parser instead of the resulting Declarations. They should derive from IParseTreeInspection instead, which enables passing the ParseTreeResults generated in the Inspector for further analysis. The actual inspection work for these inspections is performed in the Inspector where the IParseTree in the ParserState is rewalked once for all IParseTreeInspections (to improve performance).

The Description property returns a format string that is used for all inspection results as a description - for example, the description for VariableNotUsedInspection returns a localized string similar to "Variable '{0}' is not used.".

The InspectionType property determines the type of inspection - this can be any of the CodeInspectionType enum values:

public enum CodeInspectionType
{
    LanguageOpportunities,
    MaintainabilityAndReadabilityIssues,
    CodeQualityIssues
}

The constructor must assign the default Severity level. Most inspections should default to Warning.

###Guidelines for default severity level

  • Don't use DoNotShow. That level effectively disables an inspection, and that should be done by the user.
  • Don't use Hint level either. Leave that level for the user to use for inspections deemed of lower priority, but that they don't want to disable.
  • Do use Warning if you're unsure.
  • Only default an inspection to Error severity for inspections that can detect bugs and runtime errors. For example, an object variable is assigned without the Set keyword.
  • Consider defaulting to Warning severity an inspection that could potentially detect a bug, but not a runtime error. For example, a function that doesn't return a value - it's definitely a smell worth looking into, but not necessarily a bug.
  • Consider defaulting to Suggestion severity an inspection when the quick-fix involves a refactoring.
Clone this wiki locally