Open RikkiGibson opened 4 years ago
Because the constructor in use does not match any well-known signature, we should produce an obsolete warning with no custom message and a "warning" severity.
Due to outlined reason we should ignore the attribute. All well-known attributes are supposed to be ignored if we do not recognize constructor's signature
You're saying we should produce no diagnostic at all?
You're saying we should produce no diagnostic at all?
Yes, as if there was no attribute. We don't recognize constructor, we do not know what it means
@RikkiGibson how did you find this? By accident or because of my feature request? My feature deliberately didn't add any constructors to ObsoleteAttribute
, hence my question to avoid any confusion.
@terrajobst found in the course of testing the additions to ObsoleteAttribute. Suspected a bug when reviewing the existing code that gathers information from a usage of ObsoleteAttribute.
The compiler implementation of the new feature is still built around looking for well-known properties on the attribute, as was specified, not looking for any new well-known constructors.
The problem is probably not specific to the Obsolete attribute, likely it is affecting all well-known attributes. If I remember correctly, we had a stronger signature check at one point, but that probably got relaxed due to some circularity issue in some scenario for some attribute and we never got back to tighten the check or otherwise make the attribute cracking code robust to malformed declarations.
Another situation that we should handle more gracefully is when an enum type used as argument has incorrect underlying type. For example, this program causes a crash because underlying type of the enum in uint instead of int:
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Numerics;
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Text.RegularExpressions;
namespace Windows.Foundation.Metadata
{
[AttributeUsage(AttributeTargets.Delegate | AttributeTargets.Enum | AttributeTargets.Event | AttributeTargets.Field | AttributeTargets.Interface | AttributeTargets.Method | AttributeTargets.Property | AttributeTargets.Class | AttributeTargets.Struct, AllowMultiple = true)]
public sealed class DeprecatedAttribute: Attribute
{
public DeprecatedAttribute(string message, DeprecationType type, uint version){}
public DeprecatedAttribute(string message, DeprecationType type, uint version, Platform platform){}
public DeprecatedAttribute(string message, DeprecationType type, uint version, string contract){}
}
public enum Platform : uint
{
Windows = 0,
WindowsPhone = 0x1,
}
public enum DeprecationType : uint
{
Deprecate = 0,
Remove = 0x1,
}
}
[Obsolete]
class C{}
[Windows.Foundation.Metadata.Deprecated("Test message", Windows.Foundation.Metadata.DeprecationType.Deprecate, 2, Windows.Foundation.Metadata.Platform.Windows)]
class Class
{
}
public class Program
{
static void Main(string[] args)
{
var c = new C();
var cls = new Class();
}
}
Version Used: 441c154
Steps to Reproduce:
Expected Behavior: Because the constructor in use does not match any well-known signature, we should
produce an obsolete warning with no custom message and a "warning" severityproduce no diagnostic at all on usage ofC.M
.Actual Behavior: Crash:
We assume that an ObsoleteAttribute constructor with a sufficient number of parameters will have certain types for those parameters.
https://github.com/dotnet/roslyn/blob/1490d43ed98e45d67fbaa60b6a3f2f31cc01885e/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs#L291
/cc @AlekseyTs