From 883128f05f10f83842cc4566401be8844c5cdf53 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Tue, 17 Mar 2020 16:44:34 -0700 Subject: [PATCH 01/21] WIP for customizable obsolete diagnostics --- .../Errors/CustomObsoleteDiagnosticInfo.cs | 98 +++++++++++++ .../Symbols/ObsoleteAttributeHelpers.cs | 11 +- .../AttributeTests_WellKnownAttributes.cs | 131 ++++++++++++++++++ .../Portable/Diagnostic/DiagnosticInfo.cs | 4 +- .../Core/Portable/MetadataReader/PEModule.cs | 9 +- .../Symbols/Attributes/CommonAttributeData.cs | 22 ++- .../Attributes/ObsoleteAttributeData.cs | 16 ++- .../Diagnostics/DiagnosticDescription.cs | 5 +- 8 files changed, 274 insertions(+), 22 deletions(-) create mode 100644 src/Compilers/CSharp/Portable/Errors/CustomObsoleteDiagnosticInfo.cs diff --git a/src/Compilers/CSharp/Portable/Errors/CustomObsoleteDiagnosticInfo.cs b/src/Compilers/CSharp/Portable/Errors/CustomObsoleteDiagnosticInfo.cs new file mode 100644 index 0000000000000..064cb8e99bba1 --- /dev/null +++ b/src/Compilers/CSharp/Portable/Errors/CustomObsoleteDiagnosticInfo.cs @@ -0,0 +1,98 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +#nullable enable + +using System; + +namespace Microsoft.CodeAnalysis.CSharp.Errors +{ + internal class CustomObsoleteDiagnosticInfo : DiagnosticInfo + { + internal ObsoleteAttributeData Data { get; } + + internal CustomObsoleteDiagnosticInfo(Symbol obsoletedSymbol, ObsoleteAttributeData data, bool inCollectionInitializer) + : base(CSharp.MessageProvider.Instance, (int)GetErrorCode(data, inCollectionInitializer), arguments: GetArguments(obsoletedSymbol, data)) + { + Data = data; + } + + private static ErrorCode GetErrorCode(ObsoleteAttributeData data, bool inCollectionInitializer) + { + // dev11 had a bug in this area (i.e. always produce a warning when there's no message) and we have to match it. + if (data.Message is null) + { + return inCollectionInitializer ? ErrorCode.WRN_DeprecatedCollectionInitAdd : ErrorCode.WRN_DeprecatedSymbol; + } + + return (data.IsError, inCollectionInitializer) switch + { + (true, true) => ErrorCode.ERR_DeprecatedCollectionInitAddStr, + (true, false) => ErrorCode.ERR_DeprecatedSymbolStr, + (false, true) => ErrorCode.WRN_DeprecatedCollectionInitAddStr, + (false, false) => ErrorCode.WRN_DeprecatedSymbolStr + }; + } + + private static object[] GetArguments(Symbol obsoletedSymbol, ObsoleteAttributeData obsoleteAttributeData) + { + var message = obsoleteAttributeData.Message; + if (message is object) + { + return new object[] { obsoletedSymbol, message }; + } + else + { + return new object[] { obsoletedSymbol }; + } + } + + public override string MessageIdentifier + { + get + { + var id = Data.DiagnosticId; + if (!string.IsNullOrEmpty(id)) + { + return id; + } + + return base.MessageIdentifier; + } + } + + public override DiagnosticDescriptor Descriptor + { + get + { + var baseDescriptor = base.Descriptor; + var diagnosticId = Data.DiagnosticId; + var urlFormat = Data.UrlFormat; + if (diagnosticId is null && urlFormat is null) + { + return baseDescriptor; + } + + var id = MessageIdentifier; + var helpLinkUri = urlFormat is null + ? baseDescriptor.HelpLinkUri + : string.Format(urlFormat, id); + + // TODO: we expect some users to repeatedly use + // the same diagnostic IDs and url format values for many symbols. + // do we want to cache similar diagnostic descriptors? + return new DiagnosticDescriptor( + id: id, + title: baseDescriptor.Title, + messageFormat: baseDescriptor.MessageFormat, + category: baseDescriptor.Category, + defaultSeverity: baseDescriptor.DefaultSeverity, + isEnabledByDefault: baseDescriptor.IsEnabledByDefault, + description: baseDescriptor.Description, + helpLinkUri: helpLinkUri, + customTags: baseDescriptor.CustomTags.AsImmutable()); + } + } + } +} diff --git a/src/Compilers/CSharp/Portable/Symbols/ObsoleteAttributeHelpers.cs b/src/Compilers/CSharp/Portable/Symbols/ObsoleteAttributeHelpers.cs index 1c0283204cfb7..360966848bf63 100644 --- a/src/Compilers/CSharp/Portable/Symbols/ObsoleteAttributeHelpers.cs +++ b/src/Compilers/CSharp/Portable/Symbols/ObsoleteAttributeHelpers.cs @@ -5,6 +5,7 @@ using System.Diagnostics; using System.Reflection.Metadata; using System.Threading; +using Microsoft.CodeAnalysis.CSharp.Errors; using Microsoft.CodeAnalysis.CSharp.Symbols.Metadata.PE; namespace Microsoft.CodeAnalysis.CSharp.Symbols @@ -160,17 +161,11 @@ internal static DiagnosticInfo CreateObsoleteDiagnostic(Symbol symbol, BinderFla if (data.Message == null) { - // It seems like we should be able to assert that data.IsError is false, but we can't because dev11 had - // a bug in this area (i.e. always produce a warning when there's no message) and we have to match it. - // Debug.Assert(!data.IsError); - return new CSDiagnosticInfo(isColInit ? ErrorCode.WRN_DeprecatedCollectionInitAdd : ErrorCode.WRN_DeprecatedSymbol, symbol); + return new CustomObsoleteDiagnosticInfo(symbol, data, isColInit); } else { - ErrorCode errorCode = data.IsError - ? (isColInit ? ErrorCode.ERR_DeprecatedCollectionInitAddStr : ErrorCode.ERR_DeprecatedSymbolStr) - : (isColInit ? ErrorCode.WRN_DeprecatedCollectionInitAddStr : ErrorCode.WRN_DeprecatedSymbolStr); - return new CSDiagnosticInfo(errorCode, symbol, data.Message); + return new CustomObsoleteDiagnosticInfo(symbol, data, isColInit); } } } diff --git a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs index 7a56c6ec4b639..95db47186065d 100644 --- a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs +++ b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs @@ -8155,6 +8155,137 @@ static void Main(string[] args) Diagnostic(ErrorCode.WRN_DeprecatedSymbolStr, "c ?? 1").WithArguments("Convertible.implicit operator int(Convertible)", "To int")); } + const string ObsoleteAttributeSource = @" +#nullable enable +namespace System +{ + public class ObsoleteAttribute : Attribute + { + public ObsoleteAttribute() { } + public ObsoleteAttribute(string? message) { } + public ObsoleteAttribute(string? message, bool error) { } + + public string? DiagnosticId { get; set; } + public string? UrlFormat { get; set; } + } +} +"; + + [Fact, WorkItem(42119, "https://github.com/dotnet/roslyn/issues/42119")] + public void Obsolete_CustomDiagnosticId_01() + { + var source = @" +using System; +#pragma warning disable 436 + +class C +{ + [Obsolete(DiagnosticId = ""TEST1"")] + void M1() { } + + void M2() + { + M1(); // 1 + } +} +"; + + var comp = CreateCompilation(new[] { ObsoleteAttributeSource, source }); + var diags = comp.GetDiagnostics(); + + var diag = diags.Single(); + Assert.Equal("TEST1", diag.Id); + Assert.Equal(string.Empty, diag.Descriptor.HelpLinkUri); + + diags.Verify( + // (12,9): warning TEST1: 'C.M1()' is obsolete + // M1(); // 1 + Diagnostic("TEST1", "M1()").WithArguments("C.M1()").WithLocation(12, 9)); + } + + [Fact, WorkItem(42119, "https://github.com/dotnet/roslyn/issues/42119")] + public void Obsolete_CustomDiagnosticId_02() + { + var source = @" +using System; +#pragma warning disable 436 + +class C +{ + [Obsolete(""don't use"", error: false, DiagnosticId = ""TEST1"", UrlFormat = ""https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/compiler-messages/{0}"")] + void M1() { } + + void M2() + { + M1(); // 1 + } +} +"; + + var comp = CreateCompilation(new[] { ObsoleteAttributeSource, source }); + var diags = comp.GetDiagnostics(); + + var diag = diags.Single(); + Assert.Equal("TEST1", diag.Id); + Assert.Equal("https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/compiler-messages/TEST1", diag.Descriptor.HelpLinkUri); + + diags.Verify( + // (12,9): warning TEST1: 'C.M1()' is obsolete: 'don't use' + // M1(); // 1 + Diagnostic("TEST1", "M1()").WithArguments("C.M1()", "don't use").WithLocation(12, 9)); + } + + [Fact, WorkItem(42119, "https://github.com/dotnet/roslyn/issues/42119")] + public void Obsolete_CustomDiagnosticId_Suppression_01() + { + var source = @" +using System; +#pragma warning disable 436 + +class C +{ + [Obsolete(""don't use"", error: false, DiagnosticId = ""TEST1"")] + void M1() { } + + [Obsolete] + void M2() { } + + void M3() + { + M1(); // 1 + M2(); // 2 + +#pragma warning disable TEST1 + M1(); + M2(); // 3 +#pragma warning restore TEST1 + +#pragma warning disable 162 + M1(); // 4 + M2(); + } +} +"; + + var comp = CreateCompilation(new[] { ObsoleteAttributeSource, source }); + comp.VerifyDiagnostics( + // (15,9): warning TEST1: 'C.M1()' is obsolete: 'don't use' + // M1(); // 1 + Diagnostic("TEST1", "M1()").WithArguments("C.M1()", "don't use").WithLocation(15, 9), + // (16,9): warning CS0612: 'C.M2()' is obsolete + // M2(); // 2 + Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "M2()").WithArguments("C.M2()").WithLocation(16, 9), + // (20,9): warning CS0612: 'C.M2()' is obsolete + // M2(); // 3 + Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "M2()").WithArguments("C.M2()").WithLocation(20, 9), + // (24,9): warning TEST1: 'C.M1()' is obsolete: 'don't use' + // M1(); // 4 + Diagnostic("TEST1", "M1()").WithArguments("C.M1()", "don't use").WithLocation(24, 9), + // (25,9): warning CS0612: 'C.M2()' is obsolete + // M2(); + Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "M2()").WithArguments("C.M2()").WithLocation(25, 9)); + } + [Fact] [WorkItem(656345, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/656345")] public void ConditionalLazyObsoleteDiagnostic() diff --git a/src/Compilers/Core/Portable/Diagnostic/DiagnosticInfo.cs b/src/Compilers/Core/Portable/Diagnostic/DiagnosticInfo.cs index 4bbcd1155cc45..65f50d20a33a6 100644 --- a/src/Compilers/Core/Portable/Diagnostic/DiagnosticInfo.cs +++ b/src/Compilers/Core/Portable/Diagnostic/DiagnosticInfo.cs @@ -199,7 +199,7 @@ protected DiagnosticInfo(ObjectReader reader) /// public int Code { get { return _errorCode; } } - public DiagnosticDescriptor Descriptor + public virtual DiagnosticDescriptor Descriptor { get { @@ -314,7 +314,7 @@ public virtual IReadOnlyList AdditionalLocations /// Get the message id (for example "CS1001") for the message. This includes both the error number /// and a prefix identifying the source. /// - public string MessageIdentifier + public virtual string MessageIdentifier { get { diff --git a/src/Compilers/Core/Portable/MetadataReader/PEModule.cs b/src/Compilers/Core/Portable/MetadataReader/PEModule.cs index 19381da6ec0bf..2fa60f3c6824b 100644 --- a/src/Compilers/Core/Portable/MetadataReader/PEModule.cs +++ b/src/Compilers/Core/Portable/MetadataReader/PEModule.cs @@ -1320,14 +1320,14 @@ private ObsoleteAttributeData TryExtractObsoleteDataFromAttribute(AttributeInfo { case 0: // ObsoleteAttribute() - return new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, message: null, isError: false); + return new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, message: null, isError: false, diagnosticId: null, urlFormat: null); case 1: // ObsoleteAttribute(string) string message; if (TryExtractStringValueFromAttribute(attributeInfo.Handle, out message)) { - return new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, message, isError: false); + return new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, message, isError: false, diagnosticId: null, urlFormat: null); } return null; @@ -1629,7 +1629,8 @@ private static bool CrackObsoleteAttributeData(out ObsoleteAttributeData value, if (CrackStringInAttributeValue(out message, ref sig) && sig.RemainingBytes >= 1) { bool isError = sig.ReadBoolean(); - value = new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, message, isError); + // TODO: do we need to read in the diagnosticId/urlFormat when cracking the attribute? + value = new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, message, isError, diagnosticId: null, urlFormat: null); return true; } @@ -1642,7 +1643,7 @@ private static bool CrackDeprecatedAttributeData(out ObsoleteAttributeData value StringAndInt args; if (CrackStringAndIntInAttributeValue(out args, ref sig)) { - value = new ObsoleteAttributeData(ObsoleteAttributeKind.Deprecated, args.StringValue, args.IntValue == 1); + value = new ObsoleteAttributeData(ObsoleteAttributeKind.Deprecated, args.StringValue, args.IntValue == 1, diagnosticId: null, urlFormat: null); return true; } diff --git a/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs b/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs index bf7e205cb0722..40a78e74aad56 100644 --- a/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs +++ b/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs @@ -292,7 +292,25 @@ private ObsoleteAttributeData DecodeObsoleteAttribute() } } - return new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, message, isError); + TypedConstant diagnosticId = default; + TypedConstant urlFormat = default; + foreach (var pair in this.CommonNamedArguments) + { + switch (pair.Key) + { + case "DiagnosticId": + diagnosticId = pair.Value; + break; + case "UrlFormat": + urlFormat = pair.Value; + break; + default: + // unknown property specified on ObsoleteAttribute + break; + } + } + + return new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, message, isError, (string?)diagnosticId.ValueInternal, (string?)urlFormat.ValueInternal); } /// @@ -316,7 +334,7 @@ private ObsoleteAttributeData DecodeDeprecatedAttribute() isError = ((int)args[1].ValueInternal == 1); } - return new ObsoleteAttributeData(ObsoleteAttributeKind.Deprecated, message, isError); + return new ObsoleteAttributeData(ObsoleteAttributeKind.Deprecated, message, isError, diagnosticId: null, urlFormat: null); } /// diff --git a/src/Compilers/Core/Portable/Symbols/Attributes/ObsoleteAttributeData.cs b/src/Compilers/Core/Portable/Symbols/Attributes/ObsoleteAttributeData.cs index a535a587c202e..1063084e959aa 100644 --- a/src/Compilers/Core/Portable/Symbols/Attributes/ObsoleteAttributeData.cs +++ b/src/Compilers/Core/Portable/Symbols/Attributes/ObsoleteAttributeData.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +#nullable enable + using System; namespace Microsoft.CodeAnalysis @@ -20,14 +22,16 @@ internal enum ObsoleteAttributeKind /// internal sealed class ObsoleteAttributeData { - public static readonly ObsoleteAttributeData Uninitialized = new ObsoleteAttributeData(ObsoleteAttributeKind.Uninitialized, null, false); - public static readonly ObsoleteAttributeData Experimental = new ObsoleteAttributeData(ObsoleteAttributeKind.Experimental, null, false); + public static readonly ObsoleteAttributeData Uninitialized = new ObsoleteAttributeData(ObsoleteAttributeKind.Uninitialized, message: null, isError: false, diagnosticId: null, urlFormat: null); + public static readonly ObsoleteAttributeData Experimental = new ObsoleteAttributeData(ObsoleteAttributeKind.Experimental, message: null, isError: false, diagnosticId: null, urlFormat: null); - public ObsoleteAttributeData(ObsoleteAttributeKind kind, string message, bool isError) + public ObsoleteAttributeData(ObsoleteAttributeKind kind, string? message, bool isError, string? diagnosticId, string? urlFormat) { Kind = kind; Message = message; IsError = isError; + DiagnosticId = diagnosticId; + UrlFormat = urlFormat; } public readonly ObsoleteAttributeKind Kind; @@ -41,7 +45,11 @@ public ObsoleteAttributeData(ObsoleteAttributeKind kind, string message, bool is /// /// The message that will be shown when an error/warning is created for . /// - public readonly string Message; + public readonly string? Message; + + public readonly string? DiagnosticId; + + public readonly string? UrlFormat; internal bool IsUninitialized { diff --git a/src/Test/Utilities/Portable/Diagnostics/DiagnosticDescription.cs b/src/Test/Utilities/Portable/Diagnostics/DiagnosticDescription.cs index 1bd1d4181652d..043ef4c65e776 100644 --- a/src/Test/Utilities/Portable/Diagnostics/DiagnosticDescription.cs +++ b/src/Test/Utilities/Portable/Diagnostics/DiagnosticDescription.cs @@ -13,6 +13,7 @@ using Xunit; using Roslyn.Test.Utilities; using Microsoft.CodeAnalysis.PooledObjects; +using Microsoft.CodeAnalysis.CSharp; namespace Microsoft.CodeAnalysis.Test.Utilities { @@ -116,7 +117,7 @@ public DiagnosticDescription(Diagnostic d, bool errorCodeOnly, bool includeDefau _effectiveSeverityOpt = includeEffectiveSeverity ? d.Severity : (DiagnosticSeverity?)null; DiagnosticWithInfo dinfo = null; - if (d.Code == 0) + if (d.Code == 0 || !d.Id.StartsWith("CS")) { _code = d.Id; _errorCodeType = typeof(string); @@ -421,7 +422,7 @@ public static string GetAssertText(DiagnosticDescription[] expected, IEnumerable { const int CSharp = 1; const int VisualBasic = 2; - var language = actual.Any() && actual.First().Id.StartsWith("CS", StringComparison.Ordinal) ? CSharp : VisualBasic; + var language = actual.Any() && actual.First() is CSDiagnostic ? CSharp : VisualBasic; var includeDiagnosticMessagesAsComments = (language == CSharp); int indentDepth = (language == CSharp) ? 4 : 1; var includeDefaultSeverity = expected.Any() && expected.All(d => d.DefaultSeverity != null); From 1eb6df738d65a0d9472689f64afe63bde4b84cbd Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 18 Mar 2020 12:36:02 -0700 Subject: [PATCH 02/21] Add custom tag for custom Obsolete diagnostic --- .../Portable/Errors/CustomObsoleteDiagnosticInfo.cs | 11 ++++++++++- .../Portable/Diagnostic/WellKnownDiagnosticTags.cs | 6 ++++++ src/Compilers/Core/Portable/PublicAPI.Unshipped.txt | 3 ++- .../Portable/Diagnostics/DiagnosticDescription.cs | 2 +- 4 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Errors/CustomObsoleteDiagnosticInfo.cs b/src/Compilers/CSharp/Portable/Errors/CustomObsoleteDiagnosticInfo.cs index 064cb8e99bba1..e6cf6e85259da 100644 --- a/src/Compilers/CSharp/Portable/Errors/CustomObsoleteDiagnosticInfo.cs +++ b/src/Compilers/CSharp/Portable/Errors/CustomObsoleteDiagnosticInfo.cs @@ -5,6 +5,8 @@ #nullable enable using System; +using System.Collections.Immutable; +using Microsoft.CodeAnalysis.PooledObjects; namespace Microsoft.CodeAnalysis.CSharp.Errors { @@ -79,6 +81,13 @@ public override DiagnosticDescriptor Descriptor ? baseDescriptor.HelpLinkUri : string.Format(urlFormat, id); + var customTags = ArrayBuilder.GetInstance(); + foreach (var tag in baseDescriptor.CustomTags) + { + customTags.Add(tag); + } + customTags.Add(WellKnownDiagnosticTags.CustomObsolete); + // TODO: we expect some users to repeatedly use // the same diagnostic IDs and url format values for many symbols. // do we want to cache similar diagnostic descriptors? @@ -91,7 +100,7 @@ public override DiagnosticDescriptor Descriptor isEnabledByDefault: baseDescriptor.IsEnabledByDefault, description: baseDescriptor.Description, helpLinkUri: helpLinkUri, - customTags: baseDescriptor.CustomTags.AsImmutable()); + customTags: customTags.ToImmutableAndFree()); } } } diff --git a/src/Compilers/Core/Portable/Diagnostic/WellKnownDiagnosticTags.cs b/src/Compilers/Core/Portable/Diagnostic/WellKnownDiagnosticTags.cs index 4bd3cb530c77e..59c01c0db909d 100644 --- a/src/Compilers/Core/Portable/Diagnostic/WellKnownDiagnosticTags.cs +++ b/src/Compilers/Core/Portable/Diagnostic/WellKnownDiagnosticTags.cs @@ -44,5 +44,11 @@ public static class WellKnownDiagnosticTags /// Indicates that the diagnostic is related to an exception thrown by a . /// public const string AnalyzerException = nameof(AnalyzerException); + + /// + /// Indicates that the diagnostic is an obsolete diagnostic with a custom ID or URL + /// specified by the 'DiagnosticId' or 'UrlFormat' properties on 'ObsoleteAttribute'. + /// + public const string CustomObsolete = nameof(CustomObsolete); } } diff --git a/src/Compilers/Core/Portable/PublicAPI.Unshipped.txt b/src/Compilers/Core/Portable/PublicAPI.Unshipped.txt index 1d57d1cc953f8..9eba7331187f2 100644 --- a/src/Compilers/Core/Portable/PublicAPI.Unshipped.txt +++ b/src/Compilers/Core/Portable/PublicAPI.Unshipped.txt @@ -1,2 +1,3 @@ Microsoft.CodeAnalysis.CommandLineSourceFile.CommandLineSourceFile(string path, bool isScript, bool isInputRedirected) -> void -Microsoft.CodeAnalysis.CommandLineSourceFile.IsInputRedirected.get -> bool \ No newline at end of file +Microsoft.CodeAnalysis.CommandLineSourceFile.IsInputRedirected.get -> bool +const Microsoft.CodeAnalysis.WellKnownDiagnosticTags.CustomObsolete = "CustomObsolete" -> string diff --git a/src/Test/Utilities/Portable/Diagnostics/DiagnosticDescription.cs b/src/Test/Utilities/Portable/Diagnostics/DiagnosticDescription.cs index 043ef4c65e776..cd3b226716174 100644 --- a/src/Test/Utilities/Portable/Diagnostics/DiagnosticDescription.cs +++ b/src/Test/Utilities/Portable/Diagnostics/DiagnosticDescription.cs @@ -117,7 +117,7 @@ public DiagnosticDescription(Diagnostic d, bool errorCodeOnly, bool includeDefau _effectiveSeverityOpt = includeEffectiveSeverity ? d.Severity : (DiagnosticSeverity?)null; DiagnosticWithInfo dinfo = null; - if (d.Code == 0 || !d.Id.StartsWith("CS")) + if (d.Code == 0 || d.Descriptor.CustomTags.Contains(WellKnownDiagnosticTags.CustomObsolete)) { _code = d.Id; _errorCodeType = typeof(string); From 530691759bad361f529a398b0c471711c308a32d Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 18 Mar 2020 15:06:39 -0700 Subject: [PATCH 03/21] Test some more source scenarios --- .../Errors/CustomObsoleteDiagnosticInfo.cs | 22 ++- .../AttributeTests_WellKnownAttributes.cs | 162 +++++++++++++++++- .../Diagnostics/DiagnosticDescription.cs | 6 +- 3 files changed, 176 insertions(+), 14 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Errors/CustomObsoleteDiagnosticInfo.cs b/src/Compilers/CSharp/Portable/Errors/CustomObsoleteDiagnosticInfo.cs index e6cf6e85259da..47d751efb9b7d 100644 --- a/src/Compilers/CSharp/Portable/Errors/CustomObsoleteDiagnosticInfo.cs +++ b/src/Compilers/CSharp/Portable/Errors/CustomObsoleteDiagnosticInfo.cs @@ -4,13 +4,11 @@ #nullable enable -using System; -using System.Collections.Immutable; using Microsoft.CodeAnalysis.PooledObjects; namespace Microsoft.CodeAnalysis.CSharp.Errors { - internal class CustomObsoleteDiagnosticInfo : DiagnosticInfo + internal sealed class CustomObsoleteDiagnosticInfo : DiagnosticInfo { internal ObsoleteAttributeData Data { get; } @@ -77,9 +75,21 @@ public override DiagnosticDescriptor Descriptor } var id = MessageIdentifier; - var helpLinkUri = urlFormat is null - ? baseDescriptor.HelpLinkUri - : string.Format(urlFormat, id); + var helpLinkUri = baseDescriptor.HelpLinkUri; + + if (urlFormat is object) + { + try + { + helpLinkUri = string.Format(urlFormat, id); + } + catch + { + // TODO: should we report a meta-diagnostic of some kind when the string.Format fails? + // also, should we do some validation of the 'UrlFormat' values provided in source to prevent people from shipping + // obsoleted symbols with malformed 'UrlFormat' values? + } + } var customTags = ArrayBuilder.GetInstance(); foreach (var tag in baseDescriptor.CustomTags) diff --git a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs index 95db47186065d..9326defb15f82 100644 --- a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs +++ b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs @@ -8195,6 +8195,7 @@ void M2() var diag = diags.Single(); Assert.Equal("TEST1", diag.Id); + Assert.Equal(ErrorCode.WRN_DeprecatedSymbol, (ErrorCode)diag.Code); Assert.Equal(string.Empty, diag.Descriptor.HelpLinkUri); diags.Verify( @@ -8210,6 +8211,99 @@ public void Obsolete_CustomDiagnosticId_02() using System; #pragma warning disable 436 +class C +{ + [Obsolete(UrlFormat = ""https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/compiler-messages/{0}"")] + void M1() { } + + void M2() + { + M1(); // 1 + } +} +"; + + var comp = CreateCompilation(new[] { ObsoleteAttributeSource, source }); + var diags = comp.GetDiagnostics(); + + var diag = diags.Single(); + Assert.Equal("https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/compiler-messages/CS0612", diag.Descriptor.HelpLinkUri); + + diags.Verify( + // (12,9): warning CS0612: 'C.M1()' is obsolete + // M1(); // 1 + Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "M1()").WithArguments("C.M1()").WithLocation(12, 9)); + } + + [Fact, WorkItem(42119, "https://github.com/dotnet/roslyn/issues/42119")] + public void Obsolete_CustomDiagnosticId_03() + { + var source = @" +using System; +#pragma warning disable 436 + +class C +{ + [Obsolete(UrlFormat = ""https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/compiler-messages/{0}/{1}"")] + void M1() { } + + void M2() + { + M1(); // 1 + } +} +"; + + var comp = CreateCompilation(new[] { ObsoleteAttributeSource, source }); + var diags = comp.GetDiagnostics(); + + var diag = diags.Single(); + Assert.Equal("", diag.Descriptor.HelpLinkUri); + + diags.Verify( + // (12,9): warning CS0612: 'C.M1()' is obsolete + // M1(); // 1 + Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "M1()").WithArguments("C.M1()").WithLocation(12, 9)); + } + + [Fact, WorkItem(42119, "https://github.com/dotnet/roslyn/issues/42119")] + public void Obsolete_CustomDiagnosticId_04() + { + var source = @" +using System; +#pragma warning disable 436 + +class C +{ + [Obsolete(UrlFormat = ""https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/compiler-messages/CS0612"")] + void M1() { } + + void M2() + { + M1(); // 1 + } +} +"; + + var comp = CreateCompilation(new[] { ObsoleteAttributeSource, source }); + var diags = comp.GetDiagnostics(); + + var diag = diags.Single(); + Assert.Equal("https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/compiler-messages/CS0612", diag.Descriptor.HelpLinkUri); + + diags.Verify( + // (12,9): warning CS0612: 'C.M1()' is obsolete + // M1(); // 1 + Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "M1()").WithArguments("C.M1()").WithLocation(12, 9)); + } + + [Fact, WorkItem(42119, "https://github.com/dotnet/roslyn/issues/42119")] + public void Obsolete_CustomDiagnosticId_99() + { + var source = @" +using System; +#pragma warning disable 436 + class C { [Obsolete(""don't use"", error: false, DiagnosticId = ""TEST1"", UrlFormat = ""https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/compiler-messages/{0}"")] @@ -8227,6 +8321,7 @@ void M2() var diag = diags.Single(); Assert.Equal("TEST1", diag.Id); + Assert.Equal(ErrorCode.WRN_DeprecatedSymbolStr, (ErrorCode)diag.Code); Assert.Equal("https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/compiler-messages/TEST1", diag.Descriptor.HelpLinkUri); diags.Verify( @@ -8260,7 +8355,7 @@ void M3() M2(); // 3 #pragma warning restore TEST1 -#pragma warning disable 162 +#pragma warning disable 612 M1(); // 4 M2(); } @@ -8280,10 +8375,67 @@ void M3() Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "M2()").WithArguments("C.M2()").WithLocation(20, 9), // (24,9): warning TEST1: 'C.M1()' is obsolete: 'don't use' // M1(); // 4 - Diagnostic("TEST1", "M1()").WithArguments("C.M1()", "don't use").WithLocation(24, 9), - // (25,9): warning CS0612: 'C.M2()' is obsolete - // M2(); - Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "M2()").WithArguments("C.M2()").WithLocation(25, 9)); + Diagnostic("TEST1", "M1()").WithArguments("C.M1()", "don't use").WithLocation(24, 9)); + } + + [Fact, WorkItem(42119, "https://github.com/dotnet/roslyn/issues/42119")] + public void Obsolete_CustomDiagnosticId_FromMetadata_01() + { + var source1 = @" +using System; +#pragma warning disable 436 + +public class C1 +{ + [Obsolete(""don't use"", error: false, DiagnosticId = ""TEST1"")] + public void M1() { } + + [Obsolete] + public void M2() { } +} +"; + + var source2 = @" +class C2 : C1 +{ + void M3() + { + M1(); // 1 + M2(); // 2 + +#pragma warning disable TEST1 + M1(); + M2(); // 3 +#pragma warning restore TEST1 + +#pragma warning disable 612 + M1(); // 4 + M2(); + } +}"; + var comp1 = CreateCompilation(new[] { ObsoleteAttributeSource, source1 }); + verify(comp1.ToMetadataReference()); + + // FIXME + //verify(comp1.EmitToImageReference()); + + void verify(MetadataReference reference) + { + var comp2 = CreateCompilation(source2, references: new[] { reference }); + comp2.VerifyDiagnostics( + // (6,9): warning TEST1: 'C1.M1()' is obsolete: 'don't use' + // M1(); // 1 + Diagnostic("TEST1", "M1()").WithArguments("C1.M1()", "don't use").WithLocation(6, 9), + // (7,9): warning CS0612: 'C1.M2()' is obsolete + // M2(); // 2 + Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "M2()").WithArguments("C1.M2()").WithLocation(7, 9), + // (11,9): warning CS0612: 'C1.M2()' is obsolete + // M2(); // 3 + Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "M2()").WithArguments("C1.M2()").WithLocation(11, 9), + // (15,9): warning TEST1: 'C1.M1()' is obsolete: 'don't use' + // M1(); // 4 + Diagnostic("TEST1", "M1()").WithArguments("C1.M1()", "don't use").WithLocation(15, 9)); + } } [Fact] diff --git a/src/Test/Utilities/Portable/Diagnostics/DiagnosticDescription.cs b/src/Test/Utilities/Portable/Diagnostics/DiagnosticDescription.cs index cd3b226716174..0af28b8b96cb0 100644 --- a/src/Test/Utilities/Portable/Diagnostics/DiagnosticDescription.cs +++ b/src/Test/Utilities/Portable/Diagnostics/DiagnosticDescription.cs @@ -14,6 +14,7 @@ using Roslyn.Test.Utilities; using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Errors; namespace Microsoft.CodeAnalysis.Test.Utilities { @@ -116,15 +117,14 @@ public DiagnosticDescription(Diagnostic d, bool errorCodeOnly, bool includeDefau _defaultSeverityOpt = includeDefaultSeverity ? d.DefaultSeverity : (DiagnosticSeverity?)null; _effectiveSeverityOpt = includeEffectiveSeverity ? d.Severity : (DiagnosticSeverity?)null; - DiagnosticWithInfo dinfo = null; - if (d.Code == 0 || d.Descriptor.CustomTags.Contains(WellKnownDiagnosticTags.CustomObsolete)) + DiagnosticWithInfo dinfo = d as DiagnosticWithInfo; + if (d.Code == 0 || dinfo?.Info is CustomObsoleteDiagnosticInfo { Data: { DiagnosticId: object _ } }) { _code = d.Id; _errorCodeType = typeof(string); } else { - dinfo = d as DiagnosticWithInfo; if (dinfo == null) { _code = d.Code; From 0872c6eb8750ef29fe7586e976a3ad819b1db7af Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 18 Mar 2020 17:07:17 -0700 Subject: [PATCH 04/21] Read some obsolete properties from metadata --- .../AttributeTests_WellKnownAttributes.cs | 142 +++++++++++++++++- .../Core/Portable/MetadataReader/PEModule.cs | 116 +++++++++++--- 2 files changed, 237 insertions(+), 21 deletions(-) diff --git a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs index 9326defb15f82..3535e46c0d578 100644 --- a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs +++ b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs @@ -8415,9 +8415,7 @@ void M3() }"; var comp1 = CreateCompilation(new[] { ObsoleteAttributeSource, source1 }); verify(comp1.ToMetadataReference()); - - // FIXME - //verify(comp1.EmitToImageReference()); + verify(comp1.EmitToImageReference()); void verify(MetadataReference reference) { @@ -8438,6 +8436,144 @@ void verify(MetadataReference reference) } } + [Fact, WorkItem(42119, "https://github.com/dotnet/roslyn/issues/42119")] + public void Obsolete_CustomDiagnosticId_FromMetadata_02() + { + var source1 = @" +using System; +#pragma warning disable 436 + +public class C1 +{ + [Obsolete(DiagnosticId = ""TEST1"")] + public void M1() { } + + [Obsolete(""don't use"", DiagnosticId = ""TEST2"")] + public void M2() { } + + [Obsolete(""don't use"", error: false, DiagnosticId = ""TEST3"")] + public void M3() { } +} +"; + + var source2 = @" +class C2 : C1 +{ + void M4() + { + M1(); // 1 + M2(); // 2 + M3(); // 3 + } +}"; + var comp1 = CreateCompilation(new[] { ObsoleteAttributeSource, source1 }); + verify(comp1.ToMetadataReference()); + verify(comp1.EmitToImageReference()); + + void verify(MetadataReference reference) + { + var comp2 = CreateCompilation(source2, references: new[] { reference }); + comp2.VerifyDiagnostics( + // (6,9): warning TEST1: 'C1.M1()' is obsolete + // M1(); // 1 + Diagnostic("TEST1", "M1()").WithArguments("C1.M1()").WithLocation(6, 9), + // (7,9): warning TEST2: 'C1.M2()' is obsolete: 'don't use' + // M2(); // 2 + Diagnostic("TEST2", "M2()").WithArguments("C1.M2()", "don't use").WithLocation(7, 9), + // (8,9): warning TEST3: 'C1.M3()' is obsolete: 'don't use' + // M3(); // 3 + Diagnostic("TEST3", "M3()").WithArguments("C1.M3()", "don't use").WithLocation(8, 9)); + } + } + + [Fact, WorkItem(42119, "https://github.com/dotnet/roslyn/issues/42119")] + public void Obsolete_CustomDiagnosticId_FromMetadata_03() + { + var source1 = @" +using System; +#pragma warning disable 436 + +public class C1 +{ + [Obsolete(DiagnosticId = ""TEST1"", UrlFormat = ""https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/compiler-messages/{0}"")] + public void M1() { } +} +"; + + var source2 = @" +class C2 : C1 +{ + void M2() + { + M1(); // 1 + } +}"; + var comp1 = CreateCompilation(new[] { ObsoleteAttributeSource, source1 }); + verify(comp1.ToMetadataReference()); + verify(comp1.EmitToImageReference()); + + void verify(MetadataReference reference) + { + var comp2 = CreateCompilation(source2, references: new[] { reference }); + var diags = comp2.GetDiagnostics(); + + var diag = diags.Single(); + Assert.Equal("https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/compiler-messages/TEST1", diag.Descriptor.HelpLinkUri); + + diags.Verify( + // (6,9): warning TEST1: 'C1.M1()' is obsolete + // M1(); // 1 + Diagnostic("TEST1", "M1()").WithArguments("C1.M1()").WithLocation(6, 9)); + } + } + + [Fact(Skip = "TODO: make behavior consistent between source and metadata"), WorkItem(42119, "https://github.com/dotnet/roslyn/issues/42119")] + public void Obsolete_CustomDiagnosticId_BadMetadata_01() + { + var source1 = @" +using System; +#pragma warning disable 436 + +namespace System +{ + public class ObsoleteAttribute + { + public bool Flag { get; set; } + public string DiagnosticId { get; set; } + } +} + +public class C1 +{ + [Obsolete(Flag = false, DiagnosticId = ""TEST1"")] + public void M1() { } +} +"; + + var source2 = @" +class C2 : C1 +{ + void M2() + { + M1(); // 1 + } +}"; + var comp1 = CreateCompilation(source1); + verify(comp1.ToMetadataReference()); + verify(comp1.EmitToImageReference()); + + void verify(MetadataReference reference) + { + var comp2 = CreateCompilation(source2, references: new[] { reference }); + + // TODO: we could try to skip over properties of types that we don't recognize, but it only applies for bad user-defined Obsolete attributes. + comp2.VerifyDiagnostics( + // (6,9): warning TEST1: 'C1.M1()' is obsolete + // M1(); // 1 + Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "M1()").WithArguments("C1.M1()").WithLocation(6, 9)); + } + } + [Fact] [WorkItem(656345, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/656345")] public void ConditionalLazyObsoleteDiagnostic() diff --git a/src/Compilers/Core/Portable/MetadataReader/PEModule.cs b/src/Compilers/Core/Portable/MetadataReader/PEModule.cs index 2fa60f3c6824b..d367e1d99396e 100644 --- a/src/Compilers/Core/Portable/MetadataReader/PEModule.cs +++ b/src/Compilers/Core/Portable/MetadataReader/PEModule.cs @@ -7,6 +7,7 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Reflection; using System.Reflection.Metadata; @@ -94,6 +95,8 @@ internal sealed class PEModule : IDisposable private static readonly AttributeValueExtractor> s_attributeByteArrayValueExtractor = CrackByteArrayInAttributeValue; private static readonly AttributeValueExtractor> s_attributeStringArrayValueExtractor = CrackStringArrayInAttributeValue; private static readonly AttributeValueExtractor s_attributeObsoleteDataExtractor = CrackObsoleteAttributeData; + private static readonly AttributeValueExtractor s_attributeObsoleteDataExtractorString = CrackObsoleteAttributeDataString; + private static readonly AttributeValueExtractor s_attributeObsoleteDataExtractorStringBool = CrackObsoleteAttributeDataStringBool; private static readonly AttributeValueExtractor s_attributeDeprecatedDataExtractor = CrackDeprecatedAttributeData; private static readonly AttributeValueExtractor s_attributeBoolAndStringArrayValueExtractor = CrackBoolAndStringArrayInAttributeValue; @@ -1312,36 +1315,31 @@ private ArrayBuilder ExtractStringValuesFromAttributes(List(CustomAttributeHandle handle, out T // TODO: error checking offset in range BlobReader reader = MetadataReader.GetBlobReader(valueBlob); - if (reader.Length > 4) + if (reader.Length >= 4) { // check prolog if (reader.ReadByte() == 1 && reader.ReadByte() == 0) @@ -1623,14 +1621,46 @@ internal bool IsNoPiaLocalType( } } +#nullable enable + /// + /// Cracks the data on an Obsolete attribute using the parameterless constructor. + /// private static bool CrackObsoleteAttributeData(out ObsoleteAttributeData value, ref BlobReader sig) + { + CrackObsoleteProperties(out var diagnosticId, out var urlFormat, ref sig); + value = new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, message: null, isError: false, diagnosticId: diagnosticId, urlFormat: urlFormat); + return true; + } + + /// + /// Cracks the data on an Obsolete attribute using the Obsolete(string) constructor. + /// + private static bool CrackObsoleteAttributeDataString(out ObsoleteAttributeData? value, ref BlobReader sig) + { + string message; + if (CrackStringInAttributeValue(out message, ref sig)) + { + CrackObsoleteProperties(out var diagnosticId, out var urlFormat, ref sig); + value = new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, message, isError: false, diagnosticId: diagnosticId, urlFormat: urlFormat); + return true; + } + + value = null; + return false; + } + + /// + /// Cracks the data on an Obsolete attribute using the Obsolete(string, bool) constructor. + /// + private static bool CrackObsoleteAttributeDataStringBool(out ObsoleteAttributeData? value, ref BlobReader sig) { string message; if (CrackStringInAttributeValue(out message, ref sig) && sig.RemainingBytes >= 1) { bool isError = sig.ReadBoolean(); - // TODO: do we need to read in the diagnosticId/urlFormat when cracking the attribute? - value = new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, message, isError, diagnosticId: null, urlFormat: null); + + CrackObsoleteProperties(out var diagnosticId, out var urlFormat, ref sig); + value = new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, message, isError, diagnosticId: diagnosticId, urlFormat: urlFormat); return true; } @@ -1638,6 +1668,56 @@ private static bool CrackObsoleteAttributeData(out ObsoleteAttributeData value, return false; } + private static void CrackObsoleteProperties(out string? diagnosticId, out string? urlFormat, ref BlobReader sig) + { + diagnosticId = null; + urlFormat = null; + + // See CIL spec section II.23.3 Custom attributes + + // Next is a description of the optional “named” fields and properties. + // This starts with NumNamed– an unsigned int16 giving the number of “named” properties or fields that follow. + var numNamed = sig.ReadUInt16(); + for (int i = 0; i < numNamed; i++) + { + // A NamedArg is simply a FixedArg preceded by information to identify which field or property it represents. + // PROPERTY is the single byte 0x54. + const int PROPERTY = 0x54; + var fieldOrProperty = sig.ReadByte(); + if (fieldOrProperty != PROPERTY) + { + return; + } + + const int ELEMENT_TYPE_STRING = 0x0e; + var fieldOrPropType = sig.ReadByte(); + if (fieldOrPropType != ELEMENT_TYPE_STRING) + { + return; + } + + // The FieldOrPropName is the name of the field or property, stored as a SerString. + if (!CrackStringInAttributeValue(out string fieldOrPropName, ref sig) || + !CrackStringInAttributeValue(out string fixedArg, ref sig)) + { + return; + } + + switch (fieldOrPropName) + { + case "DiagnosticId": + diagnosticId = fixedArg; + break; + case "UrlFormat": + urlFormat = fixedArg; + break; + } + + // TODO: do we want to tolerate custom definitions of ObsoleteAttribute with different properties types besides string? + } + } +#nullable restore + private static bool CrackDeprecatedAttributeData(out ObsoleteAttributeData value, ref BlobReader sig) { StringAndInt args; From 30bd18287559ef93886f9cbc6aa0b83eb0dc0894 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Thu, 19 Mar 2020 17:53:52 -0700 Subject: [PATCH 05/21] Address some feedback --- .../Errors/CustomObsoleteDiagnosticInfo.cs | 98 ++++--- .../Symbols/ObsoleteAttributeHelpers.cs | 10 +- .../AttributeTests_WellKnownAttributes.cs | 100 ++++++- .../Core/Portable/MetadataReader/PEModule.cs | 261 +++++++++++++++--- .../Symbols/Attributes/CommonAttributeData.cs | 39 ++- .../Attributes/ObsoleteAttributeData.cs | 20 ++ .../Diagnostics/DiagnosticDescription.cs | 5 +- 7 files changed, 430 insertions(+), 103 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Errors/CustomObsoleteDiagnosticInfo.cs b/src/Compilers/CSharp/Portable/Errors/CustomObsoleteDiagnosticInfo.cs index 47d751efb9b7d..c773968ce25c9 100644 --- a/src/Compilers/CSharp/Portable/Errors/CustomObsoleteDiagnosticInfo.cs +++ b/src/Compilers/CSharp/Portable/Errors/CustomObsoleteDiagnosticInfo.cs @@ -4,6 +4,10 @@ #nullable enable +using System.Collections; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Threading; using Microsoft.CodeAnalysis.PooledObjects; namespace Microsoft.CodeAnalysis.CSharp.Errors @@ -62,56 +66,78 @@ public override string MessageIdentifier } } + private DiagnosticDescriptor? _descriptor; public override DiagnosticDescriptor Descriptor { get { - var baseDescriptor = base.Descriptor; - var diagnosticId = Data.DiagnosticId; - var urlFormat = Data.UrlFormat; - if (diagnosticId is null && urlFormat is null) + if (_descriptor == null) { - return baseDescriptor; + Interlocked.CompareExchange(ref _descriptor, CreateDescriptor(), null); } - var id = MessageIdentifier; - var helpLinkUri = baseDescriptor.HelpLinkUri; + return _descriptor; + } + } + + private DiagnosticDescriptor CreateDescriptor() + { + var baseDescriptor = base.Descriptor; + var diagnosticId = Data.DiagnosticId; + var urlFormat = Data.UrlFormat; + if (diagnosticId is null && urlFormat is null) + { + return baseDescriptor; + } - if (urlFormat is object) + var id = MessageIdentifier; + var helpLinkUri = baseDescriptor.HelpLinkUri; + + if (urlFormat is object) + { + try { - try - { - helpLinkUri = string.Format(urlFormat, id); - } - catch - { - // TODO: should we report a meta-diagnostic of some kind when the string.Format fails? - // also, should we do some validation of the 'UrlFormat' values provided in source to prevent people from shipping - // obsoleted symbols with malformed 'UrlFormat' values? - } + helpLinkUri = string.Format(urlFormat, id); } + catch + { + // TODO: should we report a meta-diagnostic of some kind when the string.Format fails? + // also, should we do some validation of the 'UrlFormat' values provided in source to prevent people from shipping + // obsoleted symbols with malformed 'UrlFormat' values? + } + } - var customTags = ArrayBuilder.GetInstance(); - foreach (var tag in baseDescriptor.CustomTags) + ImmutableArray customTags; + if (diagnosticId is null) + { + customTags = baseDescriptor.CustomTags.ToImmutableArray(); + } + else + { + var capacity = 1; + if (baseDescriptor.CustomTags is ICollection { Count: int count }) { - customTags.Add(tag); + capacity += count; } - customTags.Add(WellKnownDiagnosticTags.CustomObsolete); - - // TODO: we expect some users to repeatedly use - // the same diagnostic IDs and url format values for many symbols. - // do we want to cache similar diagnostic descriptors? - return new DiagnosticDescriptor( - id: id, - title: baseDescriptor.Title, - messageFormat: baseDescriptor.MessageFormat, - category: baseDescriptor.Category, - defaultSeverity: baseDescriptor.DefaultSeverity, - isEnabledByDefault: baseDescriptor.IsEnabledByDefault, - description: baseDescriptor.Description, - helpLinkUri: helpLinkUri, - customTags: customTags.ToImmutableAndFree()); + var tagsBuilder = ArrayBuilder.GetInstance(capacity); + tagsBuilder.AddRange(baseDescriptor.CustomTags); + tagsBuilder.Add(WellKnownDiagnosticTags.CustomObsolete); + customTags = tagsBuilder.ToImmutableAndFree(); } + + // TODO: we expect some users to repeatedly use + // the same diagnostic IDs and url format values for many symbols. + // do we want to cache similar diagnostic descriptors? + return new DiagnosticDescriptor( + id: id, + title: baseDescriptor.Title, + messageFormat: baseDescriptor.MessageFormat, + category: baseDescriptor.Category, + defaultSeverity: baseDescriptor.DefaultSeverity, + isEnabledByDefault: baseDescriptor.IsEnabledByDefault, + description: baseDescriptor.Description, + helpLinkUri: helpLinkUri, + customTags: customTags); } } } diff --git a/src/Compilers/CSharp/Portable/Symbols/ObsoleteAttributeHelpers.cs b/src/Compilers/CSharp/Portable/Symbols/ObsoleteAttributeHelpers.cs index 360966848bf63..4ade5d40c8a0e 100644 --- a/src/Compilers/CSharp/Portable/Symbols/ObsoleteAttributeHelpers.cs +++ b/src/Compilers/CSharp/Portable/Symbols/ObsoleteAttributeHelpers.cs @@ -158,15 +158,7 @@ internal static DiagnosticInfo CreateObsoleteDiagnostic(Symbol symbol, BinderFla // Issue a specialized diagnostic for add methods of collection initializers bool isColInit = location.Includes(BinderFlags.CollectionInitializerAddMethod); - - if (data.Message == null) - { - return new CustomObsoleteDiagnosticInfo(symbol, data, isColInit); - } - else - { - return new CustomObsoleteDiagnosticInfo(symbol, data, isColInit); - } + return new CustomObsoleteDiagnosticInfo(symbol, data, isColInit); } } } diff --git a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs index 3535e46c0d578..9de78b10af3e7 100644 --- a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs +++ b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs @@ -8297,6 +8297,46 @@ void M2() Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "M1()").WithArguments("C.M1()").WithLocation(12, 9)); } + [Fact, WorkItem(42119, "https://github.com/dotnet/roslyn/issues/42119")] + public void Obsolete_CustomDiagnosticId_BadAttribute_01() + { + var source = @" +using System; +#pragma warning disable 436 + +namespace System +{ + public class ObsoleteAttribute : Attribute + { + public string DiagnosticId; + public string DiagnosticId { get; set; } // 1 + } +} + +class C +{ + [Obsolete(DiagnosticId = ""TEST1"")] // 2 + void M1() { } + + void M2() + { + M1(); + } +} +"; + + var comp = CreateCompilation(source); + var diags = comp.GetDiagnostics(); + + diags.Verify( + // (10,23): error CS0102: The type 'ObsoleteAttribute' already contains a definition for 'DiagnosticId' + // public string DiagnosticId { get; set; } // 1 + Diagnostic(ErrorCode.ERR_DuplicateNameInClass, "DiagnosticId").WithArguments("System.ObsoleteAttribute", "DiagnosticId").WithLocation(10, 23), + // (16,15): error CS0229: Ambiguity between 'ObsoleteAttribute.DiagnosticId' and 'ObsoleteAttribute.DiagnosticId' + // [Obsolete(DiagnosticId = "TEST1")] // 2 + Diagnostic(ErrorCode.ERR_AmbigMember, "DiagnosticId").WithArguments("System.ObsoleteAttribute.DiagnosticId", "System.ObsoleteAttribute.DiagnosticId").WithLocation(16, 15)); + } + [Fact, WorkItem(42119, "https://github.com/dotnet/roslyn/issues/42119")] public void Obsolete_CustomDiagnosticId_99() { @@ -8414,6 +8454,8 @@ void M3() } }"; var comp1 = CreateCompilation(new[] { ObsoleteAttributeSource, source1 }); + comp1.VerifyDiagnostics(); + verify(comp1.ToMetadataReference()); verify(comp1.EmitToImageReference()); @@ -8467,6 +8509,8 @@ void M4() } }"; var comp1 = CreateCompilation(new[] { ObsoleteAttributeSource, source1 }); + comp1.VerifyDiagnostics(); + verify(comp1.ToMetadataReference()); verify(comp1.EmitToImageReference()); @@ -8509,6 +8553,8 @@ void M2() } }"; var comp1 = CreateCompilation(new[] { ObsoleteAttributeSource, source1 }); + comp1.VerifyDiagnostics(); + verify(comp1.ToMetadataReference()); verify(comp1.EmitToImageReference()); @@ -8527,7 +8573,7 @@ void verify(MetadataReference reference) } } - [Fact(Skip = "TODO: make behavior consistent between source and metadata"), WorkItem(42119, "https://github.com/dotnet/roslyn/issues/42119")] + [Fact, WorkItem(42119, "https://github.com/dotnet/roslyn/issues/42119")] public void Obsolete_CustomDiagnosticId_BadMetadata_01() { var source1 = @" @@ -8536,7 +8582,7 @@ public void Obsolete_CustomDiagnosticId_BadMetadata_01() namespace System { - public class ObsoleteAttribute + public class ObsoleteAttribute : Attribute { public bool Flag { get; set; } public string DiagnosticId { get; set; } @@ -8559,6 +8605,8 @@ void M2() } }"; var comp1 = CreateCompilation(source1); + comp1.VerifyDiagnostics(); + verify(comp1.ToMetadataReference()); verify(comp1.EmitToImageReference()); @@ -8566,10 +8614,56 @@ void verify(MetadataReference reference) { var comp2 = CreateCompilation(source2, references: new[] { reference }); - // TODO: we could try to skip over properties of types that we don't recognize, but it only applies for bad user-defined Obsolete attributes. comp2.VerifyDiagnostics( // (6,9): warning TEST1: 'C1.M1()' is obsolete // M1(); // 1 + Diagnostic("TEST1", "M1()").WithArguments("C1.M1()").WithLocation(6, 9)); + } + } + + [Fact, WorkItem(42119, "https://github.com/dotnet/roslyn/issues/42119")] + public void Obsolete_CustomDiagnosticId_BadMetadata_02() + { + var source1 = @" +using System; +#pragma warning disable 436 + +namespace System +{ + public class ObsoleteAttribute : Attribute + { + public string DiagnosticId; + } +} + +public class C1 +{ + [Obsolete(DiagnosticId = ""TEST1"")] + public void M1() { } +} +"; + + var source2 = @" +class C2 : C1 +{ + void M2() + { + M1(); // 1 + } +}"; + var comp1 = CreateCompilation(source1); + comp1.VerifyDiagnostics(); + + verify(comp1.ToMetadataReference()); + verify(comp1.EmitToImageReference()); + + void verify(MetadataReference reference) + { + var comp2 = CreateCompilation(source2, references: new[] { reference }); + + comp2.VerifyDiagnostics( + // (6,9): warning CS0612: 'C1.M1()' is obsolete + // M1(); // 1 Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "M1()").WithArguments("C1.M1()").WithLocation(6, 9)); } } diff --git a/src/Compilers/Core/Portable/MetadataReader/PEModule.cs b/src/Compilers/Core/Portable/MetadataReader/PEModule.cs index d367e1d99396e..87982ffaedc20 100644 --- a/src/Compilers/Core/Portable/MetadataReader/PEModule.cs +++ b/src/Compilers/Core/Portable/MetadataReader/PEModule.cs @@ -94,7 +94,6 @@ internal sealed class PEModule : IDisposable private static readonly AttributeValueExtractor> s_attributeBoolArrayValueExtractor = CrackBoolArrayInAttributeValue; private static readonly AttributeValueExtractor> s_attributeByteArrayValueExtractor = CrackByteArrayInAttributeValue; private static readonly AttributeValueExtractor> s_attributeStringArrayValueExtractor = CrackStringArrayInAttributeValue; - private static readonly AttributeValueExtractor s_attributeObsoleteDataExtractor = CrackObsoleteAttributeData; private static readonly AttributeValueExtractor s_attributeObsoleteDataExtractorString = CrackObsoleteAttributeDataString; private static readonly AttributeValueExtractor s_attributeObsoleteDataExtractorStringBool = CrackObsoleteAttributeDataStringBool; private static readonly AttributeValueExtractor s_attributeDeprecatedDataExtractor = CrackDeprecatedAttributeData; @@ -1319,14 +1318,12 @@ private ArrayBuilder ExtractStringValuesFromAttributes(List ExtractStringValuesFromAttributes(List= 4) + { + // check prolog + if (reader.ReadInt16() == 1) + { + var (diagnosticId, urlFormat) = CrackObsoleteProperties(ref reader); + return new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, message: null, isError: false, diagnosticId, urlFormat); + } + } + } + } + catch (BadImageFormatException) + { } + + return null; + } + + /// + /// Decodes attribute argument type from attribute blob (called FieldOrPropType in the spec). + /// + /// If the encoded argument type is invalid. + /// An exception from metadata reader. + private static (SerializationTypeCode typeCode, SerializationTypeCode elementTypeCode) DecodeCustomAttributeFieldOrPropTypeOrThrow(ref BlobReader argReader) + { + var typeCode = argReader.ReadSerializationTypeCode(); + + // Spec: + // The FieldOrPropType (typeCode) shall be exactly one of: ELEMENT_TYPE_BOOLEAN, + // ELEMENT_TYPE_CHAR, ELEMENT_TYPE_I1, ELEMENT_TYPE_U1, ELEMENT_TYPE_I2, + // ELEMENT_TYPE_U2, ELEMENT_TYPE_I4, ELEMENT_TYPE_U4, ELEMENT_TYPE_I8, + // ELEMENT_TYPE_U8, ELEMENT_TYPE_R4, ELEMENT_TYPE_R8, ELEMENT_TYPE_STRING. + // + // A single-dimensional, zero-based array is specified as a single byte 0x1D followed + // by the FieldOrPropType of the element type. (See §II.23.1.16) An enum is + // specified as a single byte 0x55 followed by a SerString. + // + + // todo: this could be simplified for the "skip" case. + if (typeCode == SerializationTypeCode.SZArray) + { + var elementTypeCode = argReader.ReadSerializationTypeCode(); + if (elementTypeCode == SerializationTypeCode.SZArray) + { + // nested array not allowed: + throw new UnsupportedSignatureContent(); + } + + return (typeCode, elementTypeCode); + } + + switch (typeCode) + { + case SerializationTypeCode.Enum: + if (!CrackStringInAttributeValue(out _, ref argReader)) + { + throw new UnsupportedSignatureContent(); + } + + return (typeCode, elementTypeCode: SerializationTypeCode.Invalid); + + case SerializationTypeCode.TaggedObject: + case SerializationTypeCode.Type: + case SerializationTypeCode.String: + case SerializationTypeCode.Boolean: + case SerializationTypeCode.Char: + case SerializationTypeCode.SByte: + case SerializationTypeCode.Byte: + case SerializationTypeCode.Int16: + case SerializationTypeCode.UInt16: + case SerializationTypeCode.Int32: + case SerializationTypeCode.UInt32: + case SerializationTypeCode.Int64: + case SerializationTypeCode.UInt64: + case SerializationTypeCode.Single: + case SerializationTypeCode.Double: + return (typeCode, elementTypeCode: SerializationTypeCode.Invalid); + } + + throw new UnsupportedSignatureContent(); + } + + /// If the encoded attribute argument is invalid. + /// An exception from metadata reader. + private static void SkipCustomAttributeElementOrThrow(ref BlobReader argReader, SerializationTypeCode typeCode) + { + if (typeCode == SerializationTypeCode.TaggedObject) + { + // Spec: If the parameter kind is System.Object, the value stored represents the "boxed" instance of that value-type. + var (argTypeCode, elementTypeCode) = DecodeCustomAttributeFieldOrPropTypeOrThrow(ref argReader); + + if (argTypeCode == SerializationTypeCode.SZArray) + { + SkipCustomAttributeElementArrayOrThrow(ref argReader, elementTypeCode); + return; + } + } + + SkipCustomAttributePrimitiveElementOrThrow(ref argReader, typeCode); + } + + /// If the given is invalid. + /// An exception from metadata reader. + private static void SkipCustomAttributePrimitiveElementOrThrow(ref BlobReader argReader, SerializationTypeCode typeCode) + { + switch (typeCode) + { + case SerializationTypeCode.Boolean: + case SerializationTypeCode.SByte: + case SerializationTypeCode.Byte: + argReader.ReadByte(); + break; + + case SerializationTypeCode.Int16: + case SerializationTypeCode.UInt16: + case SerializationTypeCode.Char: + argReader.ReadInt16(); + break; + + case SerializationTypeCode.Int32: + case SerializationTypeCode.UInt32: + case SerializationTypeCode.Single: + argReader.ReadInt32(); + break; + + case SerializationTypeCode.Int64: + case SerializationTypeCode.UInt64: + case SerializationTypeCode.Double: + argReader.ReadInt64(); + break; + + case SerializationTypeCode.String: + case SerializationTypeCode.Type: + if (!CrackStringInAttributeValue(out _, ref argReader)) + { + throw new UnsupportedSignatureContent(); + } + + break; + + default: + throw new UnsupportedSignatureContent(); + } + } + + /// If the encoded attribute argument is invalid. + /// An exception from metadata reader. + private static void SkipCustomAttributeElementArrayOrThrow(ref BlobReader argReader, SerializationTypeCode elementTypeCode) + { + int count = argReader.ReadInt32(); + for (int i = 0; i < count; i++) + { + SkipCustomAttributeElementOrThrow(ref argReader, elementTypeCode); + } + } #nullable restore private ObsoleteAttributeData TryExtractDeprecatedDataFromAttribute(AttributeInfo attributeInfo) @@ -1519,7 +1681,7 @@ private bool TryExtractValueFromAttribute(CustomAttributeHandle handle, out T // TODO: error checking offset in range BlobReader reader = MetadataReader.GetBlobReader(valueBlob); - if (reader.Length >= 4) + if (reader.Length > 4) { // check prolog if (reader.ReadByte() == 1 && reader.ReadByte() == 0) @@ -1622,16 +1784,6 @@ internal bool IsNoPiaLocalType( } #nullable enable - /// - /// Cracks the data on an Obsolete attribute using the parameterless constructor. - /// - private static bool CrackObsoleteAttributeData(out ObsoleteAttributeData value, ref BlobReader sig) - { - CrackObsoleteProperties(out var diagnosticId, out var urlFormat, ref sig); - value = new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, message: null, isError: false, diagnosticId: diagnosticId, urlFormat: urlFormat); - return true; - } - /// /// Cracks the data on an Obsolete attribute using the Obsolete(string) constructor. /// @@ -1640,7 +1792,7 @@ private static bool CrackObsoleteAttributeDataString(out ObsoleteAttributeData? string message; if (CrackStringInAttributeValue(out message, ref sig)) { - CrackObsoleteProperties(out var diagnosticId, out var urlFormat, ref sig); + var (diagnosticId, urlFormat) = CrackObsoleteProperties(ref sig); value = new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, message, isError: false, diagnosticId: diagnosticId, urlFormat: urlFormat); return true; } @@ -1659,7 +1811,7 @@ private static bool CrackObsoleteAttributeDataStringBool(out ObsoleteAttributeDa { bool isError = sig.ReadBoolean(); - CrackObsoleteProperties(out var diagnosticId, out var urlFormat, ref sig); + var (diagnosticId, urlFormat) = CrackObsoleteProperties(ref sig); value = new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, message, isError, diagnosticId: diagnosticId, urlFormat: urlFormat); return true; } @@ -1668,53 +1820,72 @@ private static bool CrackObsoleteAttributeDataStringBool(out ObsoleteAttributeDa return false; } - private static void CrackObsoleteProperties(out string? diagnosticId, out string? urlFormat, ref BlobReader sig) + /// + /// Gets the well-known optional named properties on ObsoleteAttribute, if present. + /// Both 'diagnosticId' and 'urlFormat' may be present, or only one, or neither. + /// + /// + /// Failure to find any of these properties does not imply failure to decode the ObsoleteAttribute, + /// so we don't return a value indicating success or failure. + /// + private static (string? diagnosticId, string? urlFormat) CrackObsoleteProperties(ref BlobReader sig) { - diagnosticId = null; - urlFormat = null; + string? diagnosticId = null; + string? urlFormat = null; // See CIL spec section II.23.3 Custom attributes // Next is a description of the optional “named” fields and properties. // This starts with NumNamed– an unsigned int16 giving the number of “named” properties or fields that follow. var numNamed = sig.ReadUInt16(); - for (int i = 0; i < numNamed; i++) + for (int i = 0; i < numNamed && (diagnosticId is null || urlFormat is null); i++) { - // A NamedArg is simply a FixedArg preceded by information to identify which field or property it represents. - // PROPERTY is the single byte 0x54. - const int PROPERTY = 0x54; - var fieldOrProperty = sig.ReadByte(); - if (fieldOrProperty != PROPERTY) + // Ecma-335 23.3 - A NamedArg is simply a FixedArg preceded by information to identify which field or + // property it represents. [Note: Recall that the CLI allows fields and properties to have the same name; so + // we require a means to disambiguate such situations. end note] FIELD is the single byte 0x53. PROPERTY is + // the single byte 0x54. + + var kind = (CustomAttributeNamedArgumentKind)sig.ReadCompressedInteger(); + if (kind != CustomAttributeNamedArgumentKind.Field && kind != CustomAttributeNamedArgumentKind.Property) { - return; + throw new UnsupportedSignatureContent(); } - const int ELEMENT_TYPE_STRING = 0x0e; - var fieldOrPropType = sig.ReadByte(); - if (fieldOrPropType != ELEMENT_TYPE_STRING) + var (typeCode, elementTypeCode) = DecodeCustomAttributeFieldOrPropTypeOrThrow(ref sig); + + if (!CrackStringInAttributeValue(out var name, ref sig)) { - return; + throw new UnsupportedSignatureContent(); } - // The FieldOrPropName is the name of the field or property, stored as a SerString. - if (!CrackStringInAttributeValue(out string fieldOrPropName, ref sig) || - !CrackStringInAttributeValue(out string fixedArg, ref sig)) + if (typeCode == SerializationTypeCode.String && kind == CustomAttributeNamedArgumentKind.Property) { - return; - } + if (!CrackStringInAttributeValue(out var value, ref sig)) + { + throw new UnsupportedSignatureContent(); + } - switch (fieldOrPropName) + switch (name) + { + case "DiagnosticId": + diagnosticId = value; + break; + case "UrlFormat": + urlFormat = value; + break; + } + } + else if (typeCode == SerializationTypeCode.SZArray) { - case "DiagnosticId": - diagnosticId = fixedArg; - break; - case "UrlFormat": - urlFormat = fixedArg; - break; + SkipCustomAttributeElementArrayOrThrow(ref sig, elementTypeCode); + } + else + { + SkipCustomAttributeElementOrThrow(ref sig, typeCode); } - - // TODO: do we want to tolerate custom definitions of ObsoleteAttribute with different properties types besides string? } + + return (diagnosticId, urlFormat); } #nullable restore diff --git a/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs b/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs index 40a78e74aad56..8ef8f0556de21 100644 --- a/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs +++ b/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs @@ -9,6 +9,7 @@ using System.Collections.Immutable; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; +using System.Linq; using System.Reflection; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; @@ -292,25 +293,47 @@ private ObsoleteAttributeData DecodeObsoleteAttribute() } } - TypedConstant diagnosticId = default; - TypedConstant urlFormat = default; + Debug.Assert(AttributeClass is object); + + string? diagnosticId = null; + string? urlFormat = null; foreach (var pair in this.CommonNamedArguments) { switch (pair.Key) { - case "DiagnosticId": - diagnosticId = pair.Value; + case "DiagnosticId" when isProperty("DiagnosticId"): + diagnosticId = pair.Value.ValueInternal as string; break; - case "UrlFormat": - urlFormat = pair.Value; + case "UrlFormat" when isProperty("UrlFormat"): + urlFormat = pair.Value.ValueInternal as string; break; default: - // unknown property specified on ObsoleteAttribute + // unknown property or field specified on ObsoleteAttribute break; } + + if (diagnosticId is object && urlFormat is object) + { + break; + } } - return new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, message, isError, (string?)diagnosticId.ValueInternal, (string?)urlFormat.ValueInternal); + return new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, message, isError, diagnosticId, urlFormat); + + // note: it is disallowed to declare a property and a field + // with the same name in C# or VB source, even if it is allowed in IL. + bool isProperty(string name) + { + foreach (var member in AttributeClass.GetMembers(name)) + { + if (member.Kind == SymbolKind.Property) + { + return true; + } + } + + return false; + } } /// diff --git a/src/Compilers/Core/Portable/Symbols/Attributes/ObsoleteAttributeData.cs b/src/Compilers/Core/Portable/Symbols/Attributes/ObsoleteAttributeData.cs index 1063084e959aa..913c2bb0e3f6c 100644 --- a/src/Compilers/Core/Portable/Symbols/Attributes/ObsoleteAttributeData.cs +++ b/src/Compilers/Core/Portable/Symbols/Attributes/ObsoleteAttributeData.cs @@ -47,8 +47,28 @@ public ObsoleteAttributeData(ObsoleteAttributeKind kind, string? message, bool i /// public readonly string? Message; + /// + /// The custom diagnostic ID to use for obsolete diagnostics. + /// If null, diagnostics are produced using the compiler default diagnostic IDs. + /// public readonly string? DiagnosticId; + /// + /// + /// The custom help URL format string for obsolete diagnostics. + /// Expected to contain zero or one format items. + /// + /// + /// When specified, the obsolete diagnostic's will be produced + /// by formatting this string using the as the single argument. + /// + /// + /// + /// e.g. with a value "TEST1", + /// and a value ,
+ /// the diagnostic will have the HelpLinkUri . + /// + /// public readonly string? UrlFormat; internal bool IsUninitialized diff --git a/src/Test/Utilities/Portable/Diagnostics/DiagnosticDescription.cs b/src/Test/Utilities/Portable/Diagnostics/DiagnosticDescription.cs index 0af28b8b96cb0..24a732ebeb7cb 100644 --- a/src/Test/Utilities/Portable/Diagnostics/DiagnosticDescription.cs +++ b/src/Test/Utilities/Portable/Diagnostics/DiagnosticDescription.cs @@ -117,14 +117,15 @@ public DiagnosticDescription(Diagnostic d, bool errorCodeOnly, bool includeDefau _defaultSeverityOpt = includeDefaultSeverity ? d.DefaultSeverity : (DiagnosticSeverity?)null; _effectiveSeverityOpt = includeEffectiveSeverity ? d.Severity : (DiagnosticSeverity?)null; - DiagnosticWithInfo dinfo = d as DiagnosticWithInfo; - if (d.Code == 0 || dinfo?.Info is CustomObsoleteDiagnosticInfo { Data: { DiagnosticId: object _ } }) + DiagnosticWithInfo dinfo = null; + if (d.Code == 0 || d.Descriptor.CustomTags.Contains(WellKnownDiagnosticTags.CustomObsolete)) { _code = d.Id; _errorCodeType = typeof(string); } else { + dinfo = d as DiagnosticWithInfo; if (dinfo == null) { _code = d.Code; From 5d060eaf74959d0bbf74764e470abb893bd47831 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Tue, 24 Mar 2020 18:16:09 -0700 Subject: [PATCH 06/21] Use MetadataDecoder.GetAttributeData for ObsoleteAttribute --- .../Symbols/ObsoleteAttributeHelpers.cs | 3 +- .../Attributes/AttributeTests_IsByRefLike.cs | 2 +- .../AttributeTests_WellKnownAttributes.cs | 82 ++++++++++ .../MetadataReader/MetadataDecoder.cs | 21 ++- .../Core/Portable/MetadataReader/PEModule.cs | 152 +++--------------- .../Symbols/ObsoleteAttributeHelpers.vb | 2 +- 6 files changed, 128 insertions(+), 134 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/ObsoleteAttributeHelpers.cs b/src/Compilers/CSharp/Portable/Symbols/ObsoleteAttributeHelpers.cs index 4ade5d40c8a0e..07dd36f838fdd 100644 --- a/src/Compilers/CSharp/Portable/Symbols/ObsoleteAttributeHelpers.cs +++ b/src/Compilers/CSharp/Portable/Symbols/ObsoleteAttributeHelpers.cs @@ -40,8 +40,7 @@ internal static void InitializeObsoleteDataFromMetadata(ref ObsoleteAttributeDat /// internal static ObsoleteAttributeData GetObsoleteDataFromMetadata(EntityHandle token, PEModuleSymbol containingModule, bool ignoreByRefLikeMarker) { - ObsoleteAttributeData obsoleteAttributeData; - obsoleteAttributeData = containingModule.Module.TryGetDeprecatedOrExperimentalOrObsoleteAttribute(token, ignoreByRefLikeMarker); + var obsoleteAttributeData = containingModule.Module.TryGetDeprecatedOrExperimentalOrObsoleteAttribute(token, new MetadataDecoder(containingModule), ignoreByRefLikeMarker); Debug.Assert(obsoleteAttributeData == null || !obsoleteAttributeData.IsUninitialized); return obsoleteAttributeData; } diff --git a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_IsByRefLike.cs b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_IsByRefLike.cs index 92a30d3ea1fc1..5200e667aa5b5 100644 --- a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_IsByRefLike.cs +++ b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_IsByRefLike.cs @@ -999,7 +999,7 @@ private static void AssertReferencedIsByRefLike(TypeSymbol type, bool hasObsolet Assert.Empty(peType.GetAttributes()); var peModule = (PEModuleSymbol)peType.ContainingModule; - var obsoleteAttribute = peModule.Module.TryGetDeprecatedOrExperimentalOrObsoleteAttribute(peType.Handle, ignoreByRefLikeMarker: false); + var obsoleteAttribute = peModule.Module.TryGetDeprecatedOrExperimentalOrObsoleteAttribute(peType.Handle, new MetadataDecoder(peModule), ignoreByRefLikeMarker: false); if (hasObsolete) { diff --git a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs index 9de78b10af3e7..68dc788294320 100644 --- a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs +++ b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs @@ -8668,6 +8668,88 @@ void verify(MetadataReference reference) } } + [Fact, WorkItem(42119, "https://github.com/dotnet/roslyn/issues/42119")] + public void Obsolete_CustomDiagnosticId_BadMetadata_03() + { + var source1 = @" +using System; +#pragma warning disable 436 + +public enum E1 { A, B } + +namespace System +{ + public class ObsoleteAttribute : Attribute + { + public byte Byte { get; set; } + public sbyte SByte { get; set; } + public bool Bool { get; set; } + public short Short { get; set; } + public ushort UShort { get; set; } + public char Char { get; set; } + public int Int { get; set; } + public uint UInt { get; set; } + public float Float { get; set; } + public long Long { get; set; } + public ulong ULong { get; set; } + public double Double { get; set; } + public E1 Enum { get; set; } + public string DiagnosticId { get; set; } + } +} + +public class C1 +{ + [Obsolete( + Byte = 0, + SByte = 0, + Bool = false, + Short = 0, + UShort = 0, + Char = '\0', + Int = 0, + UInt = 0, + Float = 0, + Long = 0, + ULong = 0, + Double = 0, + Enum = E1.A, + DiagnosticId = ""TEST1"")] + public void M1() { } +} +"; + + var source2 = @" +class C2 : C1 +{ + void M2() + { + M1(); // 1 + } +}"; + var comp1 = CreateCompilation(source1); + comp1.VerifyDiagnostics(); + + verify(comp1.ToMetadataReference()); + verify(comp1.EmitToImageReference()); + + void verify(MetadataReference reference) + { + var comp2 = CreateCompilation(source2, references: new[] { reference }); + + comp2.VerifyDiagnostics( + // (6,9): warning TEST1: 'C1.M1()' is obsolete + // M1(); // 1 + Diagnostic("TEST1", "M1()").WithArguments("C1.M1()").WithLocation(6, 9)); + } + } + + // Missing tests: + // - duplicate named arguments + // - named argument has well-known name but bad type, non-null value + // - named argument has well-known name but bad type, null value + // - ?? + [Fact] [WorkItem(656345, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/656345")] public void ConditionalLazyObsoleteDiagnostic() diff --git a/src/Compilers/Core/Portable/MetadataReader/MetadataDecoder.cs b/src/Compilers/Core/Portable/MetadataReader/MetadataDecoder.cs index 1917597eac5ea..a0eae24bd54d6 100644 --- a/src/Compilers/Core/Portable/MetadataReader/MetadataDecoder.cs +++ b/src/Compilers/Core/Portable/MetadataReader/MetadataDecoder.cs @@ -1602,7 +1602,7 @@ private static TypedConstantKind GetPrimitiveOrEnumTypedConstantKind(TypeSymbol /// If the encoded named argument is invalid. /// An exception from metadata reader. - private KeyValuePair DecodeCustomAttributeNamedArgumentOrThrow(ref BlobReader argReader) + private (KeyValuePair, bool isProperty) DecodeCustomAttributeNamedArgumentOrThrow(ref BlobReader argReader) { // Ecma-335 23.3 - A NamedArg is simply a FixedArg preceded by information to identify which field or // property it represents. [Note: Recall that the CLI allows fields and properties to have the same name; so @@ -1629,7 +1629,7 @@ private KeyValuePair DecodeCustomAttributeNamedArgumentOr ? DecodeCustomAttributeElementArrayOrThrow(ref argReader, elementTypeCode, elementType, type) : DecodeCustomAttributeElementOrThrow(ref argReader, typeCode, type); - return new KeyValuePair(name, value); + return (new KeyValuePair(name, value), kind == CustomAttributeNamedArgumentKind.Property); } internal bool IsTargetAttribute( @@ -1671,11 +1671,22 @@ internal bool GetCustomAttribute( CustomAttributeHandle handle, out TypedConstant[] positionalArgs, out KeyValuePair[] namedArgs) + { + return GetCustomAttribute(handle, out positionalArgs, out namedArgs, out _); + } + + + internal bool GetCustomAttribute( + CustomAttributeHandle handle, + out TypedConstant[] positionalArgs, + out KeyValuePair[] namedArgs, + out bool[] namedArgIsProperty) { try { positionalArgs = Array.Empty(); - namedArgs = Array.Empty>(); + namedArgs = Array.Empty>(); + namedArgIsProperty = Array.Empty(); // We could call decoder.GetSignature and use that to decode the arguments. However, materializing the // constructor signature is more work. We try to decode the arguments directly from the metadata bytes. @@ -1727,10 +1738,11 @@ internal bool GetCustomAttribute( if (namedParamCount > 0) { namedArgs = new KeyValuePair[namedParamCount]; + namedArgIsProperty = new bool[namedParamCount]; for (int i = 0; i < namedArgs.Length; i++) { - namedArgs[i] = DecodeCustomAttributeNamedArgumentOrThrow(ref argsReader); + (namedArgs[i], namedArgIsProperty[i]) = DecodeCustomAttributeNamedArgumentOrThrow(ref argsReader); } } @@ -1741,6 +1753,7 @@ internal bool GetCustomAttribute( { positionalArgs = Array.Empty(); namedArgs = Array.Empty>(); + namedArgIsProperty = Array.Empty(); } return false; diff --git a/src/Compilers/Core/Portable/MetadataReader/PEModule.cs b/src/Compilers/Core/Portable/MetadataReader/PEModule.cs index 87982ffaedc20..a9f7877f7ddfa 100644 --- a/src/Compilers/Core/Portable/MetadataReader/PEModule.cs +++ b/src/Compilers/Core/Portable/MetadataReader/PEModule.cs @@ -17,6 +17,7 @@ using System.Security.Cryptography; using System.Threading; using Microsoft.CodeAnalysis.PooledObjects; +using Microsoft.CodeAnalysis.Symbols; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis @@ -94,8 +95,6 @@ internal sealed class PEModule : IDisposable private static readonly AttributeValueExtractor> s_attributeBoolArrayValueExtractor = CrackBoolArrayInAttributeValue; private static readonly AttributeValueExtractor> s_attributeByteArrayValueExtractor = CrackByteArrayInAttributeValue; private static readonly AttributeValueExtractor> s_attributeStringArrayValueExtractor = CrackStringArrayInAttributeValue; - private static readonly AttributeValueExtractor s_attributeObsoleteDataExtractorString = CrackObsoleteAttributeDataString; - private static readonly AttributeValueExtractor s_attributeObsoleteDataExtractorStringBool = CrackObsoleteAttributeDataStringBool; private static readonly AttributeValueExtractor s_attributeDeprecatedDataExtractor = CrackDeprecatedAttributeData; private static readonly AttributeValueExtractor s_attributeBoolAndStringArrayValueExtractor = CrackBoolAndStringArrayInAttributeValue; @@ -1067,9 +1066,15 @@ internal bool HasIsByRefLikeAttribute(EntityHandle token) internal const string ByRefLikeMarker = "Types with embedded references are not supported in this version of your compiler."; - internal ObsoleteAttributeData TryGetDeprecatedOrExperimentalOrObsoleteAttribute( + internal ObsoleteAttributeData TryGetDeprecatedOrExperimentalOrObsoleteAttribute( EntityHandle token, + MetadataDecoder decoder, bool ignoreByRefLikeMarker) + where ModuleSymbol : class, IModuleSymbolInternal + where TypeSymbol : class, Symbol, ITypeSymbolInternal + where MethodSymbol : class, Symbol, IMethodSymbolInternal + where FieldSymbol : class, Symbol, IFieldSymbolInternal + where Symbol : class, ISymbolInternal { AttributeInfo info; @@ -1080,9 +1085,9 @@ internal ObsoleteAttributeData TryGetDeprecatedOrExperimentalOrObsoleteAttribute } info = FindTargetAttribute(token, AttributeDescription.ObsoleteAttribute); - if (info.HasValue) + if (info.HasValue && decoder.GetCustomAttribute(info.Handle, out TypedConstant[] positionalArgs, out KeyValuePair[] namedArgs, out bool[] namedArgIsProperty)) { - ObsoleteAttributeData obsoleteData = TryExtractObsoleteDataFromAttribute(info); + ObsoleteAttributeData obsoleteData = TryExtractObsoleteDataFromAttribute(positionalArgs, namedArgs, namedArgIsProperty); switch (obsoleteData?.Message) { case ByRefLikeMarker when ignoreByRefLikeMarker: @@ -1315,53 +1320,23 @@ private ArrayBuilder ExtractStringValuesFromAttributes(List[] namedArgs, bool[] namedArgIsProperty) { - Debug.Assert(attributeInfo.HasValue); - ObsoleteAttributeData? obsoleteData; - switch (attributeInfo.SignatureIndex) + var (diagnosticId, urlFormat) = ExtractObsoleteProperties(namedArgs, namedArgIsProperty); + switch (positionalArgs.Length) { case 0: // ObsoleteAttribute() - return TryExtractParameterlessObsoleteData(attributeInfo.Handle); + return new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, message: null, isError: false, diagnosticId, urlFormat); case 1: // ObsoleteAttribute(string) - TryExtractValueFromAttribute(attributeInfo.Handle, out obsoleteData, s_attributeObsoleteDataExtractorString); - return obsoleteData; - case 2: + return new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, positionalArgs[0].ValueInternal as string, isError: false, diagnosticId, urlFormat); + case 2 when positionalArgs[1].ValueInternal is bool isError: // ObsoleteAttribute(string, bool) - TryExtractValueFromAttribute(attributeInfo.Handle, out obsoleteData, s_attributeObsoleteDataExtractorStringBool); - return obsoleteData; + return new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, positionalArgs[0].ValueInternal as string, isError, diagnosticId, urlFormat); default: - throw ExceptionUtilities.UnexpectedValue(attributeInfo.SignatureIndex); - } - } - - private ObsoleteAttributeData? TryExtractParameterlessObsoleteData(CustomAttributeHandle handle) - { - Debug.Assert(!handle.IsNil); - try - { - BlobHandle valueBlob = GetCustomAttributeValueOrThrow(handle); - if (!valueBlob.IsNil) - { - BlobReader reader = MetadataReader.GetBlobReader(valueBlob); - - if (reader.Length >= 4) - { - // check prolog - if (reader.ReadInt16() == 1) - { - var (diagnosticId, urlFormat) = CrackObsoleteProperties(ref reader); - return new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, message: null, isError: false, diagnosticId, urlFormat); - } - } - } + return null; } - catch (BadImageFormatException) - { } - - return null; } /// @@ -1784,105 +1759,30 @@ internal bool IsNoPiaLocalType( } #nullable enable - /// - /// Cracks the data on an Obsolete attribute using the Obsolete(string) constructor. - /// - private static bool CrackObsoleteAttributeDataString(out ObsoleteAttributeData? value, ref BlobReader sig) - { - string message; - if (CrackStringInAttributeValue(out message, ref sig)) - { - var (diagnosticId, urlFormat) = CrackObsoleteProperties(ref sig); - value = new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, message, isError: false, diagnosticId: diagnosticId, urlFormat: urlFormat); - return true; - } - - value = null; - return false; - } - - /// - /// Cracks the data on an Obsolete attribute using the Obsolete(string, bool) constructor. - /// - private static bool CrackObsoleteAttributeDataStringBool(out ObsoleteAttributeData? value, ref BlobReader sig) - { - string message; - if (CrackStringInAttributeValue(out message, ref sig) && sig.RemainingBytes >= 1) - { - bool isError = sig.ReadBoolean(); - - var (diagnosticId, urlFormat) = CrackObsoleteProperties(ref sig); - value = new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, message, isError, diagnosticId: diagnosticId, urlFormat: urlFormat); - return true; - } - - value = null; - return false; - } /// /// Gets the well-known optional named properties on ObsoleteAttribute, if present. /// Both 'diagnosticId' and 'urlFormat' may be present, or only one, or neither. /// - /// - /// Failure to find any of these properties does not imply failure to decode the ObsoleteAttribute, - /// so we don't return a value indicating success or failure. - /// - private static (string? diagnosticId, string? urlFormat) CrackObsoleteProperties(ref BlobReader sig) + private static (string? diagnosticId, string? urlFormat) ExtractObsoleteProperties(KeyValuePair[] namedArgs, bool[] namedArgIsProperty) { string? diagnosticId = null; string? urlFormat = null; - // See CIL spec section II.23.3 Custom attributes - - // Next is a description of the optional “named” fields and properties. - // This starts with NumNamed– an unsigned int16 giving the number of “named” properties or fields that follow. - var numNamed = sig.ReadUInt16(); - for (int i = 0; i < numNamed && (diagnosticId is null || urlFormat is null); i++) + for (var i = 0; i < namedArgs.Length && (diagnosticId is null || urlFormat is null); i++) { - // Ecma-335 23.3 - A NamedArg is simply a FixedArg preceded by information to identify which field or - // property it represents. [Note: Recall that the CLI allows fields and properties to have the same name; so - // we require a means to disambiguate such situations. end note] FIELD is the single byte 0x53. PROPERTY is - // the single byte 0x54. - - var kind = (CustomAttributeNamedArgumentKind)sig.ReadCompressedInteger(); - if (kind != CustomAttributeNamedArgumentKind.Field && kind != CustomAttributeNamedArgumentKind.Property) + var (name, value) = namedArgs[i]; + if (namedArgIsProperty[i]) { - throw new UnsupportedSignatureContent(); - } - - var (typeCode, elementTypeCode) = DecodeCustomAttributeFieldOrPropTypeOrThrow(ref sig); - - if (!CrackStringInAttributeValue(out var name, ref sig)) - { - throw new UnsupportedSignatureContent(); - } - - if (typeCode == SerializationTypeCode.String && kind == CustomAttributeNamedArgumentKind.Property) - { - if (!CrackStringInAttributeValue(out var value, ref sig)) + if (diagnosticId is null && name == "DiagnosticId") { - throw new UnsupportedSignatureContent(); + diagnosticId = value.ValueInternal as string; } - - switch (name) + else if (urlFormat is null && name == "UrlFormat") { - case "DiagnosticId": - diagnosticId = value; - break; - case "UrlFormat": - urlFormat = value; - break; + urlFormat = value.ValueInternal as string; } } - else if (typeCode == SerializationTypeCode.SZArray) - { - SkipCustomAttributeElementArrayOrThrow(ref sig, elementTypeCode); - } - else - { - SkipCustomAttributeElementOrThrow(ref sig, typeCode); - } } return (diagnosticId, urlFormat); diff --git a/src/Compilers/VisualBasic/Portable/Symbols/ObsoleteAttributeHelpers.vb b/src/Compilers/VisualBasic/Portable/Symbols/ObsoleteAttributeHelpers.vb index 7d6125a938f2c..ceee3bf3c900e 100644 --- a/src/Compilers/VisualBasic/Portable/Symbols/ObsoleteAttributeHelpers.vb +++ b/src/Compilers/VisualBasic/Portable/Symbols/ObsoleteAttributeHelpers.vb @@ -33,7 +33,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols Friend Shared Function GetObsoleteDataFromMetadata(token As EntityHandle, containingModule As PEModuleSymbol) As ObsoleteAttributeData Dim obsoleteAttributeData As ObsoleteAttributeData = Nothing ' ignoreByRefLikeMarker := False, since VB does not support ref-like types - obsoleteAttributeData = containingModule.Module.TryGetDeprecatedOrExperimentalOrObsoleteAttribute(token, ignoreByRefLikeMarker:=False) + obsoleteAttributeData = containingModule.Module.TryGetDeprecatedOrExperimentalOrObsoleteAttribute(token, New MetadataDecoder(containingModule), ignoreByRefLikeMarker:=False) Debug.Assert(obsoleteAttributeData Is Nothing OrElse Not obsoleteAttributeData.IsUninitialized) Return obsoleteAttributeData End Function From 4c5a6953bf8d78467c986396009bc9c73a5319a8 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 25 Mar 2020 12:56:13 -0700 Subject: [PATCH 07/21] Use MetadataDecoder in a more granular way --- .../AttributeTests_WellKnownAttributes.cs | 117 +++++++++- .../MetadataReader/MetadataDecoder.cs | 25 +- .../Core/Portable/MetadataReader/PEModule.cs | 220 ++++++------------ 3 files changed, 190 insertions(+), 172 deletions(-) diff --git a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs index 68dc788294320..d59e74a23c496 100644 --- a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs +++ b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs @@ -8633,12 +8633,13 @@ namespace System public class ObsoleteAttribute : Attribute { public string DiagnosticId; + public string UrlFormat; } } public class C1 { - [Obsolete(DiagnosticId = ""TEST1"")] + [Obsolete(DiagnosticId = ""TEST1"", UrlFormat = ""TEST2"")] public void M1() { } } "; @@ -8660,11 +8661,14 @@ void M2() void verify(MetadataReference reference) { var comp2 = CreateCompilation(source2, references: new[] { reference }); - - comp2.VerifyDiagnostics( + var diags = comp2.GetDiagnostics(); + diags.Verify( // (6,9): warning CS0612: 'C1.M1()' is obsolete // M1(); // 1 Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "M1()").WithArguments("C1.M1()").WithLocation(6, 9)); + + var diag = diags.Single(); + Assert.Equal("", diag.Descriptor.HelpLinkUri); } } @@ -8744,6 +8748,113 @@ void verify(MetadataReference reference) } } + [Fact, WorkItem(42119, "https://github.com/dotnet/roslyn/issues/42119")] + public void Obsolete_CustomDiagnosticId_BadMetadata_04() + { + var source1 = @" +using System; +#pragma warning disable 436 + +public enum E1 { A, B } + +namespace System +{ + public class ObsoleteAttribute : Attribute + { + public int[] Int { get; set; } + public E1[] Enum { get; set; } + public string DiagnosticId { get; set; } + } +} + +public class C1 +{ + [Obsolete( + Int = new[] { 0, 1, 2 }, + Enum = new[] { E1.A, E1.B }, + DiagnosticId = ""TEST1"")] + public void M1() { } +} +"; + + var source2 = @" +class C2 : C1 +{ + void M2() + { + M1(); // 1 + } +}"; + var comp1 = CreateCompilation(source1); + comp1.VerifyDiagnostics(); + + // https://github.com/dotnet/roslyn/issues/42771 + //verify(comp1.ToMetadataReference()); + verify(comp1.EmitToImageReference()); + + void verify(MetadataReference reference) + { + var comp2 = CreateCompilation(source2, references: new[] { reference }); + + comp2.VerifyDiagnostics( + // (6,9): warning TEST1: 'C1.M1()' is obsolete + // M1(); // 1 + Diagnostic("TEST1", "M1()").WithArguments("C1.M1()").WithLocation(6, 9)); + } + } + + [Fact, WorkItem(42119, "https://github.com/dotnet/roslyn/issues/42119")] + public void Obsolete_CustomDiagnosticId_BadMetadata_05() + { + var source1 = @" +using System; +#pragma warning disable 436 + +namespace System +{ + public class ObsoleteAttribute : Attribute + { + public char[] DiagnosticId { get; set; } + public char[] UrlFormat { get; set; } + } +} + +public class C1 +{ + [Obsolete(DiagnosticId = new[] { 'A' }, UrlFormat = new[] { 'B' })] + public void M1() { } +} +"; + + var source2 = @" +class C2 : C1 +{ + void M2() + { + M1(); // 1 + } +}"; + var comp1 = CreateCompilation(source1); + comp1.VerifyDiagnostics(); + + // https://github.com/dotnet/roslyn/issues/42771 + //verify(comp1.ToMetadataReference()); + verify(comp1.EmitToImageReference()); + + void verify(MetadataReference reference) + { + var comp2 = CreateCompilation(source2, references: new[] { reference }); + var diags = comp2.GetDiagnostics(); + diags.Verify( + // (6,9): warning CS0612: 'C1.M1()' is obsolete + // M1(); // 1 + Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "M1()").WithArguments("C1.M1()").WithLocation(6, 9)); + + var diag = diags.Single(); + Assert.Equal("", diag.Descriptor.HelpLinkUri); + } + } + // Missing tests: // - duplicate named arguments // - named argument has well-known name but bad type, non-null value diff --git a/src/Compilers/Core/Portable/MetadataReader/MetadataDecoder.cs b/src/Compilers/Core/Portable/MetadataReader/MetadataDecoder.cs index a0eae24bd54d6..79f8fcc08feb1 100644 --- a/src/Compilers/Core/Portable/MetadataReader/MetadataDecoder.cs +++ b/src/Compilers/Core/Portable/MetadataReader/MetadataDecoder.cs @@ -70,9 +70,15 @@ internal LocalInfo WithSignature(byte[] signature) public bool IsPinned => (Constraints & LocalSlotConstraints.Pinned) != 0; } + internal interface IAttributeNamedArgumentDecoder + { + (KeyValuePair, bool isProperty) DecodeCustomAttributeNamedArgumentOrThrow(ref BlobReader argReader); + } + #nullable enable internal abstract class MetadataDecoder : - TypeNameDecoder + TypeNameDecoder, + IAttributeNamedArgumentDecoder where ModuleSymbol : class, IModuleSymbolInternal where TypeSymbol : class, Symbol, ITypeSymbolInternal where MethodSymbol : class, Symbol, IMethodSymbolInternal @@ -1602,7 +1608,7 @@ private static TypedConstantKind GetPrimitiveOrEnumTypedConstantKind(TypeSymbol /// If the encoded named argument is invalid. /// An exception from metadata reader. - private (KeyValuePair, bool isProperty) DecodeCustomAttributeNamedArgumentOrThrow(ref BlobReader argReader) + public (KeyValuePair, bool isProperty) DecodeCustomAttributeNamedArgumentOrThrow(ref BlobReader argReader) { // Ecma-335 23.3 - A NamedArg is simply a FixedArg preceded by information to identify which field or // property it represents. [Note: Recall that the CLI allows fields and properties to have the same name; so @@ -1671,22 +1677,11 @@ internal bool GetCustomAttribute( CustomAttributeHandle handle, out TypedConstant[] positionalArgs, out KeyValuePair[] namedArgs) - { - return GetCustomAttribute(handle, out positionalArgs, out namedArgs, out _); - } - - - internal bool GetCustomAttribute( - CustomAttributeHandle handle, - out TypedConstant[] positionalArgs, - out KeyValuePair[] namedArgs, - out bool[] namedArgIsProperty) { try { positionalArgs = Array.Empty(); namedArgs = Array.Empty>(); - namedArgIsProperty = Array.Empty(); // We could call decoder.GetSignature and use that to decode the arguments. However, materializing the // constructor signature is more work. We try to decode the arguments directly from the metadata bytes. @@ -1738,11 +1733,10 @@ internal bool GetCustomAttribute( if (namedParamCount > 0) { namedArgs = new KeyValuePair[namedParamCount]; - namedArgIsProperty = new bool[namedParamCount]; for (int i = 0; i < namedArgs.Length; i++) { - (namedArgs[i], namedArgIsProperty[i]) = DecodeCustomAttributeNamedArgumentOrThrow(ref argsReader); + (namedArgs[i], _) = DecodeCustomAttributeNamedArgumentOrThrow(ref argsReader); } } @@ -1753,7 +1747,6 @@ internal bool GetCustomAttribute( { positionalArgs = Array.Empty(); namedArgs = Array.Empty>(); - namedArgIsProperty = Array.Empty(); } return false; diff --git a/src/Compilers/Core/Portable/MetadataReader/PEModule.cs b/src/Compilers/Core/Portable/MetadataReader/PEModule.cs index a9f7877f7ddfa..72a5e6f5c478a 100644 --- a/src/Compilers/Core/Portable/MetadataReader/PEModule.cs +++ b/src/Compilers/Core/Portable/MetadataReader/PEModule.cs @@ -17,7 +17,6 @@ using System.Security.Cryptography; using System.Threading; using Microsoft.CodeAnalysis.PooledObjects; -using Microsoft.CodeAnalysis.Symbols; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis @@ -1066,15 +1065,10 @@ internal bool HasIsByRefLikeAttribute(EntityHandle token) internal const string ByRefLikeMarker = "Types with embedded references are not supported in this version of your compiler."; - internal ObsoleteAttributeData TryGetDeprecatedOrExperimentalOrObsoleteAttribute( + internal ObsoleteAttributeData TryGetDeprecatedOrExperimentalOrObsoleteAttribute( EntityHandle token, - MetadataDecoder decoder, + IAttributeNamedArgumentDecoder decoder, bool ignoreByRefLikeMarker) - where ModuleSymbol : class, IModuleSymbolInternal - where TypeSymbol : class, Symbol, ITypeSymbolInternal - where MethodSymbol : class, Symbol, IMethodSymbolInternal - where FieldSymbol : class, Symbol, IFieldSymbolInternal - where Symbol : class, ISymbolInternal { AttributeInfo info; @@ -1085,9 +1079,9 @@ internal ObsoleteAttributeData TryGetDeprecatedOrExperimentalOrObsoleteAttribute } info = FindTargetAttribute(token, AttributeDescription.ObsoleteAttribute); - if (info.HasValue && decoder.GetCustomAttribute(info.Handle, out TypedConstant[] positionalArgs, out KeyValuePair[] namedArgs, out bool[] namedArgIsProperty)) + if (info.HasValue) { - ObsoleteAttributeData obsoleteData = TryExtractObsoleteDataFromAttribute(positionalArgs, namedArgs, namedArgIsProperty); + ObsoleteAttributeData obsoleteData = TryExtractObsoleteDataFromAttribute(info, decoder); switch (obsoleteData?.Message) { case ByRefLikeMarker when ignoreByRefLikeMarker: @@ -1320,161 +1314,77 @@ private ArrayBuilder ExtractStringValuesFromAttributes(List[] namedArgs, bool[] namedArgIsProperty) + private ObsoleteAttributeData? TryExtractObsoleteDataFromAttribute(AttributeInfo attributeInfo, IAttributeNamedArgumentDecoder decoder) { - var (diagnosticId, urlFormat) = ExtractObsoleteProperties(namedArgs, namedArgIsProperty); - switch (positionalArgs.Length) + Debug.Assert(attributeInfo.HasValue); + if (!TryGetAttributeReader(attributeInfo.Handle, out var sig)) + { + return null; + } + + string? message = null; + bool isError = false; + switch (attributeInfo.SignatureIndex) { case 0: // ObsoleteAttribute() - return new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, message: null, isError: false, diagnosticId, urlFormat); + break; case 1: // ObsoleteAttribute(string) - return new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, positionalArgs[0].ValueInternal as string, isError: false, diagnosticId, urlFormat); - case 2 when positionalArgs[1].ValueInternal is bool isError: + if (sig.RemainingBytes > 0 && CrackStringInAttributeValue(out message, ref sig)) + { + break; + } + + return null; + case 2: // ObsoleteAttribute(string, bool) - return new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, positionalArgs[0].ValueInternal as string, isError, diagnosticId, urlFormat); - default: + if (sig.RemainingBytes > 0 && CrackStringInAttributeValue(out message, ref sig) && + sig.RemainingBytes > 0 && CrackBooleanInAttributeValue(out isError, ref sig)) + { + break; + } + return null; + default: + throw ExceptionUtilities.UnexpectedValue(attributeInfo.SignatureIndex); } - } - /// - /// Decodes attribute argument type from attribute blob (called FieldOrPropType in the spec). - /// - /// If the encoded argument type is invalid. - /// An exception from metadata reader. - private static (SerializationTypeCode typeCode, SerializationTypeCode elementTypeCode) DecodeCustomAttributeFieldOrPropTypeOrThrow(ref BlobReader argReader) - { - var typeCode = argReader.ReadSerializationTypeCode(); - - // Spec: - // The FieldOrPropType (typeCode) shall be exactly one of: ELEMENT_TYPE_BOOLEAN, - // ELEMENT_TYPE_CHAR, ELEMENT_TYPE_I1, ELEMENT_TYPE_U1, ELEMENT_TYPE_I2, - // ELEMENT_TYPE_U2, ELEMENT_TYPE_I4, ELEMENT_TYPE_U4, ELEMENT_TYPE_I8, - // ELEMENT_TYPE_U8, ELEMENT_TYPE_R4, ELEMENT_TYPE_R8, ELEMENT_TYPE_STRING. - // - // A single-dimensional, zero-based array is specified as a single byte 0x1D followed - // by the FieldOrPropType of the element type. (See §II.23.1.16) An enum is - // specified as a single byte 0x55 followed by a SerString. - // - - // todo: this could be simplified for the "skip" case. - if (typeCode == SerializationTypeCode.SZArray) + try { - var elementTypeCode = argReader.ReadSerializationTypeCode(); - if (elementTypeCode == SerializationTypeCode.SZArray) - { - // nested array not allowed: - throw new UnsupportedSignatureContent(); - } - - return (typeCode, elementTypeCode); + (string? diagnosticId, string? urlFormat) = sig.RemainingBytes > 0 ? CrackObsoleteProperties(ref sig, decoder) : default; + return new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, message, isError, diagnosticId, urlFormat); } - - switch (typeCode) + catch (Exception e) when (e is BadImageFormatException || e is UnsupportedSignatureContent) { - case SerializationTypeCode.Enum: - if (!CrackStringInAttributeValue(out _, ref argReader)) - { - throw new UnsupportedSignatureContent(); - } - - return (typeCode, elementTypeCode: SerializationTypeCode.Invalid); - - case SerializationTypeCode.TaggedObject: - case SerializationTypeCode.Type: - case SerializationTypeCode.String: - case SerializationTypeCode.Boolean: - case SerializationTypeCode.Char: - case SerializationTypeCode.SByte: - case SerializationTypeCode.Byte: - case SerializationTypeCode.Int16: - case SerializationTypeCode.UInt16: - case SerializationTypeCode.Int32: - case SerializationTypeCode.UInt32: - case SerializationTypeCode.Int64: - case SerializationTypeCode.UInt64: - case SerializationTypeCode.Single: - case SerializationTypeCode.Double: - return (typeCode, elementTypeCode: SerializationTypeCode.Invalid); + return null; } - - throw new UnsupportedSignatureContent(); } - /// If the encoded attribute argument is invalid. - /// An exception from metadata reader. - private static void SkipCustomAttributeElementOrThrow(ref BlobReader argReader, SerializationTypeCode typeCode) + private bool TryGetAttributeReader(CustomAttributeHandle handle, out BlobReader blobReader) { - if (typeCode == SerializationTypeCode.TaggedObject) + Debug.Assert(!handle.IsNil); + try { - // Spec: If the parameter kind is System.Object, the value stored represents the "boxed" instance of that value-type. - var (argTypeCode, elementTypeCode) = DecodeCustomAttributeFieldOrPropTypeOrThrow(ref argReader); - - if (argTypeCode == SerializationTypeCode.SZArray) + var valueBlob = GetCustomAttributeValueOrThrow(handle); + if (!valueBlob.IsNil) { - SkipCustomAttributeElementArrayOrThrow(ref argReader, elementTypeCode); - return; - } - } - - SkipCustomAttributePrimitiveElementOrThrow(ref argReader, typeCode); - } - - /// If the given is invalid. - /// An exception from metadata reader. - private static void SkipCustomAttributePrimitiveElementOrThrow(ref BlobReader argReader, SerializationTypeCode typeCode) - { - switch (typeCode) - { - case SerializationTypeCode.Boolean: - case SerializationTypeCode.SByte: - case SerializationTypeCode.Byte: - argReader.ReadByte(); - break; - - case SerializationTypeCode.Int16: - case SerializationTypeCode.UInt16: - case SerializationTypeCode.Char: - argReader.ReadInt16(); - break; - - case SerializationTypeCode.Int32: - case SerializationTypeCode.UInt32: - case SerializationTypeCode.Single: - argReader.ReadInt32(); - break; - - case SerializationTypeCode.Int64: - case SerializationTypeCode.UInt64: - case SerializationTypeCode.Double: - argReader.ReadInt64(); - break; - - case SerializationTypeCode.String: - case SerializationTypeCode.Type: - if (!CrackStringInAttributeValue(out _, ref argReader)) + blobReader = MetadataReader.GetBlobReader(valueBlob); + if (blobReader.Length >= 4) { - throw new UnsupportedSignatureContent(); + // check prolog + if (blobReader.ReadInt16() == 1) + { + return true; + } } - - break; - - default: - throw new UnsupportedSignatureContent(); + } } - } + catch (BadImageFormatException) + { } - /// If the encoded attribute argument is invalid. - /// An exception from metadata reader. - private static void SkipCustomAttributeElementArrayOrThrow(ref BlobReader argReader, SerializationTypeCode elementTypeCode) - { - int count = argReader.ReadInt32(); - for (int i = 0; i < count; i++) - { - SkipCustomAttributeElementOrThrow(ref argReader, elementTypeCode); - } + blobReader = default; + return false; } #nullable restore @@ -1759,29 +1669,33 @@ internal bool IsNoPiaLocalType( } #nullable enable - /// /// Gets the well-known optional named properties on ObsoleteAttribute, if present. /// Both 'diagnosticId' and 'urlFormat' may be present, or only one, or neither. /// - private static (string? diagnosticId, string? urlFormat) ExtractObsoleteProperties(KeyValuePair[] namedArgs, bool[] namedArgIsProperty) + /// + /// Failure to find any of these properties does not imply failure to decode the ObsoleteAttribute, + /// so we don't return a value indicating success or failure. + /// + private static (string? diagnosticId, string? urlFormat) CrackObsoleteProperties(ref BlobReader sig, IAttributeNamedArgumentDecoder decoder) { string? diagnosticId = null; string? urlFormat = null; - for (var i = 0; i < namedArgs.Length && (diagnosticId is null || urlFormat is null); i++) + // See CIL spec section II.23.3 Custom attributes + + // Next is a description of the optional “named” fields and properties. + // This starts with NumNamed– an unsigned int16 giving the number of “named” properties or fields that follow. + var numNamed = sig.ReadUInt16(); + for (int i = 0; i < numNamed && (diagnosticId is null || urlFormat is null); i++) { - var (name, value) = namedArgs[i]; - if (namedArgIsProperty[i]) + var ((name, value), isProperty) = decoder.DecodeCustomAttributeNamedArgumentOrThrow(ref sig); + if (value.Type?.SpecialType == SpecialType.System_String && isProperty) { if (diagnosticId is null && name == "DiagnosticId") - { - diagnosticId = value.ValueInternal as string; - } + diagnosticId = (string?)value.ValueInternal; else if (urlFormat is null && name == "UrlFormat") - { - urlFormat = value.ValueInternal as string; - } + urlFormat = (string?)value.ValueInternal; } } From 92fe2f229014c7cf7281ccaf1fd24a64039ccb53 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 25 Mar 2020 16:54:45 -0700 Subject: [PATCH 08/21] Add duplicate argument test --- .../AttributeTests_WellKnownAttributes.cs | 143 +++++++++++++++++- 1 file changed, 141 insertions(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs index d59e74a23c496..4ed44e1169e14 100644 --- a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs +++ b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs @@ -8821,7 +8821,7 @@ public class ObsoleteAttribute : Attribute public class C1 { - [Obsolete(DiagnosticId = new[] { 'A' }, UrlFormat = new[] { 'B' })] + [Obsolete(DiagnosticId = new[] { 'A' })] public void M1() { } } "; @@ -8855,8 +8855,147 @@ void verify(MetadataReference reference) } } + [Fact, WorkItem(42119, "https://github.com/dotnet/roslyn/issues/42119")] + public void Obsolete_CustomDiagnosticId_BadMetadata_06() + { + // In this program C1.M1 has an ObsoleteAttribute with multiple values provided for DiagnosticId and UrlFormat + var ilSource = @" +.assembly extern mscorlib +{ + .publickeytoken = (B7 7A 5C 56 19 34 E0 89 ) // .z\V.4.. + .ver 4:0:0:0 +} + +.class public auto ansi beforefieldinit C1 + extends [mscorlib]System.Object +{ + .method public hidebysig instance void + M1() cil managed + { + .custom instance void System.ObsoleteAttribute::.ctor() = ( 01 00 04 00 // .... + 54 0E 0C 44 69 61 67 6E 6F 73 74 69 63 49 64 01 41 // T..DiagnosticId.A + 54 0E 0C 44 69 61 67 6E 6F 73 74 69 63 49 64 01 42 // T..DiagnosticId.B + 54 0E 09 55 72 6C 46 6F 72 6D 61 74 01 43 // T..UrlFormat.C + 54 0E 09 55 72 6C 46 6F 72 6D 61 74 01 44 ) // T..UrlFormat.D + // Code size 2 (0x2) + .maxstack 8 + IL_0000: nop + IL_0001: ret + } // end of method C1::M1 + + .method public hidebysig specialname rtspecialname + instance void .ctor() cil managed + { + // Code size 8 (0x8) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: call instance void [mscorlib]System.Object::.ctor() + IL_0006: nop + IL_0007: ret + } // end of method C1::.ctor + +} // end of class C1 + +.class public auto ansi beforefieldinit System.ObsoleteAttribute + extends [mscorlib]System.Attribute +{ + .field private string 'k__BackingField' + .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + .custom instance void [mscorlib]System.Diagnostics.DebuggerBrowsableAttribute::.ctor(valuetype [mscorlib]System.Diagnostics.DebuggerBrowsableState) = ( 01 00 00 00 00 00 00 00 ) + .field private string 'k__BackingField' + .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + .custom instance void [mscorlib]System.Diagnostics.DebuggerBrowsableAttribute::.ctor(valuetype [mscorlib]System.Diagnostics.DebuggerBrowsableState) = ( 01 00 00 00 00 00 00 00 ) + .method public hidebysig specialname instance string + get_DiagnosticId() cil managed + { + .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + // Code size 7 (0x7) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: ldfld string System.ObsoleteAttribute::'k__BackingField' + IL_0006: ret + } // end of method ObsoleteAttribute::get_DiagnosticId + + .method public hidebysig specialname instance void + set_DiagnosticId(string 'value') cil managed + { + .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + // Code size 8 (0x8) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: ldarg.1 + IL_0002: stfld string System.ObsoleteAttribute::'k__BackingField' + IL_0007: ret + } // end of method ObsoleteAttribute::set_DiagnosticId + + .method public hidebysig specialname instance string + get_UrlFormat() cil managed + { + .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + // Code size 7 (0x7) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: ldfld string System.ObsoleteAttribute::'k__BackingField' + IL_0006: ret + } // end of method ObsoleteAttribute::get_UrlFormat + + .method public hidebysig specialname instance void + set_UrlFormat(string 'value') cil managed + { + .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + // Code size 8 (0x8) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: ldarg.1 + IL_0002: stfld string System.ObsoleteAttribute::'k__BackingField' + IL_0007: ret + } // end of method ObsoleteAttribute::set_UrlFormat + + .method public hidebysig specialname rtspecialname + instance void .ctor() cil managed + { + // Code size 8 (0x8) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: call instance void [mscorlib]System.Attribute::.ctor() + IL_0006: nop + IL_0007: ret + } // end of method ObsoleteAttribute::.ctor + + .property instance string DiagnosticId() + { + .get instance string System.ObsoleteAttribute::get_DiagnosticId() + .set instance void System.ObsoleteAttribute::set_DiagnosticId(string) + } // end of property ObsoleteAttribute::DiagnosticId + .property instance string UrlFormat() + { + .get instance string System.ObsoleteAttribute::get_UrlFormat() + .set instance void System.ObsoleteAttribute::set_UrlFormat(string) + } // end of property ObsoleteAttribute::UrlFormat +} // end of class System.ObsoleteAttribute +"; + + var csSource = @" +class C2 : C1 +{ + void M2() + { + M1(); // 1 + } +}"; + var ilComp = CompileIL(ilSource); + var comp = CreateCompilation(csSource, references: new[] { ilComp }); + var diags = comp.GetDiagnostics(); + diags.Verify( + // (6,9): warning A: 'C1.M1()' is obsolete + // M1(); // 1 + Diagnostic("A", "M1()").WithArguments("C1.M1()").WithLocation(6, 9)); + + var diag = diags.Single(); + Assert.Equal("C", diag.Descriptor.HelpLinkUri); + } + // Missing tests: - // - duplicate named arguments // - named argument has well-known name but bad type, non-null value // - named argument has well-known name but bad type, null value // - ?? From 8558e502d603027268e3775168239902426d401b Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 25 Mar 2020 18:00:43 -0700 Subject: [PATCH 09/21] Add abstract IsProperty helper --- .../Emitter/Model/AttributeDataAdapter.cs | 16 +++++++++++++ .../Symbols/Attributes/CommonAttributeData.cs | 24 ++++++------------- .../Symbols/Attributes/AttributeData.vb | 12 ++++++++++ 3 files changed, 35 insertions(+), 17 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Emitter/Model/AttributeDataAdapter.cs b/src/Compilers/CSharp/Portable/Emitter/Model/AttributeDataAdapter.cs index 19aaa90cb23d1..528c35843f486 100644 --- a/src/Compilers/CSharp/Portable/Emitter/Model/AttributeDataAdapter.cs +++ b/src/Compilers/CSharp/Portable/Emitter/Model/AttributeDataAdapter.cs @@ -173,6 +173,22 @@ private Cci.IMetadataNamedArgument CreateMetadataNamedArgument(string name, Type return new MetadataNamedArgument(symbol, moduleBeingBuilt.Translate(type, syntaxNodeOpt: (CSharpSyntaxNode)context.SyntaxNodeOpt, diagnostics: context.Diagnostics), value); } + private protected override bool IsProperty(string memberName) + { + if (AttributeClass is object) + { + foreach (var member in AttributeClass.GetMembers(memberName)) + { + if (member.Kind == SymbolKind.Property) + { + return true; + } + } + } + + return false; + } + private Symbol LookupName(string name) { var type = this.AttributeClass; diff --git a/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs b/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs index 8ef8f0556de21..0b9424d09b74a 100644 --- a/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs +++ b/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs @@ -301,10 +301,10 @@ private ObsoleteAttributeData DecodeObsoleteAttribute() { switch (pair.Key) { - case "DiagnosticId" when isProperty("DiagnosticId"): + case "DiagnosticId" when IsProperty("DiagnosticId"): diagnosticId = pair.Value.ValueInternal as string; break; - case "UrlFormat" when isProperty("UrlFormat"): + case "UrlFormat" when IsProperty("UrlFormat"): urlFormat = pair.Value.ValueInternal as string; break; default: @@ -319,23 +319,13 @@ private ObsoleteAttributeData DecodeObsoleteAttribute() } return new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, message, isError, diagnosticId, urlFormat); - - // note: it is disallowed to declare a property and a field - // with the same name in C# or VB source, even if it is allowed in IL. - bool isProperty(string name) - { - foreach (var member in AttributeClass.GetMembers(name)) - { - if (member.Kind == SymbolKind.Property) - { - return true; - } - } - - return false; - } } + // Note: it is disallowed to declare a property and a field + // with the same name in C# or VB source, even if it is allowed in IL. + // We use an abstract method to prevent having to realize the public symbols just to decode obsolete attributes. + private protected abstract bool IsProperty(string memberName); + /// /// Decode the arguments to DeprecatedAttribute. DeprecatedAttribute can have 3 or 4 arguments. /// diff --git a/src/Compilers/VisualBasic/Portable/Symbols/Attributes/AttributeData.vb b/src/Compilers/VisualBasic/Portable/Symbols/Attributes/AttributeData.vb index e63da252f40fd..cb6abfd33178a 100644 --- a/src/Compilers/VisualBasic/Portable/Symbols/Attributes/AttributeData.vb +++ b/src/Compilers/VisualBasic/Portable/Symbols/Attributes/AttributeData.vb @@ -488,6 +488,18 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols Return Me.GetConstructorArgument(Of String)(0, SpecialType.System_String) End Function + + Private Protected Overrides Function IsProperty(memberName As String) As Boolean + If AttributeClass IsNot Nothing Then + For Each member In AttributeClass.GetMembers(memberName) + If member.Kind = SymbolKind.Property Then + Return True + End If + Next + End If + + Return False + End Function #End Region ''' From 6fd34b48922375fbfcdf8a8fda54268a0b8f3c5e Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 25 Mar 2020 18:19:23 -0700 Subject: [PATCH 10/21] Add missing tests --- .../Emitter/Model/AttributeDataAdapter.cs | 4 +- .../AttributeTests_WellKnownAttributes.cs | 101 ++++++++++++++++-- .../Symbols/Attributes/CommonAttributeData.cs | 6 +- .../Symbols/Attributes/AttributeData.vb | 5 +- 4 files changed, 103 insertions(+), 13 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Emitter/Model/AttributeDataAdapter.cs b/src/Compilers/CSharp/Portable/Emitter/Model/AttributeDataAdapter.cs index 528c35843f486..71206afd345c9 100644 --- a/src/Compilers/CSharp/Portable/Emitter/Model/AttributeDataAdapter.cs +++ b/src/Compilers/CSharp/Portable/Emitter/Model/AttributeDataAdapter.cs @@ -173,13 +173,13 @@ private Cci.IMetadataNamedArgument CreateMetadataNamedArgument(string name, Type return new MetadataNamedArgument(symbol, moduleBeingBuilt.Translate(type, syntaxNodeOpt: (CSharpSyntaxNode)context.SyntaxNodeOpt, diagnostics: context.Diagnostics), value); } - private protected override bool IsProperty(string memberName) + private protected override bool IsStringProperty(string memberName) { if (AttributeClass is object) { foreach (var member in AttributeClass.GetMembers(memberName)) { - if (member.Kind == SymbolKind.Property) + if (member is PropertySymbol { Type: { SpecialType: SpecialType.System_String } }) { return true; } diff --git a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs index 4ed44e1169e14..3ac68b4b0e6bb 100644 --- a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs +++ b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs @@ -8573,6 +8573,49 @@ void verify(MetadataReference reference) } } + [Fact, WorkItem(42119, "https://github.com/dotnet/roslyn/issues/42119")] + public void Obsolete_CustomDiagnosticId_FromMetadata_04() + { + var source1 = @" +using System; +#pragma warning disable 436 + +public class C1 +{ + [Obsolete(DiagnosticId = null, UrlFormat = null)] + public void M1() { } +} +"; + + var source2 = @" +class C2 : C1 +{ + void M2() + { + M1(); // 1 + } +}"; + var comp1 = CreateCompilation(new[] { ObsoleteAttributeSource, source1 }); + comp1.VerifyDiagnostics(); + + verify(comp1.ToMetadataReference()); + verify(comp1.EmitToImageReference()); + + void verify(MetadataReference reference) + { + var comp2 = CreateCompilation(source2, references: new[] { reference }); + var diags = comp2.GetDiagnostics(); + + var diag = diags.Single(); + Assert.Equal("", diag.Descriptor.HelpLinkUri); + + diags.Verify( + // (6,9): warning CS0612: 'C1.M1()' is obsolete + // M1(); // 1 + Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "M1()").WithArguments("C1.M1()").WithLocation(6, 9)); + } + } + [Fact, WorkItem(42119, "https://github.com/dotnet/roslyn/issues/42119")] public void Obsolete_CustomDiagnosticId_BadMetadata_01() { @@ -8821,7 +8864,7 @@ public class ObsoleteAttribute : Attribute public class C1 { - [Obsolete(DiagnosticId = new[] { 'A' })] + [Obsolete(DiagnosticId = new[] { 'A' }, UrlFormat = new[] { 'B' })] public void M1() { } } "; @@ -8857,6 +8900,57 @@ void verify(MetadataReference reference) [Fact, WorkItem(42119, "https://github.com/dotnet/roslyn/issues/42119")] public void Obsolete_CustomDiagnosticId_BadMetadata_06() + { + var source1 = @" +using System; +#pragma warning disable 436 + +namespace System +{ + public class ObsoleteAttribute : Attribute + { + public char[] DiagnosticId { get; set; } + public char[] UrlFormat { get; set; } + } +} + +public class C1 +{ + [Obsolete(DiagnosticId = null, UrlFormat = null)] + public void M1() { } +} +"; + + var source2 = @" +class C2 : C1 +{ + void M2() + { + M1(); // 1 + } +}"; + var comp1 = CreateCompilation(source1); + comp1.VerifyDiagnostics(); + + verify(comp1.ToMetadataReference()); + verify(comp1.EmitToImageReference()); + + void verify(MetadataReference reference) + { + var comp2 = CreateCompilation(source2, references: new[] { reference }); + var diags = comp2.GetDiagnostics(); + diags.Verify( + // (6,9): warning CS0612: 'C1.M1()' is obsolete + // M1(); // 1 + Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "M1()").WithArguments("C1.M1()").WithLocation(6, 9)); + + var diag = diags.Single(); + Assert.Equal("", diag.Descriptor.HelpLinkUri); + } + } + + [Fact, WorkItem(42119, "https://github.com/dotnet/roslyn/issues/42119")] + public void Obsolete_CustomDiagnosticId_BadMetadata_07() { // In this program C1.M1 has an ObsoleteAttribute with multiple values provided for DiagnosticId and UrlFormat var ilSource = @" @@ -8995,11 +9089,6 @@ void M2() Assert.Equal("C", diag.Descriptor.HelpLinkUri); } - // Missing tests: - // - named argument has well-known name but bad type, non-null value - // - named argument has well-known name but bad type, null value - // - ?? - [Fact] [WorkItem(656345, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/656345")] public void ConditionalLazyObsoleteDiagnostic() diff --git a/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs b/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs index 0b9424d09b74a..bf54d7218f21a 100644 --- a/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs +++ b/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs @@ -301,10 +301,10 @@ private ObsoleteAttributeData DecodeObsoleteAttribute() { switch (pair.Key) { - case "DiagnosticId" when IsProperty("DiagnosticId"): + case "DiagnosticId" when IsStringProperty("DiagnosticId"): diagnosticId = pair.Value.ValueInternal as string; break; - case "UrlFormat" when IsProperty("UrlFormat"): + case "UrlFormat" when IsStringProperty("UrlFormat"): urlFormat = pair.Value.ValueInternal as string; break; default: @@ -324,7 +324,7 @@ private ObsoleteAttributeData DecodeObsoleteAttribute() // Note: it is disallowed to declare a property and a field // with the same name in C# or VB source, even if it is allowed in IL. // We use an abstract method to prevent having to realize the public symbols just to decode obsolete attributes. - private protected abstract bool IsProperty(string memberName); + private protected abstract bool IsStringProperty(string memberName); /// /// Decode the arguments to DeprecatedAttribute. DeprecatedAttribute can have 3 or 4 arguments. diff --git a/src/Compilers/VisualBasic/Portable/Symbols/Attributes/AttributeData.vb b/src/Compilers/VisualBasic/Portable/Symbols/Attributes/AttributeData.vb index cb6abfd33178a..92718def704cc 100644 --- a/src/Compilers/VisualBasic/Portable/Symbols/Attributes/AttributeData.vb +++ b/src/Compilers/VisualBasic/Portable/Symbols/Attributes/AttributeData.vb @@ -489,10 +489,11 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols Return Me.GetConstructorArgument(Of String)(0, SpecialType.System_String) End Function - Private Protected Overrides Function IsProperty(memberName As String) As Boolean + Private Protected Overrides Function IsStringProperty(memberName As String) As Boolean If AttributeClass IsNot Nothing Then For Each member In AttributeClass.GetMembers(memberName) - If member.Kind = SymbolKind.Property Then + Dim prop = TryCast(member, PropertySymbol) + If prop?.Type.SpecialType = SpecialType.System_String Then Return True End If Next From 86c5e4cab998d145de642338f4d82b6956c16d97 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 25 Mar 2020 18:32:00 -0700 Subject: [PATCH 11/21] VB WIP --- .../AttributeTests_ObsoleteAttribute.vb | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/src/Compilers/VisualBasic/Test/Emit/Attributes/AttributeTests_ObsoleteAttribute.vb b/src/Compilers/VisualBasic/Test/Emit/Attributes/AttributeTests_ObsoleteAttribute.vb index 250552b070efc..ba47d5146ee17 100644 --- a/src/Compilers/VisualBasic/Test/Emit/Attributes/AttributeTests_ObsoleteAttribute.vb +++ b/src/Compilers/VisualBasic/Test/Emit/Attributes/AttributeTests_ObsoleteAttribute.vb @@ -1146,6 +1146,52 @@ End Class Diagnostic(ERRID.WRN_UseOfObsoleteSymbolNoMessage1, "a.b.c2(Of Integer).E(Of Integer)").WithArguments("A.B.C2(Of Integer).E(Of Integer)")) End Sub + + Public Sub TestObsoleteAttributeCustomProperties() + Dim source = + + + Sub M1() + End Sub + + Sub M2() + M1() + End Sub +End Class + +Namespace System + Public Class ObsoleteAttribute + Inherits Attribute + + Public Sub New() + End Sub + + Public Sub New(message As String) + End Sub + + Public Sub New(message As String, isError As Boolean) + End Sub + + Public Property DiagnosticId As String + Public Property UrlFormat As String + End Class +End Namespace + +]]> + + + + Dim comp = CreateCompilationWithMscorlib40(source) + Dim diags = comp.GetDiagnostics() + diags.Verify(Diagnostic("TEST1", "M1()").WithArguments("Public Sub M1()").WithLocation(9, 9)) + + Dim diag = diags.Single() + Assert.Equal("TEST2", diag.Descriptor.HelpLinkUri) + End Sub + Public Sub TestObsoleteInAlias() From fb7d4209452b9542aff420cb98b7985bc8ca74e0 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Thu, 26 Mar 2020 12:03:15 -0700 Subject: [PATCH 12/21] add more tests, fix build error --- .../AttributeTests_WellKnownAttributes.cs | 251 +++++++++++++++++- .../Core/Portable/MetadataReader/PEModule.cs | 32 ++- .../Symbols/Attributes/CommonAttributeData.cs | 22 +- 3 files changed, 274 insertions(+), 31 deletions(-) diff --git a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs index 3ac68b4b0e6bb..e73aa81dce5e1 100644 --- a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs +++ b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs @@ -8338,7 +8338,7 @@ void M2() } [Fact, WorkItem(42119, "https://github.com/dotnet/roslyn/issues/42119")] - public void Obsolete_CustomDiagnosticId_99() + public void Obsolete_CustomDiagnosticId_05() { var source = @" using System; @@ -8370,6 +8370,56 @@ void M2() Diagnostic("TEST1", "M1()").WithArguments("C.M1()", "don't use").WithLocation(12, 9)); } + [Fact, WorkItem(42119, "https://github.com/dotnet/roslyn/issues/42119")] + public void Obsolete_CustomDiagnosticId_BadAttribute_02() + { + var source = @" +using System; +#pragma warning disable 436 + +namespace System +{ + public class ObsoleteAttribute : Attribute + { + public string DiagnosticId { get; set; } + public string UrlFormat { get; set; } + } +} + +class C +{ + [Obsolete( + DiagnosticId = ""A"", + DiagnosticId = ""B"", // 1 + UrlFormat = ""C"", + UrlFormat = ""D"")] // 2 + void M1() { } + + void M2() + { + M1(); // 3 + } +} +"; + + var comp = CreateCompilation(source); + var diags = comp.GetDiagnostics(); + + diags.Verify( + // (18,9): error CS0643: 'DiagnosticId' duplicate named attribute argument + // DiagnosticId = "B", // 1 + Diagnostic(ErrorCode.ERR_DuplicateNamedAttributeArgument, @"DiagnosticId = ""B""").WithArguments("DiagnosticId").WithLocation(18, 9), + // (20,9): error CS0643: 'UrlFormat' duplicate named attribute argument + // UrlFormat = "D")] // 2 + Diagnostic(ErrorCode.ERR_DuplicateNamedAttributeArgument, @"UrlFormat = ""D""").WithArguments("UrlFormat").WithLocation(20, 9), + // (25,9): warning A: 'C.M1()' is obsolete + // M1(); // 3 + Diagnostic("A", "M1()").WithArguments("C.M1()").WithLocation(25, 9)); + + var diag = diags.Last(); + Assert.Equal("C", diag.Descriptor.HelpLinkUri); + } + [Fact, WorkItem(42119, "https://github.com/dotnet/roslyn/issues/42119")] public void Obsolete_CustomDiagnosticId_Suppression_01() { @@ -9089,6 +9139,205 @@ void M2() Assert.Equal("C", diag.Descriptor.HelpLinkUri); } + [Fact, WorkItem(42119, "https://github.com/dotnet/roslyn/issues/42119")] + public void Obsolete_CustomDiagnosticId_BadMetadata_08() + { + // In this program C1.M1 has an ObsoleteAttribute with a malformed value provided for a named argument + var ilSource = @" +.assembly extern mscorlib +{ + .publickeytoken = (B7 7A 5C 56 19 34 E0 89 ) // .z\V.4.. + .ver 4:0:0:0 +} + +.class public auto ansi beforefieldinit C1 + extends [mscorlib]System.Object +{ + .method public hidebysig instance void + M1() cil managed + { + .custom instance void System.ObsoleteAttribute::.ctor() = ( 01 00 02 00 // .... + 54 0E 0C 44 69 61 67 6E 6F 73 74 69 63 49 64 01 41 // T..DiagnosticId.A + 0E 09 55 72 6C 46 6F 72 6D 61 74 01 42 ) // ..UrlFormat.B + // Code size 2 (0x2) + .maxstack 8 + IL_0000: nop + IL_0001: ret + } // end of method C1::M1 + + .method public hidebysig specialname rtspecialname + instance void .ctor() cil managed + { + // Code size 8 (0x8) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: call instance void [mscorlib]System.Object::.ctor() + IL_0006: nop + IL_0007: ret + } // end of method C1::.ctor + +} // end of class C1 + +.class public auto ansi beforefieldinit System.ObsoleteAttribute + extends [mscorlib]System.Attribute +{ + .field private string 'k__BackingField' + .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + .custom instance void [mscorlib]System.Diagnostics.DebuggerBrowsableAttribute::.ctor(valuetype [mscorlib]System.Diagnostics.DebuggerBrowsableState) = ( 01 00 00 00 00 00 00 00 ) + .field private string 'k__BackingField' + .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + .custom instance void [mscorlib]System.Diagnostics.DebuggerBrowsableAttribute::.ctor(valuetype [mscorlib]System.Diagnostics.DebuggerBrowsableState) = ( 01 00 00 00 00 00 00 00 ) + .method public hidebysig specialname instance string + get_DiagnosticId() cil managed + { + .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + // Code size 7 (0x7) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: ldfld string System.ObsoleteAttribute::'k__BackingField' + IL_0006: ret + } // end of method ObsoleteAttribute::get_DiagnosticId + + .method public hidebysig specialname instance void + set_DiagnosticId(string 'value') cil managed + { + .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + // Code size 8 (0x8) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: ldarg.1 + IL_0002: stfld string System.ObsoleteAttribute::'k__BackingField' + IL_0007: ret + } // end of method ObsoleteAttribute::set_DiagnosticId + + .method public hidebysig specialname instance string + get_UrlFormat() cil managed + { + .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + // Code size 7 (0x7) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: ldfld string System.ObsoleteAttribute::'k__BackingField' + IL_0006: ret + } // end of method ObsoleteAttribute::get_UrlFormat + + .method public hidebysig specialname instance void + set_UrlFormat(string 'value') cil managed + { + .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + // Code size 8 (0x8) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: ldarg.1 + IL_0002: stfld string System.ObsoleteAttribute::'k__BackingField' + IL_0007: ret + } // end of method ObsoleteAttribute::set_UrlFormat + + .method public hidebysig specialname rtspecialname + instance void .ctor() cil managed + { + // Code size 8 (0x8) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: call instance void [mscorlib]System.Attribute::.ctor() + IL_0006: nop + IL_0007: ret + } // end of method ObsoleteAttribute::.ctor + + .property instance string DiagnosticId() + { + .get instance string System.ObsoleteAttribute::get_DiagnosticId() + .set instance void System.ObsoleteAttribute::set_DiagnosticId(string) + } // end of property ObsoleteAttribute::DiagnosticId + .property instance string UrlFormat() + { + .get instance string System.ObsoleteAttribute::get_UrlFormat() + .set instance void System.ObsoleteAttribute::set_UrlFormat(string) + } // end of property ObsoleteAttribute::UrlFormat +} // end of class System.ObsoleteAttribute +"; + + var csSource = @" +class C2 : C1 +{ + void M2() + { + M1(); // 1 + } +}"; + var ilComp = CompileIL(ilSource); + var comp = CreateCompilation(csSource, references: new[] { ilComp }); + var diags = comp.GetDiagnostics(); + diags.Verify( + // (6,9): warning A: 'C1.M1()' is obsolete + // M1(); // 1 + Diagnostic("A", "M1()").WithArguments("C1.M1()").WithLocation(6, 9)); + + var diag = diags.Single(); + Assert.Equal("", diag.Descriptor.HelpLinkUri); + } + + [Fact, WorkItem(42119, "https://github.com/dotnet/roslyn/issues/42119")] + public void Obsolete_CustomDiagnosticId_BadMetadata_09() + { + var source1 = @" +using System; +#pragma warning disable 436 + +namespace System +{ + public class ObsoleteAttribute : Attribute + { + public object DiagnosticId { get; set; } + public object UrlFormat { get; set; } + } +} + +public class C1 +{ + [Obsolete(DiagnosticId = ""A"", UrlFormat = ""B"")] + public void M1() { } +} +"; + + var source2 = @" +class C2 : C1 +{ + void M2() + { + M1(); // 1 + } +}"; + var comp1 = CreateCompilation(source1); + comp1.VerifyDiagnostics(); + + + var comp2 = CreateCompilation(source2, references: new[] { comp1.ToMetadataReference() }); + var diags = comp2.GetDiagnostics(); + + diags.Verify( + // (6,9): warning CS0612: 'C1.M1()' is obsolete + // M1(); // 1 + Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "M1()").WithArguments("C1.M1()").WithLocation(6, 9)); + + var diag = diags.Single(); + Assert.Equal("", diag.Descriptor.HelpLinkUri); + + + comp2 = CreateCompilation(source2, references: new[] { comp1.EmitToImageReference() }); + diags = comp2.GetDiagnostics(); + + // Perhaps we should not accept the arguments when the well-known property + // is of an unexpected type that is assignable from string. + diags.Verify( + // (6,9): warning A: 'C1.M1()' is obsolete + // M1(); // 1 + Diagnostic("A", "M1()").WithArguments("C1.M1()").WithLocation(6, 9)); + + diag = diags.Single(); + Assert.Equal("B", diag.Descriptor.HelpLinkUri); + } + [Fact] [WorkItem(656345, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/656345")] public void ConditionalLazyObsoleteDiagnostic() diff --git a/src/Compilers/Core/Portable/MetadataReader/PEModule.cs b/src/Compilers/Core/Portable/MetadataReader/PEModule.cs index 72a5e6f5c478a..65ac15a1bbd3e 100644 --- a/src/Compilers/Core/Portable/MetadataReader/PEModule.cs +++ b/src/Compilers/Core/Portable/MetadataReader/PEModule.cs @@ -1350,15 +1350,8 @@ private ArrayBuilder ExtractStringValuesFromAttributes(List 0 ? CrackObsoleteProperties(ref sig, decoder) : default; - return new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, message, isError, diagnosticId, urlFormat); - } - catch (Exception e) when (e is BadImageFormatException || e is UnsupportedSignatureContent) - { - return null; - } + (string? diagnosticId, string? urlFormat) = sig.RemainingBytes > 0 ? CrackObsoleteProperties(ref sig, decoder) : default; + return new ObsoleteAttributeData(ObsoleteAttributeKind.Obsolete, message, isError, diagnosticId, urlFormat); } private bool TryGetAttributeReader(CustomAttributeHandle handle, out BlobReader blobReader) @@ -1686,18 +1679,23 @@ private static (string? diagnosticId, string? urlFormat) CrackObsoleteProperties // Next is a description of the optional “named” fields and properties. // This starts with NumNamed– an unsigned int16 giving the number of “named” properties or fields that follow. - var numNamed = sig.ReadUInt16(); - for (int i = 0; i < numNamed && (diagnosticId is null || urlFormat is null); i++) + try { - var ((name, value), isProperty) = decoder.DecodeCustomAttributeNamedArgumentOrThrow(ref sig); - if (value.Type?.SpecialType == SpecialType.System_String && isProperty) + var numNamed = sig.ReadUInt16(); + for (int i = 0; i < numNamed && (diagnosticId is null || urlFormat is null); i++) { - if (diagnosticId is null && name == "DiagnosticId") - diagnosticId = (string?)value.ValueInternal; - else if (urlFormat is null && name == "UrlFormat") - urlFormat = (string?)value.ValueInternal; + var ((name, value), isProperty) = decoder.DecodeCustomAttributeNamedArgumentOrThrow(ref sig); + if (value.TypeInternal?.SpecialType == SpecialType.System_String && isProperty) + { + if (diagnosticId is null && name == "DiagnosticId") + diagnosticId = (string?)value.ValueInternal; + else if (urlFormat is null && name == "UrlFormat") + urlFormat = (string?)value.ValueInternal; + } } } + catch (BadImageFormatException) { } + catch (UnsupportedSignatureContent) { } return (diagnosticId, urlFormat); } diff --git a/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs b/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs index bf54d7218f21a..144e52b28ff7e 100644 --- a/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs +++ b/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs @@ -297,19 +297,15 @@ private ObsoleteAttributeData DecodeObsoleteAttribute() string? diagnosticId = null; string? urlFormat = null; - foreach (var pair in this.CommonNamedArguments) + foreach (var (name, value) in this.CommonNamedArguments) { - switch (pair.Key) + if (diagnosticId is null && name == "DiagnosticId" && IsStringProperty("DiagnosticId")) { - case "DiagnosticId" when IsStringProperty("DiagnosticId"): - diagnosticId = pair.Value.ValueInternal as string; - break; - case "UrlFormat" when IsStringProperty("UrlFormat"): - urlFormat = pair.Value.ValueInternal as string; - break; - default: - // unknown property or field specified on ObsoleteAttribute - break; + diagnosticId = value.ValueInternal as string; + } + else if (urlFormat is null && name == "UrlFormat" && IsStringProperty("UrlFormat")) + { + urlFormat = value.ValueInternal as string; } if (diagnosticId is object && urlFormat is object) @@ -323,8 +319,8 @@ private ObsoleteAttributeData DecodeObsoleteAttribute() // Note: it is disallowed to declare a property and a field // with the same name in C# or VB source, even if it is allowed in IL. - // We use an abstract method to prevent having to realize the public symbols just to decode obsolete attributes. - private protected abstract bool IsStringProperty(string memberName); + // We use a virtual method and override to prevent having to realize the public symbols just to decode obsolete attributes. + private protected virtual bool IsStringProperty(string memberName) => throw ExceptionUtilities.Unreachable; /// /// Decode the arguments to DeprecatedAttribute. DeprecatedAttribute can have 3 or 4 arguments. From eb0252488443c09470b230c7c9dd4a9bb6b508dd Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Thu, 26 Mar 2020 13:58:49 -0700 Subject: [PATCH 13/21] Seal overrides --- .../CSharp/Portable/Emitter/Model/AttributeDataAdapter.cs | 2 +- .../VisualBasic/Portable/Symbols/Attributes/AttributeData.vb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Emitter/Model/AttributeDataAdapter.cs b/src/Compilers/CSharp/Portable/Emitter/Model/AttributeDataAdapter.cs index 71206afd345c9..dec66beb93198 100644 --- a/src/Compilers/CSharp/Portable/Emitter/Model/AttributeDataAdapter.cs +++ b/src/Compilers/CSharp/Portable/Emitter/Model/AttributeDataAdapter.cs @@ -173,7 +173,7 @@ private Cci.IMetadataNamedArgument CreateMetadataNamedArgument(string name, Type return new MetadataNamedArgument(symbol, moduleBeingBuilt.Translate(type, syntaxNodeOpt: (CSharpSyntaxNode)context.SyntaxNodeOpt, diagnostics: context.Diagnostics), value); } - private protected override bool IsStringProperty(string memberName) + private protected sealed override bool IsStringProperty(string memberName) { if (AttributeClass is object) { diff --git a/src/Compilers/VisualBasic/Portable/Symbols/Attributes/AttributeData.vb b/src/Compilers/VisualBasic/Portable/Symbols/Attributes/AttributeData.vb index 92718def704cc..c8ef42b9f5883 100644 --- a/src/Compilers/VisualBasic/Portable/Symbols/Attributes/AttributeData.vb +++ b/src/Compilers/VisualBasic/Portable/Symbols/Attributes/AttributeData.vb @@ -489,7 +489,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols Return Me.GetConstructorArgument(Of String)(0, SpecialType.System_String) End Function - Private Protected Overrides Function IsStringProperty(memberName As String) As Boolean + Private Protected NotOverridable Overrides Function IsStringProperty(memberName As String) As Boolean If AttributeClass IsNot Nothing Then For Each member In AttributeClass.GetMembers(memberName) Dim prop = TryCast(member, PropertySymbol) From 3f871e7ec41bc9ad7e59750e2524c3bd57a4dfd2 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Thu, 26 Mar 2020 14:31:23 -0700 Subject: [PATCH 14/21] Fix non-string property with string argument --- .../AttributeTests_WellKnownAttributes.cs | 36 +++++++------------ .../MetadataReader/MetadataDecoder.cs | 10 +++--- .../Core/Portable/MetadataReader/PEModule.cs | 8 ++--- 3 files changed, 22 insertions(+), 32 deletions(-) diff --git a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs index e73aa81dce5e1..8afa4dd65d851 100644 --- a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs +++ b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs @@ -9311,31 +9311,21 @@ void M2() var comp1 = CreateCompilation(source1); comp1.VerifyDiagnostics(); + verify(comp1.ToMetadataReference()); + verify(comp1.EmitToImageReference()); + void verify(MetadataReference reference) + { + var comp2 = CreateCompilation(source2, references: new[] { reference }); + var diags = comp2.GetDiagnostics(); - var comp2 = CreateCompilation(source2, references: new[] { comp1.ToMetadataReference() }); - var diags = comp2.GetDiagnostics(); - - diags.Verify( - // (6,9): warning CS0612: 'C1.M1()' is obsolete - // M1(); // 1 - Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "M1()").WithArguments("C1.M1()").WithLocation(6, 9)); - - var diag = diags.Single(); - Assert.Equal("", diag.Descriptor.HelpLinkUri); - - - comp2 = CreateCompilation(source2, references: new[] { comp1.EmitToImageReference() }); - diags = comp2.GetDiagnostics(); - - // Perhaps we should not accept the arguments when the well-known property - // is of an unexpected type that is assignable from string. - diags.Verify( - // (6,9): warning A: 'C1.M1()' is obsolete - // M1(); // 1 - Diagnostic("A", "M1()").WithArguments("C1.M1()").WithLocation(6, 9)); + diags.Verify( + // (6,9): warning CS0612: 'C1.M1()' is obsolete + // M1(); // 1 + Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "M1()").WithArguments("C1.M1()").WithLocation(6, 9)); - diag = diags.Single(); - Assert.Equal("B", diag.Descriptor.HelpLinkUri); + var diag = diags.Single(); + Assert.Equal("", diag.Descriptor.HelpLinkUri); + } } [Fact] diff --git a/src/Compilers/Core/Portable/MetadataReader/MetadataDecoder.cs b/src/Compilers/Core/Portable/MetadataReader/MetadataDecoder.cs index 79f8fcc08feb1..64886d2e47acb 100644 --- a/src/Compilers/Core/Portable/MetadataReader/MetadataDecoder.cs +++ b/src/Compilers/Core/Portable/MetadataReader/MetadataDecoder.cs @@ -70,12 +70,12 @@ internal LocalInfo WithSignature(byte[] signature) public bool IsPinned => (Constraints & LocalSlotConstraints.Pinned) != 0; } +#nullable enable internal interface IAttributeNamedArgumentDecoder { - (KeyValuePair, bool isProperty) DecodeCustomAttributeNamedArgumentOrThrow(ref BlobReader argReader); + (KeyValuePair, bool isProperty, SerializationTypeCode typeCode) DecodeCustomAttributeNamedArgumentOrThrow(ref BlobReader argReader); } -#nullable enable internal abstract class MetadataDecoder : TypeNameDecoder, IAttributeNamedArgumentDecoder @@ -1608,7 +1608,7 @@ private static TypedConstantKind GetPrimitiveOrEnumTypedConstantKind(TypeSymbol /// If the encoded named argument is invalid. /// An exception from metadata reader. - public (KeyValuePair, bool isProperty) DecodeCustomAttributeNamedArgumentOrThrow(ref BlobReader argReader) + public (KeyValuePair, bool isProperty, SerializationTypeCode typeCode) DecodeCustomAttributeNamedArgumentOrThrow(ref BlobReader argReader) { // Ecma-335 23.3 - A NamedArg is simply a FixedArg preceded by information to identify which field or // property it represents. [Note: Recall that the CLI allows fields and properties to have the same name; so @@ -1635,7 +1635,7 @@ private static TypedConstantKind GetPrimitiveOrEnumTypedConstantKind(TypeSymbol ? DecodeCustomAttributeElementArrayOrThrow(ref argReader, elementTypeCode, elementType, type) : DecodeCustomAttributeElementOrThrow(ref argReader, typeCode, type); - return (new KeyValuePair(name, value), kind == CustomAttributeNamedArgumentKind.Property); + return (new KeyValuePair(name, value), kind == CustomAttributeNamedArgumentKind.Property, typeCode); } internal bool IsTargetAttribute( @@ -1736,7 +1736,7 @@ internal bool GetCustomAttribute( for (int i = 0; i < namedArgs.Length; i++) { - (namedArgs[i], _) = DecodeCustomAttributeNamedArgumentOrThrow(ref argsReader); + (namedArgs[i], _, _) = DecodeCustomAttributeNamedArgumentOrThrow(ref argsReader); } } diff --git a/src/Compilers/Core/Portable/MetadataReader/PEModule.cs b/src/Compilers/Core/Portable/MetadataReader/PEModule.cs index 65ac15a1bbd3e..dc21d81a09b6f 100644 --- a/src/Compilers/Core/Portable/MetadataReader/PEModule.cs +++ b/src/Compilers/Core/Portable/MetadataReader/PEModule.cs @@ -1684,13 +1684,13 @@ private static (string? diagnosticId, string? urlFormat) CrackObsoleteProperties var numNamed = sig.ReadUInt16(); for (int i = 0; i < numNamed && (diagnosticId is null || urlFormat is null); i++) { - var ((name, value), isProperty) = decoder.DecodeCustomAttributeNamedArgumentOrThrow(ref sig); - if (value.TypeInternal?.SpecialType == SpecialType.System_String && isProperty) + var ((name, value), isProperty, typeCode) = decoder.DecodeCustomAttributeNamedArgumentOrThrow(ref sig); + if (typeCode == SerializationTypeCode.String && isProperty && value.ValueInternal is string stringValue) { if (diagnosticId is null && name == "DiagnosticId") - diagnosticId = (string?)value.ValueInternal; + diagnosticId = stringValue; else if (urlFormat is null && name == "UrlFormat") - urlFormat = (string?)value.ValueInternal; + urlFormat = stringValue; } } } From a393f103d4cba202d95cfb988650d5b4ed273239 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Thu, 26 Mar 2020 16:41:30 -0700 Subject: [PATCH 15/21] Recognize custom ObsoleteAttribute in VB --- .../Symbols/ObsoleteAttributeHelpers.cs | 20 +++- .../CustomObsoleteDiagnosticInfo.cs | 45 +------- .../Portable/Errors/ErrorFactories.vb | 4 + .../Symbols/ObsoleteAttributeHelpers.vb | 8 +- .../AttributeTests_ObsoleteAttribute.vb | 106 +++++++++++++++++- .../Diagnostics/DiagnosticDescription.cs | 1 - 6 files changed, 135 insertions(+), 49 deletions(-) rename src/Compilers/{CSharp/Portable/Errors => Core/Portable/Diagnostic}/CustomObsoleteDiagnosticInfo.cs (58%) diff --git a/src/Compilers/CSharp/Portable/Symbols/ObsoleteAttributeHelpers.cs b/src/Compilers/CSharp/Portable/Symbols/ObsoleteAttributeHelpers.cs index 07dd36f838fdd..9cf458bb97c9b 100644 --- a/src/Compilers/CSharp/Portable/Symbols/ObsoleteAttributeHelpers.cs +++ b/src/Compilers/CSharp/Portable/Symbols/ObsoleteAttributeHelpers.cs @@ -5,7 +5,6 @@ using System.Diagnostics; using System.Reflection.Metadata; using System.Threading; -using Microsoft.CodeAnalysis.CSharp.Errors; using Microsoft.CodeAnalysis.CSharp.Symbols.Metadata.PE; namespace Microsoft.CodeAnalysis.CSharp.Symbols @@ -156,8 +155,23 @@ internal static DiagnosticInfo CreateObsoleteDiagnostic(Symbol symbol, BinderFla } // Issue a specialized diagnostic for add methods of collection initializers - bool isColInit = location.Includes(BinderFlags.CollectionInitializerAddMethod); - return new CustomObsoleteDiagnosticInfo(symbol, data, isColInit); + var isColInit = location.Includes(BinderFlags.CollectionInitializerAddMethod); + var errorCode = (message: data.Message, isError: data.IsError, isColInit) switch + { + // dev11 had a bug in this area (i.e. always produce a warning when there's no message) and we have to match it. + (message: null, isError: _, isColInit: true) => ErrorCode.WRN_DeprecatedCollectionInitAdd, + (message: null, isError: _, isColInit: false) => ErrorCode.WRN_DeprecatedSymbol, + (message: { }, isError: true, isColInit: true) => ErrorCode.ERR_DeprecatedCollectionInitAddStr, + (message: { }, isError: true, isColInit: false) => ErrorCode.ERR_DeprecatedSymbolStr, + (message: { }, isError: false, isColInit: true) => ErrorCode.WRN_DeprecatedCollectionInitAddStr, + (message: { }, isError: false, isColInit: false) => ErrorCode.WRN_DeprecatedSymbolStr + }; + + var arguments = data.Message is string message + ? new object[] { symbol, message } + : new object[] { symbol }; + + return new CustomObsoleteDiagnosticInfo(MessageProvider.Instance, (int)errorCode, data, arguments); } } } diff --git a/src/Compilers/CSharp/Portable/Errors/CustomObsoleteDiagnosticInfo.cs b/src/Compilers/Core/Portable/Diagnostic/CustomObsoleteDiagnosticInfo.cs similarity index 58% rename from src/Compilers/CSharp/Portable/Errors/CustomObsoleteDiagnosticInfo.cs rename to src/Compilers/Core/Portable/Diagnostic/CustomObsoleteDiagnosticInfo.cs index c773968ce25c9..c7d5948aba63f 100644 --- a/src/Compilers/CSharp/Portable/Errors/CustomObsoleteDiagnosticInfo.cs +++ b/src/Compilers/Core/Portable/Diagnostic/CustomObsoleteDiagnosticInfo.cs @@ -10,48 +10,18 @@ using System.Threading; using Microsoft.CodeAnalysis.PooledObjects; -namespace Microsoft.CodeAnalysis.CSharp.Errors +namespace Microsoft.CodeAnalysis { - internal sealed class CustomObsoleteDiagnosticInfo : DiagnosticInfo + internal class CustomObsoleteDiagnosticInfo : DiagnosticInfo { internal ObsoleteAttributeData Data { get; } - internal CustomObsoleteDiagnosticInfo(Symbol obsoletedSymbol, ObsoleteAttributeData data, bool inCollectionInitializer) - : base(CSharp.MessageProvider.Instance, (int)GetErrorCode(data, inCollectionInitializer), arguments: GetArguments(obsoletedSymbol, data)) + internal CustomObsoleteDiagnosticInfo(CommonMessageProvider messageProvider, int errorCode, ObsoleteAttributeData data, params object[] arguments) + : base(messageProvider, errorCode, arguments) { Data = data; } - private static ErrorCode GetErrorCode(ObsoleteAttributeData data, bool inCollectionInitializer) - { - // dev11 had a bug in this area (i.e. always produce a warning when there's no message) and we have to match it. - if (data.Message is null) - { - return inCollectionInitializer ? ErrorCode.WRN_DeprecatedCollectionInitAdd : ErrorCode.WRN_DeprecatedSymbol; - } - - return (data.IsError, inCollectionInitializer) switch - { - (true, true) => ErrorCode.ERR_DeprecatedCollectionInitAddStr, - (true, false) => ErrorCode.ERR_DeprecatedSymbolStr, - (false, true) => ErrorCode.WRN_DeprecatedCollectionInitAddStr, - (false, false) => ErrorCode.WRN_DeprecatedSymbolStr - }; - } - - private static object[] GetArguments(Symbol obsoletedSymbol, ObsoleteAttributeData obsoleteAttributeData) - { - var message = obsoleteAttributeData.Message; - if (message is object) - { - return new object[] { obsoletedSymbol, message }; - } - else - { - return new object[] { obsoletedSymbol }; - } - } - public override string MessageIdentifier { get @@ -101,9 +71,7 @@ private DiagnosticDescriptor CreateDescriptor() } catch { - // TODO: should we report a meta-diagnostic of some kind when the string.Format fails? - // also, should we do some validation of the 'UrlFormat' values provided in source to prevent people from shipping - // obsoleted symbols with malformed 'UrlFormat' values? + // if string.Format fails we just want to use the default (non-user specified) URI. } } @@ -125,9 +93,6 @@ private DiagnosticDescriptor CreateDescriptor() customTags = tagsBuilder.ToImmutableAndFree(); } - // TODO: we expect some users to repeatedly use - // the same diagnostic IDs and url format values for many symbols. - // do we want to cache similar diagnostic descriptors? return new DiagnosticDescriptor( id: id, title: baseDescriptor.Title, diff --git a/src/Compilers/VisualBasic/Portable/Errors/ErrorFactories.vb b/src/Compilers/VisualBasic/Portable/Errors/ErrorFactories.vb index f48186a910472..065bf8badbec4 100644 --- a/src/Compilers/VisualBasic/Portable/Errors/ErrorFactories.vb +++ b/src/Compilers/VisualBasic/Portable/Errors/ErrorFactories.vb @@ -73,6 +73,10 @@ Namespace Microsoft.CodeAnalysis.VisualBasic Return New DiagnosticInfo(MessageProvider.Instance, id, arguments) End Function + Public Shared Function ObsoleteErrorInfo(id As ERRID, data As ObsoleteAttributeData, ParamArray arguments As Object()) As CustomObsoleteDiagnosticInfo + Return New CustomObsoleteDiagnosticInfo(MessageProvider.Instance, id, data, arguments) + End Function + Public Shared Function ErrorInfo(id As ERRID, ByRef syntaxToken As SyntaxToken) As DiagnosticInfo Return ErrorInfo(id, SyntaxFacts.GetText(syntaxToken.Kind)) End Function diff --git a/src/Compilers/VisualBasic/Portable/Symbols/ObsoleteAttributeHelpers.vb b/src/Compilers/VisualBasic/Portable/Symbols/ObsoleteAttributeHelpers.vb index ceee3bf3c900e..a6eeb6ec76e1e 100644 --- a/src/Compilers/VisualBasic/Portable/Symbols/ObsoleteAttributeHelpers.vb +++ b/src/Compilers/VisualBasic/Portable/Symbols/ObsoleteAttributeHelpers.vb @@ -129,17 +129,17 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols Dim accessorString = If(accessorSymbol.MethodKind = MethodKind.PropertyGet, "Get", "Set") If String.IsNullOrEmpty(data.Message) Then - Return ErrorFactory.ErrorInfo(If(data.IsError, ERRID.ERR_UseOfObsoletePropertyAccessor2, ERRID.WRN_UseOfObsoletePropertyAccessor2), + Return ErrorFactory.ObsoleteErrorInfo(If(data.IsError, ERRID.ERR_UseOfObsoletePropertyAccessor2, ERRID.WRN_UseOfObsoletePropertyAccessor2), data, accessorString, accessorSymbol.AssociatedSymbol) Else - Return ErrorFactory.ErrorInfo(If(data.IsError, ERRID.ERR_UseOfObsoletePropertyAccessor3, ERRID.WRN_UseOfObsoletePropertyAccessor3), + Return ErrorFactory.ObsoleteErrorInfo(If(data.IsError, ERRID.ERR_UseOfObsoletePropertyAccessor3, ERRID.WRN_UseOfObsoletePropertyAccessor3), data, accessorString, accessorSymbol.AssociatedSymbol, data.Message) End If Else If String.IsNullOrEmpty(data.Message) Then - Return ErrorFactory.ErrorInfo(If(data.IsError, ERRID.ERR_UseOfObsoleteSymbolNoMessage1, ERRID.WRN_UseOfObsoleteSymbolNoMessage1), symbol) + Return ErrorFactory.ObsoleteErrorInfo(If(data.IsError, ERRID.ERR_UseOfObsoleteSymbolNoMessage1, ERRID.WRN_UseOfObsoleteSymbolNoMessage1), data, symbol) Else - Return ErrorFactory.ErrorInfo(If(data.IsError, ERRID.ERR_UseOfObsoleteSymbol2, ERRID.WRN_UseOfObsoleteSymbol2), symbol, data.Message) + Return ErrorFactory.ObsoleteErrorInfo(If(data.IsError, ERRID.ERR_UseOfObsoleteSymbol2, ERRID.WRN_UseOfObsoleteSymbol2), data, symbol, data.Message) End If End If diff --git a/src/Compilers/VisualBasic/Test/Emit/Attributes/AttributeTests_ObsoleteAttribute.vb b/src/Compilers/VisualBasic/Test/Emit/Attributes/AttributeTests_ObsoleteAttribute.vb index ba47d5146ee17..fbb3ccbc42ad9 100644 --- a/src/Compilers/VisualBasic/Test/Emit/Attributes/AttributeTests_ObsoleteAttribute.vb +++ b/src/Compilers/VisualBasic/Test/Emit/Attributes/AttributeTests_ObsoleteAttribute.vb @@ -1146,7 +1146,7 @@ End Class Diagnostic(ERRID.WRN_UseOfObsoleteSymbolNoMessage1, "a.b.c2(Of Integer).E(Of Integer)").WithArguments("A.B.C2(Of Integer).E(Of Integer)")) End Sub - + Public Sub TestObsoleteAttributeCustomProperties() Dim source = @@ -1192,6 +1192,110 @@ End Namespace Assert.Equal("TEST2", diag.Descriptor.HelpLinkUri) End Sub + + Public Sub TestObsoleteAttributeCustomPropertiesFromMetadata() + Dim source1 = + + + Sub M1() + End Sub +End Class + +Namespace System + Public Class ObsoleteAttribute + Inherits Attribute + + Public Sub New() + End Sub + + Public Sub New(message As String) + End Sub + + Public Sub New(message As String, isError As Boolean) + End Sub + + Public Property DiagnosticId As String + Public Property UrlFormat As String + End Class +End Namespace + +]]> + + + + Dim source2 = + + + + + + Dim comp1 = CreateCompilationWithMscorlib40(source1) + comp1.VerifyDiagnostics() + + Dim comp2 = CreateCompilationWithMscorlib40(source2, references:={comp1.EmitToImageReference()}) + Dim diags = comp2.GetDiagnostics() + diags.Verify(Diagnostic("TEST1", "M1()").WithArguments("Public Sub M1()").WithLocation(4, 9)) + + Dim diag = diags.Single() + Assert.Equal("TEST2", diag.Descriptor.HelpLinkUri) + End Sub + + + Public Sub TestObsoleteAttributeDuplicateProperties() + Dim source = + + + Sub M1() + End Sub + + Sub M2() + M1() + End Sub +End Class + +Namespace System + Public Class ObsoleteAttribute + Inherits Attribute + + Public Sub New() + End Sub + + Public Sub New(message As String) + End Sub + + Public Sub New(message As String, isError As Boolean) + End Sub + + Public Property DiagnosticId As String + Public Property UrlFormat As String + End Class +End Namespace + +]]> + + + + Dim comp = CreateCompilationWithMscorlib40(source) + Dim diags = comp.GetDiagnostics() + diags.Verify(Diagnostic("TEST1", "M1()").WithArguments("Public Sub M1()").WithLocation(9, 9)) + + Dim diag = diags.Single() + Assert.Equal("TEST3", diag.Descriptor.HelpLinkUri) + End Sub + Public Sub TestObsoleteInAlias() diff --git a/src/Test/Utilities/Portable/Diagnostics/DiagnosticDescription.cs b/src/Test/Utilities/Portable/Diagnostics/DiagnosticDescription.cs index 24a732ebeb7cb..cd3b226716174 100644 --- a/src/Test/Utilities/Portable/Diagnostics/DiagnosticDescription.cs +++ b/src/Test/Utilities/Portable/Diagnostics/DiagnosticDescription.cs @@ -14,7 +14,6 @@ using Roslyn.Test.Utilities; using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.CSharp; -using Microsoft.CodeAnalysis.CSharp.Errors; namespace Microsoft.CodeAnalysis.Test.Utilities { From 54cdd2ed0605a2aa2475e1503aa8ada5ea0bc652 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Fri, 27 Mar 2020 11:25:26 -0700 Subject: [PATCH 16/21] Remove unnecessary assert --- .../Core/Portable/Symbols/Attributes/CommonAttributeData.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs b/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs index 144e52b28ff7e..7f5d2c21b54ee 100644 --- a/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs +++ b/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs @@ -293,8 +293,6 @@ private ObsoleteAttributeData DecodeObsoleteAttribute() } } - Debug.Assert(AttributeClass is object); - string? diagnosticId = null; string? urlFormat = null; foreach (var (name, value) in this.CommonNamedArguments) From 8bc4393a9bce0f24c6c34f927d84875a5584d041 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Fri, 27 Mar 2020 12:39:03 -0700 Subject: [PATCH 17/21] Address feedback --- .../Emitter/Model/AttributeDataAdapter.cs | 16 ---------------- .../Portable/Symbols/Attributes/AttributeData.cs | 16 ++++++++++++++++ .../Diagnostic/CustomObsoleteDiagnosticInfo.cs | 2 +- .../Core/Portable/MetadataReader/PEModule.cs | 12 ++++++++---- .../Symbols/Attributes/CommonAttributeData.cs | 4 ++++ 5 files changed, 29 insertions(+), 21 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Emitter/Model/AttributeDataAdapter.cs b/src/Compilers/CSharp/Portable/Emitter/Model/AttributeDataAdapter.cs index dec66beb93198..19aaa90cb23d1 100644 --- a/src/Compilers/CSharp/Portable/Emitter/Model/AttributeDataAdapter.cs +++ b/src/Compilers/CSharp/Portable/Emitter/Model/AttributeDataAdapter.cs @@ -173,22 +173,6 @@ private Cci.IMetadataNamedArgument CreateMetadataNamedArgument(string name, Type return new MetadataNamedArgument(symbol, moduleBeingBuilt.Translate(type, syntaxNodeOpt: (CSharpSyntaxNode)context.SyntaxNodeOpt, diagnostics: context.Diagnostics), value); } - private protected sealed override bool IsStringProperty(string memberName) - { - if (AttributeClass is object) - { - foreach (var member in AttributeClass.GetMembers(memberName)) - { - if (member is PropertySymbol { Type: { SpecialType: SpecialType.System_String } }) - { - return true; - } - } - } - - return false; - } - private Symbol LookupName(string name) { var type = this.AttributeClass; diff --git a/src/Compilers/CSharp/Portable/Symbols/Attributes/AttributeData.cs b/src/Compilers/CSharp/Portable/Symbols/Attributes/AttributeData.cs index ea0b8ecc50c5b..3744ad9fd32c9 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Attributes/AttributeData.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Attributes/AttributeData.cs @@ -620,6 +620,22 @@ internal string DecodeGuidAttribute(AttributeSyntax? nodeOpt, DiagnosticBag diag return guidString; } + private protected sealed override bool IsStringProperty(string memberName) + { + if (AttributeClass is object) + { + foreach (var member in AttributeClass.GetMembers(memberName)) + { + if (member is PropertySymbol { Type: { SpecialType: SpecialType.System_String } }) + { + return true; + } + } + } + + return false; + } + #endregion /// diff --git a/src/Compilers/Core/Portable/Diagnostic/CustomObsoleteDiagnosticInfo.cs b/src/Compilers/Core/Portable/Diagnostic/CustomObsoleteDiagnosticInfo.cs index c7d5948aba63f..6a05906262524 100644 --- a/src/Compilers/Core/Portable/Diagnostic/CustomObsoleteDiagnosticInfo.cs +++ b/src/Compilers/Core/Portable/Diagnostic/CustomObsoleteDiagnosticInfo.cs @@ -12,7 +12,7 @@ namespace Microsoft.CodeAnalysis { - internal class CustomObsoleteDiagnosticInfo : DiagnosticInfo + internal sealed class CustomObsoleteDiagnosticInfo : DiagnosticInfo { internal ObsoleteAttributeData Data { get; } diff --git a/src/Compilers/Core/Portable/MetadataReader/PEModule.cs b/src/Compilers/Core/Portable/MetadataReader/PEModule.cs index dc21d81a09b6f..b58e9402a96b2 100644 --- a/src/Compilers/Core/Portable/MetadataReader/PEModule.cs +++ b/src/Compilers/Core/Portable/MetadataReader/PEModule.cs @@ -1675,12 +1675,12 @@ private static (string? diagnosticId, string? urlFormat) CrackObsoleteProperties string? diagnosticId = null; string? urlFormat = null; - // See CIL spec section II.23.3 Custom attributes - - // Next is a description of the optional “named” fields and properties. - // This starts with NumNamed– an unsigned int16 giving the number of “named” properties or fields that follow. try { + // See CIL spec section II.23.3 Custom attributes + // + // Next is a description of the optional “named” fields and properties. + // This starts with NumNamed– an unsigned int16 giving the number of “named” properties or fields that follow. var numNamed = sig.ReadUInt16(); for (int i = 0; i < numNamed && (diagnosticId is null || urlFormat is null); i++) { @@ -1688,9 +1688,13 @@ private static (string? diagnosticId, string? urlFormat) CrackObsoleteProperties if (typeCode == SerializationTypeCode.String && isProperty && value.ValueInternal is string stringValue) { if (diagnosticId is null && name == "DiagnosticId") + { diagnosticId = stringValue; + } else if (urlFormat is null && name == "UrlFormat") + { urlFormat = stringValue; + } } } } diff --git a/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs b/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs index 7f5d2c21b54ee..ea0ed79f2921a 100644 --- a/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs +++ b/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs @@ -317,7 +317,11 @@ private ObsoleteAttributeData DecodeObsoleteAttribute() // Note: it is disallowed to declare a property and a field // with the same name in C# or VB source, even if it is allowed in IL. + // // We use a virtual method and override to prevent having to realize the public symbols just to decode obsolete attributes. + // Ideally we would use an abstract method, but that would require making the method visible to + // public consumers who inherit from this class, which we don't want to do. + // Therefore we just make it a 'private protected virtual' method instead. private protected virtual bool IsStringProperty(string memberName) => throw ExceptionUtilities.Unreachable; /// From 1fdb590ab3a493f0bf7b2f163eb0f0fa1ce403af Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Fri, 27 Mar 2020 12:42:34 -0700 Subject: [PATCH 18/21] Fix doc comment --- .../Core/Portable/Diagnostic/WellKnownDiagnosticTags.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Compilers/Core/Portable/Diagnostic/WellKnownDiagnosticTags.cs b/src/Compilers/Core/Portable/Diagnostic/WellKnownDiagnosticTags.cs index 59c01c0db909d..b7ad29b34eb06 100644 --- a/src/Compilers/Core/Portable/Diagnostic/WellKnownDiagnosticTags.cs +++ b/src/Compilers/Core/Portable/Diagnostic/WellKnownDiagnosticTags.cs @@ -46,8 +46,8 @@ public static class WellKnownDiagnosticTags public const string AnalyzerException = nameof(AnalyzerException); /// - /// Indicates that the diagnostic is an obsolete diagnostic with a custom ID or URL - /// specified by the 'DiagnosticId' or 'UrlFormat' properties on 'ObsoleteAttribute'. + /// Indicates that the diagnostic is an obsolete diagnostic with a custom ID + /// specified by the 'DiagnosticId' property on 'ObsoleteAttribute'. /// public const string CustomObsolete = nameof(CustomObsolete); } From ef236212a9fe6b7b4c2dc3dfed850c2f7a044183 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Fri, 27 Mar 2020 16:47:53 -0700 Subject: [PATCH 19/21] Add VB tests --- .../AttributeTests_WellKnownAttributes.cs | 23 +- .../AttributeTests_ObsoleteAttribute.vb | 1210 ++++++++++++++++- 2 files changed, 1160 insertions(+), 73 deletions(-) diff --git a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs index 8afa4dd65d851..a9aa674a56de7 100644 --- a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs +++ b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs @@ -8377,15 +8377,6 @@ public void Obsolete_CustomDiagnosticId_BadAttribute_02() using System; #pragma warning disable 436 -namespace System -{ - public class ObsoleteAttribute : Attribute - { - public string DiagnosticId { get; set; } - public string UrlFormat { get; set; } - } -} - class C { [Obsolete( @@ -8402,19 +8393,19 @@ void M2() } "; - var comp = CreateCompilation(source); + var comp = CreateCompilation(new[] { ObsoleteAttributeSource, source }); var diags = comp.GetDiagnostics(); diags.Verify( - // (18,9): error CS0643: 'DiagnosticId' duplicate named attribute argument + // (9,9): error CS0643: 'DiagnosticId' duplicate named attribute argument // DiagnosticId = "B", // 1 - Diagnostic(ErrorCode.ERR_DuplicateNamedAttributeArgument, @"DiagnosticId = ""B""").WithArguments("DiagnosticId").WithLocation(18, 9), - // (20,9): error CS0643: 'UrlFormat' duplicate named attribute argument + Diagnostic(ErrorCode.ERR_DuplicateNamedAttributeArgument, @"DiagnosticId = ""B""").WithArguments("DiagnosticId").WithLocation(9, 9), + // (11,9): error CS0643: 'UrlFormat' duplicate named attribute argument // UrlFormat = "D")] // 2 - Diagnostic(ErrorCode.ERR_DuplicateNamedAttributeArgument, @"UrlFormat = ""D""").WithArguments("UrlFormat").WithLocation(20, 9), - // (25,9): warning A: 'C.M1()' is obsolete + Diagnostic(ErrorCode.ERR_DuplicateNamedAttributeArgument, @"UrlFormat = ""D""").WithArguments("UrlFormat").WithLocation(11, 9), + // (16,9): warning A: 'C.M1()' is obsolete // M1(); // 3 - Diagnostic("A", "M1()").WithArguments("C.M1()").WithLocation(25, 9)); + Diagnostic("A", "M1()").WithArguments("C.M1()").WithLocation(16, 9)); var diag = diags.Last(); Assert.Equal("C", diag.Descriptor.HelpLinkUri); diff --git a/src/Compilers/VisualBasic/Test/Emit/Attributes/AttributeTests_ObsoleteAttribute.vb b/src/Compilers/VisualBasic/Test/Emit/Attributes/AttributeTests_ObsoleteAttribute.vb index fbb3ccbc42ad9..d92abc63bb47f 100644 --- a/src/Compilers/VisualBasic/Test/Emit/Attributes/AttributeTests_ObsoleteAttribute.vb +++ b/src/Compilers/VisualBasic/Test/Emit/Attributes/AttributeTests_ObsoleteAttribute.vb @@ -1146,22 +1146,7 @@ End Class Diagnostic(ERRID.WRN_UseOfObsoleteSymbolNoMessage1, "a.b.c2(Of Integer).E(Of Integer)").WithArguments("A.B.C2(Of Integer).E(Of Integer)")) End Sub - - Public Sub TestObsoleteAttributeCustomProperties() - Dim source = - - - Sub M1() - End Sub - - Sub M2() - M1() - End Sub -End Class - + ReadOnly ObsoleteAttributeSource As XElement = + + + Public Sub Obsolete_CustomDiagnosticId_01() + + Dim source = + + + Sub M1() + End Sub + Sub M2() + M1() + End Sub +End Class ]]> + <%= ObsoleteAttributeSource %> Dim comp = CreateCompilationWithMscorlib40(source) @@ -1189,75 +1193,109 @@ End Namespace diags.Verify(Diagnostic("TEST1", "M1()").WithArguments("Public Sub M1()").WithLocation(9, 9)) Dim diag = diags.Single() - Assert.Equal("TEST2", diag.Descriptor.HelpLinkUri) + Assert.Equal("", diag.Descriptor.HelpLinkUri) End Sub - - Public Sub TestObsoleteAttributeCustomPropertiesFromMetadata() - Dim source1 = + + Public Sub Obsolete_CustomDiagnosticId_02() + + Dim source = - +Class C1 + Sub M1() End Sub + + Sub M2() + M1() + End Sub End Class +]]> + + <%= ObsoleteAttributeSource %> + -Namespace System - Public Class ObsoleteAttribute - Inherits Attribute - - Public Sub New() - End Sub - - Public Sub New(message As String) - End Sub - - Public Sub New(message As String, isError As Boolean) + Dim comp = CreateCompilationWithMscorlib40(source) + Dim diags = comp.GetDiagnostics() + diags.Verify(Diagnostic(ERRID.WRN_UseOfObsoleteSymbolNoMessage1, "M1()").WithArguments("Public Sub M1()").WithLocation(9, 9)) + + Dim diag = diags.Single() + Assert.Equal("https://docs.microsoft.com/en-us/dotnet/visual-basic/language-reference/compiler-messages/BC40008", diag.Descriptor.HelpLinkUri) End Sub - - Public Property DiagnosticId As String - Public Property UrlFormat As String - End Class -End Namespace + + Public Sub Obsolete_CustomDiagnosticId_03() + + Dim source = + + + Sub M1() + End Sub + + Sub M2() + M1() + End Sub +End Class ]]> + <%= ObsoleteAttributeSource %> - Dim source2 = + Dim comp = CreateCompilationWithMscorlib40(source) + Dim diags = comp.GetDiagnostics() + diags.Verify(Diagnostic(ERRID.WRN_UseOfObsoleteSymbolNoMessage1, "M1()").WithArguments("Public Sub M1()").WithLocation(9, 9)) + + Dim diag = diags.Single() + Assert.Equal("", diag.Descriptor.HelpLinkUri) + End Sub + + + Public Sub Obsolete_CustomDiagnosticId_04() + + Dim source = - + Sub M1() + End Sub + Sub M2() M1() End Sub End Class ]]> + <%= ObsoleteAttributeSource %> - Dim comp1 = CreateCompilationWithMscorlib40(source1) - comp1.VerifyDiagnostics() - - Dim comp2 = CreateCompilationWithMscorlib40(source2, references:={comp1.EmitToImageReference()}) - Dim diags = comp2.GetDiagnostics() - diags.Verify(Diagnostic("TEST1", "M1()").WithArguments("Public Sub M1()").WithLocation(4, 9)) + Dim comp = CreateCompilationWithMscorlib40(source) + Dim diags = comp.GetDiagnostics() + diags.Verify(Diagnostic(ERRID.WRN_UseOfObsoleteSymbolNoMessage1, "M1()").WithArguments("Public Sub M1()").WithLocation(9, 9)) Dim diag = diags.Single() - Assert.Equal("TEST2", diag.Descriptor.HelpLinkUri) + Assert.Equal("https://docs.microsoft.com/en-us/dotnet/visual-basic/language-reference/compiler-messages/elementname-is-obsolete-visual-basic-warning", diag.Descriptor.HelpLinkUri) End Sub - - Public Sub TestObsoleteAttributeDuplicateProperties() + + Public Sub Obsolete_CustomDiagnosticId_BadAttribute_01() + Dim source = - + Sub M1() End Sub @@ -1279,21 +1317,1079 @@ Namespace System Public Sub New(message As String, isError As Boolean) End Sub + Public Dim DiagnosticId As String Public Property DiagnosticId As String - Public Property UrlFormat As String End Class End Namespace +]]> + + + Dim comp = CreateCompilationWithMscorlib40(source) + Dim diags = comp.GetDiagnostics() + diags.Verify( + Diagnostic(ERRID.ERR_MetadataMembersAmbiguous3, "DiagnosticId").WithArguments("DiagnosticId", "class", "System.ObsoleteAttribute").WithLocation(4, 15), + Diagnostic(ERRID.ERR_MultiplyDefinedType3, "DiagnosticId").WithArguments("DiagnosticId", "Public DiagnosticId As String", "class").WithLocation(27, 25)) + End Sub + + + Public Sub Obsolete_CustomDiagnosticId_05() + + Dim source = + + + Sub M1() + End Sub + Sub M2() + M1() + End Sub +End Class ]]> + <%= ObsoleteAttributeSource %> Dim comp = CreateCompilationWithMscorlib40(source) Dim diags = comp.GetDiagnostics() - diags.Verify(Diagnostic("TEST1", "M1()").WithArguments("Public Sub M1()").WithLocation(9, 9)) + diags.Verify(Diagnostic("TEST1", "M1()").WithArguments("Public Sub M1()", "don't use").WithLocation(9, 9)) Dim diag = diags.Single() - Assert.Equal("TEST3", diag.Descriptor.HelpLinkUri) + Assert.Equal("TEST1", diag.Id) + Assert.Equal(ERRID.WRN_UseOfObsoleteSymbol2, DirectCast(diag.Code, ERRID)) + Assert.Equal("https://docs.microsoft.com/en-us/dotnet/visual-basic/language-reference/compiler-messages/TEST1", diag.Descriptor.HelpLinkUri) + End Sub + + + Public Sub Obsolete_CustomDiagnosticId_BadAttribute_02() + Dim source = + + + Sub M1() + End Sub + + Sub M2() + M1() + End Sub +End Class +]]> + <%= ObsoleteAttributeSource %> + + + Dim comp = CreateCompilationWithMscorlib40(source) + Dim diags = comp.GetDiagnostics() + diags.Verify(Diagnostic("A", "M1()").WithArguments("Public Sub M1()").WithLocation(9, 9)) + + Dim diag = diags.Single() + Assert.Equal("C", diag.Descriptor.HelpLinkUri) + End Sub + + + Public Sub Obsolete_CustomDiagnosticId_Suppression_01() + Dim source = + + + Sub M1() + End Sub + + + Sub M2() + End Sub + + Sub M3() + M1() + M2() + +#Disable Warning TEST1 + M1() + M2() +#Enable Warning TEST1 + +#Disable Warning BC40008 + M1() + M2() + End Sub +End Class +]]> + <%= ObsoleteAttributeSource %> + + + Dim comp = CreateCompilationWithMscorlib40(source) + comp.VerifyDiagnostics( + Diagnostic("TEST1", "M1()").WithArguments("Public Sub M1()", "don't use").WithLocation(13, 9), + Diagnostic(ERRID.WRN_UseOfObsoleteSymbolNoMessage1, "M2()").WithArguments("Public Sub M2()").WithLocation(14, 9), + Diagnostic(ERRID.WRN_UseOfObsoleteSymbolNoMessage1, "M2()").WithArguments("Public Sub M2()").WithLocation(18, 9), + Diagnostic("TEST1", "M1()").WithArguments("Public Sub M1()", "don't use").WithLocation(22, 9)) + End Sub + + + Public Sub Obsolete_CustomDiagnosticId_FromMetadata_01() + Dim source1 = + + + Public Sub M1() + End Sub + + + Public Sub M2() + End Sub +End Class +]]> + <%= ObsoleteAttributeSource %> + + + Dim source2 = + + + + + Dim comp1 = CreateCompilationWithMscorlib40(source1) + comp1.VerifyDiagnostics() + + Dim expected = { + Diagnostic("TEST1", "M1()").WithArguments("Public Sub M1()", "don't use").WithLocation(5, 9), + Diagnostic(ERRID.WRN_UseOfObsoleteSymbolNoMessage1, "M2()").WithArguments("Public Sub M2()").WithLocation(6, 9), + Diagnostic(ERRID.WRN_UseOfObsoleteSymbolNoMessage1, "M2()").WithArguments("Public Sub M2()").WithLocation(10, 9), + Diagnostic("TEST1", "M1()").WithArguments("Public Sub M1()", "don't use").WithLocation(14, 9) + } + + Dim comp2 = CreateCompilationWithMscorlib40(source2, references:={comp1.ToMetadataReference()}) + comp2.VerifyDiagnostics(expected) + + comp2 = CreateCompilationWithMscorlib40(source2, references:={comp1.EmitToImageReference()}) + comp2.VerifyDiagnostics(expected) + End Sub + + + Public Sub Obsolete_CustomDiagnosticId_FromMetadata_02() + Dim source1 = + + + Public Sub M1() + End Sub + + + Public Sub M2() + End Sub + + + Public Sub M3() + End Sub +End Class +]]> + <%= ObsoleteAttributeSource %> + + + Dim source2 = + + + + + Dim comp1 = CreateCompilationWithMscorlib40(source1) + comp1.VerifyDiagnostics() + + Dim expected = { + Diagnostic("TEST1", "M1()").WithArguments("Public Sub M1()").WithLocation(5, 9), + Diagnostic("TEST2", "M2()").WithArguments("Public Sub M2()", "don't use").WithLocation(6, 9), + Diagnostic("TEST3", "M3()").WithArguments("Public Sub M3()", "don't use").WithLocation(7, 9) + } + + Dim comp2 = CreateCompilationWithMscorlib40(source2, references:={comp1.ToMetadataReference()}) + comp2.VerifyDiagnostics(expected) + + comp2 = CreateCompilationWithMscorlib40(source2, references:={comp1.EmitToImageReference()}) + comp2.VerifyDiagnostics(expected) + End Sub + + + Public Sub Obsolete_CustomDiagnosticId_FromMetadata_03() + Dim source1 = + + + Public Sub M1() + End Sub +End Class +]]> + <%= ObsoleteAttributeSource %> + + + Dim source2 = + + + + + Dim comp1 = CreateCompilationWithMscorlib40(source1) + comp1.VerifyDiagnostics() + + + Dim comp2 = CreateCompilationWithMscorlib40(source2, references:={comp1.ToMetadataReference()}) + Dim diags = comp2.GetDiagnostics() + diags.Verify(Diagnostic("TEST1", "M1()").WithArguments("Public Sub M1()").WithLocation(5, 9)) + + Dim diag = diags.Single() + Assert.Equal("https://docs.microsoft.com/en-us/dotnet/visual-basic/language-reference/compiler-messages/TEST1", diag.Descriptor.HelpLinkUri) + + + comp2 = CreateCompilationWithMscorlib40(source2, references:={comp1.EmitToImageReference()}) + diags = comp2.GetDiagnostics() + diags.Verify(Diagnostic("TEST1", "M1()").WithArguments("Public Sub M1()").WithLocation(5, 9)) + + diag = diags.Single() + Assert.Equal("https://docs.microsoft.com/en-us/dotnet/visual-basic/language-reference/compiler-messages/TEST1", diag.Descriptor.HelpLinkUri) + End Sub + + + Public Sub Obsolete_CustomDiagnosticId_FromMetadata_04() + Dim source1 = + + + Public Sub M1() + End Sub +End Class +]]> + <%= ObsoleteAttributeSource %> + + + Dim source2 = + + + + + Dim comp1 = CreateCompilationWithMscorlib40(source1) + comp1.VerifyDiagnostics() + + + Dim comp2 = CreateCompilationWithMscorlib40(source2, references:={comp1.ToMetadataReference()}) + Dim diags = comp2.GetDiagnostics() + diags.Verify(Diagnostic(ERRID.WRN_UseOfObsoleteSymbolNoMessage1, "M1()").WithArguments("Public Sub M1()").WithLocation(5, 9)) + + Dim diag = diags.Single() + Assert.Equal("", diag.Descriptor.HelpLinkUri) + + + comp2 = CreateCompilationWithMscorlib40(source2, references:={comp1.EmitToImageReference()}) + diags = comp2.GetDiagnostics() + diags.Verify(Diagnostic(ERRID.WRN_UseOfObsoleteSymbolNoMessage1, "M1()").WithArguments("Public Sub M1()").WithLocation(5, 9)) + + diag = diags.Single() + Assert.Equal("", diag.Descriptor.HelpLinkUri) + End Sub + + + Public Sub Obsolete_CustomDiagnosticId_BadMetadata_01() + Dim source1 = + + + Public Sub M1() + End Sub +End Class +]]> + + + + Dim source2 = + + + + + Dim comp1 = CreateCompilationWithMscorlib40(source1) + comp1.VerifyDiagnostics() + + + Dim comp2 = CreateCompilationWithMscorlib40(source2, references:={comp1.ToMetadataReference()}) + Dim diags = comp2.GetDiagnostics() + diags.Verify(Diagnostic("TEST1", "M1()").WithArguments("Public Sub M1()").WithLocation(5, 9)) + + Dim diag = diags.Single() + Assert.Equal("", diag.Descriptor.HelpLinkUri) + + + comp2 = CreateCompilationWithMscorlib40(source2, references:={comp1.EmitToImageReference()}) + diags = comp2.GetDiagnostics() + diags.Verify(Diagnostic("TEST1", "M1()").WithArguments("Public Sub M1()").WithLocation(5, 9)) + + diag = diags.Single() + Assert.Equal("", diag.Descriptor.HelpLinkUri) + End Sub + + + Public Sub Obsolete_CustomDiagnosticId_BadMetadata_02() + Dim source1 = + + + Public Sub M1() + End Sub +End Class +]]> + + + + Dim source2 = + + + + + Dim comp1 = CreateCompilationWithMscorlib40(source1) + comp1.VerifyDiagnostics() + + + Dim comp2 = CreateCompilationWithMscorlib40(source2, references:={comp1.ToMetadataReference()}) + Dim diags = comp2.GetDiagnostics() + diags.Verify(Diagnostic(ERRID.WRN_UseOfObsoleteSymbolNoMessage1, "M1()").WithArguments("Public Sub M1()").WithLocation(5, 9)) + + Dim diag = diags.Single() + Assert.Equal("", diag.Descriptor.HelpLinkUri) + + + comp2 = CreateCompilationWithMscorlib40(source2, references:={comp1.EmitToImageReference()}) + diags = comp2.GetDiagnostics() + diags.Verify(Diagnostic(ERRID.WRN_UseOfObsoleteSymbolNoMessage1, "M1()").WithArguments("Public Sub M1()").WithLocation(5, 9)) + + diag = diags.Single() + Assert.Equal("", diag.Descriptor.HelpLinkUri) + End Sub + + + Public Sub Obsolete_CustomDiagnosticId_BadMetadata_03() + Dim source1 = + + + Public Sub M1() + End Sub +End Class +]]> + + + Dim source2 = + + + + + Dim comp1 = CreateCompilationWithMscorlib40(source1) + comp1.VerifyDiagnostics() + + + Dim comp2 = CreateCompilationWithMscorlib40(source2, references:={comp1.ToMetadataReference()}) + Dim diags = comp2.GetDiagnostics() + diags.Verify(Diagnostic("TEST1", "M1()").WithArguments("Public Sub M1()").WithLocation(5, 9)) + + Dim diag = diags.Single() + Assert.Equal("", diag.Descriptor.HelpLinkUri) + + + comp2 = CreateCompilationWithMscorlib40(source2, references:={comp1.EmitToImageReference()}) + diags = comp2.GetDiagnostics() + diags.Verify(Diagnostic("TEST1", "M1()").WithArguments("Public Sub M1()").WithLocation(5, 9)) + + diag = diags.Single() + Assert.Equal("", diag.Descriptor.HelpLinkUri) + End Sub + + + Public Sub Obsolete_CustomDiagnosticId_BadMetadata_04() + Dim source1 = + + + Public Sub M1() + End Sub +End Class +]]> + + + Dim source2 = + + + + + Dim comp1 = CreateCompilationWithMscorlib40(source1) + comp1.VerifyDiagnostics() + + + Dim comp2 = CreateCompilationWithMscorlib40(source2, references:={comp1.ToMetadataReference()}) + Dim diags = comp2.GetDiagnostics() + diags.Verify(Diagnostic("TEST1", "M1()").WithArguments("Public Sub M1()").WithLocation(5, 9)) + + Dim diag = diags.Single() + Assert.Equal("", diag.Descriptor.HelpLinkUri) + + + comp2 = CreateCompilationWithMscorlib40(source2, references:={comp1.EmitToImageReference()}) + diags = comp2.GetDiagnostics() + diags.Verify(Diagnostic("TEST1", "M1()").WithArguments("Public Sub M1()").WithLocation(5, 9)) + + diag = diags.Single() + Assert.Equal("", diag.Descriptor.HelpLinkUri) + End Sub + + + Public Sub Obsolete_CustomDiagnosticId_BadMetadata_05() + Dim source1 = + + + Public Sub M1() + End Sub +End Class +]]> + + + Dim source2 = + + + + + Dim comp1 = CreateCompilationWithMscorlib40(source1) + comp1.VerifyDiagnostics() + + + Dim comp2 = CreateCompilationWithMscorlib40(source2, references:={comp1.ToMetadataReference()}) + Dim diags = comp2.GetDiagnostics() + diags.Verify(Diagnostic(ERRID.WRN_UseOfObsoleteSymbolNoMessage1, "M1()").WithArguments("Public Sub M1()").WithLocation(5, 9)) + + Dim diag = diags.Single() + Assert.Equal("", diag.Descriptor.HelpLinkUri) + + + comp2 = CreateCompilationWithMscorlib40(source2, references:={comp1.EmitToImageReference()}) + diags = comp2.GetDiagnostics() + diags.Verify(Diagnostic(ERRID.WRN_UseOfObsoleteSymbolNoMessage1, "M1()").WithArguments("Public Sub M1()").WithLocation(5, 9)) + + diag = diags.Single() + Assert.Equal("", diag.Descriptor.HelpLinkUri) + End Sub + + + Public Sub Obsolete_CustomDiagnosticId_BadMetadata_06() + Dim source1 = + + + Public Sub M1() + End Sub +End Class +]]> + + + Dim source2 = + + + + + Dim comp1 = CreateCompilationWithMscorlib40(source1) + comp1.VerifyDiagnostics() + + + Dim comp2 = CreateCompilationWithMscorlib40(source2, references:={comp1.ToMetadataReference()}) + Dim diags = comp2.GetDiagnostics() + diags.Verify(Diagnostic(ERRID.WRN_UseOfObsoleteSymbolNoMessage1, "M1()").WithArguments("Public Sub M1()").WithLocation(5, 9)) + + Dim diag = diags.Single() + Assert.Equal("", diag.Descriptor.HelpLinkUri) + + + comp2 = CreateCompilationWithMscorlib40(source2, references:={comp1.EmitToImageReference()}) + diags = comp2.GetDiagnostics() + diags.Verify(Diagnostic(ERRID.WRN_UseOfObsoleteSymbolNoMessage1, "M1()").WithArguments("Public Sub M1()").WithLocation(5, 9)) + + diag = diags.Single() + Assert.Equal("", diag.Descriptor.HelpLinkUri) + End Sub + + + Public Sub Obsolete_CustomDiagnosticId_BadMetadata_07() + + ' In this program C1.M1 has an ObsoleteAttribute with multiple values provided for DiagnosticId and UrlFormat + Dim ilSource = " +.assembly extern mscorlib +{ + .publickeytoken = (B7 7A 5C 56 19 34 E0 89 ) // .z\V.4.. + .ver 4:0:0:0 +} + +.class public auto ansi beforefieldinit C1 + extends [mscorlib]System.Object +{ + .method public hidebysig instance void + M1() cil managed + { + .custom instance void System.ObsoleteAttribute::.ctor() = ( 01 00 04 00 // .... + 54 0E 0C 44 69 61 67 6E 6F 73 74 69 63 49 64 01 41 // T..DiagnosticId.A + 54 0E 0C 44 69 61 67 6E 6F 73 74 69 63 49 64 01 42 // T..DiagnosticId.B + 54 0E 09 55 72 6C 46 6F 72 6D 61 74 01 43 // T..UrlFormat.C + 54 0E 09 55 72 6C 46 6F 72 6D 61 74 01 44 ) // T..UrlFormat.D + // Code size 2 (0x2) + .maxstack 8 + IL_0000: nop + IL_0001: ret + } // end of method C1::M1 + + .method public hidebysig specialname rtspecialname + instance void .ctor() cil managed + { + // Code size 8 (0x8) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: call instance void [mscorlib]System.Object::.ctor() + IL_0006: nop + IL_0007: ret + } // end of method C1::.ctor + +} // end of class C1 + +.class public auto ansi beforefieldinit System.ObsoleteAttribute + extends [mscorlib]System.Attribute +{ + .field private string 'k__BackingField' + .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + .custom instance void [mscorlib]System.Diagnostics.DebuggerBrowsableAttribute::.ctor(valuetype [mscorlib]System.Diagnostics.DebuggerBrowsableState) = ( 01 00 00 00 00 00 00 00 ) + .field private string 'k__BackingField' + .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + .custom instance void [mscorlib]System.Diagnostics.DebuggerBrowsableAttribute::.ctor(valuetype [mscorlib]System.Diagnostics.DebuggerBrowsableState) = ( 01 00 00 00 00 00 00 00 ) + .method public hidebysig specialname instance string + get_DiagnosticId() cil managed + { + .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + // Code size 7 (0x7) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: ldfld string System.ObsoleteAttribute::'k__BackingField' + IL_0006: ret + } // end of method ObsoleteAttribute::get_DiagnosticId + + .method public hidebysig specialname instance void + set_DiagnosticId(string 'value') cil managed + { + .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + // Code size 8 (0x8) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: ldarg.1 + IL_0002: stfld string System.ObsoleteAttribute::'k__BackingField' + IL_0007: ret + } // end of method ObsoleteAttribute::set_DiagnosticId + + .method public hidebysig specialname instance string + get_UrlFormat() cil managed + { + .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + // Code size 7 (0x7) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: ldfld string System.ObsoleteAttribute::'k__BackingField' + IL_0006: ret + } // end of method ObsoleteAttribute::get_UrlFormat + + .method public hidebysig specialname instance void + set_UrlFormat(string 'value') cil managed + { + .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + // Code size 8 (0x8) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: ldarg.1 + IL_0002: stfld string System.ObsoleteAttribute::'k__BackingField' + IL_0007: ret + } // end of method ObsoleteAttribute::set_UrlFormat + + .method public hidebysig specialname rtspecialname + instance void .ctor() cil managed + { + // Code size 8 (0x8) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: call instance void [mscorlib]System.Attribute::.ctor() + IL_0006: nop + IL_0007: ret + } // end of method ObsoleteAttribute::.ctor + + .property instance string DiagnosticId() + { + .get instance string System.ObsoleteAttribute::get_DiagnosticId() + .set instance void System.ObsoleteAttribute::set_DiagnosticId(string) + } // end of property ObsoleteAttribute::DiagnosticId + .property instance string UrlFormat() + { + .get instance string System.ObsoleteAttribute::get_UrlFormat() + .set instance void System.ObsoleteAttribute::set_UrlFormat(string) + } // end of property ObsoleteAttribute::UrlFormat +} // end of class System.ObsoleteAttribute +" + + Dim source2 = + + + + + + Dim ilComp = CompileIL(ilSource) + Dim comp2 = CreateCompilationWithMscorlib40(source2, references:={ilComp}) + Dim diags = comp2.GetDiagnostics() + diags.Verify(Diagnostic("A", "M1()").WithArguments("Public Overloads Sub M1()").WithLocation(5, 9)) + + Dim diag = diags.Single() + Assert.Equal("C", diag.Descriptor.HelpLinkUri) + End Sub + + + Public Sub Obsolete_CustomDiagnosticId_BadMetadata_08() + + ' In this program C1.M1 has an ObsoleteAttribute with multiple values provided for DiagnosticId and UrlFormat + Dim ilSource = " +.assembly extern mscorlib +{ + .publickeytoken = (B7 7A 5C 56 19 34 E0 89 ) // .z\V.4.. + .ver 4:0:0:0 +} + +.class public auto ansi beforefieldinit C1 + extends [mscorlib]System.Object +{ + .method public hidebysig instance void + M1() cil managed + { + .custom instance void System.ObsoleteAttribute::.ctor() = ( 01 00 02 00 // .... + 54 0E 0C 44 69 61 67 6E 6F 73 74 69 63 49 64 01 41 // T..DiagnosticId.A + 0E 09 55 72 6C 46 6F 72 6D 61 74 01 42 ) // ..UrlFormat.B + // Code size 2 (0x2) + .maxstack 8 + IL_0000: nop + IL_0001: ret + } // end of method C1::M1 + + .method public hidebysig specialname rtspecialname + instance void .ctor() cil managed + { + // Code size 8 (0x8) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: call instance void [mscorlib]System.Object::.ctor() + IL_0006: nop + IL_0007: ret + } // end of method C1::.ctor + +} // end of class C1 + +.class public auto ansi beforefieldinit System.ObsoleteAttribute + extends [mscorlib]System.Attribute +{ + .field private string 'k__BackingField' + .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + .custom instance void [mscorlib]System.Diagnostics.DebuggerBrowsableAttribute::.ctor(valuetype [mscorlib]System.Diagnostics.DebuggerBrowsableState) = ( 01 00 00 00 00 00 00 00 ) + .field private string 'k__BackingField' + .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + .custom instance void [mscorlib]System.Diagnostics.DebuggerBrowsableAttribute::.ctor(valuetype [mscorlib]System.Diagnostics.DebuggerBrowsableState) = ( 01 00 00 00 00 00 00 00 ) + .method public hidebysig specialname instance string + get_DiagnosticId() cil managed + { + .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + // Code size 7 (0x7) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: ldfld string System.ObsoleteAttribute::'k__BackingField' + IL_0006: ret + } // end of method ObsoleteAttribute::get_DiagnosticId + + .method public hidebysig specialname instance void + set_DiagnosticId(string 'value') cil managed + { + .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + // Code size 8 (0x8) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: ldarg.1 + IL_0002: stfld string System.ObsoleteAttribute::'k__BackingField' + IL_0007: ret + } // end of method ObsoleteAttribute::set_DiagnosticId + + .method public hidebysig specialname instance string + get_UrlFormat() cil managed + { + .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + // Code size 7 (0x7) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: ldfld string System.ObsoleteAttribute::'k__BackingField' + IL_0006: ret + } // end of method ObsoleteAttribute::get_UrlFormat + + .method public hidebysig specialname instance void + set_UrlFormat(string 'value') cil managed + { + .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + // Code size 8 (0x8) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: ldarg.1 + IL_0002: stfld string System.ObsoleteAttribute::'k__BackingField' + IL_0007: ret + } // end of method ObsoleteAttribute::set_UrlFormat + + .method public hidebysig specialname rtspecialname + instance void .ctor() cil managed + { + // Code size 8 (0x8) + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: call instance void [mscorlib]System.Attribute::.ctor() + IL_0006: nop + IL_0007: ret + } // end of method ObsoleteAttribute::.ctor + + .property instance string DiagnosticId() + { + .get instance string System.ObsoleteAttribute::get_DiagnosticId() + .set instance void System.ObsoleteAttribute::set_DiagnosticId(string) + } // end of property ObsoleteAttribute::DiagnosticId + .property instance string UrlFormat() + { + .get instance string System.ObsoleteAttribute::get_UrlFormat() + .set instance void System.ObsoleteAttribute::set_UrlFormat(string) + } // end of property ObsoleteAttribute::UrlFormat +} // end of class System.ObsoleteAttribute +" + + Dim source2 = + + + + + + Dim ilComp = CompileIL(ilSource) + Dim comp2 = CreateCompilationWithMscorlib40(source2, references:={ilComp}) + Dim diags = comp2.GetDiagnostics() + diags.Verify(Diagnostic("A", "M1()").WithArguments("Public Overloads Sub M1()").WithLocation(5, 9)) + + Dim diag = diags.Single() + Assert.Equal("", diag.Descriptor.HelpLinkUri) + End Sub + + + Public Sub Obsolete_CustomDiagnosticId_BadMetadata_09() + Dim source1 = + + + Public Sub M1() + End Sub +End Class +]]> + + + Dim source2 = + + + + + Dim comp1 = CreateCompilationWithMscorlib40(source1) + comp1.VerifyDiagnostics() + + + Dim comp2 = CreateCompilationWithMscorlib40(source2, references:={comp1.ToMetadataReference()}) + Dim diags = comp2.GetDiagnostics() + diags.Verify(Diagnostic(ERRID.WRN_UseOfObsoleteSymbolNoMessage1, "M1()").WithArguments("Public Sub M1()").WithLocation(5, 9)) + + Dim diag = diags.Single() + Assert.Equal("", diag.Descriptor.HelpLinkUri) + + + comp2 = CreateCompilationWithMscorlib40(source2, references:={comp1.EmitToImageReference()}) + diags = comp2.GetDiagnostics() + diags.Verify(Diagnostic(ERRID.WRN_UseOfObsoleteSymbolNoMessage1, "M1()").WithArguments("Public Sub M1()").WithLocation(5, 9)) + + diag = diags.Single() + Assert.Equal("", diag.Descriptor.HelpLinkUri) End Sub From c7836ddbb020bf9f7282ad04be3a6d52a642a747 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Fri, 27 Mar 2020 16:58:18 -0700 Subject: [PATCH 20/21] Address some feedback --- .../Core/Portable/Diagnostic/CustomObsoleteDiagnosticInfo.cs | 2 +- src/Compilers/Core/Portable/MetadataReader/MetadataDecoder.cs | 4 ++-- src/Compilers/Core/Portable/MetadataReader/PEModule.cs | 4 ++-- .../Core/Portable/Symbols/Attributes/CommonAttributeData.cs | 4 ++-- .../Core/Portable/Symbols/Attributes/ObsoleteAttributeData.cs | 3 +++ 5 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/Compilers/Core/Portable/Diagnostic/CustomObsoleteDiagnosticInfo.cs b/src/Compilers/Core/Portable/Diagnostic/CustomObsoleteDiagnosticInfo.cs index 6a05906262524..fd84587df9884 100644 --- a/src/Compilers/Core/Portable/Diagnostic/CustomObsoleteDiagnosticInfo.cs +++ b/src/Compilers/Core/Portable/Diagnostic/CustomObsoleteDiagnosticInfo.cs @@ -14,6 +14,7 @@ namespace Microsoft.CodeAnalysis { internal sealed class CustomObsoleteDiagnosticInfo : DiagnosticInfo { + private DiagnosticDescriptor? _descriptor; internal ObsoleteAttributeData Data { get; } internal CustomObsoleteDiagnosticInfo(CommonMessageProvider messageProvider, int errorCode, ObsoleteAttributeData data, params object[] arguments) @@ -36,7 +37,6 @@ public override string MessageIdentifier } } - private DiagnosticDescriptor? _descriptor; public override DiagnosticDescriptor Descriptor { get diff --git a/src/Compilers/Core/Portable/MetadataReader/MetadataDecoder.cs b/src/Compilers/Core/Portable/MetadataReader/MetadataDecoder.cs index 64886d2e47acb..ebebd83203f15 100644 --- a/src/Compilers/Core/Portable/MetadataReader/MetadataDecoder.cs +++ b/src/Compilers/Core/Portable/MetadataReader/MetadataDecoder.cs @@ -73,7 +73,7 @@ internal LocalInfo WithSignature(byte[] signature) #nullable enable internal interface IAttributeNamedArgumentDecoder { - (KeyValuePair, bool isProperty, SerializationTypeCode typeCode) DecodeCustomAttributeNamedArgumentOrThrow(ref BlobReader argReader); + (KeyValuePair nameValuePair, bool isProperty, SerializationTypeCode typeCode) DecodeCustomAttributeNamedArgumentOrThrow(ref BlobReader argReader); } internal abstract class MetadataDecoder : @@ -1608,7 +1608,7 @@ private static TypedConstantKind GetPrimitiveOrEnumTypedConstantKind(TypeSymbol /// If the encoded named argument is invalid. /// An exception from metadata reader. - public (KeyValuePair, bool isProperty, SerializationTypeCode typeCode) DecodeCustomAttributeNamedArgumentOrThrow(ref BlobReader argReader) + public (KeyValuePair nameValuePair, bool isProperty, SerializationTypeCode typeCode) DecodeCustomAttributeNamedArgumentOrThrow(ref BlobReader argReader) { // Ecma-335 23.3 - A NamedArg is simply a FixedArg preceded by information to identify which field or // property it represents. [Note: Recall that the CLI allows fields and properties to have the same name; so diff --git a/src/Compilers/Core/Portable/MetadataReader/PEModule.cs b/src/Compilers/Core/Portable/MetadataReader/PEModule.cs index b58e9402a96b2..60cba00e65dde 100644 --- a/src/Compilers/Core/Portable/MetadataReader/PEModule.cs +++ b/src/Compilers/Core/Portable/MetadataReader/PEModule.cs @@ -1687,11 +1687,11 @@ private static (string? diagnosticId, string? urlFormat) CrackObsoleteProperties var ((name, value), isProperty, typeCode) = decoder.DecodeCustomAttributeNamedArgumentOrThrow(ref sig); if (typeCode == SerializationTypeCode.String && isProperty && value.ValueInternal is string stringValue) { - if (diagnosticId is null && name == "DiagnosticId") + if (diagnosticId is null && name == ObsoleteAttributeData.DiagnosticIdPropertyName) { diagnosticId = stringValue; } - else if (urlFormat is null && name == "UrlFormat") + else if (urlFormat is null && name == ObsoleteAttributeData.UrlFormatPropertyName) { urlFormat = stringValue; } diff --git a/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs b/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs index ea0ed79f2921a..0162b5c8e6453 100644 --- a/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs +++ b/src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs @@ -297,11 +297,11 @@ private ObsoleteAttributeData DecodeObsoleteAttribute() string? urlFormat = null; foreach (var (name, value) in this.CommonNamedArguments) { - if (diagnosticId is null && name == "DiagnosticId" && IsStringProperty("DiagnosticId")) + if (diagnosticId is null && name == ObsoleteAttributeData.DiagnosticIdPropertyName && IsStringProperty(ObsoleteAttributeData.DiagnosticIdPropertyName)) { diagnosticId = value.ValueInternal as string; } - else if (urlFormat is null && name == "UrlFormat" && IsStringProperty("UrlFormat")) + else if (urlFormat is null && name == ObsoleteAttributeData.UrlFormatPropertyName && IsStringProperty(ObsoleteAttributeData.UrlFormatPropertyName)) { urlFormat = value.ValueInternal as string; } diff --git a/src/Compilers/Core/Portable/Symbols/Attributes/ObsoleteAttributeData.cs b/src/Compilers/Core/Portable/Symbols/Attributes/ObsoleteAttributeData.cs index 913c2bb0e3f6c..072aa235bf329 100644 --- a/src/Compilers/Core/Portable/Symbols/Attributes/ObsoleteAttributeData.cs +++ b/src/Compilers/Core/Portable/Symbols/Attributes/ObsoleteAttributeData.cs @@ -25,6 +25,9 @@ internal sealed class ObsoleteAttributeData public static readonly ObsoleteAttributeData Uninitialized = new ObsoleteAttributeData(ObsoleteAttributeKind.Uninitialized, message: null, isError: false, diagnosticId: null, urlFormat: null); public static readonly ObsoleteAttributeData Experimental = new ObsoleteAttributeData(ObsoleteAttributeKind.Experimental, message: null, isError: false, diagnosticId: null, urlFormat: null); + public const string DiagnosticIdPropertyName = "DiagnosticId"; + public const string UrlFormatPropertyName = "UrlFormat"; + public ObsoleteAttributeData(ObsoleteAttributeKind kind, string? message, bool isError, string? diagnosticId, string? urlFormat) { Kind = kind; From b7a9893998197f1d1cac0fb3a287345a88ef8fe6 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Mon, 30 Mar 2020 14:35:38 -0700 Subject: [PATCH 21/21] Make test constants private --- .../Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs | 2 +- .../Test/Emit/Attributes/AttributeTests_ObsoleteAttribute.vb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs index a9aa674a56de7..d282326520946 100644 --- a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs +++ b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_WellKnownAttributes.cs @@ -8155,7 +8155,7 @@ static void Main(string[] args) Diagnostic(ErrorCode.WRN_DeprecatedSymbolStr, "c ?? 1").WithArguments("Convertible.implicit operator int(Convertible)", "To int")); } - const string ObsoleteAttributeSource = @" + private const string ObsoleteAttributeSource = @" #nullable enable namespace System { diff --git a/src/Compilers/VisualBasic/Test/Emit/Attributes/AttributeTests_ObsoleteAttribute.vb b/src/Compilers/VisualBasic/Test/Emit/Attributes/AttributeTests_ObsoleteAttribute.vb index d92abc63bb47f..6a4b5f6404664 100644 --- a/src/Compilers/VisualBasic/Test/Emit/Attributes/AttributeTests_ObsoleteAttribute.vb +++ b/src/Compilers/VisualBasic/Test/Emit/Attributes/AttributeTests_ObsoleteAttribute.vb @@ -1146,7 +1146,7 @@ End Class Diagnostic(ERRID.WRN_UseOfObsoleteSymbolNoMessage1, "a.b.c2(Of Integer).E(Of Integer)").WithArguments("A.B.C2(Of Integer).E(Of Integer)")) End Sub - ReadOnly ObsoleteAttributeSource As XElement =