diff --git a/.github/instructions/csharp.instructions.md b/.github/instructions/csharp.instructions.md index 0c9b6fbf7..c88669513 100644 --- a/.github/instructions/csharp.instructions.md +++ b/.github/instructions/csharp.instructions.md @@ -8,6 +8,20 @@ applyTo: '**/*.cs' - **For complex changes, see the Decision Trees section below** +**MANDATORY: Only Valid C# Code in All Tests** + +> **You MUST NEVER write or include code in analyzer or code fix tests that produces C# compiler errors.** +> - All test code must be valid, compilable C#. +> - Do not write tests for static, const, readonly, or event members if the code would not compile. +> - Do not include code that triggers CSxxxx errors (e.g., invalid member access, missing members, or illegal syntax). +> - If a scenario cannot be expressed as valid C#, it is not a valid test case for analyzers or code fixes. +> - Any test that fails to compile is an immediate failure and must be removed or rewritten. + +**Rationale:** +- Roslyn analyzers and code fixes only operate on valid, successfully parsed C# code. Compiler errors prevent analyzers from running and invalidate the test scenario. +- Writing invalid code in tests causes build failures, test failures, and wastes review/CI resources. +- This is a non-negotiable rule. If you are unsure whether a test is valid C#, STOP and request expert guidance. + ## Primary Instructions Always read and apply the instructions in [.github/copilot-instructions.md](../copilot-instructions.md) when working on C# source or project files. @@ -68,9 +82,10 @@ If you update guidance in copilot-instructions.md that affects C# development, e - Update AnalyzerReleases.Unshipped.md 5. **Update project files if needed** 6. **Run all validations (build, test, lint, Codacy, etc.)** -7. **Prepare PR with validation evidence for each file type** -8. **If any diagnostic span or test fails more than once, STOP and escalate** -9. **If uncertain about Roslyn APIs, Moq semantics, or workflow, escalate** +7. **Validate code coverage of your changed code** +8. **Prepare PR with validation evidence for each file type** +9. **If any diagnostic span or test fails more than once, STOP and escalate** +10. **If uncertain about Roslyn APIs, Moq semantics, or workflow, escalate** ## Test Data & Sample Inputs/Outputs @@ -87,3 +102,4 @@ If you update guidance in copilot-instructions.md that affects C# development, e - Data-driven tests for all fixable patterns - Performance test if analyzer is non-trivial - Document test data rationale in comments or PR description +- Code coverage is automatically generated when `dotnet test --settings ./build/targets/tests/test.runsettings` is run and placed in `./artifacts/TestResults/coverage/Cobertura.xml` diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 21d33a622..7989e78d7 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -44,6 +44,7 @@ jobs: env: IS_CODACY_COVERAGE_ALLOWED: ${{ secrets.CODACY_PROJECT_TOKEN != '' }} IS_QLTY_COVERAGE_ALLOWED: ${{ secrets.QLTY_COVERAGE_TOKEN != '' }} + IS_TARGET_MAIN: ${{ github.ref == 'refs/heads/main' }} RUN_FULL_PERF: ${{ (github.event_name == 'schedule' && github.ref == 'refs/heads/main') || (github.event_name == 'workflow_dispatch' && github.event.inputs.run_performance == 'true') }} FORCE_PERF_BASELINE: ${{ github.event.inputs.force_baseline }} # This is also used in PerfCore.ps1 to determine if the baseline should be forced @@ -55,11 +56,25 @@ jobs: - name: Setup, Restore, and Build Solution uses: ./.github/actions/setup-restore-build + - name: Restore Code Coverage history + uses: actions/download-artifact@v4 + with: + name: CoverageHistory-${{ matrix.os }} + path: ./artifacts/TestResults/coveragehistory + continue-on-error: true + - name: Test run: dotnet test --no-build --configuration Release --settings ./build/targets/tests/test.runsettings env: REPORTGENERATOR_LICENSE: ${{ secrets.REPORTGENERATOR_LICENSE }} + - name: Upload coverage history + uses: actions/upload-artifact@v4 + if: success() && env.IS_TARGET_MAIN == 'true' + with: + name: CoverageHistory-${{ matrix.os }} + path: ./artifacts/TestResults/coveragehistory + - name: Upload binlogs uses: actions/upload-artifact@v4 if: success() || failure() @@ -173,6 +188,7 @@ jobs: $resultsDir = "artifacts/performance/perfResults/baseline/results" if (Test-Path $resultsDir) { Add-Content -Path $env:GITHUB_STEP_SUMMARY -Value "### Baseline Performance Results" + Add-Content -Path $env:GITHUB_STEP_SUMMARY -Value "Baseline SHA: ${{ steps.get-baseline-sha.outputs.sha }}" $files = Get-ChildItem -Path $resultsDir -Filter "*-report-github.md" | Sort-Object Name foreach ($file in $files) { Get-Content $file.FullName | Out-File -Append -FilePath $env:GITHUB_STEP_SUMMARY @@ -201,3 +217,11 @@ jobs: name: performance-${{ matrix.os }} path: | ./artifacts/performance/** + + - name: Upload artifacts + uses: actions/upload-artifact@v4 + if: success() || failure() + with: + name: artifacts-${{ matrix.os }} + path: ./artifacts + if-no-files-found: error diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5483fa5ba..0540e02f9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -283,6 +283,10 @@ ci(workflow): add performance testing to nightly builds Run all unit tests: `dotnet test --settings ./build/targets/tests/test.runsettings` All tests must pass. PRs with failing tests will be closed. +- **Code Coverage:** + Code coverage is generated automatically if you use the `test.runsettings` from above, which produces Cobertura format. If you wish to have a different format, you can specify on the command line. Example: + `dotnet test --collect:"XPlat Code Coverage" --settings ./build/targets/tests/test.runsettings` + PRs must not reduce coverage for critical paths without justification. - **Codacy Analysis:** Run Codacy CLI analysis on all changed files. Fix all reported issues before submitting the PR. - **Evidence Required:** @@ -290,6 +294,7 @@ ci(workflow): add performance testing to nightly builds - `dotnet format` - `dotnet build` - `dotnet test` + - **Code coverage report summary** - Codacy analysis (if issues were found and fixed) - **No Received Files:** Remove any `*.received.*` files before committing. @@ -665,16 +670,18 @@ Before submitting a PR, ensure: **What Constitutes Validation Evidence:** - Test execution logs showing all tests pass +- **Code coverage report summary and/or screenshots showing coverage for changed code** - Performance benchmark results (if applicable) - Screenshots of successful CI runs - Manual testing results for UI changes -- Code coverage reports for new features **Evidence Format:** - Include logs, screenshots, or links to CI runs - Provide clear, readable evidence - Ensure evidence is recent and relevant -- Link to specific test results or benchmarks +- Link to specific test results, code coverage reports, or benchmarks + +**MANDATORY:** Always validate code coverage after making code or test changes. Code coverage validation is a required step before yielding, submitting, or completing any task. Coverage must be reported as part of the validation evidence in PRs and reviews. ### Evidence of Moq Version Awareness diff --git a/build/targets/tests/Tests.targets b/build/targets/tests/Tests.targets index ea4462f20..cb4923a75 100644 --- a/build/targets/tests/Tests.targets +++ b/build/targets/tests/Tests.targets @@ -6,6 +6,7 @@ <_TestCoverageGlob>$(ArtifactsTestResultsPath)/**/*.cobertura.xml <_TestCoverageReportDirectory>$(ArtifactsTestResultsPath)/coverage + <_TestCoverageHistoryDirectory>$(ArtifactsTestResultsPath)/coveragehistory @@ -30,6 +31,7 @@ ProjectDirectory="$(MSBuildProjectDirectory)" ReportFiles="@(_CoverageFiles)" TargetDirectory="$(_TestCoverageReportDirectory)" - ReportTypes="MarkdownSummaryGithub;Cobertura;HtmlInline" /> + ReportTypes="MarkdownSummaryGithub;Cobertura;HtmlInline" + HistoryDirectory="$(_TestCoverageHistoryDirectory)" /> diff --git a/build/targets/tests/test.runsettings b/build/targets/tests/test.runsettings index 58c393e58..1316d6504 100644 --- a/build/targets/tests/test.runsettings +++ b/build/targets/tests/test.runsettings @@ -10,6 +10,8 @@ False False + Obsolete,GeneratedCodeAttribute,CompilerGeneratedAttribute + true ^set_.* ^.*Test.* + ^Microsoft\.Testing.* + ^System\.Diagnostics.* diff --git a/src/Analyzers/LinqToMocksExpressionShouldBeValidAnalyzer.cs b/src/Analyzers/LinqToMocksExpressionShouldBeValidAnalyzer.cs index 9828eb770..6659bf082 100644 --- a/src/Analyzers/LinqToMocksExpressionShouldBeValidAnalyzer.cs +++ b/src/Analyzers/LinqToMocksExpressionShouldBeValidAnalyzer.cs @@ -1,5 +1,4 @@ using Microsoft.CodeAnalysis.Operations; -using Moq.Analyzers.Common; namespace Moq.Analyzers; diff --git a/src/Analyzers/MockRepositoryVerifyAnalyzer.cs b/src/Analyzers/MockRepositoryVerifyAnalyzer.cs index ffcfcf44c..ef24c40c5 100644 --- a/src/Analyzers/MockRepositoryVerifyAnalyzer.cs +++ b/src/Analyzers/MockRepositoryVerifyAnalyzer.cs @@ -1,5 +1,3 @@ -using System.Diagnostics.CodeAnalysis; -using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Operations; namespace Moq.Analyzers; diff --git a/src/Analyzers/VerifyShouldBeUsedOnlyForOverridableMembersAnalyzer.cs b/src/Analyzers/VerifyShouldBeUsedOnlyForOverridableMembersAnalyzer.cs index 74b52140d..ba1313652 100644 --- a/src/Analyzers/VerifyShouldBeUsedOnlyForOverridableMembersAnalyzer.cs +++ b/src/Analyzers/VerifyShouldBeUsedOnlyForOverridableMembersAnalyzer.cs @@ -1,4 +1,3 @@ -using System.Diagnostics.CodeAnalysis; using Microsoft.CodeAnalysis.Operations; namespace Moq.Analyzers; diff --git a/src/Common/EventSyntaxExtensions.cs b/src/Common/EventSyntaxExtensions.cs index 368a028cc..0d16b7966 100644 --- a/src/Common/EventSyntaxExtensions.cs +++ b/src/Common/EventSyntaxExtensions.cs @@ -1,6 +1,3 @@ -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CSharp.Syntax; - namespace Moq.Analyzers.Common; /// diff --git a/tests/Moq.Analyzers.Test/Common/DiagnosticEditPropertiesTests.cs b/tests/Moq.Analyzers.Test/Common/DiagnosticEditPropertiesTests.cs index 988b4b17a..18ef2aa15 100644 --- a/tests/Moq.Analyzers.Test/Common/DiagnosticEditPropertiesTests.cs +++ b/tests/Moq.Analyzers.Test/Common/DiagnosticEditPropertiesTests.cs @@ -11,6 +11,61 @@ public void ToImmutableDictionary_RoundTripsProperties() Assert.Equal("2", dict[DiagnosticEditProperties.EditPositionKey]); } + [Fact] + public void ToImmutableDictionary_AllEnumValues_RoundTrips() + { + foreach (DiagnosticEditProperties.EditType type in Enum.GetValues(typeof(DiagnosticEditProperties.EditType))) + { + DiagnosticEditProperties props = new DiagnosticEditProperties { TypeOfEdit = type, EditPosition = 0 }; + ImmutableDictionary dict = props.ToImmutableDictionary(); + Assert.Equal(type.ToString(), dict[DiagnosticEditProperties.EditTypeKey]); + Assert.Equal("0", dict[DiagnosticEditProperties.EditPositionKey]); + Assert.True(DiagnosticEditProperties.TryGetFromImmutableDictionary(dict, out DiagnosticEditProperties? roundTripped)); + Assert.Equal(type, roundTripped!.TypeOfEdit); + } + } + + [Fact] + public void ToImmutableDictionary_HandlesNegativeAndLargeEditPosition() + { + DiagnosticEditProperties propsNeg = new DiagnosticEditProperties { TypeOfEdit = DiagnosticEditProperties.EditType.Insert, EditPosition = -1 }; + ImmutableDictionary dictNeg = propsNeg.ToImmutableDictionary(); + Assert.Equal("-1", dictNeg[DiagnosticEditProperties.EditPositionKey]); + Assert.True(DiagnosticEditProperties.TryGetFromImmutableDictionary(dictNeg, out DiagnosticEditProperties? roundTrippedNeg)); + Assert.Equal(-1, roundTrippedNeg!.EditPosition); + + DiagnosticEditProperties propsLarge = new DiagnosticEditProperties { TypeOfEdit = DiagnosticEditProperties.EditType.Replace, EditPosition = int.MaxValue }; + ImmutableDictionary dictLarge = propsLarge.ToImmutableDictionary(); + Assert.Equal(int.MaxValue.ToString(), dictLarge[DiagnosticEditProperties.EditPositionKey]); + Assert.True(DiagnosticEditProperties.TryGetFromImmutableDictionary(dictLarge, out DiagnosticEditProperties? roundTrippedLarge)); + Assert.Equal(int.MaxValue, roundTrippedLarge!.EditPosition); + } + + [Fact] + public void TryGetFromImmutableDictionary_IgnoresExtraKeys() + { + ImmutableDictionary dict = ImmutableDictionary.Empty + .Add(DiagnosticEditProperties.EditTypeKey, "Insert") + .Add(DiagnosticEditProperties.EditPositionKey, "1") + .Add("ExtraKey", "ExtraValue"); + Assert.True(DiagnosticEditProperties.TryGetFromImmutableDictionary(dict, out DiagnosticEditProperties? props)); + Assert.Equal(DiagnosticEditProperties.EditType.Insert, props!.TypeOfEdit); + Assert.Equal(1, props.EditPosition); + } + + [Fact] + public void DiagnosticEditProperties_EqualityAndImmutability() + { + DiagnosticEditProperties a = new DiagnosticEditProperties { TypeOfEdit = DiagnosticEditProperties.EditType.Insert, EditPosition = 1 }; + DiagnosticEditProperties b = new DiagnosticEditProperties { TypeOfEdit = DiagnosticEditProperties.EditType.Insert, EditPosition = 1 }; + DiagnosticEditProperties c = new DiagnosticEditProperties { TypeOfEdit = DiagnosticEditProperties.EditType.Replace, EditPosition = 1 }; + Assert.Equal(a, b); + Assert.NotEqual(a, c); + Assert.True(a.Equals(b)); + Assert.False(a.Equals(c)); + Assert.Equal(a.GetHashCode(), b.GetHashCode()); + } + [Fact] public void TryGetFromImmutableDictionary_Succeeds_WithValidDictionary() { diff --git a/tests/Moq.Analyzers.Test/Common/DiagnosticExtensionsTests.cs b/tests/Moq.Analyzers.Test/Common/DiagnosticExtensionsTests.cs new file mode 100644 index 000000000..1257c6d39 --- /dev/null +++ b/tests/Moq.Analyzers.Test/Common/DiagnosticExtensionsTests.cs @@ -0,0 +1,58 @@ +namespace Moq.Analyzers.Test.Common; + +public class DiagnosticExtensionsTests +{ + [Fact] + public void CreateDiagnostic_FromSyntaxNode_Basic() + { + SyntaxTree tree = CSharpSyntaxTree.ParseText("class C { void M() {} }"); + SyntaxNode root = tree.GetRoot(); + DiagnosticDescriptor rule = new DiagnosticDescriptor("TEST0001", "Test", "Test message", "Test", DiagnosticSeverity.Warning, true); + Diagnostic diag = root.CreateDiagnostic(rule); + Assert.Equal("TEST0001", diag.Id); + Assert.Equal(DiagnosticSeverity.Warning, diag.Severity); + } + + [Fact] + public void CreateDiagnostic_FromLocation_WithProperties() + { + SyntaxTree tree = CSharpSyntaxTree.ParseText("class C { void M() {} }"); + Location loc = tree.GetRoot().GetLocation(); + DiagnosticDescriptor rule = new DiagnosticDescriptor("TEST0002", "Test2", "Test message 2", "Test", DiagnosticSeverity.Info, true); + ImmutableDictionary properties = ImmutableDictionary.Empty.Add("Key", "Value"); + Diagnostic diag = loc.CreateDiagnostic(rule, properties); + Assert.Equal("TEST0002", diag.Id); + Assert.Equal("Value", diag.Properties["Key"]); + } + + [Fact] + public void CreateDiagnostic_FromOperation_DelegatesToSyntax() + { + SyntaxTree tree = CSharpSyntaxTree.ParseText("class C { void M() {} }"); + SyntaxNode root = tree.GetRoot(); + CSharpCompilation compilation = CSharpCompilation.Create("Test", new[] { tree }); + SemanticModel model = compilation.GetSemanticModel(tree); + MethodDeclarationSyntax methodDecl = root.DescendantNodes().OfType().First(); + Microsoft.CodeAnalysis.IOperation? operation = model.GetOperation(methodDecl); + DiagnosticDescriptor rule = new DiagnosticDescriptor("TEST0003", "Test3", "Test message 3", "Test", DiagnosticSeverity.Error, true); + Diagnostic diag = operation!.CreateDiagnostic(rule); + Assert.Equal("TEST0003", diag.Id); + Assert.Equal(DiagnosticSeverity.Error, diag.Severity); + } + + [Fact] + public void CreateDiagnostic_FromOperation_WithProperties() + { + SyntaxTree tree = CSharpSyntaxTree.ParseText("class C { void M() {} }"); + SyntaxNode root = tree.GetRoot(); + CSharpCompilation compilation = CSharpCompilation.Create("Test", new[] { tree }); + SemanticModel model = compilation.GetSemanticModel(tree); + MethodDeclarationSyntax methodDecl = root.DescendantNodes().OfType().First(); + Microsoft.CodeAnalysis.IOperation? operation = model.GetOperation(methodDecl); + DiagnosticDescriptor rule = new DiagnosticDescriptor("TEST0004", "Test4", "Test message 4", "Test", DiagnosticSeverity.Warning, true); + ImmutableDictionary properties = ImmutableDictionary.Empty.Add("Key2", "Value2"); + Diagnostic diag = operation!.CreateDiagnostic(rule, properties); + Assert.Equal("TEST0004", diag.Id); + Assert.Equal("Value2", diag.Properties["Key2"]); + } +} diff --git a/tests/Moq.Analyzers.Test/Common/MoqVerificationHelpersTests.cs b/tests/Moq.Analyzers.Test/Common/MoqVerificationHelpersTests.cs new file mode 100644 index 000000000..4da0a8c36 --- /dev/null +++ b/tests/Moq.Analyzers.Test/Common/MoqVerificationHelpersTests.cs @@ -0,0 +1,86 @@ +using Microsoft.CodeAnalysis.Operations; + +namespace Moq.Analyzers.Test.Common; + +public class MoqVerificationHelpersTests +{ + [Fact] + public void ExtractLambdaFromArgument_ReturnsLambda_ForDirectLambda() + { + string code = @"class C { void M() { System.Action a = () => { }; } }"; + SyntaxTree tree = CSharpSyntaxTree.ParseText(code); + CSharpCompilation compilation = CSharpCompilation.Create("Test", new[] { tree }, new[] { MetadataReference.CreateFromFile(typeof(object).Assembly.Location) }); + SemanticModel model = compilation.GetSemanticModel(tree); + ParenthesizedLambdaExpressionSyntax lambda = tree.GetRoot().DescendantNodes().OfType().First(); + IOperation? operation = model.GetOperation(lambda); + IAnonymousFunctionOperation? result = MoqVerificationHelpers.ExtractLambdaFromArgument(operation!); + Assert.NotNull(result); + Assert.IsAssignableFrom(result); + } + + [Fact] + public void ExtractLambdaFromArgument_ReturnsLambda_ForSimpleLambda() + { + string code = @"class C { void M() { System.Func f = x => x.ToString(); } }"; + SyntaxTree tree = CSharpSyntaxTree.ParseText(code); + CSharpCompilation compilation = CSharpCompilation.Create("Test", new[] { tree }, new[] { MetadataReference.CreateFromFile(typeof(object).Assembly.Location) }); + SemanticModel model = compilation.GetSemanticModel(tree); + SimpleLambdaExpressionSyntax lambda = tree.GetRoot().DescendantNodes().OfType().First(); + IOperation? operation = model.GetOperation(lambda); + IAnonymousFunctionOperation? result = MoqVerificationHelpers.ExtractLambdaFromArgument(operation!); + Assert.NotNull(result); + Assert.IsAssignableFrom(result); + } + + [Fact] + public void ExtractLambdaFromArgument_ReturnsNull_ForMethodGroupConversion() + { + string code = @"class C { void M() { System.Action a = M2; } void M2() { } }"; + SyntaxTree tree = CSharpSyntaxTree.ParseText(code); + CSharpCompilation compilation = CSharpCompilation.Create("Test", new[] { tree }, new[] { MetadataReference.CreateFromFile(typeof(object).Assembly.Location) }); + SemanticModel model = compilation.GetSemanticModel(tree); + IdentifierNameSyntax methodGroup = tree.GetRoot().DescendantNodes().OfType().First(id => string.Equals(id.Identifier.Text, "M2", StringComparison.Ordinal)); + IOperation? operation = model.GetOperation(methodGroup); + IAnonymousFunctionOperation? result = MoqVerificationHelpers.ExtractLambdaFromArgument(operation!); + Assert.Null(result); + } + + [Fact] + public void ExtractLambdaFromArgument_ReturnsNull_ForNullInput() + { + // Suppress CS8625: Cannot convert null literal to non-nullable reference type +#pragma warning disable CS8625 + IAnonymousFunctionOperation? result = MoqVerificationHelpers.ExtractLambdaFromArgument(null); +#pragma warning restore CS8625 + Assert.Null(result); + } + + [Fact] + public void ExtractLambdaFromArgument_ReturnsLambda_ForNestedLambda() + { + string code = @"class C { void M() { System.Func f = x => () => { }; } }"; + SyntaxTree tree = CSharpSyntaxTree.ParseText(code); + CSharpCompilation compilation = CSharpCompilation.Create("Test", new[] { tree }, new[] { MetadataReference.CreateFromFile(typeof(object).Assembly.Location) }); + SemanticModel model = compilation.GetSemanticModel(tree); + + // Find the inner (nested) lambda + ParenthesizedLambdaExpressionSyntax innerLambda = tree.GetRoot().DescendantNodes().OfType().First(); + IOperation? operation = model.GetOperation(innerLambda); + IAnonymousFunctionOperation? result = MoqVerificationHelpers.ExtractLambdaFromArgument(operation!); + Assert.NotNull(result); + Assert.IsAssignableFrom(result); + } + + [Fact] + public void ExtractLambdaFromArgument_ReturnsNull_ForNonLambda() + { + string code = @"class C { void M() { int x = 1; } }"; + SyntaxTree tree = CSharpSyntaxTree.ParseText(code); + CSharpCompilation compilation = CSharpCompilation.Create("Test", new[] { tree }, new[] { MetadataReference.CreateFromFile(typeof(object).Assembly.Location) }); + SemanticModel model = compilation.GetSemanticModel(tree); + LiteralExpressionSyntax literal = tree.GetRoot().DescendantNodes().OfType().First(); + IOperation? operation = model.GetOperation(literal); + IAnonymousFunctionOperation? result = MoqVerificationHelpers.ExtractLambdaFromArgument(operation!); + Assert.Null(result); + } +} diff --git a/tests/Moq.Analyzers.Test/EventSetupHandlerShouldMatchEventTypeAnalyzerTests.cs b/tests/Moq.Analyzers.Test/EventSetupHandlerShouldMatchEventTypeAnalyzerTests.cs index a810f02af..d671232b4 100644 --- a/tests/Moq.Analyzers.Test/EventSetupHandlerShouldMatchEventTypeAnalyzerTests.cs +++ b/tests/Moq.Analyzers.Test/EventSetupHandlerShouldMatchEventTypeAnalyzerTests.cs @@ -4,27 +4,72 @@ namespace Moq.Analyzers.Test; public class EventSetupHandlerShouldMatchEventTypeAnalyzerTests { + // Only one version of each static data source method + public static IEnumerable InvalidTestData() + { + return new object[][] + { + // null assignment is a valid C# event handler operation, but not a Moq error + ["""mockProvider.SetupAdd(x => x.StringEvent += null);"""], + + // Mismatched event handler type, compiler emits CS0029 for Action to Action (span 23,36,23,71) + ["""mockProvider.SetupAdd(x => {|CS0029:x.StringEvent += It.IsAny>()|});"""], + + // Method group assignment, compiler emits CS0131 (not assignable) + ["""mockProvider.SetupAdd(x => {|CS0131:x.ToString()|} += It.IsAny());"""], + + // Mismatched event handler type, compiler emits CS0029 for Action to EventHandler (span 23,36,23,71) + ["""mockProvider.SetupAdd(x => {|CS0029:x.CustomEvent += It.IsAny()|});"""], + }.WithNamespaces().WithNewMoqReferenceAssemblyGroups(); + } + public static IEnumerable ValidTestData() { return new object[][] { - // Valid: Action event with It.IsAny> ["""mockProvider.SetupAdd(x => x.StringEvent += It.IsAny>());"""], - - // Valid: EventHandler event with It.IsAny> ["""mockProvider.SetupAdd(x => x.CustomEvent += It.IsAny>());"""], - - // Valid: Action event with It.IsAny ["""mockProvider.SetupAdd(x => x.SimpleEvent += It.IsAny());"""], - - // Valid: SetupRemove with correct handler type ["""mockProvider.SetupRemove(x => x.StringEvent -= It.IsAny>());"""], - - // Valid: Custom delegate with correct handler type ["""mockProvider.SetupAdd(x => x.CustomDelegate += It.IsAny());"""], }.WithNamespaces().WithNewMoqReferenceAssemblyGroups(); } + [Theory] + [MemberData(nameof(InvalidTestData))] + public async Task ShouldHandleInvalidEventSetups(string referenceAssemblyGroup, string @namespace, string setupCall) + { + await Verifier.VerifyAnalyzerAsync( + $$""" + {{@namespace}} + + internal class CustomArgs : EventArgs + { + public string Value { get; set; } + } + + internal delegate void MyDelegate(string value); + + internal interface ITestInterface + { + event Action StringEvent; + event EventHandler CustomEvent; + event Action SimpleEvent; + event MyDelegate CustomDelegate; + } + + internal class UnitTest + { + private void Test() + { + var mockProvider = new Mock(); + {{setupCall}} + } + } + """, + referenceAssemblyGroup); + } + [Theory] [MemberData(nameof(ValidTestData))] public async Task ShouldNotReportDiagnosticForValidEventSetup(string referenceAssemblyGroup, string @namespace, string setupCall) diff --git a/tests/Moq.Analyzers.Test/IsRaisesMethodTests.cs b/tests/Moq.Analyzers.Test/IsRaisesMethodTests.cs index be0003037..f957898eb 100644 --- a/tests/Moq.Analyzers.Test/IsRaisesMethodTests.cs +++ b/tests/Moq.Analyzers.Test/IsRaisesMethodTests.cs @@ -1,5 +1,3 @@ -using Verifier = Moq.Analyzers.Test.Helpers.AllAnalyzersVerifier; - namespace Moq.Analyzers.Test; /// diff --git a/tests/Moq.Analyzers.Test/LinqToMocksExpressionShouldBeValidAnalyzerTests.cs b/tests/Moq.Analyzers.Test/LinqToMocksExpressionShouldBeValidAnalyzerTests.cs index 9b9a637d1..d5f3cd6e5 100644 --- a/tests/Moq.Analyzers.Test/LinqToMocksExpressionShouldBeValidAnalyzerTests.cs +++ b/tests/Moq.Analyzers.Test/LinqToMocksExpressionShouldBeValidAnalyzerTests.cs @@ -4,31 +4,51 @@ namespace Moq.Analyzers.Test; public class LinqToMocksExpressionShouldBeValidAnalyzerTests(ITestOutputHelper output) { + private readonly ITestOutputHelper output = output; + + // Only one version of each static data source method + public static IEnumerable EdgeCaseExpressionTestData() + { + return new object[][] + { + // Existing edge cases + ["""Mock.Of(null);"""], + ["""Mock.Of(r => 42 == 42);"""], + ["""Mock.Of(r => r != null);"""], + ["""Mock.Of(r => new Func(() => 1)() == 1);"""], + ["""Mock.Of(r => {|Moq1302:object.Equals(r, null)|});"""], + + // New diverse edge cases + + // Using a conditional operator (valid) + ["""Mock.Of(r => r.IsAuthenticated ? true : false);"""], + + // Using a coalesce operator (valid) + ["""Mock.Of(r => (r.Name ?? "default") == "test");"""], + + // Using a cast (valid) + ["""Mock.Of(r => ((object)r) != null);"""], + + // Using a delegate invocation (valid) + ["""Mock.Of(r => new System.Func(x => true)(r));"""], + + // Using a discard (valid) + ["""Mock.Of(_ => true);"""], + }.WithNamespaces().WithMoqReferenceAssemblyGroups(); + } + public static IEnumerable ValidExpressionTestData() { return new object[][] { - // Interface properties - should be valid ["""Mock.Of(r => r.IsAuthenticated == true);"""], ["""Mock.Of(r => r.Name == "test");"""], - - // Interface methods - should be valid ["""Mock.Of(s => s.GetData() == "result");"""], ["""Mock.Of(s => s.Calculate(1, 2) == 3);"""], - - // Virtual properties - should be valid ["""Mock.Of(b => b.VirtualProperty == "test");"""], - - // Virtual methods - should be valid ["""Mock.Of(b => b.VirtualMethod() == true);"""], - - // Abstract properties - should be valid ["""Mock.Of(a => a.AbstractProperty == 42);"""], - - // Abstract methods - should be valid ["""Mock.Of(a => a.AbstractMethod() == "result");"""], - - // Simple boolean expressions without member access ["""Mock.Of(r => true);"""], ["""Mock.Of(r => false);"""], }.WithNamespaces().WithMoqReferenceAssemblyGroups(); @@ -38,16 +58,9 @@ public static IEnumerable InvalidExpressionTestData() { return new object[][] { - // Non-virtual properties - should trigger diagnostic ["""Mock.Of(c => {|Moq1302:c.NonVirtualProperty|} == "test");"""], - - // Non-virtual methods - should trigger diagnostic ["""Mock.Of(c => {|Moq1302:c.NonVirtualMethod()|} == true);"""], - - // Sealed class properties ["""Mock.Of(s => {|Moq1302:s.Property|} == "value");"""], - - // Static methods (if they could be called in LINQ expressions) ["""Mock.Of(c => {|Moq1302:c.InstanceMethod()|} == 42);"""], }.WithNamespaces().WithMoqReferenceAssemblyGroups(); } @@ -56,11 +69,35 @@ public static IEnumerable ComplexExpressionTestData() { return new object[][] { - // Field reference - should trigger diagnostic ["""Mock.Of(c => {|Moq1302:c.Field|} == 5);"""], }.WithNamespaces().WithMoqReferenceAssemblyGroups(); } + [Theory] + [MemberData(nameof(EdgeCaseExpressionTestData))] + public async Task ShouldHandleEdgeCaseLinqToMocksExpressions(string referenceAssemblyGroup, string @namespace, string mockExpression) + { + await Verifier.VerifyAnalyzerAsync( + $$""" + {{@namespace}} + + public interface IRepository + { + bool IsAuthenticated { get; set; } + string Name { get; set; } + } + + internal class UnitTest + { + private void Test() + { + var mock = {{mockExpression}} + } + } + """, + referenceAssemblyGroup); + } + [Theory] [MemberData(nameof(ValidExpressionTestData))] public async Task ShouldNotReportDiagnosticForValidLinqToMocksExpressions(string referenceAssemblyGroup, string @namespace, string mockExpression) diff --git a/tests/Moq.Analyzers.Test/MethodSetupShouldSpecifyReturnValueAnalyzerTests.cs b/tests/Moq.Analyzers.Test/MethodSetupShouldSpecifyReturnValueAnalyzerTests.cs index d1fe53d06..c5fdb8ca5 100644 --- a/tests/Moq.Analyzers.Test/MethodSetupShouldSpecifyReturnValueAnalyzerTests.cs +++ b/tests/Moq.Analyzers.Test/MethodSetupShouldSpecifyReturnValueAnalyzerTests.cs @@ -6,9 +6,26 @@ public class MethodSetupShouldSpecifyReturnValueAnalyzerTests(ITestOutputHelper { public static IEnumerable TestData() { + IEnumerable edge = + [ + + // Null lambda (should not crash, should not report diagnostic) + ["""new Mock().Setup(null);"""], + + // Constant expression (should not report diagnostic) + ["""new Mock().Setup(x => 42);"""], + + // Setup with method group (should report Moq1203 diagnostic) + ["""{|Moq1203:new Mock().Setup(x => x.ToString())|};"""], + + // Setup with nested lambda (should report Moq1203 diagnostic) + ["""{|Moq1203:new Mock().Setup(x => new Func(() => 1)())|};"""], + ]; + // Test cases where a method setup should specify return value but doesn't - IEnumerable both = new object[][] - { + IEnumerable both = + [ + // Method with return type should specify return value ["""{|Moq1203:new Mock().Setup(x => x.DoSomething("test"))|};"""], ["""{|Moq1203:new Mock().Setup(x => x.GetValue())|};"""], @@ -28,9 +45,9 @@ public static IEnumerable TestData() // Property setups should not trigger this analyzer ["""new Mock().Setup(x => x.Name);"""], ["""new Mock().SetupGet(x => x.Name);"""], - }.WithNamespaces().WithMoqReferenceAssemblyGroups(); + ]; - return both; + return both.Union(edge).WithNamespaces().WithMoqReferenceAssemblyGroups(); } [Theory] diff --git a/tests/Moq.Analyzers.Test/RedundantTimesSpecificationAnalyzerTests.cs b/tests/Moq.Analyzers.Test/RedundantTimesSpecificationAnalyzerTests.cs index 0c8fa3eae..44431b482 100644 --- a/tests/Moq.Analyzers.Test/RedundantTimesSpecificationAnalyzerTests.cs +++ b/tests/Moq.Analyzers.Test/RedundantTimesSpecificationAnalyzerTests.cs @@ -1,4 +1,3 @@ -using Moq.Analyzers.Test.Helpers; using Verifier = Moq.Analyzers.Test.Helpers.AnalyzerVerifier; namespace Moq.Analyzers.Test; @@ -7,38 +6,69 @@ public class RedundantTimesSpecificationAnalyzerTests(ITestOutputHelper output) { public static IEnumerable TestData() { - IEnumerable both = new object[][] - { + IEnumerable both = + [ + // Should detect redundant Times.AtLeastOnce() - ["""new Mock().Verify(x => x.TestMethod(), {|Moq1420:Times.AtLeastOnce()|});"""], + ["new Mock().Verify(x => x.TestMethod(), {|Moq1420:Times.AtLeastOnce()|});"], + + // Should NOT detect redundant for Times.AtMostOnce() + ["new Mock().Verify(x => x.TestMethod(), Times.AtMostOnce());"], + + // Should NOT detect redundant for Times.AtLeast(1) + ["new Mock().Verify(x => x.TestMethod(), Times.AtLeast(1));"], + + // Should NOT detect redundant for Times.AtMost(1) + ["new Mock().Verify(x => x.TestMethod(), Times.AtMost(1));"], // Should NOT detect other Times specifications - ["""new Mock().Verify(x => x.TestMethod(), Times.Never());"""], - ["""new Mock().Verify(x => x.TestMethod(), Times.Once());"""], - ["""new Mock().Verify(x => x.TestMethod(), Times.Exactly(3));"""], + ["new Mock().Verify(x => x.TestMethod(), Times.Never());"], + ["new Mock().Verify(x => x.TestMethod(), Times.Once());"], + ["new Mock().Verify(x => x.TestMethod(), Times.Exactly(3));"], + + // Should NOT detect variable/expression-based Times + ["int times = 2; new Mock().Verify(x => x.TestMethod(), Times.Exactly(times));"], + ["int times = 2; new Mock().Verify(x => x.TestMethod(), Times.AtLeast(times));"], // Should NOT detect Verify calls without Times parameter (default behavior) - ["""new Mock().Verify(x => x.TestMethod());"""], + ["new Mock().Verify(x => x.TestMethod());"], // Should detect redundant Times.AtLeastOnce() in VerifyGet - ["""new Mock().VerifyGet(x => x.TestProperty, {|Moq1420:Times.AtLeastOnce()|});"""], + ["new Mock().VerifyGet(x => x.TestProperty, {|Moq1420:Times.AtLeastOnce()|});"], + + // Should NOT detect redundant for Times.AtMostOnce() in VerifyGet + ["new Mock().VerifyGet(x => x.TestProperty, Times.AtMostOnce());"], // Should NOT detect other Times in VerifyGet - ["""new Mock().VerifyGet(x => x.TestProperty, Times.Never());"""], - ["""new Mock().VerifyGet(x => x.TestProperty);"""], - }.WithNamespaces().WithMoqReferenceAssemblyGroups(); + ["new Mock().VerifyGet(x => x.TestProperty, Times.Never());"], + ["new Mock().VerifyGet(x => x.TestProperty);"], + + // Should NOT detect variable/expression-based Times in VerifyGet + ["int times = 2; new Mock().VerifyGet(x => x.TestProperty, Times.Exactly(times));"], + + // Should detect redundant Times with message argument + ["new Mock().Verify(x => x.TestMethod(), {|Moq1420:Times.AtLeastOnce()|}, \"should be called at least once\");"], + ["new Mock().VerifyGet(x => x.TestProperty, {|Moq1420:Times.AtLeastOnce()|}, \"should be called at least once\");"], + ]; + + IEnumerable newMoqOnly = + [ - IEnumerable newMoqOnly = new object[][] - { // Should detect redundant Times.AtLeastOnce() in VerifySet (only available in new Moq versions) - ["""new Mock().VerifySet(x => { x.TestProperty = It.IsAny(); }, {|Moq1420:Times.AtLeastOnce()|});"""], + ["new Mock().VerifySet(x => { x.TestProperty = It.IsAny(); }, {|Moq1420:Times.AtLeastOnce()|});"], + + // Should NOT detect redundant for Times.AtMostOnce() in VerifySet + ["new Mock().VerifySet(x => { x.TestProperty = It.IsAny(); }, Times.AtMostOnce());"], // Should NOT detect other Times in VerifySet - ["""new Mock().VerifySet(x => { x.TestProperty = It.IsAny(); }, Times.Never());"""], - ["""new Mock().VerifySet(x => { x.TestProperty = It.IsAny(); });"""], - }.WithNamespaces().WithNewMoqReferenceAssemblyGroups(); + ["new Mock().VerifySet(x => { x.TestProperty = It.IsAny(); }, Times.Never());"], + ["new Mock().VerifySet(x => { x.TestProperty = It.IsAny(); });"], + + // Should NOT detect variable/expression-based Times in VerifySet + ["int times = 2; new Mock().VerifySet(x => { x.TestProperty = It.IsAny(); }, Times.Exactly(times));"], + ]; - return both.Concat(newMoqOnly); + return both.Concat(newMoqOnly).WithNamespaces().WithMoqReferenceAssemblyGroups(); } [Theory] diff --git a/tests/Moq.Analyzers.Test/RefOutCallbackTests.cs b/tests/Moq.Analyzers.Test/RefOutCallbackTests.cs index 02cbe90b2..06aee9f5e 100644 --- a/tests/Moq.Analyzers.Test/RefOutCallbackTests.cs +++ b/tests/Moq.Analyzers.Test/RefOutCallbackTests.cs @@ -1,6 +1,3 @@ -using Moq.Analyzers.Test.Helpers; - -using AnalyzerVerifier = Moq.Analyzers.Test.Helpers.AnalyzerVerifier; using Verifier = Moq.Analyzers.Test.Helpers.CodeFixVerifier; namespace Moq.Analyzers.Test; diff --git a/tests/Moq.Analyzers.Test/ReturnsAsyncShouldBeUsedForAsyncMethodsAnalyzerTests.cs b/tests/Moq.Analyzers.Test/ReturnsAsyncShouldBeUsedForAsyncMethodsAnalyzerTests.cs index 5f10774df..22ff730d3 100644 --- a/tests/Moq.Analyzers.Test/ReturnsAsyncShouldBeUsedForAsyncMethodsAnalyzerTests.cs +++ b/tests/Moq.Analyzers.Test/ReturnsAsyncShouldBeUsedForAsyncMethodsAnalyzerTests.cs @@ -1,4 +1,3 @@ -using Moq.Analyzers.Test.Helpers; using Verifier = Moq.Analyzers.Test.Helpers.AnalyzerVerifier; namespace Moq.Analyzers.Test; diff --git a/tests/Moq.Analyzers.Test/VerifyShouldBeUsedOnlyForOverridableMembersAnalyzerTests.cs b/tests/Moq.Analyzers.Test/VerifyShouldBeUsedOnlyForOverridableMembersAnalyzerTests.cs index 8a66beb1c..cc9393fcf 100644 --- a/tests/Moq.Analyzers.Test/VerifyShouldBeUsedOnlyForOverridableMembersAnalyzerTests.cs +++ b/tests/Moq.Analyzers.Test/VerifyShouldBeUsedOnlyForOverridableMembersAnalyzerTests.cs @@ -1,4 +1,3 @@ -using Moq.Analyzers.Test.Helpers; using Verifier = Moq.Analyzers.Test.Helpers.AnalyzerVerifier; using Verify = Moq.Analyzers.Test.Helpers.CodeFixVerifier; @@ -392,4 +391,34 @@ public void Test() await Verify.VerifyCodeFixAsync(test, test, referenceAssemblyGroup); } + + [Theory] + [MemberData(nameof(MoqReferenceAssemblyGroups))] + public async Task NoFixForExplicitInterfaceImplementation(string referenceAssemblyGroup) + { + string test = """ + + public interface IMyInterface + { + void MyMethod(); + } + + public class MyClass : IMyInterface + { + void IMyInterface.MyMethod() { } + } + + public class MyTest + { + public void Test() + { + var mock = new Mock(); + // This is not a valid Moq pattern, but ensure analyzer/fixer does not crash or offer a fix + mock.Verify(x => ((IMyInterface)x).MyMethod()); + } + } + """; + + await Verify.VerifyCodeFixAsync(test, test, referenceAssemblyGroup); + } }