nsubstitute / nsubstitute.analyzers Goto Github PK
View Code? Open in Web Editor NEWRoslyn analysers for NSubstitute.
License: MIT License
Roslyn analysers for NSubstitute.
License: MIT License
Originated from #99
As reported in here nsubstitute/NSubstitute#499 (comment) NSubstitute.Analyzers fails to find non-virtual member calls when substituting for class but assigning it to interface e.g
IUnitOfWorkCoreCqrs cqrsUow = Substitute.ForPartsOf<CqrsUow>();
cqrsUow.Received(1).RegisterAfterCommitCallBackItem(Arg.Any<Func<Task>>());
In order to correctly report non-virtual member calls, we have to find actual substitute assignment
IUnitOfWorkCoreCqrs cqrsUow = Substitute.ForPartsOf<CqrsUow>();
or
IUnitOfWorkCoreCqrs cqrsUow = null;
cqrsUow = Substitute.ForPartsOf<CqrsUow>();
based on the type infered from expression, we should be able to enhance CanBeSetuped
method.
Possible roslyn API to use
var dataFlowAnalysis = syntaxNodeContext.SemanticModel.AnalyzeDataFlow(accessedMember);
Possible problems
People often use Arg.Any<int>()
in real calls (rather than .Returns
stub or .Received
assertion) to represent "I don't mind what input is used for this test".
This case is show in the documentation. There is also an example in this post (search for "Another example").
This causes problems for NSubstitute because it tries to use these argument matchers the next time it gets a call (it will consider the next call to be an attempt to stub, rather than a real call), and it can cause confusing test behaviour or test errors.
Proposed title:
Argument matcher used without call to .Returns or .Received
Proposed description (from docs):
Argument matchers should only be used when setting return values or checking received calls. Using Arg.Is or Arg.Any without a call to .Returns or Received() can cause your tests to behave in unexpected ways.
Using an analyser for this came up in NSub#314, so I thought I'd raise this issue here for discussion and to see how practical it would be to implement.
Introduce a project for measuring the performance of all of the VB and CSharp diagnostic analyzers
As a follow up of #66 which introduces diagnostics for internal member substitution, this one is all about adding code fix providers to fix the diagnostics. Possible fixes are listed in before mentioned task
Describe the bug
We have to strong name all our assemblies.
NS2003 does not recognize InternalsVisibleTo syntax with publickey.
To Reproduce
our AssembyInfo.cs contains following line:
[assembly: InternalsVisibleTo("DynamicProxyGenAssembly2, PublicKey=0024000004800000940000000602000000240000525341310004000001000100c547cac37abd99c8db225ef2f6c8a3602f3b3606cc9891605d02baa56104f4cfc0734aa39b93bf7852f7d9266654753cc297e7d2edfe0bac1cdcf9f717241550e0a7b191195b7667bb4f64bcb8e2121380fd1d9d46ad2d92d2d15605093924cceaf74c4861eff62abf69b9291ed0a340e113be11e6a7d3113e92484cf7045cc7")]
Expected behaviour
A clear and concise description of what you expected to happen, compared to what actually happened.
Currently i receive following warning:
Warning NS2003
Can not substitute for internal type. To substitute for internal type expose your type to DynamicProxyGenAssembly2 via [assembly: InternalsVisibleTo("DynamicProxyGenAssembly2")]
I'd expect not to receive any warning when using strong key.
Environment:
NSubstitute.Analyzers.CSharp 1.0.6
Platform: Windows10 - .net 4.6.2
Additional context
public static bool InternalsVisibleToProxyGenerator(this ISymbol typeSymbol)
{
return typeSymbol.ContainingAssembly != null &&
typeSymbol.ContainingAssembly.GetAttributes()
.Any(att => att.AttributeClass.ToString() == MetadataNames.InternalsVisibleToAttributeFullTypeName &&
att.ConstructorArguments.Any(arg => arg.Value.ToString() == MetadataNames.CastleDynamicProxyGenAssembly2Name));
}
arg.Value.ToString().StartsWith(MetadataNames.CastleDynamicProxyGenAssembly2Name) would quickly fix this issue ;-)
Analyzers should be able to detect following wrong usages
using NSubstitute;
namespace MyNamespace
{
public class Foo
{
public int Bar()
{
return 2;
}
}
public class FooTests
{
public void Test()
{
int i = 0;
var substitute = NSubstitute.Substitute.For<Foo>();
void SubstituteCall(Foo sub)
{
sub.Bar();
}
substitute.When(SubstituteCall).Do(callInfo => i++);
substitute.When(delegate(Foo sub) { sub.Bar() })
substitute.When(sub => sub.Bar())
}
}
}
and of course usages of beforementioned examples executed as ordinal method and used with WhenForAnyArgs
var substitute = NSubstitute.Substitute.For<Foo>();
var x = substitute.When(x => x.Bar());
Similarly, as in #73, assuming that Bar is a non-virtual member, NS1002 is correctly reported, however the analyzer highlights x.Bar
instead of x.Bar()
.
Not sure how feasible this is but...
Call a substitute from within the .Returns(...)
block of another substitute can cause issues:
nsubstitute/NSubstitute#26
nsubstitute/NSubstitute#357
Note that sub.Blah().Returns(x => CreateFoo())
can be ok, whereas sub.Blah().Returns(CreateFoo())
can cause the problem (when CreateFoo
also sets Returns()
values).
When searching for NSubstitute, the analyzers are listed pretty low in the search results, we should add some more tags to make the package more "recognizable". Additionally, as a part of this task, we should get rid of obsolete PackageLicenseUrl
from the csproj, and replace it with the new tag
As discussed in #77 (comment)
First of all, the analyzer is really useful - thanks for creating the project!
I think I'm incorrectly getting the Could not set value of type โฆ to argument ... because the types are incompatible
error when trying to assign a Dictionary
to a IReadOnlyDictionary
argument and was able to reproduce it with this code:
using NSubstitute;
using Xunit;
public class ExampleTest
{
public interface IExampleInterface
{
bool Method(out object value);
}
[Fact]
public void DictionaryTest()
{
IExampleInterface substitute = Substitute.For<IExampleInterface>();
substitute.Method(out Arg.Any<object>())
.Returns(ci =>
{
ci[0] = "string";
return true;
});
bool result = substitute.Method(out object value);
Assert.True(result);
Assert.Equal("string", value);
}
}
The line with ci[0] = "string";
triggers the analyser error, however, the test succeeds in passing (i.e. NSubstitute does correctly set the parameter).
I think the problem is we're checking if the type is equal instead of if the assignment will work in AbstractCallInfoAnalyzer.cs - maybe Equals
should be replaced with a call to CanCast?
I do have the attribute InternalsVisibleTo
in the file AssemblyInfo.cs
[assembly: InternalsVisibleTo("DynamicProxyGenAssembly2")]
However, the analyser adds a warning and suggests a fix when I use Substitute.ForPartsOf<MyInternalType>()
. The fix add the attribute in the file that contains the class MyInternalType. But this is not needed as the attribute is already declared in another file.
Expected behavior: it should not report a warning if there is [assembly: InternalsVisibleTo("DynamicProxyGenAssembly2")]
somewhere in the project.
I think the analyzer is incorrectly firing NS4000: Returns() is set with a method that itself calls Returns
for the following code (this is the simplest I could get the code to reproduce the warning I was getting):
public interface IExample
{
object Method();
}
public class ExampleTest
{
[Fact]
public void TypeofTestClass()
{
IExample example = Substitute.For<IExample>();
example.Method().Returns(typeof(ExampleTest));
// ^^^^^^^^^^^^^^^^^^^
}
}
This is under dotnet-core 2.1 with using NSubstitute version 4.0.0, NSubstitute.Analyzers.CSharp version 1.0.5 and xunit version 2.4.1
Granted, the above code doesn't look realistic (there's no asserts!), however, I was getting the warning in one of my actual tests and it seems to be the typeof(TheClassContainingTheTest)
that triggers it.
Let me know if there's anything I can do to help.
Lets follow NSubstitute convention and add acknowledgments .md file with the stuff which was used to build the NSubstitute.Analyzers
A current implementation of analyzers uses a lot of preprocessor directives such as
#if CSHARP
#elif VISUAL_BASIC
#endif
Maitaining this code will be painfull in the future, so the pragma conditions should be removed and replaced with better approach.
Originally reported in nsubstitute/NSubstitute#260 (comment)
The problem occurs when ReEntrantCallAnalyzer crosses boundries of given syntax tree when analyzing reentrant call.
Repro
using NSubstitute;
using NSubstitute.Core;
using System;
using Xunit;
namespace MyNamespace
{
public class UnitTest1
{
[Fact]
public void Test1()
{
var sub = Substitute.For<MyClass2>();
sub.Foo().Returns(ClassInOtherFile.Foo());
}
}
public class MyClass2
{
public virtual int Foo()
{
return 1;
}
}
public interface MyClass
{
int Foo();
}
}
Currently, each analyzer library (C# or VB version) has its own Resources.resx file which contains messages for every diagnostic. As it was noted by @dtchepak
in #14 (comment) in most of the cases, the messages are the same and they only differ in some particular scenarios. Having that in mind we should find a way to share messages across analyzer libraries and override some of them when needed. Thanks to that approach we will avoid message duplication which will make the code-base cleaner and easier to maintain
It is possible to set argument values from .Returns
and .AndDoes
. At present the one set in .Returns
wins (not sure we should always rely on this for future versions), but this can result in confusing results.
public interface ILookup { bool Lookup(string key, out int i); }
[Fact]
public void Test() {
var sub = Substitute.For<ILookup>();
sub.Lookup("blah", out Arg.Any<int>())
.Returns(x => {
x[1] = 42;
return true;
})
.AndDoes(x => x[1] = 45);
sub.Lookup("blah", out var value);
Assert.Equal(42, value); // The final values is 42 from the Returns, not 45 from AndDoes
}
CheckId | NS3006 |
Category | Argument specification |
Conflicting assignments to out/ref arguments.
A violation of this rule occurs when an out or ref argument is assigned via CallInfo
in both a Returns
and an AndDoes
call.
To fix a violation of this rule remove one of the assignments, so the argument is only assigned in either the Returns
or the AndDoes
block (not in both).
// Incorrect, argument assigned in `Returns` and `AndDoes`:
dictionary.TryGetValue("sample", out Arg.Any<int>())
.Returns(x => {
x[1] = 42;
return true;
})
.AndDoes(x => x[1] = 45);
// Correct, only assigned in `AndDoes`:
dictionary.TryGetValue("sample", out Arg.Any<int>())
.Returns(true)
.AndDoes(x => x[1] = 45);
// Correct, only assign in `Returns`:
dictionary.TryGetValue("sample", out Arg.Any<int>())
.Returns(x => {
x[1] = 42;
return true;
});
This warning can be suppressed by disabling the warning in the ruleset file for the project.
The warning can also be suppressed programmatically for an assembly:
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Argument specification", "NS3006:Conflicting argument assignments.", Justification = "Reviewed")]
Or for a specific code block:
#pragma warning disable NS3006 // Conflicting argument assignments.
// the code which produces warning
#pragma warning disable NS3006 // Conflicting argument assignments.
I've got some code like this:
var myMock = Substitute.For<IDependancy>();
myMock.When(a => a.Foo(Arg.Any<SomeClass>())).Throw(new Exception());
Since updating to version 1.0.8 we're getting the following error on the .When
line.
error NS1002: Member Any can not be intercepted. Only interface members and virtual, overriding, and abstract members can be intercepted.
Just realised that out
and ref
arguments can also be set via AndDoes
(in the same way they can be set from Returns
). This should probably also result in a NS3005 if a non-out/ref arg is set.
public interface ILookup { bool Lookup(string key, out int i); }
[Fact]
public void Test() {
var sub = Substitute.For<ILookup>();
sub.Lookup("blah", out Arg.Any<int>())
.Returns(true)
.AndDoes(x => x[0] = 45); // <-- incorrectly setting x[0] instead of x[1]. Should give NS3005
sub.Lookup("blah", out var value);
}
Using 1.0.0 C#.
On first adding the package, or first loading the project in VS, I get the following error (anonymised slightly). It seems to go away after running a build.
Error AD0001 Analyzer 'NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantSetupAnalyzer' threw an exception of type 'System.ArgumentException' with message 'SyntaxTree is not part of the compilation
Parameter name: syntaxTree'. SampleTests 1 Active Analyzer 'NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantSetupAnalyzer' threw the following exception:
'Exception occurred with following context:
Compilation: SampleTests
SyntaxTree: C:\dev\git\sample\SampleTests\SampleFixture.cs
SyntaxNode: _sample.Actions ... [InvocationExpressionSyntax]@[4394..4580) (99,12)-(99,198)
System.ArgumentException: SyntaxTree is not part of the compilation
Parameter name: syntaxTree
at Microsoft.CodeAnalysis.CSharp.CSharpCompilation.GetSemanticModel(SyntaxTree syntaxTree, Boolean ignoreAccessibility)
at Microsoft.CodeAnalysis.CSharp.CSharpCompilation.CommonGetSemanticModel(SyntaxTree syntaxTree, Boolean ignoreAccessibility)
at NSubstitute.Analyzers.Shared.DiagnosticAnalyzers.AbstractReEntrantCallFinder.<GetRelatedNodes>d__3.MoveNext()
at NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantCallFinder.ReEntrantCallVisitor.VisitRelatedSymbols(SyntaxNode syntaxNode)
at NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantCallFinder.ReEntrantCallVisitor.DefaultVisit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxVisitor.VisitIdentifierName(IdentifierNameSyntax node)
at Microsoft.CodeAnalysis.CSharp.Syntax.IdentifierNameSyntax.Accept(CSharpSyntaxVisitor visitor)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.Visit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.DefaultVisit(SyntaxNode node)
at NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantCallFinder.ReEntrantCallVisitor.DefaultVisit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxVisitor.VisitPropertyDeclaration(PropertyDeclarationSyntax node)
at Microsoft.CodeAnalysis.CSharp.Syntax.PropertyDeclarationSyntax.Accept(CSharpSyntaxVisitor visitor)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.Visit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.DefaultVisit(SyntaxNode node)
at NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantCallFinder.ReEntrantCallVisitor.DefaultVisit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxVisitor.VisitInterfaceDeclaration(InterfaceDeclarationSyntax node)
at Microsoft.CodeAnalysis.CSharp.Syntax.InterfaceDeclarationSyntax.Accept(CSharpSyntaxVisitor visitor)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.Visit(SyntaxNode node)
at NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantCallFinder.ReEntrantCallVisitor.VisitRelatedSymbols(SyntaxNode syntaxNode)
at NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantCallFinder.ReEntrantCallVisitor.DefaultVisit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxVisitor.VisitIdentifierName(IdentifierNameSyntax node)
at Microsoft.CodeAnalysis.CSharp.Syntax.IdentifierNameSyntax.Accept(CSharpSyntaxVisitor visitor)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.Visit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.DefaultVisit(SyntaxNode node)
at NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantCallFinder.ReEntrantCallVisitor.DefaultVisit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxVisitor.VisitMethodDeclaration(MethodDeclarationSyntax node)
at Microsoft.CodeAnalysis.CSharp.Syntax.MethodDeclarationSyntax.Accept(CSharpSyntaxVisitor visitor)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.Visit(SyntaxNode node)
at NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantCallFinder.ReEntrantCallVisitor.VisitRelatedSymbols(SyntaxNode syntaxNode)
at NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantCallFinder.ReEntrantCallVisitor.DefaultVisit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxVisitor.VisitIdentifierName(IdentifierNameSyntax node)
at Microsoft.CodeAnalysis.CSharp.Syntax.IdentifierNameSyntax.Accept(CSharpSyntaxVisitor visitor)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.Visit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.DefaultVisit(SyntaxNode node)
at NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantCallFinder.ReEntrantCallVisitor.DefaultVisit(SyntaxNode node)
at NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantCallFinder.ReEntrantCallVisitor.VisitInvocationExpression(InvocationExpressionSyntax node)
at Microsoft.CodeAnalysis.CSharp.Syntax.InvocationExpressionSyntax.Accept(CSharpSyntaxVisitor visitor)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.Visit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.DefaultVisit(SyntaxNode node)
at NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantCallFinder.ReEntrantCallVisitor.DefaultVisit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxVisitor.VisitEqualsValueClause(EqualsValueClauseSyntax node)
at Microsoft.CodeAnalysis.CSharp.Syntax.EqualsValueClauseSyntax.Accept(CSharpSyntaxVisitor visitor)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.Visit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.DefaultVisit(SyntaxNode node)
at NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantCallFinder.ReEntrantCallVisitor.DefaultVisit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxVisitor.VisitVariableDeclarator(VariableDeclaratorSyntax node)
at Microsoft.CodeAnalysis.CSharp.Syntax.VariableDeclaratorSyntax.Accept(CSharpSyntaxVisitor visitor)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.Visit(SyntaxNode node)
at NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantCallFinder.ReEntrantCallVisitor.VisitRelatedSymbols(SyntaxNode syntaxNode)
at NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantCallFinder.ReEntrantCallVisitor.DefaultVisit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxVisitor.VisitIdentifierName(IdentifierNameSyntax node)
at Microsoft.CodeAnalysis.CSharp.Syntax.IdentifierNameSyntax.Accept(CSharpSyntaxVisitor visitor)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.Visit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.DefaultVisit(SyntaxNode node)
at NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantCallFinder.ReEntrantCallVisitor.DefaultVisit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxVisitor.VisitInitializerExpression(InitializerExpressionSyntax node)
at Microsoft.CodeAnalysis.CSharp.Syntax.InitializerExpressionSyntax.Accept(CSharpSyntaxVisitor visitor)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.Visit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.DefaultVisit(SyntaxNode node)
at NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantCallFinder.ReEntrantCallVisitor.DefaultVisit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxVisitor.VisitImplicitArrayCreationExpression(ImplicitArrayCreationExpressionSyntax node)
at Microsoft.CodeAnalysis.CSharp.Syntax.ImplicitArrayCreationExpressionSyntax.Accept(CSharpSyntaxVisitor visitor)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.Visit(SyntaxNode node)
at NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantCallFinder.GetReEntrantSymbols(Compilation compilation, SyntaxNode rootNode)
at NSubstitute.Analyzers.Shared.DiagnosticAnalyzers.AbstractReEntrantCallFinder.GetReEntrantCalls(Compilation compilation, SyntaxNode rootNode)
at NSubstitute.Analyzers.Shared.DiagnosticAnalyzers.AbstractReEntrantSetupAnalyzer`2.AnalyzeInvocation(SyntaxNodeAnalysisContext syntaxNodeContext)
at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.<>c__44`1.<ExecuteSyntaxNodeAction>b__44_0(ValueTuple`2 data)
at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.ExecuteAndCatchIfThrows_NoLock[TArg](DiagnosticAnalyzer analyzer, Action`1 analyze, TArg argument, Nullable`1 info)
-----
'.
I just wanted to check if VS2019 is supported.
The documentation is slightly ambiguous here - https://github.com/nsubstitute/NSubstitute.Analyzers/blob/master/documentation/Compatibility.md
Does it mean that only VS2017 is supported (above version 15.7) or does it mean that anything above Visual Studio 2017 version 15.7 is supported?
Thanks.
Found this case where the Analyzers are a little too enthusiastic detecting this error. ๐ (with 1.0.0 CSharp Analyzers)
using NSubstitute;
using NUnit.Framework;
namespace NSubTestWorkshop {
public interface ISample { IValue GetValue(); }
public interface IValue { int Value { get; } }
[TestFixture]
public class SampleTests {
[Test]
public void Sample() {
var value1 = CreateValue();
var sample = Substitute.For<ISample>();
sample.GetValue().Returns(value1); // NS4000 error
Assert.AreEqual(42, sample.GetValue().Value);
}
public IValue CreateValue() {
var val = Substitute.For<IValue>();
val.Value.Returns(42);
return val;
}
}
}
~/dev/git/NSubTestWorkshop/UnitTest1.cs(39,39): Warning NS4000: Returns() is set with a method that itself calls Returns. This can cause problems with NSubstitute. Consider replacing with a lambda: Returns(x => value1). (NS4000) (NSubTestWorkshop)
This one should not cause a problem because value1
substitute is fully created and configured before being given to sample.GetValue().Returns(value1)
. This test passes if we disable the warning โ
.
This would be a problem if we instead had this:
sample.GetValue().Returns(CreateValue());
Because in that case we switch from configuring GetValue()
to configuring Value
. This modified test fails with CouldNotSetReturnDueToNoLastCallException
:
[Test]
public void Sample() {
var value1 = CreateValue();
var sample = Substitute.For<ISample>();
#pragma warning disable NS4000
sample.GetValue().Returns(CreateValue()); // <-- Analyzers correctly detect NS4000 here
#pragma warning restore NS4000
Assert.AreEqual(42, sample.GetValue().Value);
}
Repro
using NSubstitute;
namespace MyNamespace
{
public class AbstractFoo
{
public int Bar()
{
return 1;
}
}
public class Foo : AbstractFoo
{
}
public class FooTests
{
public void Test()
{
var foo = new Foo();
foo.When(xx => xx.Bar()); // xx.Bar() is not higlighted
}
}
}
Current NS002 message looks like:
Error | NS002 | Unused received check.
I'm not sure what the standard is for analyser messages in general, but I think it might be good to include some information on how to fix this in the message.
For example:
Error | NS002 | Unused received check. To fix, make sure there is a call after `Received`.
Correct: `sub.Received().SomeCall();`. Incorrect: `sub.Received();`
Add same diagnostics as in #1 but for SubstitutionContext.Current.SubstituteFactory.Createand
SubstitutionContext.Current.SubstituteFactory.CreatePartial
Even though automatic test runs are disabled in appveyor config, the tests are still being picked up by the appveyor which drastically increases the build time. We have to disable so called autoreporters in order for appveyor to stop analyzing test results. More info can be found here
There is a lot of code duplication in the codebase related to checks if a given symbol/node is part of NSubstitute. Those checks should be extracted to some sort of helper class/service so as they can be easily reusable without need of copying the code all over the places
NSubstitute 4.0.0.0
NSubstitute.Analyzers.CSharp 1.0.9
Perhaps I'm missing something but this looks like a false positive to me.
using System.Linq;
using NSubstitute;
namespace ClassLibrary3
{
public class Class1
{
public void Foo(IFoo foo, IEnumerable<Description> descriptions)
{
foreach (Description d in descriptions)
{
foo.Value.Returns(d.Value); // <- Warning NS4000 Returns() is set with a method that itself calls Returns. This can cause problems with NSubstitute. Consider replacing with a lambda: Returns(x => d.Value).
}
}
}
public class Description
{
public int Value { get; set; }
}
public interface IFoo
{
int Value { get; }
}
}
Usually, the analyzer has multiple test classes which test different usages of NSubstitute. For instance, in NonVirtualSetupAnalyzer we have separate test classes for usage of Returns method as an extension and other for an ordinary method(and many more). Test cases for that kind of stuff are usually the same, however currently TestCases attributes are copied/duplicated over all test classes for the given analyzer. This approach was originally chosen as different usages of NSubstitute might report a different location of diagnostic - so it was/is not possible to pass the same diagnostic location for all test classes.
Now as codebase is much larger than originally, it is difficult to maintain a proper set of tests - thus we should move test cases to the base test class of given analyzer and remove the duplication. This will require to mark the location of the diagnostic in the test itself (as we will have to drop expectedLine
from the test cases) - "marker" approach is used for instance in Roslynator so we can take a look at its implementation
Analyzers should be able to detect common mistakes when accessing invocation arguments via CallInfo.
using System;
using NSubstitute;
namespace MyNamespace
{
public interface Foo
{
int Bar(int x);
}
public class Bar
{
}
public class FooTests
{
public void Test()
{
var substitute = NSubstitute.Substitute.For<Foo>();
substitute.Bar(Arg.Any<int>()).Returns(callInfo =>
{
var x = (Bar)callInfo[0]; // invalid cast
var xx = callInfo[0] as Bar; // invalid cast
var xxx = (Bar)callInfo.Args()[0]; // invalid cast
var xxxx = callInfo.Args()[0] as Bar; // invalid cast
var xxxxx = callInfo[1]; // out of bounds
var xxxxxx = callInfo.Args()[1]; // out of bounds
var xxxxxxx = callInfo.ArgTypes()[1]; // out of bounds
callInfo[0] = 1; // no ref or out method
callInfo[0] = new object(); // no ref or out method, wrong type assigned
return 1;
});
}
}
}
Apart of already covered methods, NSubstitute provides a set of extension methods in NSubstitute.ReturnsExtensions class which should also be included in the analysis for violation of rule NS1000 (Non virtual substitution)
Analyzers should be able to detect following wrong usages
namespace MyNamespace
{
public class Foo
{
public int Bar()
{
return 2;
}
}
public class FooTests
{
public void Test()
{
var substitute = NSubstitute.Substitute.For<Foo>();
substitute.Received().Bar();
substitute.ReceivedWithAnyArgs().Bar()
substitute.DidNotReceive().Bar();
substitute.DidNotReceiveWithAnyArgs().Bar();
}
}
}
and of course usages of beforementioned examples executed as ordinal method
var substitute = NSubstitute.Substitute.For<Foo>();
var x = substitute.Received(1).Bar();
Assuming that Bar is non virtual member, NS1001 is correctly reported, however the analyzer highlight
substitute.Received(1)
instead of Bar()
. Alternatively it should higligt entire invocation expression substitute.Received(1).Bar();
if the previous one is not feasible
Blocks #70
Add documentation for every diagnostic rule, with a simple explanation of why it is reported and how to fix it
In some cases the NSub API can be intentionally used in a questionable manner to get a specific result.
A common example I have seen is "mocking" extensions methods. For example:
public static class MyExtensions {
public static ICounter Counter { get; set; }
public static int GetCount(this object blah) {
return Counter.GetCount(object);
}
}
We can then "mock" this extension method by cheating:
MyExtensions.Counter = Substitute.For<ICounter>();
// ...
widget.GetCount().Returns(42);
This will work because MyExtensions.GetCount()
invokes ICounter.GetCount(widget)
, so Returns
will successfully mock the ICounter
call (rather than the static, unmockable MyExtensions.GetCount()
). NSubstitute.Analyzers correctly reports this as "NS001 | Member GetCount can not be intercepted. Only interface members and virtual, overriding, and abstract members can be intercepted." Which would normally be fine, but in this case we're using this to intercept the underlying call instead.
@tpodolak's suggestion is to use a configuration file to allow developers to whitelist specific calls in their project. We still need to flesh out this idea, but here are some rough notes:
NS001
errors for MyProject.MyExtensions.GetCount
.NS001
for MyProject.MyExtensions.Get*
?NS1xx
series could be used for attempts to sub non-virtuals. NS101
can be for class methods, NS102
for class properties, NS111
for static class methods, NS120
for extension methods etc. Then we could specifically ignore NS111
for MyProject.MyClass.*
in the case we know all static members delegate to an underlying virtual method, but we still want to catch any instances of trying to substitute non-virtual class methods. (This is just an idea, it is probably sufficient just to do the basic whitelist instead.)It seems not to be clear where [assembly: InternalsVisibleTo(""DynamicProxyGenAssembly2"")] should be applied in order to allow NSubstitute to mock given internal type. Code fix provider should be able to do it for the user (if possible).
Question
I have an ordinary Test project in my solution, targeting .Net Framework 4.6.2. When I added a Nuget reference to NSubstitute.Analyzers.CSharp, I got several warnings like this:
An instance of analyzer NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.NonVirtualSetupWhenAnalyzer cannot be created from C:\Users\erizzo.nuget\packages\nsubstitute.analyzers.csharp\1.0.6\analyzers\dotnet\cs\NSubstitute.Analyzers.CSharp.dll : Could not load file or assembly 'Microsoft.CodeAnalysis.CSharp, Version=2.8.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35' or one of its dependencies. The system cannot find the file specified..
I then tried adding Nuget reference to Microsoft.CodeAnalysis.CSharp (I tried both 2.8.0 and the latest stable version, 2.10.0), but it made no difference.
Obviously, the NSubstitute analyzers are not producing any warnings, either.
Any ideas what I can do?
VisualStudio version is 15.6.7 (VS 2017)
As for today, two conventions are required to implement
Hi,
I got this exception on my project:
Severity Code Description Project File Line Suppression State Detail Description
Warning AD0001 Analyzer 'NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantSetupAnalyzer' threw an exception of type 'System.ArgumentException' with message 'SyntaxTree is not part of the compilation
Parameter name: syntaxTree'. ***** 1 Active Analyzer 'NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantSetupAnalyzer' threw the following exception:
'Exception occurred with following context:
Compilation: *****
SyntaxTree: C:\Users\Tests.cs
SyntaxNode: preferenceCollection.Get(@"t:\a.csproj" ... [InvocationExpressionSyntax]@[474..540) (15,12)-(15,78)
System.ArgumentException: SyntaxTree is not part of the compilation
Parameter name: syntaxTree
at Microsoft.CodeAnalysis.CSharp.CSharpCompilation.GetSemanticModel(SyntaxTree syntaxTree, Boolean ignoreAccessibility)
at Microsoft.CodeAnalysis.CSharp.CSharpCompilation.CommonGetSemanticModel(SyntaxTree syntaxTree, Boolean ignoreAccessibility)
at NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantCallFinder.ReEntrantCallVisitor.VisitInvocationExpression(InvocationExpressionSyntax node)
at Microsoft.CodeAnalysis.CSharp.Syntax.InvocationExpressionSyntax.Accept(CSharpSyntaxVisitor visitor)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.Visit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.DefaultVisit(SyntaxNode node)
at NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantCallFinder.ReEntrantCallVisitor.DefaultVisit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxVisitor.VisitArrowExpressionClause(ArrowExpressionClauseSyntax node)
at Microsoft.CodeAnalysis.CSharp.Syntax.ArrowExpressionClauseSyntax.Accept(CSharpSyntaxVisitor visitor)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.Visit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.DefaultVisit(SyntaxNode node)
at NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantCallFinder.ReEntrantCallVisitor.DefaultVisit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxVisitor.VisitAccessorDeclaration(AccessorDeclarationSyntax node)
at Microsoft.CodeAnalysis.CSharp.Syntax.AccessorDeclarationSyntax.Accept(CSharpSyntaxVisitor visitor)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.Visit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.DefaultVisit(SyntaxNode node)
at NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantCallFinder.ReEntrantCallVisitor.DefaultVisit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxVisitor.VisitAccessorList(AccessorListSyntax node)
at Microsoft.CodeAnalysis.CSharp.Syntax.AccessorListSyntax.Accept(CSharpSyntaxVisitor visitor)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.Visit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.DefaultVisit(SyntaxNode node)
at NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantCallFinder.ReEntrantCallVisitor.DefaultVisit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxVisitor.VisitPropertyDeclaration(PropertyDeclarationSyntax node)
at Microsoft.CodeAnalysis.CSharp.Syntax.PropertyDeclarationSyntax.Accept(CSharpSyntaxVisitor visitor)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.Visit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.DefaultVisit(SyntaxNode node)
at NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantCallFinder.ReEntrantCallVisitor.DefaultVisit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxVisitor.VisitClassDeclaration(ClassDeclarationSyntax node)
at Microsoft.CodeAnalysis.CSharp.Syntax.ClassDeclarationSyntax.Accept(CSharpSyntaxVisitor visitor)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.Visit(SyntaxNode node)
at NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantCallFinder.ReEntrantCallVisitor.VisitRelatedSymbols(SyntaxNode syntaxNode)
at NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantCallFinder.ReEntrantCallVisitor.DefaultVisit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxVisitor.VisitIdentifierName(IdentifierNameSyntax node)
at Microsoft.CodeAnalysis.CSharp.Syntax.IdentifierNameSyntax.Accept(CSharpSyntaxVisitor visitor)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.Visit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.DefaultVisit(SyntaxNode node)
at NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantCallFinder.ReEntrantCallVisitor.DefaultVisit(SyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxVisitor.VisitObjectCreationExpression(ObjectCreationExpressionSyntax node)
at Microsoft.CodeAnalysis.CSharp.Syntax.ObjectCreationExpressionSyntax.Accept(CSharpSyntaxVisitor visitor)
at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxWalker.Visit(SyntaxNode node)
at NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers.ReEntrantCallFinder.GetReEntrantSymbols(Compilation compilation, SyntaxNode rootNode)
at NSubstitute.Analyzers.Shared.DiagnosticAnalyzers.AbstractReEntrantCallFinder.GetReEntrantCalls(Compilation compilation, SyntaxNode rootNode)
at NSubstitute.Analyzers.Shared.DiagnosticAnalyzers.AbstractReEntrantSetupAnalyzer`2.AnalyzeInvocation(SyntaxNodeAnalysisContext syntaxNodeContext)
at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.<>c__44`1.<ExecuteSyntaxNodeAction>b__44_0(ValueTuple`2 data)
at Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.ExecuteAndCatchIfThrows_NoLock[TArg](DiagnosticAnalyzer analyzer, Action`1 analyze, TArg argument, Nullable`1 info)
From nsubstitute/NSubstitute#496, having a test assembly with visibility of internal members of assembly under test, but without InternalsVisibleTo("DynamicProxyGenAssembly2")
, causes errors when running the tests.
It would be good to catch this case, similar to NS2003.
From Robert's comment on NSubstitute@496:
Referring to NS2003, there are no checks on members, only on the type itself. This is the analyzer method in question AnalyzeTypeAccessability
The reproduction on that issue is a good test case to illustrate the problem (just having a type with internal members local to the test assembly with trigger the issue).
I'm not sure what category this should belong to. Could be NS5xxx
as a general usage problem, although it is also sort of similar to NS1xxx
which covers non-virtual substitution, although in this case it is a non-substitutable member due to internal
. The easiest is probably to put it in NS5xxx
, but maybe we should also consider adapting the NS1xxx
category to be something to do with "non-substitutable members" and include this?
CheckId | NS???? |
Category | ???? |
Substituting for an internal member of a class without proxies having visibility into internal members.
A violation of this rule occurs when using NSubstitute features like Returns()
, Received()
etc. with an internal member of a class, but without giving NSubstitute's proxy assembly access to internal members.
There are several ways to fix this:
internal
to public
internal
to protected internal
DynamicProxyGenAssembly2
using:[assembly: InternalsVisibleTo("DynamicProxyGenAssembly2")]
...
Even though NSubstitute.Analyzers are written against NetStandard at the moment all tests are failing when run on Linux machine. This task is all about ensuring that it is possible to do a normal development on linux based machines.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.