From fcd32792437885eebcd53c0d484aa4ea58825ffd Mon Sep 17 00:00:00 2001 From: Mark Plesko Date: Thu, 27 Jul 2023 00:19:57 -0700 Subject: [PATCH 01/11] Add check for and remove unnecessary return 100s --- .../XUnitWrapperGenerator.cs | 16 ++++++++++++++++ .../Regression/Dev11/Dev11_5437/Dev11_5437.cs | 1 - .../Dev11/DevDiv2_8863/DevDiv2_8863.cs | 1 - .../Dev11/External/dev11_149090/GcHole1.cs | 1 - .../JitBlue/Runtime_45557/Runtime_45557.cs | 2 -- 5 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs b/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs index cb85e588e2b230..f95fe3c51678b4 100644 --- a/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs +++ b/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs @@ -145,8 +145,11 @@ public void Initialize(IncrementalGeneratorInitializationContext context) } } if (!found) return; +<<<<<<< HEAD // Find methods where all returns are the literal 100 (and there is at least one return) +======= +>>>>>>> e63edc4297b (Add check for and remove unnecessary return 100s) if (method.DeclaringSyntaxReferences.IsEmpty) return; found = false; @@ -162,7 +165,20 @@ public void Initialize(IncrementalGeneratorInitializationContext context) } if (!found) return; +<<<<<<< HEAD context.ReportDiagnostic(Diagnostic.Create(Descriptors.XUWG1002, method.Locations[0])); +======= + context.ReportDiagnostic(Diagnostic.Create( + new DiagnosticDescriptor( + "XUW1003", + "All returns are constant 100", + "A test method that always returns 100 should return \"void\" instead", + "XUnitWrapperGenerator", + DiagnosticSeverity.Warning, + isEnabledByDefault: true), + method.Locations[0])); + +>>>>>>> e63edc4297b (Add check for and remove unnecessary return 100s) }); context.RegisterImplementationSourceOutput( diff --git a/src/tests/JIT/Regression/Dev11/Dev11_5437/Dev11_5437.cs b/src/tests/JIT/Regression/Dev11/Dev11_5437/Dev11_5437.cs index a95089d99f5e80..8e642a78c00e66 100644 --- a/src/tests/JIT/Regression/Dev11/Dev11_5437/Dev11_5437.cs +++ b/src/tests/JIT/Regression/Dev11/Dev11_5437/Dev11_5437.cs @@ -31,6 +31,5 @@ public static void TestEntryPoint() } } Console.WriteLine("PASSED"); - //Didn't hit the bug so return success } } diff --git a/src/tests/JIT/Regression/Dev11/DevDiv2_8863/DevDiv2_8863.cs b/src/tests/JIT/Regression/Dev11/DevDiv2_8863/DevDiv2_8863.cs index c2876c1ac70444..693a17c8f23b67 100644 --- a/src/tests/JIT/Regression/Dev11/DevDiv2_8863/DevDiv2_8863.cs +++ b/src/tests/JIT/Regression/Dev11/DevDiv2_8863/DevDiv2_8863.cs @@ -76,7 +76,6 @@ public static void TestEntryPoint() * This example will AV... * */ x64_JIT_Bug(new MyDerived()); - // Well, we made it here... should be good. } } diff --git a/src/tests/JIT/Regression/Dev11/External/dev11_149090/GcHole1.cs b/src/tests/JIT/Regression/Dev11/External/dev11_149090/GcHole1.cs index eb52ce597d6ee2..9a86fbb2b26e35 100644 --- a/src/tests/JIT/Regression/Dev11/External/dev11_149090/GcHole1.cs +++ b/src/tests/JIT/Regression/Dev11/External/dev11_149090/GcHole1.cs @@ -61,7 +61,6 @@ public static void TestEntryPoint() result = sequence.SingleOrDefault(App.CheckString); Console.WriteLine("RESULT: `{0}'", result); - //assume if run to completion, the test passes } } } diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_45557/Runtime_45557.cs b/src/tests/JIT/Regression/JitBlue/Runtime_45557/Runtime_45557.cs index 99e923d4e05775..334a97eae3778e 100644 --- a/src/tests/JIT/Regression/JitBlue/Runtime_45557/Runtime_45557.cs +++ b/src/tests/JIT/Regression/JitBlue/Runtime_45557/Runtime_45557.cs @@ -85,8 +85,6 @@ public static void TestEntryPoint() { ObjectBinderSnapshot o = ObjectBinder.GetSnapshot(); Console.WriteLine($"Test output: {o.SomeValue}"); - - // success if we got here without crashing } } } From ca96a6c2b32b6ac2cebac70e698fd9cb930804b6 Mon Sep 17 00:00:00 2001 From: Mark Plesko Date: Thu, 27 Jul 2023 01:01:18 -0700 Subject: [PATCH 02/11] restore some comments --- src/tests/JIT/Regression/Dev11/Dev11_5437/Dev11_5437.cs | 1 + src/tests/JIT/Regression/Dev11/DevDiv2_8863/DevDiv2_8863.cs | 1 + src/tests/JIT/Regression/Dev11/External/dev11_149090/GcHole1.cs | 1 + src/tests/JIT/Regression/JitBlue/Runtime_45557/Runtime_45557.cs | 2 ++ 4 files changed, 5 insertions(+) diff --git a/src/tests/JIT/Regression/Dev11/Dev11_5437/Dev11_5437.cs b/src/tests/JIT/Regression/Dev11/Dev11_5437/Dev11_5437.cs index 8e642a78c00e66..a95089d99f5e80 100644 --- a/src/tests/JIT/Regression/Dev11/Dev11_5437/Dev11_5437.cs +++ b/src/tests/JIT/Regression/Dev11/Dev11_5437/Dev11_5437.cs @@ -31,5 +31,6 @@ public static void TestEntryPoint() } } Console.WriteLine("PASSED"); + //Didn't hit the bug so return success } } diff --git a/src/tests/JIT/Regression/Dev11/DevDiv2_8863/DevDiv2_8863.cs b/src/tests/JIT/Regression/Dev11/DevDiv2_8863/DevDiv2_8863.cs index 693a17c8f23b67..c2876c1ac70444 100644 --- a/src/tests/JIT/Regression/Dev11/DevDiv2_8863/DevDiv2_8863.cs +++ b/src/tests/JIT/Regression/Dev11/DevDiv2_8863/DevDiv2_8863.cs @@ -76,6 +76,7 @@ public static void TestEntryPoint() * This example will AV... * */ x64_JIT_Bug(new MyDerived()); + // Well, we made it here... should be good. } } diff --git a/src/tests/JIT/Regression/Dev11/External/dev11_149090/GcHole1.cs b/src/tests/JIT/Regression/Dev11/External/dev11_149090/GcHole1.cs index 9a86fbb2b26e35..eb52ce597d6ee2 100644 --- a/src/tests/JIT/Regression/Dev11/External/dev11_149090/GcHole1.cs +++ b/src/tests/JIT/Regression/Dev11/External/dev11_149090/GcHole1.cs @@ -61,6 +61,7 @@ public static void TestEntryPoint() result = sequence.SingleOrDefault(App.CheckString); Console.WriteLine("RESULT: `{0}'", result); + //assume if run to completion, the test passes } } } diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_45557/Runtime_45557.cs b/src/tests/JIT/Regression/JitBlue/Runtime_45557/Runtime_45557.cs index 334a97eae3778e..99e923d4e05775 100644 --- a/src/tests/JIT/Regression/JitBlue/Runtime_45557/Runtime_45557.cs +++ b/src/tests/JIT/Regression/JitBlue/Runtime_45557/Runtime_45557.cs @@ -85,6 +85,8 @@ public static void TestEntryPoint() { ObjectBinderSnapshot o = ObjectBinder.GetSnapshot(); Console.WriteLine($"Test output: {o.SomeValue}"); + + // success if we got here without crashing } } } From cb64354778d4bd5260a94c4c48f2152fedf8e7fc Mon Sep 17 00:00:00 2001 From: Mark Plesko Date: Thu, 27 Jul 2023 01:09:49 -0700 Subject: [PATCH 03/11] Limit new checks to merged test groups --- .../XUnitWrapperGenerator.cs | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs b/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs index f95fe3c51678b4..5466eaa7129787 100644 --- a/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs +++ b/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs @@ -145,11 +145,6 @@ public void Initialize(IncrementalGeneratorInitializationContext context) } } if (!found) return; -<<<<<<< HEAD - - // Find methods where all returns are the literal 100 (and there is at least one return) -======= ->>>>>>> e63edc4297b (Add check for and remove unnecessary return 100s) if (method.DeclaringSyntaxReferences.IsEmpty) return; found = false; @@ -165,20 +160,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) } if (!found) return; -<<<<<<< HEAD context.ReportDiagnostic(Diagnostic.Create(Descriptors.XUWG1002, method.Locations[0])); -======= - context.ReportDiagnostic(Diagnostic.Create( - new DiagnosticDescriptor( - "XUW1003", - "All returns are constant 100", - "A test method that always returns 100 should return \"void\" instead", - "XUnitWrapperGenerator", - DiagnosticSeverity.Warning, - isEnabledByDefault: true), - method.Locations[0])); - ->>>>>>> e63edc4297b (Add check for and remove unnecessary return 100s) }); context.RegisterImplementationSourceOutput( @@ -203,6 +185,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) if (inMergedTestDirectory) { CheckNoEntryPoint(context, compData); + CheckTestsExist(context, methods, compData); } if (compData.OutputKind != OutputKind.ConsoleApplication) From 3c121585746e2c4c29c0ee92d2e1909951d15ff3 Mon Sep 17 00:00:00 2001 From: Mark Plesko Date: Thu, 27 Jul 2023 02:31:44 -0700 Subject: [PATCH 04/11] add rule that test methods must be public and fix tests --- .../XUnitWrapperGenerator.cs | 51 +++++++++++++++++++ .../JitBlue/GitHub_11408/GitHub_11408.cs | 2 +- .../jit64/regress/vsw/528315/simple-repro.cs | 6 +-- 3 files changed, 55 insertions(+), 4 deletions(-) diff --git a/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs b/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs index 5466eaa7129787..982dd7ba96a9ca 100644 --- a/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs +++ b/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs @@ -163,6 +163,57 @@ public void Initialize(IncrementalGeneratorInitializationContext context) context.ReportDiagnostic(Diagnostic.Create(Descriptors.XUWG1002, method.Locations[0])); }); + context.RegisterImplementationSourceOutput( + methodsInSource, + static (context, method) => + { + bool found = false; + foreach (var attr in method.GetAttributesOnSelfAndContainingSymbols()) + { + switch (attr.AttributeClass?.ToDisplayString()) + { + case "Xunit.ConditionalFactAttribute": + case "Xunit.FactAttribute": + case "Xunit.ConditionalTheoryAttribute": + case "Xunit.TheoryAttribute": + found = true; + break; + } + } + if (!found) return; + + if (method.DeclaredAccessibility != Accessibility.Public) + { + context.ReportDiagnostic(Diagnostic.Create( + new DiagnosticDescriptor( + "XUW1004", + "Test methods must be public", + "Test methods must be public. Add or change the visibility modifier of the test method to public.", + "XUnitWrapperGenerator", + DiagnosticSeverity.Error, + isEnabledByDefault: true), + method.Locations[0])); + } + + INamedTypeSymbol containingType = method.ContainingType; + while (containingType != null) + { + if (containingType.DeclaredAccessibility != Accessibility.Public) + { + context.ReportDiagnostic(Diagnostic.Create( + new DiagnosticDescriptor( + "XUW1005", + "Test classes must be public", + "Test classes must be public. Add or change the visibility modifier of the test class to public.", + "XUnitWrapperGenerator", + DiagnosticSeverity.Error, + isEnabledByDefault: true), + method.Locations[0])); + } + containingType = containingType.ContainingType; + } + }); + context.RegisterImplementationSourceOutput( allTests .Combine(context.AnalyzerConfigOptionsProvider) diff --git a/src/tests/JIT/Regression/JitBlue/GitHub_11408/GitHub_11408.cs b/src/tests/JIT/Regression/JitBlue/GitHub_11408/GitHub_11408.cs index 30a71b8e584763..8f5198e505760b 100644 --- a/src/tests/JIT/Regression/JitBlue/GitHub_11408/GitHub_11408.cs +++ b/src/tests/JIT/Regression/JitBlue/GitHub_11408/GitHub_11408.cs @@ -5,7 +5,7 @@ using System.Runtime.CompilerServices; using Xunit; -class GitHub_11408 +public class GitHub_11408 { const int Pass = 100; const int Fail = -1; diff --git a/src/tests/JIT/jit64/regress/vsw/528315/simple-repro.cs b/src/tests/JIT/jit64/regress/vsw/528315/simple-repro.cs index 7b45c5ebf647a2..307ba121d7d37b 100644 --- a/src/tests/JIT/jit64/regress/vsw/528315/simple-repro.cs +++ b/src/tests/JIT/jit64/regress/vsw/528315/simple-repro.cs @@ -2,12 +2,12 @@ // The .NET Foundation licenses this file to you under the MIT license. using Xunit; -internal enum NodeType +public enum NodeType { True, False, Not, Other } -internal class Node +public class Node { public NodeType NodeType; public Node Child; @@ -15,7 +15,7 @@ internal class Node public Node(string s) { name = s; } } -internal class NodeFactory +public class NodeFactory { public Node Conditional(Node condition, Node trueBranch, Node falseBranch) { From 7c597716fbf1b5fbc87b3f5f47a89f4578a6adb6 Mon Sep 17 00:00:00 2001 From: Mark Plesko Date: Thu, 27 Jul 2023 02:33:01 -0700 Subject: [PATCH 05/11] Remove check that any tests exist --- src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs b/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs index 982dd7ba96a9ca..35ce51bd5226cd 100644 --- a/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs +++ b/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs @@ -236,7 +236,6 @@ public void Initialize(IncrementalGeneratorInitializationContext context) if (inMergedTestDirectory) { CheckNoEntryPoint(context, compData); - CheckTestsExist(context, methods, compData); } if (compData.OutputKind != OutputKind.ConsoleApplication) From 124b2abb18a8f75ff4056460a58e7c3e8fd6cdc6 Mon Sep 17 00:00:00 2001 From: Mark Plesko Date: Thu, 27 Jul 2023 02:34:21 -0700 Subject: [PATCH 06/11] 1003->1002 --- .../XUnitWrapperGenerator/XUnitWrapperGenerator.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs b/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs index 35ce51bd5226cd..ebb6c83b562814 100644 --- a/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs +++ b/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs @@ -160,8 +160,15 @@ public void Initialize(IncrementalGeneratorInitializationContext context) } if (!found) return; - context.ReportDiagnostic(Diagnostic.Create(Descriptors.XUWG1002, method.Locations[0])); - }); + context.ReportDiagnostic(Diagnostic.Create( + new DiagnosticDescriptor( + "XUW1002", + "All returns are constant 100", + "A test method that always returns 100 should return \"void\" instead", + "XUnitWrapperGenerator", + DiagnosticSeverity.Warning, + isEnabledByDefault: true), + method.Locations[0])); context.RegisterImplementationSourceOutput( methodsInSource, From 43ca3e926cf79f61ef1cf9e3beb38259c0a8a64b Mon Sep 17 00:00:00 2001 From: Mark Plesko Date: Thu, 27 Jul 2023 02:35:13 -0700 Subject: [PATCH 07/11] 1004->1003, 1005->1004 --- .../Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs b/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs index ebb6c83b562814..4441d6d75609d5 100644 --- a/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs +++ b/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs @@ -193,7 +193,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) { context.ReportDiagnostic(Diagnostic.Create( new DiagnosticDescriptor( - "XUW1004", + "XUW1003", "Test methods must be public", "Test methods must be public. Add or change the visibility modifier of the test method to public.", "XUnitWrapperGenerator", @@ -209,7 +209,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) { context.ReportDiagnostic(Diagnostic.Create( new DiagnosticDescriptor( - "XUW1005", + "XUW1004", "Test classes must be public", "Test classes must be public. Add or change the visibility modifier of the test class to public.", "XUnitWrapperGenerator", From fbc3053e11164d69fce066ffc97243c468092e59 Mon Sep 17 00:00:00 2001 From: Mark Plesko Date: Thu, 27 Jul 2023 12:00:32 -0700 Subject: [PATCH 08/11] Limit new checks to merged test directories --- .../XUnitWrapperGenerator/XUnitWrapperGenerator.cs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs b/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs index 4441d6d75609d5..f1048a879e05f6 100644 --- a/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs +++ b/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs @@ -130,7 +130,6 @@ public void Initialize(IncrementalGeneratorInitializationContext context) { var (method, _) = data; - // Only check test methods bool found = false; foreach (var attr in method.GetAttributesOnSelfAndContainingSymbols()) { @@ -171,9 +170,17 @@ public void Initialize(IncrementalGeneratorInitializationContext context) method.Locations[0])); context.RegisterImplementationSourceOutput( - methodsInSource, - static (context, method) => + methodsInSource + .Combine(context.AnalyzerConfigOptionsProvider) + .Where(data => + { + var (_, configOptions) = data; + return configOptions.GlobalOptions.InMergedTestDirectory(); + }), + static (context, data) => { + var (method, _) = data; + bool found = false; foreach (var attr in method.GetAttributesOnSelfAndContainingSymbols()) { From da18a9622784504e3c7510be5787aac7b7ed7a27 Mon Sep 17 00:00:00 2001 From: Mark Plesko Date: Thu, 27 Jul 2023 16:54:40 -0700 Subject: [PATCH 09/11] Move XUWG1002 to static --- .../XUnitWrapperGenerator.cs | 68 +------------------ 1 file changed, 1 insertion(+), 67 deletions(-) diff --git a/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs b/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs index f1048a879e05f6..799bb37b2010d3 100644 --- a/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs +++ b/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs @@ -159,73 +159,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) } if (!found) return; - context.ReportDiagnostic(Diagnostic.Create( - new DiagnosticDescriptor( - "XUW1002", - "All returns are constant 100", - "A test method that always returns 100 should return \"void\" instead", - "XUnitWrapperGenerator", - DiagnosticSeverity.Warning, - isEnabledByDefault: true), - method.Locations[0])); - - context.RegisterImplementationSourceOutput( - methodsInSource - .Combine(context.AnalyzerConfigOptionsProvider) - .Where(data => - { - var (_, configOptions) = data; - return configOptions.GlobalOptions.InMergedTestDirectory(); - }), - static (context, data) => - { - var (method, _) = data; - - bool found = false; - foreach (var attr in method.GetAttributesOnSelfAndContainingSymbols()) - { - switch (attr.AttributeClass?.ToDisplayString()) - { - case "Xunit.ConditionalFactAttribute": - case "Xunit.FactAttribute": - case "Xunit.ConditionalTheoryAttribute": - case "Xunit.TheoryAttribute": - found = true; - break; - } - } - if (!found) return; - - if (method.DeclaredAccessibility != Accessibility.Public) - { - context.ReportDiagnostic(Diagnostic.Create( - new DiagnosticDescriptor( - "XUW1003", - "Test methods must be public", - "Test methods must be public. Add or change the visibility modifier of the test method to public.", - "XUnitWrapperGenerator", - DiagnosticSeverity.Error, - isEnabledByDefault: true), - method.Locations[0])); - } - - INamedTypeSymbol containingType = method.ContainingType; - while (containingType != null) - { - if (containingType.DeclaredAccessibility != Accessibility.Public) - { - context.ReportDiagnostic(Diagnostic.Create( - new DiagnosticDescriptor( - "XUW1004", - "Test classes must be public", - "Test classes must be public. Add or change the visibility modifier of the test class to public.", - "XUnitWrapperGenerator", - DiagnosticSeverity.Error, - isEnabledByDefault: true), - method.Locations[0])); - } - containingType = containingType.ContainingType; - } + context.ReportDiagnostic(Diagnostic.Create(Descriptors.XUWG1002, method.Locations[0])); }); context.RegisterImplementationSourceOutput( From 2fe764a019a989c3ba9dfc423c09f521e0afd40b Mon Sep 17 00:00:00 2001 From: Mark Plesko Date: Thu, 27 Jul 2023 17:16:48 -0700 Subject: [PATCH 10/11] Move XUWG1003/4 to static --- .../XUnitWrapperGenerator/Descriptors.cs | 18 ++++++++ .../XUnitWrapperGenerator.cs | 43 +++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/src/tests/Common/XUnitWrapperGenerator/Descriptors.cs b/src/tests/Common/XUnitWrapperGenerator/Descriptors.cs index 130180659693b7..f3a96b07fd483b 100644 --- a/src/tests/Common/XUnitWrapperGenerator/Descriptors.cs +++ b/src/tests/Common/XUnitWrapperGenerator/Descriptors.cs @@ -27,4 +27,22 @@ public static class Descriptors "XUnitWrapperGenerator", DiagnosticSeverity.Warning, isEnabledByDefault: true); + + public static readonly DiagnosticDescriptor XUWG1003 = + new DiagnosticDescriptor( + "XUW1003", + "Test methods must be public", + "Test methods must be public. Add or change the visibility modifier of the test method to public.", + "XUnitWrapperGenerator", + DiagnosticSeverity.Error, + isEnabledByDefault: true); + + public static readonly DiagnosticDescriptor XUWG1004 = + new DiagnosticDescriptor( + "XUW1004", + "Test classes must be public", + "Test classes must be public. Add or change the visibility modifier of the test class to public.", + "XUnitWrapperGenerator", + DiagnosticSeverity.Error, + isEnabledByDefault: true); } diff --git a/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs b/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs index 799bb37b2010d3..069eb9dabdeffa 100644 --- a/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs +++ b/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs @@ -162,6 +162,49 @@ public void Initialize(IncrementalGeneratorInitializationContext context) context.ReportDiagnostic(Diagnostic.Create(Descriptors.XUWG1002, method.Locations[0])); }); + context.RegisterImplementationSourceOutput( + methodsInSource + .Combine(context.AnalyzerConfigOptionsProvider) + .Where(data => + { + var (_, configOptions) = data; + return configOptions.GlobalOptions.InMergedTestDirectory(); + }), + static (context, data) => + { + var (method, _) = data; + + bool found = false; + foreach (var attr in method.GetAttributesOnSelfAndContainingSymbols()) + { + switch (attr.AttributeClass?.ToDisplayString()) + { + case "Xunit.ConditionalFactAttribute": + case "Xunit.FactAttribute": + case "Xunit.ConditionalTheoryAttribute": + case "Xunit.TheoryAttribute": + found = true; + break; + } + } + if (!found) return; + + if (method.DeclaredAccessibility != Accessibility.Public) + { + context.ReportDiagnostic(Diagnostic.Create(Descriptors.XUWG1003, method.Locations[0])); + } + + INamedTypeSymbol containingType = method.ContainingType; + while (containingType != null) + { + if (containingType.DeclaredAccessibility != Accessibility.Public) + { + context.ReportDiagnostic(Diagnostic.Create(Descriptors.XUWG1004, method.Locations[0])); + } + containingType = containingType.ContainingType; + } + }); + context.RegisterImplementationSourceOutput( allTests .Combine(context.AnalyzerConfigOptionsProvider) From a34e2b76834f9db39be07c33fe41ab05c21ed4b6 Mon Sep 17 00:00:00 2001 From: Mark Plesko Date: Fri, 28 Jul 2023 12:12:47 -0700 Subject: [PATCH 11/11] Limit check to merged test directories, add comments --- .../Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs b/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs index 069eb9dabdeffa..3c4828cc676b50 100644 --- a/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs +++ b/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs @@ -130,6 +130,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) { var (method, _) = data; + // Only check test methods bool found = false; foreach (var attr in method.GetAttributesOnSelfAndContainingSymbols()) { @@ -144,6 +145,8 @@ public void Initialize(IncrementalGeneratorInitializationContext context) } } if (!found) return; + + // Find methods where all returns are the literal 100 (and there is at least one return) if (method.DeclaringSyntaxReferences.IsEmpty) return; found = false;