Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ private static Task<Document> ReplaceReturnsWithReturnsAsync(
{
SimpleNameSyntax oldName = memberAccess.Name;
SimpleNameSyntax newName;

// Moq's IReturns<TMock, TResult> interface defines method-level generic overloads
// like Returns<T1>(Func<T1, TResult>). When a user writes .Returns<string>(s => 42),
// the syntax is GenericNameSyntax. Preserve type arguments to maintain developer intent.
if (oldName is GenericNameSyntax genericName)
Comment on lines +66 to 69
{
newName = SyntaxFactory.GenericName(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
using System.Collections.Immutable;

Check warning on line 1 in tests/Moq.Analyzers.Benchmarks/Moq1208ReturnsDelegateBenchmarks.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

tests/Moq.Analyzers.Benchmarks/Moq1208ReturnsDelegateBenchmarks.cs#L1

Provide an 'AssemblyVersion' attribute for assembly 'srcassembly.dll'.
using System.Diagnostics.CodeAnalysis;
using Microsoft.CodeAnalysis;

Check notice on line 3 in tests/Moq.Analyzers.Benchmarks/Moq1208ReturnsDelegateBenchmarks.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

tests/Moq.Analyzers.Benchmarks/Moq1208ReturnsDelegateBenchmarks.cs#L3

Remove this unnecessary 'using'.
using Microsoft.CodeAnalysis.Diagnostics;
using Moq.Analyzers.Benchmarks.Helpers;

namespace Moq.Analyzers.Benchmarks;

[InProcess]
[MemoryDiagnoser]
[BenchmarkCategory("Moq1208")]
public class Moq1208ReturnsDelegateBenchmarks
{
#pragma warning disable ECS0900
[Params(1, 1_000)]

Check notice on line 15 in tests/Moq.Analyzers.Benchmarks/Moq1208ReturnsDelegateBenchmarks.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

tests/Moq.Analyzers.Benchmarks/Moq1208ReturnsDelegateBenchmarks.cs#L15

Assign this magic number '1000' to a well-named variable or constant, and use that instead.
#pragma warning restore ECS0900
public int FileCount { get; set; }

[Params("Net80WithOldMoq", "Net80WithNewMoq")]
public string MoqKey { get; set; } = "Net80WithOldMoq";
Comment on lines +12 to +20
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Add XML documentation for the public benchmark API surface.

This file introduces public members without XML docs (Moq1208ReturnsDelegateBenchmarks, FileCount, MoqKey, SetupCompilation, Moq1208WithDiagnostics, Moq1208Baseline).

As per coding guidelines, **/*.cs: "Ensure all public APIs have complete XML documentation".

Also applies to: 28-28, 66-66, 83-83

🧰 Tools
🪛 GitHub Check: Codacy Static Code Analysis

[notice] 15-15: tests/Moq.Analyzers.Benchmarks/Moq1208ReturnsDelegateBenchmarks.cs#L15
Assign this magic number '1000' to a well-named variable or constant, and use that instead.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/Moq.Analyzers.Benchmarks/Moq1208ReturnsDelegateBenchmarks.cs` around
lines 12 - 20, Add XML documentation comments for the public benchmark API
surface: add a summary for the class Moq1208ReturnsDelegateBenchmarks and for
each public member FileCount and MoqKey, and add appropriate <summary>,
<returns> and <param> (if any) for the methods SetupCompilation,
Moq1208WithDiagnostics, and Moq1208Baseline so every public symbol listed has
complete XML docs; ensure summaries describe purpose, parameters and return
values where applicable and follow the project's XML doc style.


private CompilationWithAnalyzers? BaselineCompilation { get; set; }

private CompilationWithAnalyzers? TestCompilation { get; set; }

[IterationSetup]
[SuppressMessage("Usage", "VSTHRD002:Avoid problematic synchronous waits", Justification = "Async setup not supported in BenchmarkDotNet.See https://github.com/dotnet/BenchmarkDotNet/issues/2442.")]
public void SetupCompilation()
{
List<(string Name, string Content)> sources = [];
for (int index = 0; index < FileCount; index++)
{
string name = "TypeName" + index;
sources.Add((name, @$"
using System;
using System.Threading.Tasks;
using Moq;

public class AsyncClient{index}
{{
public virtual Task<int> GetValueAsync() => Task.FromResult(0);
public virtual Task<string> GetNameAsync() => Task.FromResult(string.Empty);
public virtual ValueTask<int> GetValueTaskAsync() => ValueTask.FromResult(0);
}}

internal class {name}
{{
private void Test()
{{
new Mock<AsyncClient{index}>().Setup(c => c.GetValueAsync()).Returns(() => 42);
_ = ""sample test""; // Add an expression that looks similar but does not match
}}
}}
"));
}

Microsoft.CodeAnalysis.Testing.ReferenceAssemblies referenceAssemblies = CompilationCreator.GetReferenceAssemblies(MoqKey);
(BaselineCompilation, TestCompilation) =
BenchmarkCSharpCompilationFactory
.CreateAsync<ReturnsDelegateShouldReturnTaskAnalyzer>(sources.ToArray(), referenceAssemblies)
.GetAwaiter()
.GetResult();
Comment thread
rjmurillo marked this conversation as resolved.
}

[Benchmark]
public async Task Moq1208WithDiagnostics()
{
ImmutableArray<Diagnostic> diagnostics =
(await TestCompilation!
.GetAnalysisResultAsync(CancellationToken.None)
Comment on lines +69 to +70
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Replace null-forgiving dereferences with explicit setup guards.

Line 69 and Line 86 assume setup always ran; if not, benchmark failures degrade to opaque NREs. Add explicit state checks before analysis calls.

💡 Suggested change
 [Benchmark]
 public async Task Moq1208WithDiagnostics()
 {
+    if (TestCompilation is null)
+    {
+        throw new InvalidOperationException($"{nameof(SetupCompilation)} must run before {nameof(Moq1208WithDiagnostics)}.");
+    }
+
     ImmutableArray<Diagnostic> diagnostics =
-        (await TestCompilation!
+        (await TestCompilation
         .GetAnalysisResultAsync(CancellationToken.None)
         .ConfigureAwait(false))
         .AssertValidAnalysisResult()
         .GetAllDiagnostics();
@@
 [Benchmark(Baseline = true)]
 public async Task Moq1208Baseline()
 {
+    if (BaselineCompilation is null)
+    {
+        throw new InvalidOperationException($"{nameof(SetupCompilation)} must run before {nameof(Moq1208Baseline)}.");
+    }
+
     ImmutableArray<Diagnostic> diagnostics =
-        (await BaselineCompilation!
+        (await BaselineCompilation
         .GetAnalysisResultAsync(CancellationToken.None)
         .ConfigureAwait(false))
         .AssertValidAnalysisResult()
         .GetAllDiagnostics();

Also applies to: 86-87

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/Moq.Analyzers.Benchmarks/Moq1208ReturnsDelegateBenchmarks.cs` around
lines 69 - 70, The test currently uses null-forgiving on TestCompilation (e.g.,
(await TestCompilation!.GetAnalysisResultAsync(CancellationToken.None))) which
can throw opaque NREs if setup didn't run; update the benchmark methods that
call TestCompilation!.GetAnalysisResultAsync (and any similar calls at lines
referenced) to first assert or guard that TestCompilation is not null (for
example throw a clear InvalidOperationException or use Assert.NotNull on
TestCompilation) and only then call GetAnalysisResultAsync; ensure all
occurrences (including the calls around line 86/87) have the same explicit guard
so failures surface as clear setup errors rather than NREs.

.ConfigureAwait(false))
.AssertValidAnalysisResult()
.GetAllDiagnostics();

// Each file has one Returns(() => 42) on a Task<int> method
if (diagnostics.Length != FileCount)
{
throw new InvalidOperationException($"Expected '{FileCount:N0}' analyzer diagnostics but found '{diagnostics.Length}'");
}
Comment thread
rjmurillo marked this conversation as resolved.
}

[Benchmark(Baseline = true)]
public async Task Moq1208Baseline()
{
ImmutableArray<Diagnostic> diagnostics =
(await BaselineCompilation!
.GetAnalysisResultAsync(CancellationToken.None)
.ConfigureAwait(false))
.AssertValidAnalysisResult()
.GetAllDiagnostics();

if (diagnostics.Length != 0)
{
throw new InvalidOperationException($"Expected no analyzer diagnostics but found '{diagnostics.Length}'");
}
}
Comment thread
rjmurillo marked this conversation as resolved.
Comment thread
rjmurillo marked this conversation as resolved.
}
Comment thread
qltysh[bot] marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,12 @@ public static IEnumerable<object[]> ValidWithCompilerSuppression()

// Invocation as value, not delegate: GetInt() is a call, not a method group (GH PR #942 review thread)
["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).Returns(GetInt());"""],

// Generic Returns<T> with sync lambda: target-type inference masks the mismatch (analyzer gap, see PR #1089)
["""new Mock<AsyncService>().Setup(c => c.ProcessAsync(It.IsAny<string>())).Returns<string>(s => s.Length);"""],
Comment on lines +164 to +165

// Generic Returns<T> with async lambda (no mismatch)
["""new Mock<AsyncService>().Setup(c => c.ProcessAsync(It.IsAny<string>())).Returns<string>(async s => s.Length);"""],
Comment on lines +167 to +168
};

return data.WithNamespaces().WithMoqReferenceAssemblyGroups();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found 22 lines of similar code in 2 locations (mass = 74) [qlty:similar-code]

Expand Down
Loading