GH-219 - handling protected internal constructors when analyzing NS20…

…01 diagnostic (#221)
nsubstitute · May 3, 2024 · a4d589a · a4d589a
1 parent f2ff842
commit a4d589a
Show file tree

Hide file tree

Showing 14 changed files with 291 additions and 58 deletions.
diff --git a/src/NSubstitute.Analyzers.Shared/DiagnosticAnalyzers/SubstituteConstructorAnalysis.cs b/src/NSubstitute.Analyzers.Shared/DiagnosticAnalyzers/SubstituteConstructorAnalysis.cs
@@ -93,22 +93,13 @@ private IMethodSymbol[] GetAccessibleConstructors(ITypeSymbol genericArgument)
     {
         var internalsVisibleToProxy = genericArgument.InternalsVisibleToProxyGenerator();
 
-        bool IsAccessible(IMethodSymbol symbol) => symbol.DeclaredAccessibility is Accessibility.Protected or Accessibility.Public;
-
-        bool IsVisibleToProxy(IMethodSymbol symbol)
-        {
-            if (internalsVisibleToProxy == false)
-            {
-                return false;
-            }
-
-            return symbol.DeclaredAccessibility is Accessibility.Internal or Accessibility.ProtectedOrInternal;
-        }
-
         return genericArgument.GetMembers().OfType<IMethodSymbol>().Where(symbol =>
             symbol.MethodKind == MethodKind.Constructor &&
             symbol.IsStatic == false &&
-            (IsAccessible(symbol) || IsVisibleToProxy(symbol))).ToArray();
+            (symbol.DeclaredAccessibility == Accessibility.Protected ||
+             symbol.DeclaredAccessibility == Accessibility.Public ||
+             symbol.DeclaredAccessibility == Accessibility.ProtectedOrInternal ||
+             (internalsVisibleToProxy && symbol.DeclaredAccessibility == Accessibility.Internal))).ToArray();
     }
 
     private ITypeSymbol[] GetTypeSymbols(IArrayCreationOperation arrayInitializerOperation)

diff --git a/...s.Tests.CSharp/DiagnosticAnalyzerTests/SubstituteAnalyzerTests/ForAsGenericMethodTests.cs b/...s.Tests.CSharp/DiagnosticAnalyzerTests/SubstituteAnalyzerTests/ForAsGenericMethodTests.cs
@@ -274,7 +274,7 @@ public void Test()
         await VerifyDiagnostic(source, SubstituteForWithoutAccessibleConstructorDescriptor, "Could not find accessible constructor. Make sure that type MyNamespace.Foo exposes public or protected constructors.");
     }
 
-    public override async Task ReportsDiagnostic_WhenUsedForClassWithProtectedInternalConstructor_AndInternalsVisibleToNotApplied()
+    public override async Task ReportsNoDiagnostics_WhenUsedForClassWithProtectedInternalConstructor_AndInternalsVisibleToNotApplied()
     {
         var source = @"using NSubstitute;
 
@@ -291,11 +291,11 @@ public class FooTests
     {
         public void Test()
         {
-            var substitute = [|NSubstitute.Substitute.For<Foo>()|];
+            var substitute = NSubstitute.Substitute.For<Foo>();
         }
     }
 }";
-        await VerifyDiagnostic(source, SubstituteForWithoutAccessibleConstructorDescriptor, "Could not find accessible constructor. Make sure that type MyNamespace.Foo exposes public or protected constructors.");
+        await this.VerifyNoDiagnostic(source);
     }
 
     public override async Task ReportsNoDiagnostic_WhenUsedForClassWithInternalConstructor_AndInternalsVisibleToApplied()
@@ -350,6 +350,30 @@ public void Test()
         await VerifyNoDiagnostic(source);
     }
 
