Introducing \Plug-ins folder #2935
Conversation
sync with main repo
…of tests that are green despite 0 inspections being implemented in Rubberduck.dll.
| using Ninject.Extensions.Interception.Infrastructure.Language; | ||
| using Ninject.Extensions.NamedScope; | ||
| using Rubberduck.Inspections.Abstract; | ||
| using NLog; |
There was a problem hiding this comment.
oops, this shouldn't be here
| private IEnumerable<Assembly> FindPlugins() | ||
| { | ||
| var basePath = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location); | ||
| yield return Assembly.LoadFile(Path.Combine(basePath, "Rubberduck.Inspections.dll")); |
There was a problem hiding this comment.
Rubberduck.Inspections.dll has dependencies on Rubberduck.dll, Rubberduck.Parsing.dll, Rubberduck.VBEditor.dll, Rubberduck.SmartIndenter.dll, and Rubberduck.SettingsProvider.dll; so Rubberduck.Inspections.dll is essentially a plug-in... but since it's the "core inspections package" we're loading it from the main assembly path.
| .Where(type => type.BaseType == typeof (InspectionBase)); | ||
| var inspections = assemblies | ||
| .SelectMany(a => a.GetTypes().Where(type => type.IsClass && !type.IsAbstract && type.GetInterfaces().Contains(typeof (IInspection)))) | ||
| .ToList(); |
There was a problem hiding this comment.
This means any .dll under the \Plug-ins folder can contain IInspection implementations, we'll load them, and bind them.
|
|
||
| var items = filteredResults | ||
| .Where(result => !(result is AggregateInspectionResult)) | ||
| .Where(result => result.GetType().Name != "AggregateInspectionResult") |
There was a problem hiding this comment.
makes stuff funny when renaming the type ...
We can't get the dependency in here and making this a special case smells, because we're violating LSP by special-casing this IInspectionResult...
This logic should be encapsulated into Inspection results in the first place:
var items = filteredResults
.Select(item => item.DefaultQuickFix)
.OrderByDescending(fix = fix.Selection);
also a good moment to rename items to something like defaultQuickFixes
There was a problem hiding this comment.
This is especially clean, since InspectionResultBase already implements this for us...
There was a problem hiding this comment.
@Vogel612 well spotted - I forgot to get back to that hack after moving AggregateInspectionResult to the Rubberduck.UI.Inspections namespace, which makes the aggregate results nothing more than a presentation concern (which it really is after all) - I disagree that individual inspections should be responsible for aggregating their own results.
There was a problem hiding this comment.
First of all, use nameof--that'll fix the type-renaming issue. Second, why not use the is?
There was a problem hiding this comment.
@Hosch250 because Rubberduck.dll doesn't even know Rubberduck.Inspections.dll exists.
| {40CC03E3-756C-4674-AF07-384115DEAEE2}.Release64|x64.Build.0 = Release|Any CPU | ||
| {40CC03E3-756C-4674-AF07-384115DEAEE2}.Release64|x86.ActiveCfg = Release|Any CPU | ||
| {40CC03E3-756C-4674-AF07-384115DEAEE2}.Release64|x86.Build.0 = Release|Any CPU | ||
| {AC4F1D22-D74B-45FF-AB0C-CC2A104FE023}.Debug|Any CPU.ActiveCfg = Debug|Any CPU |
There was a problem hiding this comment.
I believe we can kill all legacy configurations. I've only ever used Debug and Release for over a year; the xxxx64 configs have been useless since the unification of x86 and x64 installers IIRC.
@ThunderFrame is DebugAccess of any use?
I'll make this PR clean up these noisy configurations.
| public interface IInspectionResult : IComparable<IInspectionResult>, IComparable | ||
| { | ||
| IEnumerable<IQuickFix> QuickFixes { get; } | ||
| bool HasQuickFixes { get; } |
There was a problem hiding this comment.
Not sure what you added this for, but I'm going to kill it when I make the quick fixes specify which inspections they can fix and take an inspection result for the specific details.
There was a problem hiding this comment.
The ViewModel was working off InspectionResultBase, now IInspectionResult - I didn't want to change any functionality, just keep the code compilable... so the property had to get onto the interface.
Extracted
Rubberduck.Inspectionsnamespace into its own assembly, moved things around to keep code compilable (e.g.VariableNameValidatoris now underRubberduck.Common).With this PR Rubberduck runs any
IInspectionimplementation located in any assembly under a\Plug-insfolder (under theRubberduck.dlldirectory).