Mark Assembly.CodeBase as obsolete

[terrajobst]

Assembly.Location and Assembly.CodeBase are very similar, but not the same thing. To reduce confusion, we should mark it as obsolete.

Reason

Having two things that are similar, but not identical always causes confusion. Here is an example.

As @jkotas said:

Assembly.Location is strictly better than Assembly.CodeBase.

Assembly.CodeBase is an obsolete property. The only reason why it was included in .NET Core was .NET Framework compatibility. The original purpose of Assembly.CodeBase was CAS (Code Access Security). It was meant to describe where the assembly was downloaded from for the Internet Zone security checks. It also explains some of its weird behaviors. For example, if the assembly is loaded as byte array, it returns the location of the caller of the Assembly.Load method.

Proposed API

namespace System.Reflection
{
    public partial class Assembly
    {
        [Obsolete("Use Location instead.")]
        public virtual string CodeBase { get; }
    }
}

[terrajobst]

Video

Looks good. We should also mark it as [EditorBrowsable(Never)].

namespace System.Reflection
{
    public partial class Assembly
    {
        [Obsolete("Use Location instead.")]
        public virtual string CodeBase { get; }
    }
}

[vitek-karas]

Should we also consider obsoleting AssemblyName.CodeBase? It's not exactly the same thing, but quite related - in fact that property has no functional effect (trying to load assembly via AssemblyName will ignore the CodeBase property anyway: https://github.com/dotnet/coreclr/blob/master/src/System.Private.CoreLib/src/System/Reflection/Assembly.CoreCLR.cs#L63

[jkotas]

Assembly.EscapedCodeBase should be marked as obsolete as well.

[bitbonk]

Just out of curiosity: Since those APIs are part of .NET Standard, what implications does marking APIs as obsolete have for .NET Standard?

Since in .NET Standard, APIs cannot be removed, only added, does that mean you will only obsolete those APs but never remove them? Or will .NET Standard itself become obsolete due to the unification in .NET 5.

[jkotas]

Right, we do not have plans to remove any netstandard APIs.

[JoshSchreuder]

I started looking at this, but need a little clarification as I'm unfamiliar with the code structure.

With my change, I'm getting the following errors where CodeBase / EscapedCodeBase are used elsewhere (DesigntimeLicenseContext, Helper.Win32Unix, RoAssembly). Can I please get some tips on the best way to resolve these?

System\ComponentModel\Design\DesigntimeLicenseContext.cs(88,56): error CS0618: 'Assembly.EscapedCodeBase' is obsolete: 'EscapedCodeBase is only included for .NET Framework compatibility. Use Location instead.' [C:\Work\dotnetruntime\src\libraries\System.ComponentModel.TypeConverter\src\System.ComponentModel.TypeConverter.csproj]
System\ComponentModel\Design\DesigntimeLicenseContext.cs(111,45): error CS0618: 'Assembly.EscapedCodeBase' is obsolete: 'EscapedCodeBase is only included for .NET Framework compatibility. Use Location instead.' [C:\Work\dotnetruntime\src\libraries\System.ComponentModel.TypeConverter\src\System.ComponentModel.TypeConverter.csproj]
System\IO\IsolatedStorage\Helper.Win32Unix.cs(57,36): error CS0618: 'Assembly.CodeBase' is obsolete: 'CodeBase is only included for .NET Framework compatibility. Use Location instead.' [C:\Work\dotnetruntime\src\libraries\System.IO.IsolatedStorage\src\System.IO.IsolatedStorage.csproj]
System\Reflection\TypeLoading\Assemblies\RoAssembly.cs(44,39): error CS0672: Member 'RoAssembly.CodeBase' overrides obsolete member 'Assembly.CodeBase'. Add the Obsolete attribute to 'RoAssembly.CodeBase'. [C:\Work\dotnetruntime\src\libraries\System.Reflection.MetadataLoadContext\src\System.Reflection.MetadataLoadContext.csproj]
System\Reflection\TypeLoading\Assemblies\RoAssembly.cs(45,39): error CS0672: Member 'RoAssembly.EscapedCodeBase' overrides obsolete member 'Assembly.EscapedCodeBase'. Add the Obsolete attribute to 'RoAssembly.EscapedCodeBase'. [C:\Work\dotnetruntime\src\libraries\System.Reflection.MetadataLoadContext\src\System.Reflection.MetadataLoadContext.csproj]

Additionally, does this need something done per Better Obsoletions? How would I go about getting an aka.ms to put into list-of-obsoletions.md?

[jkotas]

System.ComponentModel.TypeConverter should be switched to use Assembly.Location. The rest should use #pragmas to disable the warnings.

does this need something done per Better Obsoletions?

@jeffhandley is working on the plan for better obsoletions IDs. You can leave the ID as TODO in the PR.

[JoshSchreuder]

Cheers @jkotas, I've raised #39261

[jkotas]

We should also mark it as [EditorBrowsable(Never)]

@terrajobst Is marking it as [EditorBrowsable(Never)] still valid with the new obsoletion plan? We are marking a lot of other methods obsolete, but we are not marking them as [EditorBrowsable(Never)]. What is special about this method that it deserves to be marked with [EditorBrowsable(Never)]?

cc @jeffhandley

[jeffhandley]

Per the non-goals of the better obsoletion spec, it is not expected that we'll mark [EditorBrowsable(Never)] in conjunction with the obsoletion but that they should be independent decisions. I took the approach to not apply the EditorBrowsable attribute in the big obsoletion PR.

I recommend we remove the EditorBrowable attributes from the obsoletions here too.

[JoshSchreuder]

Updated PR with the new Obsoletion stuff, and removed the EditorBrowsable attribute, thanks all!

[vitek-karas]

Should we also look into AssemblyName.CodeBase and AssemblyName.EscapedCodeBase - they are very similar to Assembly.CodeBase:

• Runtime fills in the same values for loaded assemblies
• When constructed manually the value is preserved
• Runtime never actually consumes these values - calling Assembly.Load(AssemblyName) completely ignores these properties on the name.

[danmoseley]

Was there an answer to this question?

Should we also look into AssemblyName.CodeBase and AssemblyName.EscapedCodeBase - they are very similar to Assembly.CodeBase:

[terrajobst]

Yes, @jkotas mentioned it above:

Assembly.EscapedCodeBase should be marked as obsolete as well.

And the PR marked it.

[vitek-karas]

What about the AssemblyName variants - they return basically the exact same thing... and AFAIK are not marked right now.