Cleanup around the ImportCommand based on review comments to PR #5274

MDoerner · MDoerner · commit 9be7204bc967 · 2019-11-25T22:34:31.000+01:00
diff --git a/Rubberduck.Core/UI/CodeExplorer/Commands/ImportCommand.cs b/Rubberduck.Core/UI/CodeExplorer/Commands/ImportCommand.cs
@@ -35,7 +35,7 @@ public class ImportCommand : CodeExplorerCommandBase
         private readonly IParseManager _parseManager;
         private readonly IProjectsProvider _projectsProvider;
         private readonly IModuleNameFromFileExtractor _moduleNameFromFileExtractor;
-        private readonly IDictionary<ComponentType, IRequiredBinaryFilesFromFileNameExtractor> _binaryFileExtractors;
+        private readonly IDictionary<ComponentType, List<IRequiredBinaryFilesFromFileNameExtractor>> _binaryFileExtractors;
         private readonly IFileExistenceChecker _fileExistenceChecker;
 
         protected readonly IDeclarationFinderProvider DeclarationFinderProvider;
@@ -58,13 +58,13 @@ public ImportCommand(
             _dialogFactory = dialogFactory;
             _parseManager = parseManager;
             _projectsProvider = projectsProvider;
-            DeclarationFinderProvider = declarationFinderProvider;
             _moduleNameFromFileExtractor = moduleNameFromFileExtractor;
             _fileExistenceChecker = fileExistenceChecker;
 
             _binaryFileExtractors = BinaryFileExtractors(binaryFileExtractors);
 
             MessageBox = messageBox;
+            DeclarationFinderProvider = declarationFinderProvider;
 
             AddToCanExecuteEvaluation(SpecialEvaluateCanExecute);
 
@@ -123,19 +123,19 @@ private IVBProject TargetProjectFromVbe()
                 : null;
         }
 
