moq.analyzers icon indicating copy to clipboard operation
moq.analyzers copied to clipboard

Expand SetupShouldOnlyBeUsedForOverridableMembersAnalyzer to add regression tests for Field and Event

Open rjmurillo opened this issue 9 months ago • 0 comments

:warning: 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 https://github.com/rjmurillo/moq.analyzers/pull/340#discussion_r1938114814

rjmurillo avatar Feb 01 '25 00:02 rjmurillo