Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create separate attribute for warning behavior differences #101220

Merged
merged 10 commits into from
Apr 26, 2024
Original file line number Diff line number Diff line change
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");
sbomer marked this conversation as resolved.
Show resolved Hide resolved
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
70 changes: 51 additions & 19 deletions src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/TestChecker.cs
Original file line number Diff line number Diff line change
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,63 @@ 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;
//break;
sbomer marked this conversation as resolved.
Show resolved Hide resolved
}

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 +281,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 +350,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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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; }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// 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, params string[] messageContains)
jtschuster marked this conversation as resolved.
Show resolved Hide resolved
: base (warningCode, messageContains) { }
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) { }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,4 @@ public static void Unused ()
{
}
}
}
}
Loading
Loading