-        private IDictionary<ComponentType, IRequiredBinaryFilesFromFileNameExtractor> BinaryFileExtractors(IEnumerable<IRequiredBinaryFilesFromFileNameExtractor> extractors)
+        private IDictionary<ComponentType, List<IRequiredBinaryFilesFromFileNameExtractor>> BinaryFileExtractors(IEnumerable<IRequiredBinaryFilesFromFileNameExtractor> extractors)
         {
-            var dict = new Dictionary<ComponentType, IRequiredBinaryFilesFromFileNameExtractor>();
+            var dict = new Dictionary<ComponentType, List<IRequiredBinaryFilesFromFileNameExtractor>>();
             foreach (var extractor in extractors)
             {
                 foreach (var componentType in extractor.SupportedComponentTypes)
-                {
-                    if (dict.ContainsKey(componentType))
+                { 
+                    if (!dict.ContainsKey(componentType))
                     {
-                        continue;
+                        dict.Add(componentType, new List<IRequiredBinaryFilesFromFileNameExtractor>());
                     }
 
-                    dict.Add(componentType, extractor);
+                    dict[componentType].Add(extractor);
                 }
             }
 
@@ -164,8 +164,7 @@ protected virtual ICollection<string> FilesToImport(object parameter)
 
                 var fileNames = dialog.FileNames;
                 var fileExtensions = fileNames.Select(Path.GetExtension);
-                var importableExtensions = ImportableExtensions;
-                if (fileExtensions.Any(fileExt => !importableExtensions.Contains(fileExt)))
+                if (fileExtensions.Any(fileExt => !ImportableExtensions.Contains(fileExt)))
                 {
                     NotifyUserAboutAbortDueToUnsupportedFileExtensions(fileNames);
                     return new List<string>();
@@ -177,6 +176,7 @@ protected virtual ICollection<string> FilesToImport(object parameter)
 
         protected virtual string DialogsTitle => RubberduckUI.ImportCommand_OpenDialog_Title;
 
+        //TODO: Gather all conflicts and report them in one error dialog instead of reporting them one at a time.
         private void NotifyUserAboutAbortDueToUnsupportedFileExtensions(IEnumerable<string> fileNames)
         {
             var firstUnsupportedFile = fileNames.First(filename => !ImportableExtensions.Contains(Path.GetExtension(filename)));
@@ -193,6 +193,7 @@ private void ImportFilesWithSuspension(ICollection<string> filesToImport, IVBPro
             {
                 if (suspendOutcome == SuspensionOutcome.UnexpectedError || suspendOutcome == SuspensionOutcome.Canceled)
                 {
+                    //This rethrows the exception with the original stack trace.
                     ExceptionDispatchInfo.Capture(suspendResult.EncounteredException).Throw();
                     return;
                 }
@@ -382,16 +383,19 @@ private Dictionary<string, ICollection<string>> RequiredBinaryFiles(ICollection<
         private ICollection<string> RequiredBinaryFiles(string filename)
         {
             var extension = Path.GetExtension(filename);
-            if (!ComponentTypesForExtension.TryGetValue(extension, out var componentTypes))
+            if (extension == null || !ComponentTypesForExtension.TryGetValue(extension, out var componentTypes))
             {
                 return new List<string>();
             }
 
             foreach (var componentType in componentTypes)
             {
-                if (_binaryFileExtractors.TryGetValue(componentType, out var binaryExtractor))
+                if (_binaryFileExtractors.TryGetValue(componentType, out var binaryExtractors))
                 {
-                    return binaryExtractor.RequiredBinaryFiles(filename, componentType);
+                    return binaryExtractors
+                        .SelectMany(binaryExtractor => binaryExtractor
+                            .RequiredBinaryFiles(filename, componentType))
+                        .ToHashSet();
                 }
             }
 
@@ -523,8 +527,8 @@ protected override void OnExecute(object parameter)
         }
 
 
-        //We only allow extensions to be imported for which we might be able to determine that the conditions are met to actually import the file.
-        //The exception are specif exceptions to the rule.
+        //We usually only allow extensions to be imported for which we might be able to determine that the conditions are met to actually import the file.
+        //However, we ignore this cautionary rule for the extensions specified in AlwaysImportableExtensions;
         protected ICollection<string> ImportableExtensions =>
             ComponentTypesForExtension.Keys
                 .Where(fileExtension => ComponentTypesForExtension.TryGetValue(fileExtension, out var componentTypes)
@@ -534,6 +538,7 @@ protected override void OnExecute(object parameter)
                 .Concat(AlwaysImportableExtensions)
                 .ToHashSet();
 
+        //TODO: Implement the binary extractors necessary to allow us to remove this member.
         protected virtual IEnumerable<string> AlwaysImportableExtensions => _vbe.Kind == VBEKind.Standalone
             ? ComponentTypesForExtension.Keys
             : Enumerable.Empty<string>();
diff --git a/Rubberduck.Parsing/VBA/DeclarationResolving/DeclarationResolveRunner.cs b/Rubberduck.Parsing/VBA/DeclarationResolving/DeclarationResolveRunner.cs
@@ -56,6 +56,7 @@ protected override void ResolveDeclarations(IReadOnlyCollection<QualifiedModuleN
             {
                 if (exception.Flatten().InnerExceptions.All(ex => ex is OperationCanceledException))
                 {
+                    //This rethrows the exception with the original stack trace.
                     ExceptionDispatchInfo.Capture(exception.InnerException ?? exception).Throw();
                 }
                 _parserStateManager.SetStatusAndFireStateChanged(this, ParserState.ResolverError, token);
diff --git a/Rubberduck.Parsing/VBA/Parsing/ParseRunner.cs b/Rubberduck.Parsing/VBA/Parsing/ParseRunner.cs
@@ -62,6 +62,7 @@ public ParseRunner(
             {
                 if (exception.Flatten().InnerExceptions.All(ex => ex is OperationCanceledException))
                 {
+                    //This rethrows the exception with the original stack trace.
                     ExceptionDispatchInfo.Capture(exception.InnerException ?? exception).Throw();
                 }
                 StateManager.SetStatusAndFireStateChanged(this, ParserState.Error, token);
diff --git a/Rubberduck.Parsing/VBA/ReferenceManagement/ReferenceResolveRunner.cs b/Rubberduck.Parsing/VBA/ReferenceManagement/ReferenceResolveRunner.cs
@@ -42,6 +42,7 @@ protected override void ResolveReferences(ICollection<KeyValuePair<QualifiedModu
             {
                 if (exception.Flatten().InnerExceptions.All(ex => ex is OperationCanceledException))
                 {
+                    //This rethrows the exception with the original stack trace.
                     ExceptionDispatchInfo.Capture(exception.InnerException ?? exception).Throw();
                 }
 
@@ -70,7 +71,8 @@ protected override void AddModuleToModuleReferences(DeclarationFinder finder, Ca
             {
                 if (exception.Flatten().InnerExceptions.All(ex => ex is OperationCanceledException))
                 {
-                    throw exception.InnerException ?? exception; //This eliminates the stack trace, but for the cancellation, this is irrelevant.
+                    //This rethrows the exception with the original stack trace.
+                    ExceptionDispatchInfo.Capture(exception.InnerException ?? exception).Throw();
                 }
 
                 _parserStateManager.SetStatusAndFireStateChanged(this, ParserState.ResolverError, token);
diff --git a/Rubberduck.Resources/RubberduckUI.de.resx b/Rubberduck.Resources/RubberduckUI.de.resx
@@ -1538,12 +1538,12 @@ Import abgebrochen.</value>
 Import abgebrochen.</value>
   </data>
   <data name="ImportCommand_BinaryAndComponentDoNotExist" xml:space="preserve">
-    <value>Für die Datei '{0}' fehlen die benötigte(n) Binary-Datei(en) '{1}' und die Zielkomponente '{2}'existiert noch nicht.
+    <value>Für die Datei '{0}' fehlen die benötigte(n) Binärdatei(en) '{1}' und die Zielkomponente '{2}' existiert noch nicht.
 
 Import abgebrochen.</value>
   </data>
   <data name="ImportCommand_BinaryDoesNotExist" xml:space="preserve">
-    <value>Für die Datei '{0}' fehlen die benötigte(n) Binary-Datei(en) '{1}'.
+    <value>Für die Datei '{0}' fehlen die benötigte(n) Binärdatei(en) '{1}'.
 
 Import abgebrochen.</value>
   </data>

Original file line number	Diff line number	Diff line change
`@@ -35,7 +35,7 @@ public class ImportCommand : CodeExplorerCommandBase`
`35`	`35`	`private readonly IParseManager _parseManager;`
`36`	`36`	`private readonly IProjectsProvider _projectsProvider;`
`37`	`37`	`private readonly IModuleNameFromFileExtractor _moduleNameFromFileExtractor;`
`38`		`- private readonly IDictionary<ComponentType, IRequiredBinaryFilesFromFileNameExtractor> _binaryFileExtractors;`
	`38`	`+ private readonly IDictionary<ComponentType, List<IRequiredBinaryFilesFromFileNameExtractor>> _binaryFileExtractors;`
`39`	`39`	`private readonly IFileExistenceChecker _fileExistenceChecker;`
`40`	`40`
`41`	`41`	`protected readonly IDeclarationFinderProvider DeclarationFinderProvider;`
`@@ -58,13 +58,13 @@ public ImportCommand(`
`58`	`58`	`_dialogFactory = dialogFactory;`
`59`	`59`	`_parseManager = parseManager;`
`60`	`60`	`_projectsProvider = projectsProvider;`
`61`		`- DeclarationFinderProvider = declarationFinderProvider;`
`62`	`61`	`_moduleNameFromFileExtractor = moduleNameFromFileExtractor;`
`63`	`62`	`_fileExistenceChecker = fileExistenceChecker;`
`64`	`63`
`65`	`64`	`_binaryFileExtractors = BinaryFileExtractors(binaryFileExtractors);`
`66`	`65`
`67`	`66`	`MessageBox = messageBox;`
	`67`	`+ DeclarationFinderProvider = declarationFinderProvider;`
`68`	`68`
`69`	`69`	`AddToCanExecuteEvaluation(SpecialEvaluateCanExecute);`
`70`	`70`
`@@ -123,19 +123,19 @@ private IVBProject TargetProjectFromVbe()`
`123`	`123`	`: null;`
`124`	`124`	`}`
`125`	`125`
`126`		`- private IDictionary<ComponentType, IRequiredBinaryFilesFromFileNameExtractor> BinaryFileExtractors(IEnumerable<IRequiredBinaryFilesFromFileNameExtractor> extractors)`
	`126`	`+ private IDictionary<ComponentType, List<IRequiredBinaryFilesFromFileNameExtractor>> BinaryFileExtractors(IEnumerable<IRequiredBinaryFilesFromFileNameExtractor> extractors)`
`127`	`127`	`{`
`128`		`- var dict = new Dictionary<ComponentType, IRequiredBinaryFilesFromFileNameExtractor>();`
	`128`	`+ var dict = new Dictionary<ComponentType, List<IRequiredBinaryFilesFromFileNameExtractor>>();`
`129`	`129`	`foreach (var extractor in extractors)`
`130`	`130`	`{`
`131`	`131`	`foreach (var componentType in extractor.SupportedComponentTypes)`
`132`		`- {`
`133`		`- if (dict.ContainsKey(componentType))`
	`132`	`+ {`
	`133`	`+ if (!dict.ContainsKey(componentType))`
`134`	`134`	`{`
`135`		`- continue;`
	`135`	`+ dict.Add(componentType, new List<IRequiredBinaryFilesFromFileNameExtractor>());`
`136`	`136`	`}`
`137`	`137`
`138`		`- dict.Add(componentType, extractor);`
	`138`	`+ dict[componentType].Add(extractor);`
`139`	`139`	`}`
`140`	`140`	`}`
`141`	`141`
`@@ -164,8 +164,7 @@ protected virtual ICollection<string> FilesToImport(object parameter)`
`164`	`164`
`165`	`165`	`var fileNames = dialog.FileNames;`
`166`	`166`	`var fileExtensions = fileNames.Select(Path.GetExtension);`
`167`		`- var importableExtensions = ImportableExtensions;`
`168`		`- if (fileExtensions.Any(fileExt => !importableExtensions.Contains(fileExt)))`
	`167`	`+ if (fileExtensions.Any(fileExt => !ImportableExtensions.Contains(fileExt)))`
`169`	`168`	`{`
`170`	`169`	`NotifyUserAboutAbortDueToUnsupportedFileExtensions(fileNames);`
`171`	`170`	`return new List<string>();`
`@@ -177,6 +176,7 @@ protected virtual ICollection<string> FilesToImport(object parameter)`
`177`	`176`
`178`	`177`	`protected virtual string DialogsTitle => RubberduckUI.ImportCommand_OpenDialog_Title;`
`179`	`178`
	`179`	`+ //TODO: Gather all conflicts and report them in one error dialog instead of reporting them one at a time.`
`180`	`180`	`private void NotifyUserAboutAbortDueToUnsupportedFileExtensions(IEnumerable<string> fileNames)`
`181`	`181`	`{`
`182`	`182`	`var firstUnsupportedFile = fileNames.First(filename => !ImportableExtensions.Contains(Path.GetExtension(filename)));`
`@@ -193,6 +193,7 @@ private void ImportFilesWithSuspension(ICollection<string> filesToImport, IVBPro`
`193`	`193`	`{`
`194`	`194`	`if (suspendOutcome == SuspensionOutcome.UnexpectedError \|\| suspendOutcome == SuspensionOutcome.Canceled)`
`195`	`195`	`{`
	`196`	`+ //This rethrows the exception with the original stack trace.`
`196`	`197`	`ExceptionDispatchInfo.Capture(suspendResult.EncounteredException).Throw();`
`197`	`198`	`return;`
`198`	`199`	`}`
`@@ -382,16 +383,19 @@ private Dictionary<string, ICollection<string>> RequiredBinaryFiles(ICollection<`
`382`	`383`	`private ICollection<string> RequiredBinaryFiles(string filename)`
`383`	`384`	`{`
`384`	`385`	`var extension = Path.GetExtension(filename);`
`385`		`- if (!ComponentTypesForExtension.TryGetValue(extension, out var componentTypes))`
	`386`	`+ if (extension == null \|\| !ComponentTypesForExtension.TryGetValue(extension, out var componentTypes))`
`386`	`387`	`{`
`387`	`388`	`return new List<string>();`
`388`	`389`	`}`
`389`	`390`
`390`	`391`	`foreach (var componentType in componentTypes)`
`391`	`392`	`{`
`392`		`- if (_binaryFileExtractors.TryGetValue(componentType, out var binaryExtractor))`
	`393`	`+ if (_binaryFileExtractors.TryGetValue(componentType, out var binaryExtractors))`
`393`	`394`	`{`
`394`		`- return binaryExtractor.RequiredBinaryFiles(filename, componentType);`
	`395`	`+ return binaryExtractors`
	`396`	`+ .SelectMany(binaryExtractor => binaryExtractor`
	`397`	`+ .RequiredBinaryFiles(filename, componentType))`
	`398`	`+ .ToHashSet();`
`395`	`399`	`}`
`396`	`400`	`}`
`397`	`401`
`@@ -523,8 +527,8 @@ protected override void OnExecute(object parameter)`
`523`	`527`	`}`
`524`	`528`
`525`	`529`
`526`		`- //We only allow extensions to be imported for which we might be able to determine that the conditions are met to actually import the file.`
`527`		`- //The exception are specif exceptions to the rule.`
	`530`	`+ //We usually only allow extensions to be imported for which we might be able to determine that the conditions are met to actually import the file.`
	`531`	`+ //However, we ignore this cautionary rule for the extensions specified in AlwaysImportableExtensions;`
`528`	`532`	`protected ICollection<string> ImportableExtensions =>`
`529`	`533`	`ComponentTypesForExtension.Keys`
`530`	`534`	`.Where(fileExtension => ComponentTypesForExtension.TryGetValue(fileExtension, out var componentTypes)`
`@@ -534,6 +538,7 @@ protected override void OnExecute(object parameter)`
`534`	`538`	`.Concat(AlwaysImportableExtensions)`
`535`	`539`	`.ToHashSet();`
`536`	`540`
	`541`	`+ //TODO: Implement the binary extractors necessary to allow us to remove this member.`
`537`	`542`	`protected virtual IEnumerable<string> AlwaysImportableExtensions => _vbe.Kind == VBEKind.Standalone`
`538`	`543`	`? ComponentTypesForExtension.Keys`
`539`	`544`	`: Enumerable.Empty<string>();`
Original file line number	Diff line number	Diff line change
`@@ -56,6 +56,7 @@ protected override void ResolveDeclarations(IReadOnlyCollection<QualifiedModuleN`
`56`	`56`	`{`
`57`	`57`	`if (exception.Flatten().InnerExceptions.All(ex => ex is OperationCanceledException))`
`58`	`58`	`{`
	`59`	`+ //This rethrows the exception with the original stack trace.`
`59`	`60`	`ExceptionDispatchInfo.Capture(exception.InnerException ?? exception).Throw();`
`60`	`61`	`}`
`61`	`62`	`_parserStateManager.SetStatusAndFireStateChanged(this, ParserState.ResolverError, token);`
Original file line number	Diff line number	Diff line change
`@@ -62,6 +62,7 @@ public ParseRunner(`
`62`	`62`	`{`
`63`	`63`	`if (exception.Flatten().InnerExceptions.All(ex => ex is OperationCanceledException))`
`64`	`64`	`{`
	`65`	`+ //This rethrows the exception with the original stack trace.`
`65`	`66`	`ExceptionDispatchInfo.Capture(exception.InnerException ?? exception).Throw();`
`66`	`67`	`}`
`67`	`68`	`StateManager.SetStatusAndFireStateChanged(this, ParserState.Error, token);`
Original file line number	Diff line number	Diff line change
`@@ -42,6 +42,7 @@ protected override void ResolveReferences(ICollection<KeyValuePair<QualifiedModu`
`42`	`42`	`{`
`43`	`43`	`if (exception.Flatten().InnerExceptions.All(ex => ex is OperationCanceledException))`
`44`	`44`	`{`
	`45`	`+ //This rethrows the exception with the original stack trace.`
`45`	`46`	`ExceptionDispatchInfo.Capture(exception.InnerException ?? exception).Throw();`
`46`	`47`	`}`
`47`	`48`
`@@ -70,7 +71,8 @@ protected override void AddModuleToModuleReferences(DeclarationFinder finder, Ca`
`70`	`71`	`{`
`71`	`72`	`if (exception.Flatten().InnerExceptions.All(ex => ex is OperationCanceledException))`
`72`	`73`	`{`
`73`		`- throw exception.InnerException ?? exception; //This eliminates the stack trace, but for the cancellation, this is irrelevant.`
	`74`	`+ //This rethrows the exception with the original stack trace.`
	`75`	`+ ExceptionDispatchInfo.Capture(exception.InnerException ?? exception).Throw();`
`74`	`76`	`}`
`75`	`77`
`76`	`78`	`_parserStateManager.SetStatusAndFireStateChanged(this, ParserState.ResolverError, token);`