+    public override async Task ReportsNoDiagnostic_WhenUsedForClassWithProtectedConstructor()
+    {
+        var source = @"using NSubstitute;
+
+namespace MyNamespace
+{
+    public class Foo
+    {
+        protected Foo()
+        {
+        }
+    }
+
+    public class FooTests
+    {
+        public void Test()
+        {
+            var substitute = NSubstitute.Substitute.For<Foo>();
+        }
+    }
+}";
+        await VerifyNoDiagnostic(source);
+    }
+
     [Fact]
     public override async Task ReportsDiagnostic_WhenPassedParametersCount_GreaterThanCtorParametersCount()
     {

diff --git a/...ests.CSharp/DiagnosticAnalyzerTests/SubstituteAnalyzerTests/ForAsNonGenericMethodTests.cs b/...ests.CSharp/DiagnosticAnalyzerTests/SubstituteAnalyzerTests/ForAsNonGenericMethodTests.cs
@@ -370,7 +370,7 @@ public void Test()
         await VerifyDiagnostic(source, SubstituteForWithoutAccessibleConstructorDescriptor, "Could not find accessible constructor. Make sure that type MyNamespace.Foo exposes public or protected constructors.");
     }
 
-    public override async Task ReportsDiagnostic_WhenUsedForClassWithProtectedInternalConstructor_AndInternalsVisibleToNotApplied()
+    public override async Task ReportsNoDiagnostics_WhenUsedForClassWithProtectedInternalConstructor_AndInternalsVisibleToNotApplied()
     {
         var source = @"using NSubstitute;
 
@@ -387,13 +387,13 @@ public class FooTests
     {
         public void Test()
         {
-            var substitute = [|NSubstitute.Substitute.For(new [] { typeof(Foo) }, null)|];
-            var otherSubstitute = [|NSubstitute.Substitute.For(typesToProxy: new [] { typeof(Foo) }, constructorArguments: null)|];
-            var yetAnotherSubstitute = [|NSubstitute.Substitute.For(constructorArguments: null, typesToProxy: new [] { typeof(Foo) })|];
+            var substitute = NSubstitute.Substitute.For(new [] { typeof(Foo) }, null);
+            var otherSubstitute = NSubstitute.Substitute.For(typesToProxy: new [] { typeof(Foo) }, constructorArguments: null);
+            var yetAnotherSubstitute = NSubstitute.Substitute.For(constructorArguments: null, typesToProxy: new [] { typeof(Foo) });
         }
     }
 }";
-        await VerifyDiagnostic(source, SubstituteForWithoutAccessibleConstructorDescriptor, "Could not find accessible constructor. Make sure that type MyNamespace.Foo exposes public or protected constructors.");
+        await VerifyNoDiagnostic(source);
     }
 
     public override async Task ReportsNoDiagnostic_WhenUsedForClassWithInternalConstructor_AndInternalsVisibleToApplied()
@@ -452,6 +452,32 @@ public void Test()
         await VerifyNoDiagnostic(source);
     }
 
+    public override async Task ReportsNoDiagnostic_WhenUsedForClassWithProtectedConstructor()
+    {
+        var source = @"using NSubstitute;
+
+namespace MyNamespace
+{
+    public class Foo
+    {
+        protected Foo()
+        {
+        }
+    }
+
+    public class FooTests
+    {
+        public void Test()
+        {
+            var substitute = NSubstitute.Substitute.For(new [] { typeof(Foo) }, null);
+            var otherSubstitute = NSubstitute.Substitute.For(typesToProxy: new [] { typeof(Foo) }, constructorArguments: null);
+            var yetAnotherSubstitute = NSubstitute.Substitute.For(constructorArguments: null, typesToProxy: new [] { typeof(Foo) });
+        }
+    }
+}";
+        await VerifyNoDiagnostic(source);
+    }
+
     [Fact]
     public override async Task ReportsDiagnostic_WhenPassedParametersCount_GreaterThanCtorParametersCount()
     {

diff --git a/...ers.Tests.CSharp/DiagnosticAnalyzerTests/SubstituteAnalyzerTests/ForPartsOfMethodTests.cs b/...ers.Tests.CSharp/DiagnosticAnalyzerTests/SubstituteAnalyzerTests/ForPartsOfMethodTests.cs
@@ -98,7 +98,7 @@ public void Test()
         await VerifyDiagnostic(source, SubstituteForWithoutAccessibleConstructorDescriptor, "Could not find accessible constructor. Make sure that type MyNamespace.Foo exposes public or protected constructors.");
     }
 
-    public override async Task ReportsDiagnostic_WhenUsedForClassWithProtectedInternalConstructor_AndInternalsVisibleToNotApplied()
+    public override async Task ReportsNoDiagnostics_WhenUsedForClassWithProtectedInternalConstructor_AndInternalsVisibleToNotApplied()
     {
         var source = @"using NSubstitute;
 
@@ -115,11 +115,11 @@ public class FooTests
     {
         public void Test()
         {
-            var substitute = [|NSubstitute.Substitute.ForPartsOf<Foo>()|];
+            var substitute = NSubstitute.Substitute.ForPartsOf<Foo>();
         }
     }
 }";
-        await VerifyDiagnostic(source, SubstituteForWithoutAccessibleConstructorDescriptor, "Could not find accessible constructor. Make sure that type MyNamespace.Foo exposes public or protected constructors.");
+        await VerifyNoDiagnostic(source);
     }
 
     public override async Task ReportsNoDiagnostic_WhenUsedForClassWithInternalConstructor_AndInternalsVisibleToApplied()
@@ -176,6 +176,30 @@ public void Test()
         await VerifyNoDiagnostic(source);
     }
 
+    public override async Task ReportsNoDiagnostic_WhenUsedForClassWithProtectedConstructor()
+    {
+        var source = @"using NSubstitute;
+
+namespace MyNamespace
+{
+    public class Foo
+    {
+        protected Foo()
+        {
+        }
+    }
+
+    public class FooTests
+    {
+        public void Test()
+        {
+            var substitute = NSubstitute.Substitute.ForPartsOf<Foo>();
+        }
+    }
+}";
+        await VerifyNoDiagnostic(source);
+    }
+
     [Fact]
     public override async Task ReportsDiagnostic_WhenPassedParametersCount_GreaterThanCtorParametersCount()
     {

diff --git a/...ts.CSharp/DiagnosticAnalyzerTests/SubstituteAnalyzerTests/SubstituteDiagnosticVerifier.cs b/...ts.CSharp/DiagnosticAnalyzerTests/SubstituteAnalyzerTests/SubstituteDiagnosticVerifier.cs
@@ -37,14 +37,17 @@ public abstract class SubstituteDiagnosticVerifier : CSharpDiagnosticVerifier, I
     public abstract Task ReportsDiagnostic_WhenUsedForClassWithInternalConstructor_AndInternalsVisibleToNotApplied();
 
     [Fact]
-    public abstract Task ReportsDiagnostic_WhenUsedForClassWithProtectedInternalConstructor_AndInternalsVisibleToNotApplied();
+    public abstract Task ReportsNoDiagnostics_WhenUsedForClassWithProtectedInternalConstructor_AndInternalsVisibleToNotApplied();
 
     [Fact]
     public abstract Task ReportsNoDiagnostic_WhenUsedForClassWithInternalConstructor_AndInternalsVisibleToApplied();
 
     [Fact]
     public abstract Task ReportsNoDiagnostic_WhenUsedForClassWithProtectedInternalConstructor_AndInternalsVisibleToApplied();
 
+    [Fact]
+    public abstract Task ReportsNoDiagnostic_WhenUsedForClassWithProtectedConstructor();
+
     [Fact]
     public abstract Task ReportsDiagnostic_WhenPassedParametersCount_GreaterThanCtorParametersCount();
 

diff --git a/...arp/DiagnosticAnalyzerTests/SubstituteAnalyzerTests/SubstituteFactoryCreateMethodTests.cs b/...arp/DiagnosticAnalyzerTests/SubstituteAnalyzerTests/SubstituteFactoryCreateMethodTests.cs
@@ -323,7 +323,7 @@ public void Test()
         await VerifyDiagnostic(source, SubstituteForWithoutAccessibleConstructorDescriptor, "Could not find accessible constructor. Make sure that type MyNamespace.Foo exposes public or protected constructors.");
     }
 
-    public override async Task ReportsDiagnostic_WhenUsedForClassWithProtectedInternalConstructor_AndInternalsVisibleToNotApplied()
+    public override async Task ReportsNoDiagnostics_WhenUsedForClassWithProtectedInternalConstructor_AndInternalsVisibleToNotApplied()
     {
         var source = @"using NSubstitute.Core;
 
@@ -340,13 +340,13 @@ public class FooTests
     {
         public void Test()
         {
-            var substitute = [|SubstitutionContext.Current.SubstituteFactory.Create(new[] {typeof(Foo)}, null)|];
-            var otherSubstitute = [|SubstitutionContext.Current.SubstituteFactory.Create(typesToProxy: new[] {typeof(Foo)}, constructorArguments: null)|];
-            var yetAnotherSubstitute = [|SubstitutionContext.Current.SubstituteFactory.Create(constructorArguments: null, typesToProxy: new[] {typeof(Foo)})|];
+            var substitute = SubstitutionContext.Current.SubstituteFactory.Create(new[] {typeof(Foo)}, null);
+            var otherSubstitute = SubstitutionContext.Current.SubstituteFactory.Create(typesToProxy: new[] {typeof(Foo)}, constructorArguments: null);
+            var yetAnotherSubstitute = SubstitutionContext.Current.SubstituteFactory.Create(constructorArguments: null, typesToProxy: new[] {typeof(Foo)});
         }
     }
 }";
-        await VerifyDiagnostic(source, SubstituteForWithoutAccessibleConstructorDescriptor, "Could not find accessible constructor. Make sure that type MyNamespace.Foo exposes public or protected constructors.");
+        await VerifyNoDiagnostic(source);
     }
 
     public override async Task ReportsNoDiagnostic_WhenUsedForClassWithInternalConstructor_AndInternalsVisibleToApplied()
@@ -405,6 +405,32 @@ public void Test()
         await VerifyNoDiagnostic(source);
     }
 
+    public override async Task ReportsNoDiagnostic_WhenUsedForClassWithProtectedConstructor()
+    {
+        var source = @"using NSubstitute.Core;
+
+namespace MyNamespace
+{
+    public class Foo
+    {
+        protected Foo()
+        {
+        }
+    }
+
+    public class FooTests
+    {
+        public void Test()
+        {
+            var substitute = SubstitutionContext.Current.SubstituteFactory.Create(new[] {typeof(Foo)}, null);
+            var otherSubstitute = SubstitutionContext.Current.SubstituteFactory.Create(typesToProxy: new[] {typeof(Foo)}, constructorArguments: null);
+            var yetAnotherSubstitute = SubstitutionContext.Current.SubstituteFactory.Create(constructorArguments: null, typesToProxy: new[] {typeof(Foo)});
+        }
+    }
+}";
+        await VerifyNoDiagnostic(source);
+    }
+
     [Fact]
     public override async Task ReportsDiagnostic_WhenPassedParametersCount_GreaterThanCtorParametersCount()
     {

diff --git a/...gnosticAnalyzerTests/SubstituteAnalyzerTests/SubstituteFactoryCreatePartialMethodTests.cs b/...gnosticAnalyzerTests/SubstituteAnalyzerTests/SubstituteFactoryCreatePartialMethodTests.cs
@@ -132,7 +132,7 @@ public void Test()
         await VerifyDiagnostic(source, SubstituteForWithoutAccessibleConstructorDescriptor, "Could not find accessible constructor. Make sure that type MyNamespace.Foo exposes public or protected constructors.");
     }
 
-    public override async Task ReportsDiagnostic_WhenUsedForClassWithProtectedInternalConstructor_AndInternalsVisibleToNotApplied()
+    public override async Task ReportsNoDiagnostics_WhenUsedForClassWithProtectedInternalConstructor_AndInternalsVisibleToNotApplied()
     {
         var source = @"using System;
 using NSubstitute.Core;
@@ -150,13 +150,13 @@ public class FooTests
     {
         public void Test()
         {
-            var substitute = [|SubstitutionContext.Current.SubstituteFactory.CreatePartial(new Type[] { typeof(Foo)}, null)|];
-            var otherSubstitute = [|SubstitutionContext.Current.SubstituteFactory.CreatePartial(typesToProxy: new Type[] { typeof(Foo)}, constructorArguments: null)|];
-            var yetAnotherSubstitute = [|SubstitutionContext.Current.SubstituteFactory.CreatePartial(constructorArguments: null, typesToProxy: new Type[] { typeof(Foo)})|];
+            var substitute = SubstitutionContext.Current.SubstituteFactory.CreatePartial(new Type[] { typeof(Foo)}, null);
+            var otherSubstitute = SubstitutionContext.Current.SubstituteFactory.CreatePartial(typesToProxy: new Type[] { typeof(Foo)}, constructorArguments: null);
+            var yetAnotherSubstitute = SubstitutionContext.Current.SubstituteFactory.CreatePartial(constructorArguments: null, typesToProxy: new Type[] { typeof(Foo)});
         }
     }
 }";
-        await VerifyDiagnostic(source, SubstituteForWithoutAccessibleConstructorDescriptor, "Could not find accessible constructor. Make sure that type MyNamespace.Foo exposes public or protected constructors.");
+        await VerifyNoDiagnostic(source);
     }
 
     public override async Task ReportsNoDiagnostic_WhenUsedForClassWithInternalConstructor_AndInternalsVisibleToApplied()
@@ -217,6 +217,33 @@ public void Test()
         await VerifyNoDiagnostic(source);
     }
 
+    public override async Task ReportsNoDiagnostic_WhenUsedForClassWithProtectedConstructor()
+    {
+        var source = @"using System;
+using NSubstitute.Core;
+
+namespace MyNamespace
+{
+    public class Foo
+    {
+        protected Foo()
+        {
+        }
+    }
+
+    public class FooTests
+    {
+        public void Test()
+        {
+            var substitute = SubstitutionContext.Current.SubstituteFactory.CreatePartial(new Type[] { typeof(Foo)}, null);
+            var otherSubstitute = SubstitutionContext.Current.SubstituteFactory.CreatePartial(typesToProxy: new Type[] { typeof(Foo)}, constructorArguments: null);
+            var yetAnotherSubstitute = SubstitutionContext.Current.SubstituteFactory.CreatePartial(constructorArguments: null, typesToProxy: new Type[] { typeof(Foo)});
+        }
+    }
+}";
+        await VerifyNoDiagnostic(source);
+    }
+
     [Fact]
     public override async Task ReportsDiagnostic_WhenPassedParametersCount_GreaterThanCtorParametersCount()
     {

diff --git a/tests/NSubstitute.Analyzers.Tests.Shared/DiagnosticAnalyzers/ISubstituteAnalyzerVerifier.cs b/tests/NSubstitute.Analyzers.Tests.Shared/DiagnosticAnalyzers/ISubstituteAnalyzerVerifier.cs
@@ -8,12 +8,14 @@ public interface ISubstituteAnalyzerVerifier
 
     Task ReportsDiagnostic_WhenUsedForClassWithInternalConstructor_AndInternalsVisibleToNotApplied();
 
-    Task ReportsDiagnostic_WhenUsedForClassWithProtectedInternalConstructor_AndInternalsVisibleToNotApplied();
+    Task ReportsNoDiagnostics_WhenUsedForClassWithProtectedInternalConstructor_AndInternalsVisibleToNotApplied();
 
     Task ReportsNoDiagnostic_WhenUsedForClassWithInternalConstructor_AndInternalsVisibleToApplied();
 
     Task ReportsNoDiagnostic_WhenUsedForClassWithProtectedInternalConstructor_AndInternalsVisibleToApplied();
 
+    Task ReportsNoDiagnostic_WhenUsedForClassWithProtectedConstructor();
+
     Task ReportsDiagnostic_WhenPassedParametersCount_GreaterThanCtorParametersCount();
 
     Task ReportsDiagnostic_WhenPassedParametersCount_LessThanCtorParametersCount();