The Project
With the planned features and project design for 2.1/3.0, we needed to give our inspections some thought. The basic inspection design had not really changed since the initial conception, except we allowed constructor parameters in 2.0 after we moved to Ninject DI instead of using reflection to create them. This post describes how quick fixes are being made independent of a inspection results.
Why?
The reason why this was so important has many sides. First, we want to make Rubberduck extensible. If you want to write an add-in for our add-in, we want you to be able to do that. We cannot provide every feature, and we especially cannot provide every inspection you would want. Part of this reason is that not every inspection is in scope for this project—you could write an inspection and quick fix using Rubberduck’s parse trees and declarations to safely correct some bad behavior that is specific to your project, or perhaps you just want to write a custom quick fix for one of our inspections, maybe you want to write an inspection that uses one of our quick fixes. This would be somewhat difficult under our previous design of:
Inspection -> IEnumerable GetInspectionResults()
IInspectionResult -> IEnumerable QuickFixes { get }
If any inspection wanted to offer a specific set of quick fixes, it had to have its own inspection result. An existing inspection could not gain more quick fixes without changing its inspection result, which required us to completely redeploy the solution.
Further consideration reveals that the quick fixes really have their own scope and that our inspections were violating the Single Responsibility Principle (SRP). An inspection’s responsibility is to find issues. An inspection result’s responsibility is to report an issue, and nothing else. A quick fix’s responsibility is to fix the issue—it doesn’t care about anything else. We needed to split these up in a way that
- Allows an inspection to have new quick fixes added at runtime
- Separates inspections and quick fixes so that
- A quick fix knows what inspections it can fix
- An inspection and its result knows nothing about which quick fixes support it
- Is clean and maintainable
The Solution
Our final solution is to leave the inspections pretty much alone, except move more of them to be IParseTreeInspection’s because we are moving from lists of declarations to making a full AST with our ANTLR parse trees. The inspection-specific result classes are now gone, and we made the following inheritance structure:
IInspectionResult -> InspectionResultBase InspectionResultBase -> DeclarationInspectionResult // works off declaration nodes IdentifierReferenceInspectionResult // works off identifier references QualifiedContextInspectionResult // works off a qualified context, which is a module name and ANTLR node
So, an inspection still reports inspection results, just like previously. However, it no longer needs to inject dependencies only used by the quick fix. Previously, we had to pass these dependencies into the quick fix through the inspection result through the inspection; this was causing some minor problems in the website, as well. Once we get our add-in structure complete, the user will be able to create a class library using Rubberduck’s features, install it to a certain folder, and Ninject will automatically load the types into Rubberduck and the inspections will be treated just like our “built-in” inspections.
An inspection result also has a simpler constructor, and only takes the information needed to report the inspection result and its scope. This is open to slight changes in the future as we stop reporting lists of results, but rather directly annotate parse trees with them. However, the beauty of it at this point is nothing else is affected—most of the inspection result is consumed by the end user only.
A quick fix now is a standalone feature exposing the following interface:
IQuickFix ->
void Fix(IInspectionResult result)
string Description(IInspectionResult result)
bool CanFixInProcedure { get }
bool CanFixInModule { get }
bool CanFixInProject { get }
IReadOnlyCollection SupportedInspections { get }
Rubberduck exposes the quick fix to the user through the IQuickFixProvider, which returns a set of quick fixes for an inspection result by checking the reported inspection type and allows the user to fix an individual inspection or all inspections of a certain type from a set of results in a certain scope. This provider is incomplete, but will allow the user to add or remove a quick fix for any inspection, other than inspection/quick fix mappings built-in to Rubberduck.
Further Considerations
At this point, one inspection is broken because it did not enable the quick fixes in certain circumstances. The solution for this is still being thought about, but it will likely involve giving the inspection result a dictionary of properties, with a special case for disabling a set of inspections:
{ “DisableInspections”, “FooInspection,BarInspection” }
Other properties can be used by the quick fixes either for performance enhancements, simplifications, or to convey information not allowed by the inspection result API.
Rubberduck News 