Skip to content

Commit

Permalink
Create separate attribute for warning behavior differences (#101220)
Browse files Browse the repository at this point in the history
To help track differences in the warning behavior of the trimming related tools, this modifies how adds UnexpectedWarning, and requires an issue link to be provided when there is a ProducedBy argument to the constructor. To enforce that either both a ProducedBy and IssueLink is provided or neither, the ProducedBy property is removed and is provided as the second to last argument, and IssueLink is provided as the last argument.

ExpectedWarning means that the correct behavior is to warn. Any attributes that expect it only from a subset of the tools must provide an issue link. (These are mostly blank strings now, though)

UnexpectedWarning means that this warning is not the correct behavior. These attributes always include a ProducedBy anrdshould link to an issue.

Changes
Look for a Tool attribute argument in the second to last position of ExpectedWarning and Unexpected warning when a ProducedBy property is not found.
Find a replace existing ExpectedWarnings to use the new constructors.
Adds issue links within AttributedMembersAccessedViaReflection.cs and in some places in ArrayDataFlow.cs
  • Loading branch information
jtschuster committed Apr 26, 2024
1 parent a0beed4 commit cdfc8f2
Show file tree
Hide file tree
Showing 79 changed files with 1,642 additions and 1,754 deletions.
Expand Up @@ -159,6 +159,8 @@ protected virtual void AdditionalChecking (TrimmedTestCaseResult linkResult, Ass

private static bool IsProducedByNativeAOT (CustomAttribute attr)
{
if (attr.ConstructorArguments.Count > 2 && attr.ConstructorArguments[^2].Type.Name == "Tool")
return ((Tool)attr.ConstructorArguments[^2].Value).HasFlag(Tool.NativeAot);
var producedBy = attr.GetPropertyValue ("ProducedBy");
return producedBy is null ? true : ((Tool) producedBy).HasFlag (Tool.NativeAot);
}
Expand Down Expand Up @@ -227,12 +229,29 @@ private void VerifyLoggedMessages (AssemblyDefinition original, TrimmingTestLogg
}
break;

case nameof (ExpectedWarningAttribute): {
case nameof (ExpectedWarningAttribute) or nameof(UnexpectedWarningAttribute): {
var expectedWarningCode = (string) attr.GetConstructorArgumentValue (0);
if (!expectedWarningCode.StartsWith ("IL")) {
Assert.Fail ($"The warning code specified in {nameof (ExpectedWarningAttribute)} must start with the 'IL' prefix. Specified value: '{expectedWarningCode}'.");
Assert.Fail ($"The warning code specified in {attr.AttributeType.Name} must start with the 'IL' prefix. Specified value: '{expectedWarningCode}'.");
}
var expectedMessageContains = ((CustomAttributeArgument[]) attr.GetConstructorArgumentValue (1)).Select (a => (string) a.Value).ToArray ();
IEnumerable<string> expectedMessageContains = attr.Constructor.Parameters switch
{
// ExpectedWarningAttribute(string warningCode, params string[] expectedMessages)
// ExpectedWarningAttribute(string warningCode, string[] expectedMessages, Tool producedBy, string issueLink)
[_, { ParameterType.IsArray: true }, ..]
=> ((CustomAttributeArgument[])attr.ConstructorArguments[1].Value)
.Select(caa => (string)caa.Value),
// ExpectedWarningAttribute(string warningCode, string expectedMessage1, string expectedMessage2, Tool producedBy, string issueLink)
[_, { ParameterType.Name: "String" }, { ParameterType.Name: "String" }, { ParameterType.Name: "Tool" }, _]
=> [(string)attr.GetConstructorArgumentValue(1), (string)attr.GetConstructorArgumentValue(2)],
// ExpectedWarningAttribute(string warningCode, string expectedMessage, Tool producedBy, string issueLink)
[_, { ParameterType.Name: "String" }, { ParameterType.Name: "Tool" }, _]
=> [(string)attr.GetConstructorArgumentValue(1)],
// ExpectedWarningAttribute(string warningCode, Tool producedBy, string issueLink)
[_, { ParameterType.Name: "Tool" }, _]
=> [],
_ => throw new UnreachableException(),
};
string fileName = (string) attr.GetPropertyValue ("FileName")!;
int? sourceLine = (int?) attr.GetPropertyValue ("SourceLine");
int? sourceColumn = (int?) attr.GetPropertyValue ("SourceColumn");
Expand Down
69 changes: 50 additions & 19 deletions src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/TestChecker.cs
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Text.RegularExpressions;
Expand Down Expand Up @@ -66,7 +67,7 @@ public void Check (bool allowMissingWarnings)
}

if (message.Length > 0) {
Assert.Fail(message);
Assert.Fail (message);
}
}

Expand Down Expand Up @@ -207,51 +208,62 @@ private void ValidateDiagnostics (CSharpSyntaxNode memberSyntax, SyntaxList<Attr

static bool IsExpectedDiagnostic (AttributeSyntax attribute)
{
switch (attribute.Name.ToString ()) {
case "ExpectedWarning":
case "LogContains":
switch (attribute.Name.ToString () + "Attribute") {
case nameof (ExpectedWarningAttribute):
case nameof (UnexpectedWarningAttribute):
case nameof (LogContainsAttribute):
var args = LinkerTestBase.GetAttributeArguments (attribute);
if (args.TryGetValue ("ProducedBy", out var producedBy)) {
// Skip if this warning is not expected to be produced by any of the analyzers that we are currently testing.
return GetProducedBy (producedBy).HasFlag (Tool.Analyzer);
}
var toolArg = args.Where (arg => arg.Key.StartsWith ('#')).Count () - 2;
if (args.TryGetValue ($"#{toolArg}", out var maybeProducedBy) && TryGetProducedBy (maybeProducedBy, out Tool producedByTool)) {
return producedByTool.HasFlag (Tool.Analyzer);
}

return true;
default:
return false;
}

static Tool GetProducedBy (ExpressionSyntax expression)
static bool TryGetProducedBy (ExpressionSyntax expression, out Tool producedBy)
{
var producedBy = (Tool) 0x0;
producedBy = (Tool) 0x0;
switch (expression) {
case BinaryExpressionSyntax binaryExpressionSyntax:
case BinaryExpressionSyntax binaryExpressionSyntax when binaryExpressionSyntax.Kind () == SyntaxKind.BitwiseOrExpression:
if (!Enum.TryParse<Tool> ((binaryExpressionSyntax.Left as MemberAccessExpressionSyntax)!.Name.Identifier.ValueText, out var besProducedBy))
throw new ArgumentException ("Expression must be a ProducedBy value", nameof (expression));
return false;
producedBy |= besProducedBy;
producedBy |= GetProducedBy (binaryExpressionSyntax.Right);
break;

case MemberAccessExpressionSyntax memberAccessExpressionSyntax:
if (!Enum.TryParse<Tool> (memberAccessExpressionSyntax.Name.Identifier.ValueText, out var maeProducedBy))
throw new ArgumentException ("Expression must be a ProducedBy value", nameof (expression));
return false;
producedBy |= maeProducedBy;
break;

default:
break;
return false;
}

return producedBy;
return true;
}

static Tool GetProducedBy (ExpressionSyntax expression)
{
return TryGetProducedBy (expression, out var tool) ? tool : throw new ArgumentException ("Expression must be a ProducedBy value", nameof (expression));
}
}

bool TryValidateExpectedDiagnostic (AttributeSyntax attribute, List<Diagnostic> diagnostics, [NotNullWhen (true)] out int? matchIndex, [NotNullWhen (false)] out string? missingDiagnosticMessage)
{
switch (attribute.Name.ToString ()) {
case "ExpectedWarning":
switch (attribute.Name.ToString () + "Attribute") {
case nameof (ExpectedWarningAttribute):
case nameof (UnexpectedWarningAttribute):
return TryValidateExpectedWarningAttribute (attribute!, diagnostics, out matchIndex, out missingDiagnosticMessage);
case "LogContains":
case nameof (LogContainsAttribute):
return TryValidateLogContainsAttribute (attribute!, diagnostics, out matchIndex, out missingDiagnosticMessage);
default:
throw new InvalidOperationException ($"Unsupported attribute type {attribute.Name}");
Expand All @@ -268,10 +280,29 @@ private bool TryValidateExpectedWarningAttribute (AttributeSyntax attribute, Lis
if (!expectedWarningCode.StartsWith ("IL"))
throw new InvalidOperationException ($"Expected warning code should start with \"IL\" prefix.");

List<string> expectedMessages = args
.Where (arg => arg.Key.StartsWith ("#") && arg.Key != "#0")
.Select (arg => LinkerTestBase.GetStringFromExpression (arg.Value, _semanticModel))
.ToList ();
List<string> expectedMessages = ((IMethodSymbol) (_semanticModel.GetSymbolInfo (attribute).Symbol!)).Parameters switch {
// ExpectedWarningAttribute(string warningCode, params string[] expectedMessages)
[_, { IsParams: true }]
=> args
.Where (arg => arg.Key.StartsWith ('#') && arg.Key != "#0")
.Select (arg => LinkerTestBase.GetStringFromExpression (arg.Value, _semanticModel))
.ToList (),
// ExpectedWarningAttribute(string warningCode, string[] expectedMessages, Tool producedBy, string issueLink)
[_, { Type.TypeKind: TypeKind.Array }, _, _]
=> ((CollectionExpressionSyntax) args["#1"]).Elements
.Select (arg => LinkerTestBase.GetStringFromExpression (((ExpressionElementSyntax) arg).Expression, _semanticModel))
.ToList (),
// ExpectedWarningAttribute(string warningCode, string expectedMessage, Tool producedBy, string issueLink)
[_, { Type.SpecialType: SpecialType.System_String }, _, _]
=> [LinkerTestBase.GetStringFromExpression (args["#1"], _semanticModel)],
// ExpectedWarningAttribute(string warningCode, string expectedMessage1, string expectedMessage2, Tool producedBy, string issueLink)
[_, { Type.SpecialType: SpecialType.System_String }, { Type.SpecialType: SpecialType.System_String }, _, _]
=> [LinkerTestBase.GetStringFromExpression (args["#1"], _semanticModel), LinkerTestBase.GetStringFromExpression (args["#2"], _semanticModel)],
// ExpectedWarningAttribute(string warningCode, Tool producedBy, string issueLink)
[_, _, _]
=> [],
_ => throw new UnreachableException (),
};

for (int i = 0; i < diagnostics.Count; i++) {
if (Matches (diagnostics[i])) {
Expand Down Expand Up @@ -318,7 +349,7 @@ private void ValidateLogDoesNotContainAttribute (AttributeSyntax attribute, IRea
Assert.False (args.ContainsKey ("#1"));
_ = LinkerTestBase.GetStringFromExpression (arg, _semanticModel);
if (LogContains (attribute, diagnosticMessages, out var matchIndex, out var findText)) {
Assert.Fail($"LogDoesNotContain failure: Text\n\"{findText}\"\nfound in diagnostic:\n {diagnosticMessages[(int) matchIndex]}");
Assert.Fail ($"LogDoesNotContain failure: Text\n\"{findText}\"\nfound in diagnostic:\n {diagnosticMessages[(int) matchIndex]}");
}
}

Expand Down
Expand Up @@ -19,12 +19,6 @@ public Task GenericParameterDataFlowMarking ()
return RunTest (allowMissingWarnings: true);
}

[Fact]
public Task MakeGenericDataflowIntrinsics ()
{
return RunTest (allowMissingWarnings: true);
}

[Fact]
public Task MethodByRefParameterDataFlow ()
{
Expand Down
Expand Up @@ -9,8 +9,27 @@ namespace Mono.Linker.Tests.Cases.Expectations.Assertions
AttributeTargets.Assembly | AttributeTargets.Struct | AttributeTargets.Class | AttributeTargets.Method | AttributeTargets.Property | AttributeTargets.Constructor | AttributeTargets.Field | AttributeTargets.Interface | AttributeTargets.Event,
AllowMultiple = true,
Inherited = false)]
/// <summary>
/// An attribute applied to a member to indicate that a warning is expected in ideal behavior, and is present in all tools
/// </summary>
public class ExpectedWarningAttribute : EnableLoggerAttribute
{
public ExpectedWarningAttribute (string warningCode, string[] messageContains, Tool producedBy, string issueLinkOrReason)
{
}

public ExpectedWarningAttribute (string warningCode, string messageContains, Tool producedBy, string issueLinkOrReason)
{
}

public ExpectedWarningAttribute (string warningCode, string messageContains, string messageContains2, Tool producedBy, string issueLinkOrReason)
{
}

public ExpectedWarningAttribute (string warningCode, Tool producedBy, string issueLinkOrReason)
{
}

public ExpectedWarningAttribute (string warningCode, params string[] messageContains)
{
}
Expand All @@ -19,12 +38,6 @@ public ExpectedWarningAttribute (string warningCode, params string[] messageCont
public int SourceLine { get; set; }
public int SourceColumn { get; set; }

/// <summary>
/// Property used by the result checkers of trimmer and analyzers to determine whether
/// the tool should have produced the specified warning on the annotated member.
/// </summary>
public Tool ProducedBy { get; set; } = Tool.TrimmerAnalyzerAndNativeAot;

public bool CompilerGeneratedCode { get; set; }
}
}
@@ -0,0 +1,26 @@
// Copyright (c) .NET Foundation and contributors. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System;

namespace Mono.Linker.Tests.Cases.Expectations.Assertions
{
[AttributeUsage (
AttributeTargets.Assembly | AttributeTargets.Struct | AttributeTargets.Class | AttributeTargets.Method | AttributeTargets.Property | AttributeTargets.Constructor | AttributeTargets.Field | AttributeTargets.Interface | AttributeTargets.Event,
AllowMultiple = true,
Inherited = false)]
/// <summary>
/// An attribute applied to a member to indicate that a warning is raised in tests, but should not be present in ideal behavior
/// </summary>
public class UnexpectedWarningAttribute : ExpectedWarningAttribute
{
public UnexpectedWarningAttribute (string warningCode, string[] messageContains, Tool producedBy, string issueLinkOrReason)
: base (warningCode, messageContains, producedBy, issueLinkOrReason) { }
public UnexpectedWarningAttribute (string warningCode, string messageContains, Tool producedBy, string issueLinkOrReason)
: base (warningCode, messageContains, producedBy, issueLinkOrReason) { }
public UnexpectedWarningAttribute (string warningCode, string messageContains, string messageContains2, Tool producedBy, string issueLinkOrReason)
: base (warningCode, messageContains, messageContains2, producedBy, issueLinkOrReason) { }
public UnexpectedWarningAttribute (string warningCode, Tool producedBy, string issueLinkOrReason)
: base (warningCode, producedBy, issueLinkOrReason) { }
}
}
Expand Up @@ -29,4 +29,4 @@ public static void Unused ()
{
}
}
}
}

0 comments on commit cdfc8f2

Please sign in to comment.