Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Moq1002 false positive when using expression constructor #234

Closed
MattKotsenas opened this issue Oct 22, 2024 · 1 comment · Fixed by #301
Closed

Moq1002 false positive when using expression constructor #234

MattKotsenas opened this issue Oct 22, 2024 · 1 comment · Fixed by #301
Assignees
Labels
bug .NET Pull requests that update .net code
Milestone

Comments

@MattKotsenas
Copy link
Collaborator

MattKotsenas commented Oct 22, 2024

This code fires Moq1002 when I believe that it shouldn't

_ = new Mock<Calculator>(() => new Calculator(), MockBehavior.Loose);

public class Calculator
{
    public int Add(int a, int b) => a + b;
}

Here's the overload that should be used: https://github.com/devlooped/moq/blob/18dc7410ad4f993ce0edd809c5dfcaa3199f13ff/src/Moq/Mock%601.cs#L200

@rjmurillo rjmurillo added .NET Pull requests that update .net code triage labels Oct 22, 2024
@rjmurillo rjmurillo added this to the vNext milestone Oct 22, 2024
@rjmurillo
Copy link
Owner

There are a few ways to declare how to new up types with Moq. Identification of which case being used needs to be fully implemented from the Moq implementation, then passed on for evaluation. Similarly, code fixes need to also understand the semantic tree to match which implementation is intended, then perform tree updates with code fixes.

@rjmurillo rjmurillo self-assigned this Dec 24, 2024
@rjmurillo rjmurillo added bug and removed triage labels Dec 24, 2024
rjmurillo added a commit that referenced this issue Dec 24, 2024
rjmurillo added a commit that referenced this issue Dec 24, 2024
When using Moq's expression features for type construction, the Moq1200
analyzer was firing unexpectedly indicating a matching constructor on
the type was not found. The `ConstructorArgumentsShouldMatchAnalyzer`
was extracting the parameters from the Mock type and comparing them to
the parameters of the constructed type as is, including the lambda and
the `MockBehavior`.

Example:

```csharp
_ = new Mock<Calculator>(() => new Calculator(), MockBehavior.Loose);
```

> See #234 for more details

This is incorrect for several reasons:

1. The parenthesized lambda is not itself a parameter for the target
type, but rather the body
2. The `MockBehavior` would not likely be a parameter on the target type

Correct analysis would be to drop the `MockBehavior` argument as with
other configurations of the `Mock<T>` type, and use the body of the
lambda. However, using the body of the lambda is not necessary. The
purpose of this analyzer is to detect errors that would not be caught at
compile time that would result in a runtime error. In this case, using a
constructor not available on the type would result in a compiler error.
As such, the constructor detection method has been updated to return a
tertiary result: `true` when there is a matching constructor found,
`false` when there is not, and `null` when the constructor is a lambda
(i.e., the constructor should be ignored).

Changes:

- Add unit test for issue #234
- Add support to ignore parameterized lambda arguments when performing
constructor detection
- Add support `MockBehavior` definitions to be in ordinal position 0 or
1

Fixes #234
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug .NET Pull requests that update .net code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants