search

mono / monodevelop

MonoDevelop is a cross platform .NET IDE
http://www.monodevelop.com
2.82k stars 1.01k forks source link

MEF composition errors are logged as warnings #4426

Open mhutch opened 6 years ago

mhutch commented 6 years ago

MEF composition errors are real errors that can cause loss of functionality, but for some reason they are only logged as info, so are not reported to telemetry.

INFO [2018-04-03 11:59:35Z]: MEF composition error: Microsoft.CodeAnalysis.Editor.Implementation.Workspaces.EditorTextFactoryService.ctor(textBufferCloneService): expected exactly 1 export of Microsoft.CodeAnalysis.Text.ITextBufferCloneService but found 0.
INFO [2018-04-03 11:59:35Z]: MEF composition error: Microsoft.CodeAnalysis.Editor.Implementation.Peek.PeekableItemSourceProvider.ctor(peekResultFactory): expected exactly 1 export of Microsoft.VisualStudio.Language.Intellisense.IPeekResultFactory but found 0.

VS bug #595965

Therzok commented 6 years ago

We're missing EditorFeatures.Text from composition: https://github.com/mono/monodevelop/blob/master/main/src/core/MonoDevelop.Ide/ExtensionModel/MonoDevelop.Ide.addin.xml#L409

Therzok commented 6 years ago

Also VS.Intellisense.dll

Therzok commented 6 years ago

@KirillOsenkov is there anything else we need to include?

Therzok commented 6 years ago

Should we be generating them automatically for things in ReferencesRoslyn/VSEditor.props?

KirillOsenkov commented 6 years ago

No, I'd like to have intentional manual control over these. Don't worry, if we were missing something important, composition would tell us ;)

On MS.CA.EF.Text.dll - good catch, but there's only one type exported: TextBufferCloneServiceFactory. Apparently it's non-essential. I'll still add it to the composition. On MS.VS.Language.intelliSense.dll - it doesn't have any MEF exports inside it.

Therzok commented 6 years ago

We're still missing IPeekResultFactory. It seems we need to supply: a) an implementation for IDocumentPeekResult b) an implementation for IExternallyBrowsablePeekResult c) an implementation for IPeekResultFactory

KirillOsenkov commented 6 years ago

There are Roslyn bugs tracking all of those. I've verified that the failures we're seeing are completely benign (i.e. not causing cascading failures, because these are the leafs in the MEF graph).

We can certainly provide stubs to shut up the warnings, but I'm not sure it's worth it? Let me know and I could do it if you feel it's cleaner this way.

mhutch commented 6 years ago

I would probably add stubs and turn the warnings into real errors so the faults get reported if composition breaks for any other reason.