From ce406f10e7e9af7716c037191fa4956ddef70f67 Mon Sep 17 00:00:00 2001 From: madelson <1269046+madelson@users.noreply.github.com> Date: Thu, 3 Nov 2022 13:58:25 -0400 Subject: [PATCH] Make ResourceManager.BaseName work as documented. (#75497) * Make ResourceManager.BaseName work as documented. This property is documented to return the "qualified namespace name and the root resource name of a resource file". However, previously when constructing a ResourceManager with a Type the BaseName property would instead return the name of the type without its namespace. fix #74918 * Remove _locationInfo field * Add additional test cases for ResourceManager.BaseName. Address feedback from https://github.com/dotnet/runtime/pull/75497#issuecomment-1262913704. * Fix BaseName test case in response to https://github.com/dotnet/runtime/pull/75497#issuecomment-1295500698 --- .../tests/ComponentResourceManagerTests.cs | 2 +- .../Resources/FileBasedResourceGroveler.cs | 5 +--- .../ManifestBasedResourceGroveler.cs | 25 +++++-------------- .../src/System/Resources/ResourceManager.cs | 16 ++++++------ .../tests/ResourceManagerTests.cs | 13 +++++++++- 5 files changed, 28 insertions(+), 33 deletions(-) diff --git a/src/libraries/System.ComponentModel.TypeConverter/tests/ComponentResourceManagerTests.cs b/src/libraries/System.ComponentModel.TypeConverter/tests/ComponentResourceManagerTests.cs index 12b72967c4089..6f3c901031909 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/tests/ComponentResourceManagerTests.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/tests/ComponentResourceManagerTests.cs @@ -24,7 +24,7 @@ public void Ctor_Default() public void Ctor_Type(Type type) { var resourceManager = new ComponentResourceManager(type); - Assert.Equal("Int32", resourceManager.BaseName); + Assert.Equal("System.Int32", resourceManager.BaseName); Assert.False(resourceManager.IgnoreCase); Assert.NotNull(resourceManager.ResourceSetType); } diff --git a/src/libraries/System.Private.CoreLib/src/System/Resources/FileBasedResourceGroveler.cs b/src/libraries/System.Private.CoreLib/src/System/Resources/FileBasedResourceGroveler.cs index ad98312fadd85..6bf0bd754fe96 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Resources/FileBasedResourceGroveler.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Resources/FileBasedResourceGroveler.cs @@ -47,10 +47,7 @@ public FileBasedResourceGroveler(ResourceManager.ResourceManagerMediator mediato // If we've hit top of the Culture tree, return. if (culture.HasInvariantCultureName) { - // We really don't think this should happen - we always - // expect the neutral locale's resources to be present. - string? locationInfo = _mediator.LocationInfo == null ? "" : _mediator.LocationInfo.FullName; - throw new MissingManifestResourceException($"{SR.MissingManifestResource_NoNeutralDisk}{Environment.NewLineConst}baseName: {_mediator.BaseNameField} locationInfo: {locationInfo} fileName: {_mediator.GetResourceFileName(culture)}"); + throw new MissingManifestResourceException($"{SR.MissingManifestResource_NoNeutralDisk}{Environment.NewLineConst}baseName: {_mediator.BaseNameField} fileName: {_mediator.GetResourceFileName(culture)}"); } } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Resources/ManifestBasedResourceGroveler.cs b/src/libraries/System.Private.CoreLib/src/System/Resources/ManifestBasedResourceGroveler.cs index fa7bfb44b4bfc..6a100e23251af 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Resources/ManifestBasedResourceGroveler.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Resources/ManifestBasedResourceGroveler.cs @@ -321,12 +321,12 @@ private static ResourceSet InternalGetResourceSetFromSerializedData(Stream store return rs; } - private Stream? GetManifestResourceStream(Assembly satellite, string fileName) + private static Stream? GetManifestResourceStream(Assembly satellite, string fileName) { Debug.Assert(satellite != null, "satellite shouldn't be null; check caller"); Debug.Assert(fileName != null, "fileName shouldn't be null; check caller"); - return satellite.GetManifestResourceStream(_mediator.LocationInfo!, fileName) ?? + return satellite.GetManifestResourceStream(fileName) ?? CaseInsensitiveManifestResourceStreamLookup(satellite, fileName); } @@ -334,22 +334,15 @@ private static ResourceSet InternalGetResourceSetFromSerializedData(Stream store // case-insensitive lookup rules. Yes, this is slow. The metadata // dev lead refuses to make all assembly manifest resource lookups case-insensitive, // even optionally case-insensitive. - private Stream? CaseInsensitiveManifestResourceStreamLookup(Assembly satellite, string name) + private static Stream? CaseInsensitiveManifestResourceStreamLookup(Assembly satellite, string name) { Debug.Assert(satellite != null, "satellite shouldn't be null; check caller"); Debug.Assert(name != null, "name shouldn't be null; check caller"); - string? nameSpace = _mediator.LocationInfo?.Namespace; - - char c = Type.Delimiter; - string resourceName = nameSpace != null && name != null ? - string.Concat(nameSpace, new ReadOnlySpan(in c), name) : - string.Concat(nameSpace, name); - string? canonicalName = null; foreach (string existingName in satellite.GetManifestResourceNames()) { - if (string.Equals(existingName, resourceName, StringComparison.InvariantCultureIgnoreCase)) + if (string.Equals(existingName, name, StringComparison.InvariantCultureIgnoreCase)) { if (canonicalName == null) { @@ -357,7 +350,7 @@ private static ResourceSet InternalGetResourceSetFromSerializedData(Stream store } else { - throw new MissingManifestResourceException(SR.Format(SR.MissingManifestResource_MultipleBlobs, resourceName, satellite.ToString())); + throw new MissingManifestResourceException(SR.Format(SR.MissingManifestResource_MultipleBlobs, name, satellite.ToString())); } } } @@ -488,16 +481,10 @@ private void HandleResourceStreamMissing(string fileName) const string MesgFailFast = System.CoreLib.Name + ResourceManager.ResFileExtension + " couldn't be found! Large parts of the BCL won't work!"; System.Environment.FailFast(MesgFailFast); } - // We really don't think this should happen - we always - // expect the neutral locale's resources to be present. - string resName = string.Empty; - if (_mediator.LocationInfo != null && _mediator.LocationInfo.Namespace != null) - resName = _mediator.LocationInfo.Namespace + Type.Delimiter; - resName += fileName; Debug.Assert(_mediator.MainAssembly != null); throw new MissingManifestResourceException( SR.Format(SR.MissingManifestResource_NoNeutralAsm, - resName, _mediator.MainAssembly.GetName().Name, GetManifestResourceNamesList(_mediator.MainAssembly))); + fileName, _mediator.MainAssembly.GetName().Name, GetManifestResourceNamesList(_mediator.MainAssembly))); } } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceManager.cs b/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceManager.cs index 20f07154bdec5..71138a8ebb706 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceManager.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceManager.cs @@ -103,7 +103,6 @@ internal sealed class CultureNameResourceSetPair private Dictionary? _resourceSets; private readonly string? _moduleDir; // For assembly-ignorant directory location - private readonly Type? _locationInfo; // For Assembly or type-based directory layout [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] private readonly Type? _userResourceSet; // Which ResourceSet instance to create @@ -227,9 +226,13 @@ public ResourceManager(Type resourceSource) if (resourceSource is not RuntimeType) throw new ArgumentException(SR.Argument_MustBeRuntimeType); - _locationInfo = resourceSource; - MainAssembly = _locationInfo.Assembly; - BaseNameField = resourceSource.Name; + MainAssembly = resourceSource.Assembly; + + string? nameSpace = resourceSource.Namespace; + char c = Type.Delimiter; + BaseNameField = nameSpace is null + ? resourceSource.Name + : string.Concat(nameSpace, new ReadOnlySpan(in c), resourceSource.Name); CommonAssemblyInit(); } @@ -407,7 +410,7 @@ protected virtual string GetResourceFileName(CultureInfo culture) { string fileName = GetResourceFileName(culture); Debug.Assert(MainAssembly != null); - Stream? stream = MainAssembly.GetManifestResourceStream(_locationInfo!, fileName); + Stream? stream = MainAssembly.GetManifestResourceStream(fileName); if (createIfNotExists && stream != null) { rs = ((ManifestBasedResourceGroveler)_resourceGroveler).CreateResourceSet(stream, MainAssembly); @@ -747,9 +750,6 @@ internal ResourceManagerMediator(ResourceManager rm) // NEEDED ONLY BY FILE-BASED internal string? ModuleDir => _rm._moduleDir; - // NEEDED BOTH BY FILE-BASED AND ASSEMBLY-BASED - internal Type? LocationInfo => _rm._locationInfo; - [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] internal Type? UserResourceSet => _rm._userResourceSet; diff --git a/src/libraries/System.Resources.ResourceManager/tests/ResourceManagerTests.cs b/src/libraries/System.Resources.ResourceManager/tests/ResourceManagerTests.cs index ebdac1cd0c48a..7b2b19bd54c59 100644 --- a/src/libraries/System.Resources.ResourceManager/tests/ResourceManagerTests.cs +++ b/src/libraries/System.Resources.ResourceManager/tests/ResourceManagerTests.cs @@ -204,8 +204,19 @@ public static void UsingResourceSet() [Fact] public static void BaseName() { - var manager = new ResourceManager("System.Resources.Tests.Resources.TestResx", typeof(ResourceManagerTests).GetTypeInfo().Assembly); + var manager = new ResourceManager("System.Resources.Tests.Resources.TestResx", typeof(ResourceManagerTests).Assembly); + Assert.Equal("System.Resources.Tests.Resources.TestResx", manager.BaseName); + + manager = new ResourceManager(typeof(System.Resources.Tests.Resources.TestResx)); Assert.Equal("System.Resources.Tests.Resources.TestResx", manager.BaseName); + + Type typeWithoutNamespace = new { }.GetType(); + Assert.Null(typeWithoutNamespace.Namespace); + manager = new ResourceManager(typeWithoutNamespace); + Assert.Equal(typeWithoutNamespace.Name, manager.BaseName); + + manager = new ResourceManager(typeof(List)); + Assert.Equal("System.Collections.Generic.List`1", manager.BaseName); } [Theory]