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

Expand SetupShouldOnlyBeUsedForOverridableMembersAnalyzer to add regression tests for Field and Event #341

Open
rjmurillo opened this issue Feb 1, 2025 · 0 comments
Labels
analyzers Change that impacts an analyzer behavior bug .NET Pull requests that update .net code triage
Milestone

Comments

@rjmurillo
Copy link
Owner

rjmurillo commented Feb 1, 2025

⚠️ Potential issue

Potential bug: Non-property/method members are silently skipped.

Lines [114-116] specifically force a true return for the default case, meaning no diagnostic is ever raised for fields, events, or other non-overridable members. For example, if a user calls Setup(x => x.myField), the analyzer won't report a diagnostic. This appears to undermine the goal of restricting Setup to only overridable members.

Why it’s a bug: Fields and other non-method/non-property members are not overridable. The analyzer's stated objective is “Setup should be used only for overridable members.”

Example:

public class Foo
{
    public int myField;
}

// In test:
var mock = new Mock<Foo>();
mock.Setup(x => x.myField).Returns(42); // Expect analyzer to warn, but it won't currently

Suggested fix: Return false in the default case, so the analyzer issues a diagnostic for members that aren’t recognized as a property or a method.

- default:
-     // If it's not a property or method, we do not issue a diagnostic
-     return true;
+ default:
+     // If it's not a property or method, it's not overridable
+     return false;

Consider adding regression tests covering fields or events to ensure the analyzer flags them correctly going forward.

Originally posted by @coderabbitai[bot] in #340 (comment)

@rjmurillo rjmurillo added .NET Pull requests that update .net code triage bug analyzers Change that impacts an analyzer behavior and removed .NET Pull requests that update .net code triage labels Feb 1, 2025
@rjmurillo rjmurillo added this to the vNext milestone Feb 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzers Change that impacts an analyzer behavior bug .NET Pull requests that update .net code triage
Projects
None yet
Development

No branches or pull requests

1 participant