icsharpcode / RefactoringEssentials

Refactoring Essentials for Visual Studio
MIT License
638 stars 120 forks source link

RedundantPrivateCodeFixProvider passes null document in DocumentChangeAction.PostProcessChangesAsync #105

Open jmarolf opened 9 years ago

jmarolf commented 9 years ago

Customer reported a stacktrace at https://github.com/dotnet/roslyn/issues/5121.

Object reference not set to an instance of an object.
   
at Microsoft.CodeAnalysis.Simplification.Simplifier.<ReduceAsync>d__11.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   
at RefactoringEssentials.DocumentChangeAction.<PostProcessChangesAsync>d__6.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   
at Microsoft.CodeAnalysis.CodeActions.CodeAction.<PostProcessChangesAsync>d__14.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   
at Microsoft.CodeAnalysis.CodeActions.CodeAction.<PostProcessAsync>d__13.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   
at Microsoft.CodeAnalysis.CodeActions.CodeAction.<GetPreviewOperationsAsync>d__6.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   
at Microsoft.CodeAnalysis.Editor.Implementation.Suggestions.SuggestedAction.<>c__DisplayClass10_0.<<GetPreviewOperationsAsync>b__0>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   
at Microsoft.CodeAnalysis.Editor.Implementation.Suggestions.SuggestedAction.<GetPreviewResultAsync>d__14.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   
at Microsoft.CodeAnalysis.Editor.Implementation.Suggestions.SuggestedAction.<>c__DisplayClass17_0.<<GetPreviewAsync>b__0>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   
at Microsoft.CodeAnalysis.Extensions.IExtensionManagerExtensions.<PerformFunctionAsync>d__3`1.MoveNext()

My plan is to make sure the Simplifier throws ArgumentNull exceptions so this is easier to track down, but let me know if you have other suggestions.

Rpinski commented 9 years ago

Hmm, I'm still wondering how this can happen. The Document instance we pass here is something we receive from Roslyn (it's an overridden method):

https://github.com/icsharpcode/RefactoringEssentials/blob/master/RefactoringEssentials/CSharp/Diagnostics/DocumentChangeAction.cs#L39

Wouldn't that mean that Roslyn has passed a null reference to us?

Rpinski commented 9 years ago

And I also don't see in our code, where we could create a null Document in this analyzer:

https://github.com/icsharpcode/RefactoringEssentials/blob/master/RefactoringEssentials/CSharp/Diagnostics/Custom/RedundantPrivateCodeFixProvider.cs#L38

https://github.com/icsharpcode/RefactoringEssentials/blob/master/RefactoringEssentials/CSharp/Diagnostics/Custom/RedundantPrivateAnalyzer.cs#L151