docs: Add XML documentation to remaining classes#17
Conversation
WalkthroughThis update introduces extensive XML documentation to a wide array of public interfaces and methods within the Mimic library's fluent API, clarifying usage, intent, and chaining behaviour. It also adds new overloads and methods to support more flexible return and exception configuration, including sequence setups and asynchronous returns. Additionally, a new documentation file, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MimicFluentAPI
participant OriginalMethod
User->>MimicFluentAPI: Setup().Returns(value/function)
Note right of MimicFluentAPI: New overloads support up to 16 parameters and async returns
User->>MimicFluentAPI: Setup().Throws(exception/factory)
User->>MimicFluentAPI: Setup().AsSequence()
User->>MimicFluentAPI: Setup().Proceed()
MimicFluentAPI->>OriginalMethod: (If Proceed called) Invoke original implementation
MimicFluentAPI-->>User: Fluent chaining for further configuration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
DrBarnabus
force-pushed
the
final-xmldocs
branch
from
1899981 to
ead8e56
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #17 +/- ##
=======================================
Coverage 92.29% 92.29%
=======================================
Files 64 64
Lines 2115 2115
Branches 365 365
=======================================
Hits 1952 1952
Misses 85 85
Partials 78 78 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
src/Mimic/Extensions/DelayableExtensions.cs (1)
42-48: Guard and document null receivers for extension methodsBoth WithDelay overloads dereference mimic without null checks. For symmetry with Random null-guarding:
- Add Guard.NotNull(mimic); near the top of each overload.
- Document for mimic.
Also applies to: 87-93
🧹 Nitpick comments (41)
src/Mimic/Fluent/IFluent.cs (1)
9-12: Nit: link Object members with for richer IntelliSenseReferencing the members as links improves navigation/tooltips. Suggested tweak:
-/// (GetType, GetHashCode, ToString, Equals) to prevent them from appearing in IntelliSense when using the fluent API, +/// (<see cref="object.GetType"/>, <see cref="object.GetHashCode"/>, <see cref="object.ToString"/>, <see cref="object.Equals(object)"/>) to prevent them from appearing in IntelliSense when using the fluent API,src/Mimic/Fluent/IGetterCallback`1.cs (1)
17-21: Clarify callback invocation semantics and null parameter behaviourMinor doc refinements to reduce ambiguity:
- Specify that the callback runs each time the getter is accessed.
- Note null handling (e.g., “Throws ArgumentNullException if callback is null”) if that’s the contract.
Example tweak:
-/// Specifies an action callback to execute when the mocked property getter is accessed. +/// Specifies an action callback to execute each time the mocked property getter is accessed. @@ -/// <param name="callback">The action to execute when the property getter is called.</param> +/// <param name="callback">The action to execute when the property getter is accessed. Must not be <c>null</c>.</param> +/// <exception cref="ArgumentNullException">Thrown if <paramref name="callback"/> is <c>null</c>.</exception>src/Mimic/Fluent/IDelayableResult.cs (1)
3-12: Docs read well; add discoverability links.The summary/remarks accurately reflect the role and inheritance. Consider adding see-also links for quicker navigation.
/// The interface provides access to setting call limits and marking setups as expected. /// </remarks> +/// <seealso cref="ILimitable"/> +/// <seealso cref="IExpected"/>src/Mimic/Fluent/IGetterReturns`1.cs (1)
17-33: Clarify per-access execution and null semantics.Docs are solid. Two small refinements improve precision:
- Explicitly state exceptions thrown by the value function propagate to the caller.
- Note null handling (e.g., whether null values are allowed and how they interact with NRT).
/// <param name="value">The value to return when the property getter is called.</param> /// <returns>A <see cref="IReturnsResult"/> that allows further configuration of the mock setup.</returns> IReturnsResult Returns(TProperty? value); @@ /// <param name="valueFunction">A function that computes the value to return when the property getter is called.</param> -/// <returns>A <see cref="IReturnsResult"/> that allows further configuration of the mock setup.</returns> +/// <returns>A <see cref="IReturnsResult"/> that allows further configuration of the mock setup.</returns> /// <remarks> -/// The function is executed each time the property getter is accessed, allowing for dynamic value computation. +/// The function is executed each time the property getter is accessed, allowing for dynamic value computation. +/// Exceptions thrown by the function propagate to the caller. +/// If <typeparamref name="TProperty"/> is a reference type, returning <see langword="null"/> is permitted and will surface as a null getter result. /// </remarks>src/Mimic/Fluent/ICallbackResult.cs (1)
3-12: Good coverage; add see-also links to related fluent continuations.The remarks align with the inherited interfaces. Consider see-also links for quicker navigation across the chain.
/// The interface provides access to throwing exceptions, adding delays, setting call limits, and marking setups as expected. /// </remarks> +/// <seealso cref="IThrows"/> +/// <seealso cref="IThrowsResult"/> +/// <seealso cref="IDelayable"/> +/// <seealso cref="ILimitable"/> +/// <seealso cref="IExpected"/>src/Mimic/Fluent/ICallbackResult`1.cs (1)
3-13: Minor wording: emphasise chaining after callback.Docs are accurate. Consider explicitly calling out that return configuration (via
IReturns<TResult>) is typically the next step after a callback, to make the chain obvious.-/// The interface provides access to configuring return values, throwing exceptions, adding delays, setting call limits, and marking setups as expected. +/// The interface provides access to configuring return values (typically the next step after a callback), throwing exceptions, adding delays, setting call limits, and marking setups as expected.src/Mimic/Fluent/IReturnsResult.cs (2)
11-16: Sharpen delay wording to match return configuration context.“Before method execution” can be misread here; in a returns context it’s clearer to tie the delay to the completed invocation/return.
-/// <item><description><see cref="IDelayable"/> - For introducing delays before method execution</description></item> +/// <item><description><see cref="IDelayable"/> - For introducing delays before completing the invocation or returning the configured result</description></item>
17-20: Add see-also links for common next steps in the chain./// It serves as the continuation point in the fluent API chain for return-value-configured setups. /// </remarks> +/// <seealso cref="ICallback"/> +/// <seealso cref="IDelayable"/> +/// <seealso cref="ILimitable"/> +/// <seealso cref="IExpected"/>src/Mimic/Fluent/ISetterSetup`2.cs (2)
10-18: Mention all inherited interfaces and clarify EditorBrowsable stateThe list omits IFluent even though the interface inherits it, and you can explicitly state EditorBrowsableState.Never for clarity.
/// to provide comprehensive configuration options specifically tailored for property setter scenarios: /// <list type="bullet"> /// <item><description><see cref="ISetterCallback{TProperty}"/> - For defining callback actions when the property setter is invoked</description></item> /// <item><description><see cref="IDelayable"/> - For introducing delays before the setter executes</description></item> /// <item><description><see cref="ILimitable"/> - For limiting the number of times the setup can be matched</description></item> /// <item><description><see cref="IExpected"/> - For defining verification expectations</description></item> +/// <item><description><see cref="IFluent"/> - Identifies this interface as part of the fluent configuration chain</description></item> /// </list> -/// This interface is marked with <see cref="EditorBrowsableAttribute"/> to hide it from IntelliSense, +/// This interface is marked with <see cref="EditorBrowsableAttribute"/> (<see cref="EditorBrowsableState.Never"/>) to hide it from IntelliSense,
19-23: Tighten the example phrasingConsider clarifying that the example shows an expression-based setup and that the assigned value is the focus of matching/validation.
-/// like <c>SetupSet(() => obj.Property = value)</c>. It provides specialised functionality for property -/// setting scenarios where you need to observe, validate, or control the assignment of property values. +/// e.g. <c>SetupSet(() => obj.Property = value)</c>. Property setter setups focus on the assigned value, +/// enabling you to observe, validate, or control that assignment.src/Mimic/Fluent/ILimitable.cs (2)
20-26: Document valid range and behaviour for executionLimitPlease specify the valid range (e.g. must be ≥ 1) and what happens for 0 or negative values (exception vs. treat as 0/∞). This avoids ambiguity for consumers.
-/// <param name="executionLimit">The maximum number of times the setup can be executed. Default is 1.</param> +/// <param name="executionLimit"> +/// The maximum number of times the setup can be executed. Must be greater than or equal to 1. Default is 1. +/// </param> +/// <exception cref="ArgumentOutOfRangeException">Thrown when <paramref name="executionLimit"/> is less than 1.</exception> /// <returns>An <see cref="IExpected"/> that allows marking the setup as expected for verification.</returns>
7-12: Minor wording polish“Internal fluent API chain” can be misread; consider simplifying to emphasise IntelliSense hiding and typical access via chaining.
-/// as it's part of the internal fluent API chain. Users typically access limit functionality -/// through method chaining on setup expressions. Setting execution limits helps ensure that mocked +/// as it's part of the fluent API chain. Users access limit functionality via method chaining on setup expressions. +/// Setting execution limits helps ensure that mocked /// methods are called the expected number of times during testing.src/Mimic/Fluent/IThrowsResult.cs (2)
12-15: Include IFluent in the inheritance list for completenessThe interface also inherits IFluent (Line 24). Consider listing it to keep the docs aligned with the signature.
/// <list type="bullet"> /// <item><description><see cref="IDelayable"/> - For introducing delays before method execution</description></item> /// <item><description><see cref="ILimitable"/> - For limiting the number of times the setup can be matched</description></item> /// <item><description><see cref="IExpected"/> - For defining verification expectations</description></item> +/// <item><description><see cref="IFluent"/> - Identifies this interface as part of the fluent chain</description></item> /// </list>
19-21: Optional nuance about callbacksNice clarification. You could also briefly state that callbacks are unavailable by design because no continuation executes after the throw.
src/Mimic/Fluent/IProceedResult.cs (1)
12-14: Tighten list wordingSmall clarity improvement to emphasise that throwing will override the proceed behaviour.
-/// <item><description><see cref="IThrows"/> - For configuring exception throwing behavior that can override the proceed behavior</description></item> +/// <item><description><see cref="IThrows"/> - Configure exception throwing that, if used, overrides the proceed behaviour</description></item>src/Mimic/Exceptions/MimicException.cs (3)
21-26: Clarify “default message” wording“Default message” can be ambiguous. Consider stating what callers should expect when message is null (e.g. the framework-provided default message).
-/// <param name="message">The message that describes the error, or <c>null</c> to use a default message.</param> +/// <param name="message"> +/// The message that describes the error, or <c>null</c> to use the framework-provided default message. +/// </param>
32-38: Same clarification for the overload with innerExceptionMirror the “framework-provided default message” wording here as well for consistency.
-/// <param name="message">The message that describes the error, or <c>null</c> to use a default message.</param> +/// <param name="message"> +/// The message that describes the error, or <c>null</c> to use the framework-provided default message. +/// </param>
15-18: Minor cref simplificationSince this type is in the same namespace, you can simplify the cref to .
-/// <value>A <see cref="Mimic.Exceptions.Reason"/> value indicating the category of error.</value> +/// <value>A <see cref="Reason"/> value indicating the category of error.</value>src/Mimic/Fluent/ICallback`1.cs (3)
9-12: Unresolved cref risk: qualify EditorBrowsableAttributeUse a fully qualified cref (e.g., System.ComponentModel.EditorBrowsableAttribute) or ensure a using for System.ComponentModel exists to avoid CS1574 in XML docs.
17-23: Consider discouraging the untyped Delegate overloadAccepting Delegate bypasses compile-time type safety. Consider:
- Documenting it as advanced and recommending the strongly-typed Action overloads.
- Optionally marking the method [EditorBrowsable(EditorBrowsableState.Advanced)] to reduce accidental use.
24-276: Document null-handling for callback parametersIf implementations guard against null callbacks, add to these overloads. If not, consider adding null checks for consistency across the fluent API.
src/Mimic/Extensions/DelayableExtensions.cs (1)
44-45: Consider allowing equal min/max delaysPermit minDelay <= maxDelay to support a fixed delay (zero range). This is a common use-case and harmless.
Also applies to: 89-90
src/Mimic/Fluent/IProceed.cs (1)
28-30: Spelling consistency: favour en-GB (“behaviour”)Docs elsewhere use British English (“behaviour”, “randomisation”). Replace “behavior” with “behaviour” here for consistency.
src/Mimic/Fluent/IGetterCallbackResult`1.cs (1)
3-13: Helpful, focused docs; minor cross-links could aid discoverabilityConsider adding to IGetterReturns{TProperty}, IThrows, IDelayable, ILimitable, and IExpected to surface the next steps in the chain.
src/Mimic/Fluent/ISequenceSetup`1.cs (1)
3-23: Spelling/style consistency: behaviour vs behaviorDocs use American spelling (“behaviors”). If the repository standard is en-GB, consider “behaviours”. Whichever you choose, apply consistently across this PR.
src/Mimic/Fluent/IDelayable.cs (2)
3-11: Good high-level summary; consider spelling consistencyThe documentation reads clearly. If en-GB is preferred in this repo, use “behaviour(s)” instead of “behavior(s)” and keep consistent across files.
23-33: Clarify call count scopeYou mention “call count (starting from 0)”. Clarify whether this count is per-setup instance, per-method across all setups, or global to the mimic to avoid ambiguity in complex test scenarios.
src/Mimic/Fluent/ISequenceDelayable.cs (1)
3-18: Clear purpose; align spelling across repoDocs are comprehensive and align with the API surface. Consider standardising on “behaviour(s)” if en-GB is the project norm.
src/Mimic/Fluent/ISetup`2.cs (1)
3-23: Minor doc nit: spelling consistencyAs with other files, consider aligning on en-GB spelling (“behaviour(s)”).
src/Mimic/Fluent/ICallback.cs (3)
57-64: Trim redundancy in remarksYou already provide full XML docs for all higher-arity overloads below. Consider removing or tightening this “additional overloads” remark to avoid duplication.
23-275: Consider documenting null handling for callback parametersIf implementations will throw on null callbacks (common), add
<exception cref="ArgumentNullException">to each overload’s documentation to set expectations. If nulls are accepted and no-ops, state that instead.
3-11: Minor: spelling consistencyFor repository-wide consistency, prefer en-GB (“behaviour”) if that’s the house style.
src/Mimic/Fluent/ISetterCallback`1.cs (1)
37-47: Minor doc clarification: exception behaviourConsider stating what happens if the callback throws (propagation vs swallowing) to set expectations for users composing callbacks with test assertions or logging.
src/Mimic/Fluent/ISequenceSetup.cs (2)
25-33: Clarify whether Proceed() advances the sequence“Proceed … then continues to allow configuration of the next step” reads ambiguously. Does calling
Proceed()itself advance to the next step, or mustNext()be called to advance? Please make this explicit in the remarks.
41-54: Next() remarks: chaining guidanceConsider adding a short example sentence to show a common chain, e.g.
Proceed().Next().Throws<...>(), to disambiguate ordering.CLAUDE.md (3)
27-35: Fix unordered list indentation (MD007)Indentation is 4 spaces; markdownlint expects 2 for nested lists. Adjusting improves rendering consistency and passes lint.
- - `Mimic<T>` - Main generic mimic class for creating mock objects - - `Setup/` - Fluent API for configuring method/property behaviours - - `Proxy/` - Castle DynamicProxy integration for runtime proxy generation - - `Expressions/` - Expression tree handling for type-safe setup syntax + - `Mimic<T>` - Main generic mimic class for creating mock objects + - `Setup/` - Fluent API for configuring method/property behaviours + - `Proxy/` - Castle DynamicProxy integration for runtime proxy generation + - `Expressions/` - Expression tree handling for type-safe setup syntaxSimilarly adjust the nested list under “Test Project”.
Also applies to: 33-35
49-54: Minor grammar/style tweaksTighten phrasing and fix subject agreement:
- - **Common**: Shared utilities and models - - Uses GitVersion for semantic versioning - - Coverlet for code coverage with Codecov integration + - **Common**: shared utilities and models + - GitVersion is used for semantic versioning + - Coverlet is used for code coverage with Codecov integration
73-77: Preposition tweak for clarity- - Castle.Core dependency for proxy generation + - Castle.Core as a dependency for proxy generationNote: Keeping en-GB spellings (Organised/Organisation) for consistency with the rest of the document.
src/Mimic/Fluent/IThrows`1.cs (2)
8-20: Doc accuracy: constraints statement is overstatedThe remark “All exception factory methods are constrained to ensure only valid Exception types are used” is not accurate for
Throws(Delegate), which accepts any delegate. Please soften to reflect runtime validation or narrow the API.Option A (doc-only):
- /// All exception factory methods are constrained to ensure only valid <see cref="Exception"/> types are used. + /// Generic factory overloads constrain <typeparamref name="TException"/> to <see cref="Exception"/>. + /// The <see cref="Delegate"/> overload is validated at runtime to ensure it produces an <see cref="Exception"/>.Option B (API tightening, if feasible):
- TNext Throws(Delegate exceptionFactory); + TNext Throws(Func<Exception> exceptionFactory);
40-46:Throws(Delegate)needs return-type and argument-shape clarificationThe
Delegateoverload can be misused. Please document:
- Expected return type (must be
Exception)- Whether method arguments are bound from the intercepted call, and how mismatches are handled (argument count/type)
- What exception is thrown if the delegate returns a non-Exception
src/Mimic/Extensions/ReturnsExtensions.cs (1)
62-99: High duplication – consider source-generation.The 15× overload pattern is verbatim repeated for both
TaskandValueTask. A small Roslyn source-generator (or T4) could emit these at build-time, shrinking maintenance cost and file noise.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
CLAUDE.md(1 hunks)src/Mimic/Exceptions/MimicException.cs(1 hunks)src/Mimic/Extensions/DelayableExtensions.cs(3 hunks)src/Mimic/Extensions/ReturnsExtensions.cs(2 hunks)src/Mimic/Fluent/ICallback.cs(1 hunks)src/Mimic/Fluent/ICallbackResult.cs(1 hunks)src/Mimic/Fluent/ICallbackResult1.cs` (1 hunks)src/Mimic/Fluent/ICallback1.cs` (1 hunks)src/Mimic/Fluent/IConditionalSetup1.cs` (1 hunks)src/Mimic/Fluent/IDelayable.cs(1 hunks)src/Mimic/Fluent/IDelayableResult.cs(1 hunks)src/Mimic/Fluent/IExpected.cs(1 hunks)src/Mimic/Fluent/IFluent.cs(1 hunks)src/Mimic/Fluent/IGetterCallbackResult1.cs` (1 hunks)src/Mimic/Fluent/IGetterCallback1.cs` (1 hunks)src/Mimic/Fluent/IGetterReturns1.cs` (1 hunks)src/Mimic/Fluent/IGetterSetup2.cs` (1 hunks)src/Mimic/Fluent/ILimitable.cs(1 hunks)src/Mimic/Fluent/IProceed.cs(1 hunks)src/Mimic/Fluent/IProceedResult.cs(1 hunks)src/Mimic/Fluent/IReturnsResult.cs(1 hunks)src/Mimic/Fluent/IReturns1.cs` (1 hunks)src/Mimic/Fluent/IReturns2.cs` (1 hunks)src/Mimic/Fluent/ISequenceDelayable.cs(1 hunks)src/Mimic/Fluent/ISequenceSetup.cs(1 hunks)src/Mimic/Fluent/ISequenceSetup1.cs` (1 hunks)src/Mimic/Fluent/ISetterCallback1.cs` (1 hunks)src/Mimic/Fluent/ISetterSetup2.cs` (1 hunks)src/Mimic/Fluent/ISetup1.cs` (1 hunks)src/Mimic/Fluent/ISetup2.cs` (1 hunks)src/Mimic/Fluent/IThrows.cs(1 hunks)src/Mimic/Fluent/IThrowsResult.cs(1 hunks)src/Mimic/Fluent/IThrows1.cs` (1 hunks)
🧰 Additional context used
🪛 LanguageTool
CLAUDE.md
[style] ~35-~35: Would you like to use the Oxford spelling “Organized”? The spelling ‘Organised’ is also correct.
Context: ... xUnit, Shouldly, and AutoFixture - Organised by feature area (Setup/, Expressions/, ...
(OXFORD_SPELLING_Z_NOT_S)
[grammar] ~51-~51: Did you mean “models use” or “model uses”?
Context: ...asks - Common: Shared utilities and models - Uses GitVersion for semantic versioning - Co...
(NNS_USES)
[style] ~64-~64: Would you like to use the Oxford spelling “Organization”? The spelling ‘Organisation’ is also correct.
Context: ... testing on net8.0 and net9.0 ### Code Organisation - Follow existing namespace patterns (...
(OXFORD_SPELLING_Z_NOT_S)
[uncategorized] ~75-~75: Possible missing preposition found.
Context: ...s are enabled for reproducible builds - Castle.Core dependency for proxy generation - ...
(AI_HYDRA_LEO_MISSING_OF)
🪛 markdownlint-cli2 (0.17.2)
CLAUDE.md
27-27: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
28-28: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
29-29: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
30-30: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
33-33: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
34-34: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
35-35: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (csharp)
- GitHub Check: Prepare (windows-latest)
🔇 Additional comments (19)
src/Mimic/Fluent/IFluent.cs (1)
3-13: Solid, clear summary and remarks; pattern aligns with hiding Object membersDocumentation accurately explains the interface’s purpose and IntelliSense-hiding intent. No functional changes introduced. LGTM.
src/Mimic/Fluent/IGetterSetup`2.cs (1)
3-14: Good, comprehensive description of the getter setup aggregatorDocs clearly state that this interface composes callbacks, returns, throws, delays, limits, and expectations and is hidden from IntelliSense. Reads well and matches the interface’s role. LGTM.
src/Mimic/Fluent/IThrows.cs (1)
3-14: Accurate and succinct explanation of the non-generic IThrowsDocumentation correctly frames this as the simplified specialisation of IThrows with IThrowsResult as the continuation. No issues spotted. LGTM.
src/Mimic/Fluent/IReturns`1.cs (1)
3-15: Clear, consistent documentation of the simplified returns interfaceWell-aligned with the fluent chain narrative and the specialisation to IReturnsResult. Nothing functionally changed; docs are helpful. LGTM.
src/Mimic/Fluent/IGetterCallback`1.cs (1)
3-12: Type-level docs are clear and consistent with fluent chainingExplains purpose, type parameter, and IntelliSense-hiding rationale. Looks good. LGTM.
src/Mimic/Fluent/ICallbackResult`1.cs (1)
14-16: Variance alignment confirmed for ICallbackResult and IReturns
BothICallbackResult<in TResult>and its base interfaces declareTResultcontravariantly, ensuring type-safety across the fluent API. No variance mismatches detected—no changes required.• src/Mimic/Fluent/IReturns
1.cs:public interface IReturns : IReturns<TResult, IReturnsResult>;• src/Mimic/Fluent/IReturns2.cs:public interface IReturns<in TResult, out TNext> : IFluent
• src/Mimic/Fluent/ICallbackResult1.cs:public interface ICallbackResult : IReturns, …`src/Mimic/Fluent/ICallback`1.cs (1)
3-76: Overall: documentation quality is solid and consistentClear summaries, param docs, and fluent-chain context. No API surface changes detected.
src/Mimic/Fluent/IExpected.cs (1)
17-25: Docs read well; intent and verification semantics are clearNo API change, explanations are precise and consistent with the fluent chain.
src/Mimic/Fluent/IProceed.cs (1)
3-18: Good clarity on intent and scenariosWell-framed use-cases (partial mocking, spies, decorators) and chaining explanation. No API change.
src/Mimic/Fluent/IGetterCallbackResult`1.cs (1)
14-16: LGTMDocs align with the interface role; no behavioural/API changes detected.
src/Mimic/Fluent/ISequenceDelayable.cs (1)
34-52: LGTM: dynamic delay semantics documented wellDynamic delay semantics and zero-based sequence index are clearly explained. This aligns with typical sequence configuration use-cases.
src/Mimic/Fluent/ISetup`2.cs (2)
29-42: AsSequence() addition reads well; verify parity with void-returning setupsThe docs are clear and helpful. Please confirm the void counterpart (
ISetup<TMimic>) also exposesAsSequence()for consistency, as referenced in the PR summary.
26-26: Ignore suggested variance changeThe type parameter
TResultmust remain contravariant (in) to match its base interfaces—bothICallback<in TResult>andIReturns<in TResult>consumeTResultas an input (e.g. callback and Returns methods). Switching toout TResultwould break those interfaces. Disregard the variance-change recommendation.Likely an incorrect or invalid review comment.
src/Mimic/Fluent/ICallback.cs (1)
16-21: LGTM: base callback overload documented clearlyClear summary and return contract; matches the fluent API pattern.
src/Mimic/Fluent/ISetup`1.cs (1)
28-41: Confirm major version bump for public interface changeAdding AsSequence() to the public ISetup and ISetup<T, TResult> interfaces is a breaking change for any external implementers.
- Internal implementations (Setup and Setup<TMimic, TResult>) have already been updated with the new member.
Please confirm that this change will be released in a new major version. If you’d rather avoid breaking external code, consider offering AsSequence as an extension method instead of adding it directly to the interface.
src/Mimic/Fluent/IThrows`1.cs (1)
47-55: Clarify null-return semantics forThrows<TException>(Func<TException?>)The
<summary>XML doc says the factory “can return null,” but it isn’t defined what happens at runtime if it does:
- Should an
ArgumentNullExceptionorInvalidOperationExceptionbe thrown?- Should it be treated as “no throw” and continue the chain?
- Or should it be considered an invalid argument and fail immediately?
Please address the following:
• Update the XML documentation in src/Mimic/Fluent/IThrows
1.cs (lines 47–55) to state exactly what occurs when the factory returns null. • I couldn’t find any existing unit tests covering a null-returning factory for this overload—could you confirm whether such tests already exist? If not, please add a test that asserts the intended behaviour when supplying() => null`.src/Mimic/Fluent/IConditionalSetup`1.cs (1)
8-12: Documentation looks good – no technical concerns.Nothing in the docs changes behaviour; the interface contract remains intact.
src/Mimic/Extensions/ReturnsExtensions.cs (1)
387-417: Missing 16-parameter overload forTaskbranch.
IReturns<TResult,TNext>exposes overloads up to 16 params; theTask<TResult>helpers stop at 15, giving an asymmetry. Add the 16-param version for parity (or drop the 16-param interface overload).src/Mimic/Fluent/IReturns`2.cs (1)
27-33: Interface documentation reads well.No functional changes detected; the added remarks improve IntelliSense without altering API surface.
1 participant
Summary by CodeRabbit
Documentation
Style