Skip to content

Load analyzer satellite assemblies - #1223

Merged
tmeschter merged 2 commits into
dotnet:masterfrom
tmeschter:HandleLocalizedAnalyzers
Mar 13, 2015
Merged

tmeschter merged 2 commits into
dotnet:masterfrom
tmeschter:HandleLocalizedAnalyzers

Conversation

@tmeschter

Copy link
Copy Markdown
Contributor

Analyzers support localized strings for titles, messages, descriptions,
etc. through the LocalizableString types. The localized strings will
generally end up in a set of "satellite" assemblies ending in
.resources.dll, one per culture. For example, if your analyzer is in an
assembly "MyAnalyzer" and you provided localized strings for French
(e.g., the fr-FR culture) then the build will typically produce
bin\Debug\MyAnalyzer.dll and bin\Debug\fr-FR\MyAnalyzer.resources.dll.

However, we currently have no way to find these satellite assemblies
when the CLR asks for them via the AppDomain.AssemblyResolve event, so
we never actually output localized strings from analyzers. The fix is to
simply look for the assemblies in culture-specific directories next to
the analyzer.

Analyzers support localized strings for titles, messages, descriptions,
etc. through the `LocalizableString` types. The localized strings will
generally end up in a set of "satellite" assemblies ending in
.resources.dll, one per culture. For example, if your analyzer is in an
assembly "MyAnalyzer" and you provided localized strings for French
(e.g., the fr-FR culture) then the build will typically produce
bin\Debug\MyAnalyzer.dll and bin\Debug\fr-FR\MyAnalyzer.resources.dll.

However, we currently have no way to find these satellite assemblies
when the CLR asks for them via the `AppDomain.AssemblyResolve` event, so
we never actually output localized strings from analyzers. The fix is to
simply look for the assemblies in culture-specific directories next to
the analyzer.
@tmeschter

Copy link
Copy Markdown
Contributor Author

@srivatsn @shyamnamboodiripad @mavasani @heejaechang @jmarolf @JohnHamby Could you take a look, please?

@jmarolf

jmarolf commented Mar 12, 2015

Copy link
Copy Markdown
Contributor

Tests?

@tmeschter

Copy link
Copy Markdown
Contributor Author

Oh, all right. This turns out to be very difficult to do in a unit test, unfortunately. There probably isn't a good way to do it without refactoring the API and separating the assembly loader from the AnalyzerFileReference. I'll have to think about that some more.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we overwrite the directory path only if this directory actually exists: Path.Combine(directoryPath, requestedAssemblyIdentity.CultureName); ? This we way we can fallback to the loadedAssembly directory if no such sub-folder exists.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally, is it also worth looking for sub-folder with name: "CultureInfo.GetCultureInfo(requestedAssemblyIdentity.CultureName)?.LCID"?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But why would we fall back?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the LCID thing - will the CLR assembly resolution for normal contexts pick up a Japanese resource if it was in the 1041 directory for example? If so, yes we should look for the LCID directory - otherwise I don't think there'll be a scenario where the user will expect it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it will not automatically look in a 1041 directory. The probing logic is described here:
https://msdn.microsoft.com/en-us/library/15hyw9x3(v=vs.100).aspx

@mavasani

Copy link
Copy Markdown
Contributor

👍

@tmeschter

Copy link
Copy Markdown
Contributor Author

For the sake of good design and testability, we need to separate the concept of an AnalyzerFileReference from the mechanism we use to load assemblies. I'll update AnalyzerFileReference to take an instance of a new interface, IAssemblyLoader, instead of the current Func<string, Assembly>.

The InMemoryAssemblyLoader will become one implementation of IAssemblyLoader and we'll have another for loading VSIX-based analyzers.

Extract out the logic used by
`AnalyzerFileReference.InMemoryAssemblyLoader` to guess where a given
assembly might be located, and add a couple of unit tests.
@tmeschter

Copy link
Copy Markdown
Contributor Author

After several attempts, I've decided there's no good way to really separate AnalyzerFileReference from the InMemoryAssemblyLoader. I've instead extracted out the code for generating a potential assembly path from an AssemblyIdentity and added unit tests just for that.

@tmeschter

Copy link
Copy Markdown
Contributor Author

@jmarolf I've added tests.

@jmarolf

jmarolf commented Mar 13, 2015

Copy link
Copy Markdown
Contributor

@tmeschter thanks! 👍

tmeschter added a commit that referenced this pull request Mar 13, 2015
@tmeschter
tmeschter merged commit 1b13e0f into dotnet:master Mar 13, 2015
JoeRobich added a commit that referenced this pull request Aug 18, 2026
…sults

Group and sort NuGet Dependency results
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants