Code cleanup

egil · egil · commit 51d6b81fe68a · 2020-12-19T17:06:26.000Z
diff --git a/.editorconfig b/.editorconfig
@@ -44,13 +44,20 @@ dotnet_diagnostic.BL0006.severity = none
 
 [*.cs]
 
+dotnet_diagnostic.IDE0022.severity = silent # IDE0022: Use block body for methods
+dotnet_diagnostic.IDE0090.severity = silent # IDE0090: Use 'new(...)'
 dotnet_diagnostic.S125.severity = suggestion # S125: Sections of code should not be commented out
 dotnet_diagnostic.S927.severity = suggestion # S927: Parameter names should match base declaration and other partial definitions
 dotnet_diagnostic.S1075.severity = suggestion # S1075: URIs should not be hardcoded
-dotnet_diagnostic.S1186.severity = suggestion # S1186: Methods should not be empty
+dotnet_diagnostic.S1186.severity = silent # S1186: Methods should not be empty
 dotnet_diagnostic.S1199.severity = suggestion # S1199: Nested code blocks should not be used 
 dotnet_diagnostic.S3925.severity = suggestion # S3925: "ISerializable" should be implemented correctly
 
-[tests/**.cs]
+[src/bunit.web/EventDispatchExtensions/**.cs]
+dotnet_diagnostic.S107.severity = none # S107: Methods should not have too many parameters
 
-dotnet_diagnostic.S3459.severity = suggestion # S3459: Unassigned members should be removed
+[tests/**.cs]
+dotnet_diagnostic.S125.severity = none # S125: Sections of code should not be commented out
+dotnet_diagnostic.CA2012.severity = none # CA2012: Use ValueTasks correctly
+dotnet_diagnostic.S3459.severity = none # S3459: Unassigned members should be removed
+dotnet_diagnostic.S1186.severity = none # S1186: Methods should not be empty
diff --git a/src/Directory.Build.props b/src/Directory.Build.props
@@ -46,9 +46,13 @@
 		<PackageReference Include="Microsoft.AspNetCore.Components" Version="5.0.0" />
 		<PackageReference Include="Microsoft.AspNetCore.Components.Web" Version="5.0.0" />
 	</ItemGroup>
-	
+
 	<ItemGroup>
 		<PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0" PrivateAssets="All" />
 		<PackageReference Include="Nerdbank.GitVersioning" Version="3.3.37" PrivateAssets="All" />
-	</ItemGroup>
+		<PackageReference Include="SonarAnalyzer.CSharp" Version="8.15.0.24505">
+			<PrivateAssets>all</PrivateAssets>
+			<IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
+		</PackageReference>
+	</ItemGroup>	
 </Project>
diff --git a/src/bunit.core/ComponentParameter.cs b/src/bunit.core/ComponentParameter.cs
@@ -1,13 +1,11 @@
 using System;
-using System.Diagnostics.CodeAnalysis;
 
 namespace Bunit
 {
 	/// <summary>
 	/// Represents a single parameter supplied to an <see cref="Microsoft.AspNetCore.Components.IComponent"/>
 	/// component under test.
 	/// </summary>    
-	[SuppressMessage("Usage", "CA2225:Operator overloads have named alternates", Justification = "<Pending>")]
 	public readonly struct ComponentParameter : IEquatable<ComponentParameter>
 	{
 		/// <summary>
diff --git a/src/bunit.core/GlobalSuppressions.cs b/src/bunit.core/GlobalSuppressions.cs
diff --git a/src/bunit.web/GlobalSuppressions.cs b/src/bunit.web/GlobalSuppressions.cs
diff --git a/src/bunit.web/JSInterop/BunitJSInterop.cs b/src/bunit.web/JSInterop/BunitJSInterop.cs
@@ -41,7 +41,9 @@ public BunitJSInterop()
 		{
 			Mode = JSRuntimeMode.Strict;
 			JSRuntime = new BunitJSRuntime(this);
+#if NET5_0
 			AddCustomHandlers();
+#endif
 		}
 
 		/// <summary>
@@ -83,13 +85,13 @@ internal virtual void RegisterInvocation(JSRuntimeInvocation invocation)
 			return result;
 		}
 
+#if NET5_0
 		private void AddCustomHandlers()
 		{
-#if NET5_0
 			AddInvocationHandler(new FocusAsyncInvocationHandler());
 			AddInvocationHandler(new VirtualizeJSRuntimeInvocationHandler());
 			AddInvocationHandler(new LooseModeJSObjectReferenceInvocationHandler(this));
-#endif
 		}
+#endif
 	}
 }
diff --git a/src/bunit.xunit/GlobalSuppressions.cs b/src/bunit.xunit/GlobalSuppressions.cs
diff --git a/tests/Directory.Build.props b/tests/Directory.Build.props
@@ -36,5 +36,9 @@
 		<PackageReference Include="AutoFixture.Xunit2" Version="4.14.0" />
 		<PackageReference Include="Xunit.Combinatorial" Version="1.4.1" />
 		<PackageReference Include="coverlet.msbuild" Version="2.9.0" PrivateAssets="All" />
+		<PackageReference Include="SonarAnalyzer.CSharp" Version="8.15.0.24505">
+			<PrivateAssets>all</PrivateAssets>
+			<IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
+		</PackageReference>
 	</ItemGroup>
 </Project>
diff --git a/tests/bunit.core.tests/ComponentParameterCollectionTest.cs b/tests/bunit.core.tests/ComponentParameterCollectionTest.cs
@@ -1,6 +1,5 @@
 using System;
 using System.Collections.Generic;
-using Bunit.TestDoubles;
 using Microsoft.AspNetCore.Components;
 using Microsoft.AspNetCore.Components.Rendering;
 using Shouldly;
diff --git a/tests/bunit.core.tests/GlobalSuppressions.cs b/tests/bunit.core.tests/GlobalSuppressions.cs
diff --git a/tests/bunit.core.tests/RazorTesting/FixtureBaseTest.cs b/tests/bunit.core.tests/RazorTesting/FixtureBaseTest.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Diagnostics.CodeAnalysis;
 using System.Threading.Tasks;
 using Shouldly;
 using Xunit;
@@ -14,6 +15,7 @@ private class FixtureComponent : FixtureBase<FixtureComponent>
 		}
 
 		[Fact(DisplayName = "Setup, SetupAsync and Test methods are called in the correct order")]
+		[SuppressMessage("Minor Bug", "S4158:Empty collections should not be accessed or iterated", Justification = "False positive!")]
 		public async Task Test001()
 		{
 			var callLog = new List<string>(3);
@@ -40,6 +42,7 @@ Task SetupAsync(FixtureComponent fixture)
 		}
 
 		[Fact(DisplayName = "Setup, SetupAsync and TestAsync methods are called in the correct order")]
+		[SuppressMessage("Minor Bug", "S4158:Empty collections should not be accessed or iterated", Justification = "False positive!")]
 		public async Task Test002()
 		{
 			var callLog = new List<string>(3);
diff --git a/tests/bunit.core.tests/Rendering/ComponentParameterTest.cs b/tests/bunit.core.tests/Rendering/ComponentParameterTest.cs
@@ -25,18 +25,18 @@ public static IEnumerable<object[]> GetEqualsTestData()
 			yield return new object[] { p1, p5, false };
 		}
 
-		[Fact(DisplayName = "Creating a cascading value throws")]
+		[Fact(DisplayName = "Creating a cascading value with null throws")]
 		public void Test001()
 		{
 			Should.Throw<ArgumentNullException>(() => ComponentParameter.CreateCascadingValue(null, null!));
-			Should.Throw<ArgumentNullException>(() => { ComponentParameter p = (null, null, true); });
+			Should.Throw<ArgumentNullException>(() => (ComponentParameter)(null, null, true));
 		}
 
 		[Fact(DisplayName = "Creating a regular parameter without a name throws")]
 		public void Test002()
 		{
 			Should.Throw<ArgumentNullException>(() => ComponentParameter.CreateParameter(null!, null));
-			Should.Throw<ArgumentNullException>(() => { ComponentParameter p = (null, null, false); });
+			Should.Throw<ArgumentNullException>(() => (ComponentParameter)(null, null, false));
 		}
 
 		[Theory(DisplayName = "Equals compares correctly")]
diff --git a/tests/bunit.testassets/GlobalSuppressions.cs b/tests/bunit.testassets/GlobalSuppressions.cs
diff --git a/tests/bunit.testassets/SampleComponents/WrapperDiv.cs b/tests/bunit.testassets/SampleComponents/WrapperDiv.cs
@@ -1,4 +1,3 @@
-using System.Threading.Tasks;
 using Microsoft.AspNetCore.Components;
 using Microsoft.AspNetCore.Components.Rendering;
 
diff --git a/tests/bunit.web.tests/Asserting/DiffAssertExtensionsTest.cs b/tests/bunit.web.tests/Asserting/DiffAssertExtensionsTest.cs
@@ -23,7 +23,7 @@ public void Test001()
 			catch (Exception ex)
 			{
 				exception = ex;
-			};
+			}
 
 			exception.ShouldBeOfType<ArgumentNullException>();
 		}
@@ -41,7 +41,7 @@ public void Test002(IReadOnlyList<IDiff> diffs)
 			catch (Exception ex)
 			{
 				exception = ex;
-			};
+			}
 
 			exception.ShouldBeOfType<ActualExpectedAssertException>();
 		}
diff --git a/tests/bunit.web.tests/EventDispatchExtensions/DetailsElementEventDispatcherExtensionsTest.cs b/tests/bunit.web.tests/EventDispatchExtensions/DetailsElementEventDispatcherExtensionsTest.cs
@@ -1,6 +1,4 @@
 #if NET5_0
-using System.Collections.Generic;
-using System.Threading.Tasks;
 using Bunit.TestAssets.SampleComponents;
 using Shouldly;
 using Xunit;
diff --git a/tests/bunit.web.tests/EventDispatchExtensions/KeyTest.cs b/tests/bunit.web.tests/EventDispatchExtensions/KeyTest.cs
@@ -1,15 +1,13 @@
 using System;
 using System.Collections.Generic;
-using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using System.Reflection;
 using Microsoft.AspNetCore.Components.Web;
 using Shouldly;
 using Xunit;
 
 namespace Bunit
-{
-	[SuppressMessage("Microsoft.Design", "CA1062:Validate arguments of public methods", Justification = "Arguments are provided from test data")]
+{	
 	public class KeyTest
 	{
 		public static IEnumerable<object[]> KeyValueTestData { get; } = GetKeyValueTestData().Select(c => new object[] { c }).ToList();
@@ -69,15 +67,12 @@ public void CanCastFromString(string value)
 			key.CommandKey.ShouldBeFalse();
 		}
 
-		[Theory(DisplayName = "Custing from null or empty string throws ArgumentNullException")]
+		[Theory(DisplayName = "Casting from null or empty string throws ArgumentNullException")]
 		[InlineData(null)]
 		[InlineData("")]
 		public void CastingFromNullStringThrowsException(string value)
 		{
-			Should.Throw<ArgumentNullException>(() =>
-			{
-				Key key = value;
-			});
+			Should.Throw<ArgumentNullException>(() => (Key)value);
 		}
 
 		[Theory(DisplayName = "Get method with value and code parameters should return initialized Key object")]
diff --git a/tests/bunit.web.tests/Extensions/RefreshingWrappedElementTest.cs b/tests/bunit.web.tests/Extensions/RefreshingWrappedElementTest.cs
@@ -70,7 +70,7 @@ public void Test030()
 
 			var btn = cut.Find("button");
 
-			btn.Click(); // remove from dom
+			Should.NotThrow(() => btn.Click());
 		}
 
 		[Fact(DisplayName = "Found element throws when its properties or methods are used after it's removed from DOM")]
diff --git a/tests/bunit.web.tests/GlobalSuppressions.cs b/tests/bunit.web.tests/GlobalSuppressions.cs
diff --git a/tests/bunit.web.tests/JSInterop/BunitJSInteropTest.cs b/tests/bunit.web.tests/JSInterop/BunitJSInteropTest.cs
@@ -1,5 +1,4 @@
 using System;
-using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
@@ -414,7 +413,6 @@ public async Task Test022()
 			plannedCatchall.Invocations.Count.ShouldBe(0);
 		}
 
-		[SuppressMessage("Reliability", "CA2012:Use ValueTasks correctly", Justification = "Not relevant for test")]
 		[Fact(DisplayName = "The last handler matching an invocation receives the invocation")]
 		public void Test030()
 		{
diff --git a/tests/bunit.web.tests/JSInterop/InvocationHandlers/VirtualizeJSRuntimeInvocationHandlerTest.cs b/tests/bunit.web.tests/JSInterop/InvocationHandlers/VirtualizeJSRuntimeInvocationHandlerTest.cs
@@ -108,7 +108,7 @@ public void Test040(int itemsInDataSource)
 		private static ICollection<string> CreateItems(int itemsToCreate)
 			=> Enumerable.Range(0, itemsToCreate).Select(x => Guid.NewGuid().ToString()).ToArray();
 
-		private ItemsProviderDelegate<string> CreateItemsProvider(int itemsInCollection)
+		private static ItemsProviderDelegate<string> CreateItemsProvider(int itemsInCollection)
 		{
 			return request =>
 			{
diff --git a/tests/bunit.web.tests/TestContextTest.cs b/tests/bunit.web.tests/TestContextTest.cs
@@ -8,13 +8,6 @@ namespace Bunit
 {
 	public class TestContextTest : TestContext
 	{
-		[Fact(DisplayName = "The placeholder IJSRuntime is overridden by a supplied mock and does not throw")]
-		public void Test022()
-		{
-			JSInterop.Mode = JSRuntimeMode.Loose;
-			RenderComponent<SimpleWithJSRuntimeDep>();
-		}
-
 		[Fact(DisplayName = "The test service provider should register a placeholder NavigationManager which throws exceptions")]
 		public void Test023()
 		{
@@ -89,9 +82,7 @@ public void Test033()
 		[Fact(DisplayName = "Can raise events from markup rendered with TestContext")]
 		public void Test040()
 		{
-			RenderComponent<ClickCounter>()
-				.Find("button")
-				.Click();
+			Should.NotThrow(() => RenderComponent<ClickCounter>().Find("button").Click());
 		}
 
 		class ReceivesCascadinValue : ComponentBase
diff --git a/tests/bunit.xunit.tests/Asserting/GenericCollectionAssertExtensionsTest.cs b/tests/bunit.xunit.tests/Asserting/GenericCollectionAssertExtensionsTest.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Diagnostics.CodeAnalysis;
 using Shouldly;
 using Xunit;
 using Xunit.Sdk;
@@ -22,7 +23,7 @@ public void Test001()
 			catch (Exception ex)
 			{
 				exception = ex;
-			};
+			}
 
 			var actual = exception.ShouldBeOfType<CollectionException>();
 			actual.ActualCount.ShouldBe(collection.Length);
@@ -31,6 +32,7 @@ public void Test001()
 
 		[Fact(DisplayName = "ShouldAllBe for Action<T> throws CollectionException if one of " +
 							"the element inspectors throws")]
+		[SuppressMessage("Minor Code Smell", "S3626:Jump statements should not be redundant", Justification = "Necessary for testing purposes.")]
 		public void Test002()
 		{
 			Exception? exception = null;
@@ -43,7 +45,7 @@ public void Test002()
 			catch (Exception ex)
 			{
 				exception = ex;
-			};
+			}
 
 			var actual = exception.ShouldBeOfType<CollectionException>();
 			actual.IndexFailurePoint.ShouldBe(1);
@@ -64,7 +66,7 @@ public void Test003()
 			catch (Exception ex)
 			{
 				exception = ex;
-			};
+			}
 
 			var actual = exception.ShouldBeOfType<CollectionException>();
 			actual.ActualCount.ShouldBe(collection.Length);
@@ -73,6 +75,7 @@ public void Test003()
 
 		[Fact(DisplayName = "ShouldAllBe for Action<T, int> throws CollectionException if one of " +
 							"the element inspectors throws")]
+		[SuppressMessage("Minor Code Smell", "S3626:Jump statements should not be redundant", Justification = "Necessary for testing purposes.")]
 		public void Test004()
 		{
 			Exception? exception = null;
@@ -85,7 +88,7 @@ public void Test004()
 			catch (Exception ex)
 			{
 				exception = ex;
-			};
+			}
 
 			var actual = exception.ShouldBeOfType<CollectionException>();
 			actual.IndexFailurePoint.ShouldBe(1);
diff --git a/tests/bunit.xunit.tests/GlobalSuppressions.cs b/tests/bunit.xunit.tests/GlobalSuppressions.cs
diff --git a/tests/bunit.xunit.tests/RazorTesting/RazorTestDiscovererTest.cs b/tests/bunit.xunit.tests/RazorTesting/RazorTestDiscovererTest.cs

Original file line number	Diff line number	Diff line change
`@@ -1,13 +1,11 @@`
`1`	`1`	`using System;`
`2`		`-using System.Diagnostics.CodeAnalysis;`
`3`	`2`
`4`	`3`	`namespace Bunit`
`5`	`4`	`{`
`6`	`5`	`/// <summary>`
`7`	`6`	`/// Represents a single parameter supplied to an <see cref="Microsoft.AspNetCore.Components.IComponent"/>`
`8`	`7`	`/// component under test.`
`9`	`8`	`/// </summary>`
`10`		`- [SuppressMessage("Usage", "CA2225:Operator overloads have named alternates", Justification = "<Pending>")]`
`11`	`9`	`public readonly struct ComponentParameter : IEquatable<ComponentParameter>`
`12`	`10`	`{`
`13`	`11`	`/// <summary>`
Original file line number	Diff line number	Diff line change
`@@ -41,7 +41,9 @@ public BunitJSInterop()`
`41`	`41`	`{`
`42`	`42`	`Mode = JSRuntimeMode.Strict;`
`43`	`43`	`JSRuntime = new BunitJSRuntime(this);`
	`44`	`+#if NET5_0`
`44`	`45`	`AddCustomHandlers();`
	`46`	`+#endif`
`45`	`47`	`}`
`46`	`48`
`47`	`49`	`/// <summary>`
`@@ -83,13 +85,13 @@ internal virtual void RegisterInvocation(JSRuntimeInvocation invocation)`
`83`	`85`	`return result;`
`84`	`86`	`}`
`85`	`87`
	`88`	`+#if NET5_0`
`86`	`89`	`private void AddCustomHandlers()`
`87`	`90`	`{`
`88`		`-#if NET5_0`
`89`	`91`	`AddInvocationHandler(new FocusAsyncInvocationHandler());`
`90`	`92`	`AddInvocationHandler(new VirtualizeJSRuntimeInvocationHandler());`
`91`	`93`	`AddInvocationHandler(new LooseModeJSObjectReferenceInvocationHandler(this));`
`92`		`-#endif`
`93`	`94`	`}`
	`95`	`+#endif`
`94`	`96`	`}`
`95`	`97`	`}`