Skip to content

Commit 3e70feb

Browse files
tilligtillig
authored
Merge pull request #12 from autofac/feature/tighten-analyzers
Add SonarAnalyzer.CSharp and tighten Roslyn analyzer rules
2 parents 7ceb06c + 7039f9d commit 3e70feb

23 files changed

Lines changed: 214 additions & 100 deletions

‎.vscode/settings.json‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
11
{
2+
"cSpell.words": [
3+
"autofac",
4+
"xunit"
5+
],
26
"omnisharp.enableEditorConfigSupport": true
37
}

‎.vscode/tasks.json‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,9 @@
11
{
22
"tasks": [
3-
{
4-
"args": [
5-
"watch",
6-
"run",
7-
"${workspaceFolder}/test/Autofac.Pooling.Tests/Autofac.Pooling.Tests.csproj",
8-
"/property:GenerateFullPaths=true",
9-
"/consoleloggerparameters:NoSummary"
10-
],
11-
"command": "dotnet",
12-
"label": "watch",
13-
"problemMatcher": "$msCompile",
14-
"type": "process"
15-
},
163
{
174
"args": [
185
"build",
6+
"Autofac.Pooling.sln",
197
"/property:GenerateFullPaths=true",
208
"/consoleloggerparameters:NoSummary"
219
],

‎Directory.Build.targets‎

Lines changed: 0 additions & 3 deletions
This file was deleted.

‎build/Source.ruleset‎

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
1-
<?xml version="1.0" encoding="utf-8"?>
2-
<RuleSet Name="Autofac Analyzer Rules" Description="Analyzer rules for Autofac assemblies." ToolsVersion="16.0">
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<RuleSet Name="Autofac Analyzer Rules" Description="Analyzer rules for Autofac source assemblies." ToolsVersion="16.0">
33
<IncludeAll Action="Warning" />
4-
<Rules AnalyzerId="Microsoft.Usage" RuleNamespace="Microsoft.Usage">
5-
<!-- Implement standard exception constructors - not all of the exception constructors (e.g., parameterless) are desired in our system. -->
4+
<Rules AnalyzerId="Microsoft.CodeAnalysis.Analyzers" RuleNamespace="Microsoft.CodeAnalysis.Analyzers">
5+
<!-- Implement standard exception constructors - not all of the exception constructors are desired. -->
66
<Rule Id="CA1032" Action="None" />
7+
<!-- Implement IDisposable correctly (must be explicitly enabled). -->
8+
<Rule Id="CA1063" Action="Warning" />
79
<!-- Avoid excessive inheritance (must be explicitly enabled). -->
810
<Rule Id="CA1501" Action="Warning" />
911
<!-- Avoid excessive complexity (must be explicitly enabled). -->
@@ -12,23 +14,45 @@
1214
<Rule Id="CA1505" Action="Warning" />
1315
<!-- Avoid excessive class coupling (must be explicitly enabled). -->
1416
<Rule Id="CA1506" Action="Warning" />
15-
<!-- Use ArgumentNullException.ThrowIfNull - this isn't available until we stop targeting netstandard. -->
17+
<!-- Use ArgumentNullException.ThrowIfNull - not available in netstandard. -->
1618
<Rule Id="CA1510" Action="None" />
17-
<!-- Use ArgumentOutOfRangeException.ThrowIfNegative - this isn't available until we stop targeting anything below net8.0. -->
19+
<!-- Use ArgumentOutOfRangeException.ThrowIfNegative - not available below net8.0. -->
1820
<Rule Id="CA1512" Action="None" />
19-
<!-- Use ObjectDisposedException.ThrowIf - this isn't available until we stop targeting anything below net8.0. -->
21+
<!-- Use ObjectDisposedException.ThrowIf - not available below net8.0. -->
2022
<Rule Id="CA1513" Action="None" />
21-
<!-- Change names to avoid reserved word overlaps (e.g., Delegate, GetType, etc.) - too many of these in the public API, we'd break if we fixed it. -->
23+
<!-- Change names to avoid reserved word overlaps - would break public API. -->
2224
<Rule Id="CA1716" Action="None" />
23-
<!-- Cache a CompositeFormat object for use in String.Format - this isn't available until we stop targeting netstandard, and we only String.Format when throwing exceptions so the work/complexity isn't justified to increase perf just for those situations. -->
25+
<!-- Cache a CompositeFormat - not available in netstandard. -->
2426
<Rule Id="CA1863" Action="None" />
25-
<!-- Implement serialization constructors - false positive when building .NET Core. -->
27+
<!-- Implement serialization constructors - false positive in .NET Core. -->
2628
<Rule Id="CA2229" Action="None" />
27-
<!-- Mark ISerializable types with SerializableAttribute - false positive when building .NET Core. -->
29+
<!-- Mark ISerializable types with SerializableAttribute - false positive in .NET Core. -->
2830
<Rule Id="CA2237" Action="None" />
29-
<!-- Prefer generic overloads to using Type parameters - many aren't available in earlier frameworks; and we do a lot of reflection work in Autofac. -->
31+
<!-- Prefer generic overloads - conflicts with reflection work in Autofac. -->
3032
<Rule Id="CA2263" Action="None" />
3133
</Rules>
34+
<Rules AnalyzerId="SonarAnalyzer.CSharp" RuleNamespace="SonarAnalyzer.CSharp">
35+
<!-- "Sonar Way" rules to match SonarQube scans. -->
36+
<Rule Id="S107" Action="Warning" />
37+
<!-- Do not use obsolete members - Autofac maintains deprecated APIs for backward compat. -->
38+
<Rule Id="S1133" Action="None" />
39+
<Rule Id="S1192" Action="Warning" />
40+
<Rule Id="S1313" Action="Warning" />
41+
<Rule Id="S2259" Action="Warning" />
42+
<Rule Id="S2583" Action="Warning" />
43+
<Rule Id="S2589" Action="Warning" />
44+
<!-- Verify reflection accessibility bypass - DI containers do extensive reflection by design. -->
45+
<Rule Id="S3011" Action="None" />
46+
<!-- Loops should use LINQ - DI container hot paths intentionally avoid LINQ allocations. -->
47+
<Rule Id="S3267" Action="None" />
48+
<Rule Id="S3776" Action="Warning" />
49+
<!-- IDisposable pattern - Autofac uses non-standard dispose patterns for container lifecycle. -->
50+
<Rule Id="S3881" Action="None" />
51+
<!-- Split method for params check + iterator - overly prescriptive for our patterns. -->
52+
<Rule Id="S4456" Action="None" />
53+
<Rule Id="S6418" Action="Warning" />
54+
<Rule Id="S6444" Action="Warning" />
55+
</Rules>
3256
<Rules AnalyzerId="StyleCop.Analyzers" RuleNamespace="StyleCop.Analyzers">
3357
<!-- Prefix local calls with this. -->
3458
<Rule Id="SA1101" Action="None" />

‎build/Test.ruleset‎

Lines changed: 90 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,14 @@
1-
<?xml version="1.0" encoding="utf-8"?>
2-
<RuleSet Name="Autofac Analyzer Rules" Description="Analyzer rules for Autofac assemblies." ToolsVersion="16.0">
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<RuleSet Name="Autofac Analyzer Rules" Description="Analyzer rules for Autofac test assemblies." ToolsVersion="16.0">
33
<IncludeAll Action="Warning" />
4-
<Rules AnalyzerId="Microsoft.Usage" RuleNamespace="Microsoft.Usage">
5-
<!-- Avoid excessive parameters on generic types (must be explicitly enabled). -->
6-
<Rule Id="CA1005" Action="Warning" />
7-
<!-- Don't catch general exceptions - test scenarios sometimes require general exception handling. -->
4+
<Rules AnalyzerId="Microsoft.CodeAnalysis.Analyzers" RuleNamespace="Microsoft.CodeAnalysis.Analyzers">
5+
<!-- Don't catch general exceptions - test scenarios sometimes require it. -->
86
<Rule Id="CA1031" Action="None" />
9-
<!-- Implement standard exception constructors - not all of the exception constructors (e.g., parameterless) are desired in our system. -->
10-
<Rule Id="CA1032" Action="None" />
11-
<!-- Avoid empty interfaces - in unit tests for service resolution, this happens a lot. -->
7+
<!-- Avoid empty interfaces - happens a lot in unit tests for service resolution. -->
128
<Rule Id="CA1040" Action="None" />
13-
<!-- Do not pass literals as localized parameters - tests don't need to localize. -->
9+
<!-- Implement IDisposable correctly (must be explicitly enabled). -->
10+
<Rule Id="CA1063" Action="Warning" />
11+
<!-- Do not pass literals as localized parameters - tests don't need to localize. -->
1412
<Rule Id="CA1303" Action="None" />
1513
<!-- Avoid excessive inheritance (must be explicitly enabled). -->
1614
<Rule Id="CA1501" Action="Warning" />
@@ -20,49 +18,105 @@
2018
<Rule Id="CA1505" Action="Warning" />
2119
<!-- Avoid excessive class coupling (must be explicitly enabled). -->
2220
<Rule Id="CA1506" Action="Warning" />
23-
<!-- Use ArgumentNullException.ThrowIfNull - this isn't available until we stop targeting netstandard. -->
24-
<Rule Id="CA1510" Action="None" />
25-
<!-- Make API types internal - causes problems with tests and test assemblies. -->
21+
<!-- Make API types internal - causes problems with tests. -->
2622
<Rule Id="CA1515" Action="None" />
27-
<!-- Remove the underscores from member name - unit test scenarios may use underscores. -->
23+
<!-- Remove underscores from member name - test naming conventions use underscore. -->
2824
<Rule Id="CA1707" Action="None" />
29-
<!-- Change names to avoid reserved word overlaps (e.g., Delegate, GetType, etc.) - too many of these in the public API, we'd break if we fixed it. -->
30-
<Rule Id="CA1716" Action="None" />
31-
<!-- Internal class that appears to never be instantiated - lots of false positives here because they're test stubs created by Autofac registrations. -->
25+
<!-- Internal class that appears to never be instantiated - false positives from DI. -->
26+
<!-- Identifiers should not have incorrect suffix - test classes use Impl, Handler, etc. -->
27+
<Rule Id="CA1711" Action="None" />
28+
<!-- Property names should not match get methods - aggregate service tests intentionally test both. -->
29+
<Rule Id="CA1721" Action="None" />
3230
<Rule Id="CA1812" Action="None" />
33-
<!-- Change Dispose() to call GC.SuppressFinalize - in tests we don't really care and it can impact readability. -->
31+
<!-- Change Dispose() to call GC.SuppressFinalize - unimportant in tests. -->
3432
<Rule Id="CA1816" Action="None" />
35-
<!-- Mark members static - test methods may not access member data but also can't be static. -->
33+
<!-- Mark members static - test methods may not access member data. -->
3634
<Rule Id="CA1822" Action="None" />
37-
<!-- Seal internal types for performance - in tests we don't really care and it gets painful to enforce. -->
35+
<!-- Use the LoggerMessage delegates - noise in tests, performance irrelevant. -->
36+
<Rule Id="CA1848" Action="None" />
37+
<!-- Seal internal types - unimportant in tests. -->
3838
<Rule Id="CA1852" Action="None" />
39-
<!-- Prefer static readonly fields over constant array arguments - constant array arguments happen a lot in unit tests for assertions and test setup. -->
39+
<!-- Prefer static readonly fields over constant arrays - happens in test setup. -->
4040
<Rule Id="CA1861" Action="None" />
41-
<!-- Cache a CompositeFormat object for use in String.Format - this makes unit tests harder to read, and performance isn't an issue. -->
41+
<!-- Cache a CompositeFormat - makes tests harder to read. -->
4242
<Rule Id="CA1863" Action="None" />
43-
<!-- Call ConfigureAwait on tasks - you shouldn't do this in unit test libraries; XUnit has an opposite analyzer. -->
43+
<!-- Call ConfigureAwait - shouldn't in test libraries; XUnit has opposing analyzer. -->
4444
<Rule Id="CA2007" Action="None" />
45-
<!-- Implement serialization constructors - false positive when building .NET Core. -->
45+
<!-- Implement serialization constructors - false positive in .NET Core. -->
46+
<!-- Do not raise reserved exception types - tests use generic exceptions for simulation. -->
47+
<Rule Id="CA2201" Action="None" />
4648
<Rule Id="CA2229" Action="None" />
4749
<!-- Use Uri instead of string parameters - strings are easier for testing. -->
4850
<Rule Id="CA2234" Action="None" />
49-
<!-- Mark ISerializable types with SerializableAttribute - false positive when building .NET Core. -->
51+
<!-- Mark ISerializable types with SerializableAttribute - false positive in .NET Core. -->
5052
<Rule Id="CA2237" Action="None" />
51-
<!-- Prefer generic overloads to using Type parameters - we do a lot of reflection work in Autofac that needs to be tested. -->
53+
<!-- Nested types should not be visible - test scenario classes use nested types extensively. -->
54+
<Rule Id="CA1034" Action="None" />
55+
<!-- Prefer generic overloads - reflection work in Autofac needs to be tested. -->
5256
<Rule Id="CA2263" Action="None" />
5357
</Rules>
58+
<Rules AnalyzerId="SonarAnalyzer.CSharp" RuleNamespace="SonarAnalyzer.CSharp">
59+
<!-- Don't use hardcoded paths or URIs - needed in several test cases. -->
60+
<Rule Id="S1075" Action="None" />
61+
<!-- Utility class should not be instantiated - test fixture classes with nested tests can't be static. -->
62+
<!-- Sections of code should not be commented out - test files may retain commented examples. -->
63+
<Rule Id="S125" Action="None" />
64+
<Rule Id="S1118" Action="None" />
65+
<!-- Don't call GC.Collect - required for weak reference and disposal tests. -->
66+
<Rule Id="S1215" Action="None" />
67+
<!-- Do not use obsolete members - tests exercise deprecated APIs to verify backward compat. -->
68+
<Rule Id="S1133" Action="None" />
69+
<!-- Remove unused private members - reflection tests need non-public members. -->
70+
<Rule Id="S1144" Action="None" />
71+
<!-- Empty method bodies - test stubs often have no-op implementations. -->
72+
<Rule Id="S1186" Action="None" />
73+
<!-- Remove unused method parameters - reflection tests declare parameters not used in the body. -->
74+
<Rule Id="S1172" Action="None" />
75+
<!-- Remove empty class - test stubs for DI resolution don't need implementation. -->
76+
<Rule Id="S2094" Action="None" />
77+
<!-- Make member static - test fixture members may not access instance data. -->
78+
<Rule Id="S2325" Action="None" />
79+
<!-- Remove unused type parameters - used in reflection testing. -->
80+
<Rule Id="S2326" Action="None" />
81+
<!-- Write-only properties - reflection tests verify property setter behavior. -->
82+
<Rule Id="S2376" Action="None" />
83+
<!-- Classes with only private constructors - reflection tests verify inaccessible constructors. -->
84+
<Rule Id="S3453" Action="None" />
85+
<!-- Remove unassigned auto-property - reflection tests verify non-public property behavior. -->
86+
<Rule Id="S3459" Action="None" />
87+
<!-- Complete the TODO - acceptable in tests for backlog items. -->
88+
<Rule Id="S1135" Action="None" />
89+
<!-- Make instance property static - false positives in data/document types. -->
90+
<Rule Id="S2335" Action="None" />
91+
<!-- Cognitive complexity (must be explicitly enabled). -->
92+
<Rule Id="S3776" Action="Warning" />
93+
<!-- IDisposable pattern - test dispose implementations are intentionally simplified. -->
94+
<Rule Id="S3881" Action="None" />
95+
<!-- Split method for params check + iterator - overly prescriptive for our patterns. -->
96+
<Rule Id="S4456" Action="None" />
97+
<!-- Don't use Thread.Sleep - required for concurrency and timing tests. -->
98+
<Rule Id="S2925" Action="None" />
99+
<!-- NSubstitute mock verification is complete without property access. -->
100+
<Rule Id="S2970" Action="None" />
101+
<!-- Fields only assigned in constructor - needed to prevent optimization in reflection tests. -->
102+
<Rule Id="S4487" Action="None" />
103+
<!-- Set route attributes at top of controller - false positive. -->
104+
<Rule Id="S6934" Action="None" />
105+
<!-- Use async dispose - sync dispose tests are intentional. -->
106+
<Rule Id="S6966" Action="None" />
107+
</Rules>
54108
<Rules AnalyzerId="StyleCop.Analyzers" RuleNamespace="StyleCop.Analyzers">
55109
<!-- Prefix local calls with this. -->
56110
<Rule Id="SA1101" Action="None" />
57111
<!-- Use built-in type alias. -->
58112
<Rule Id="SA1121" Action="None" />
59113
<!-- Use String.Empty instead of "". -->
60114
<Rule Id="SA1122" Action="None" />
61-
<!-- Enforce order of class members by member type - sometimes putting test classes/data by the test helps. -->
115+
<!-- Enforce order of class members by member type. -->
62116
<Rule Id="SA1201" Action="None" />
63-
<!-- Enforce order of class members by member visibility - sometimes putting test classes/data by the test helps. -->
117+
<!-- Enforce order of class members by visibility. -->
64118
<Rule Id="SA1202" Action="None" />
65-
<!-- Enforce order of static vs. non-static members - sometimes putting test classes/data by the test helps. -->
119+
<!-- Enforce order of static vs. non-static members. -->
66120
<Rule Id="SA1204" Action="None" />
67121
<!-- Fields can't start with underscore. -->
68122
<Rule Id="SA1309" Action="None" />
@@ -82,6 +136,13 @@
82136
<Rule Id="SA1618" Action="None" />
83137
<!-- Don't copy/paste documentation. -->
84138
<Rule Id="SA1625" Action="None" />
139+
</Rules>
140+
<Rules AnalyzerId="xUnit" RuleNamespace="xUnit">
141+
<!-- Use Assert.Single when there's only one collection entry. -->
142+
<Rule Id="xUnit2023" Action="Warning" />
143+
</Rules>
144+
<!-- IDE rules for test assemblies. -->
145+
<Rules AnalyzerId="Microsoft.CodeAnalysis.CSharp" RuleNamespace="Microsoft.CodeAnalysis.CSharp">
85146
<!-- Private member is unused - tests for reflection require members that may not get used. -->
86147
<Rule Id="IDE0051" Action="None" />
87148
<!-- Private member assigned value never read - tests for reflection require values that may not get used. -->

‎src/Autofac.Pooling/Autofac.Pooling.csproj‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
<AssemblyOriginatorKeyFile>../../Autofac.snk</AssemblyOriginatorKeyFile>
99
<SignAssembly>true</SignAssembly>
1010
<CodeAnalysisRuleSet>../../build/Source.ruleset</CodeAnalysisRuleSet>
11+
<AnalysisMode>AllEnabledByDefault</AnalysisMode>
12+
<EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>
1113
<GenerateAssemblyTitleAttribute>false</GenerateAssemblyTitleAttribute>
1214
<GenerateNeutralResourcesLanguageAttribute>false</GenerateNeutralResourcesLanguageAttribute>
1315
<GenerateAssemblyCopyrightAttribute>false</GenerateAssemblyCopyrightAttribute>
@@ -32,27 +34,29 @@
3234
<EmbedAllSources>true</EmbedAllSources>
3335
<AllowedOutputExtensionsInPackageBuildOutputFolder>$(AllowedOutputExtensionsInPackageBuildOutputFolder);.pdb</AllowedOutputExtensionsInPackageBuildOutputFolder>
3436
</PropertyGroup>
35-
37+
<ItemGroup>
38+
<Using Include="System.Diagnostics.CodeAnalysis" />
39+
</ItemGroup>
3640
<ItemGroup>
3741
<None Include="..\..\build\icon.png" Pack="true" PackagePath="\" />
3842
</ItemGroup>
39-
4043
<ItemGroup>
4144
<AdditionalFiles Include="..\..\build\stylecop.json" Link="stylecop.json" />
4245
</ItemGroup>
43-
4446
<ItemGroup>
4547
<PackageReference Include="Autofac" Version="9.1.0" />
4648
<PackageReference Include="Microsoft.Extensions.ObjectPool" Version="10.0.8" />
47-
4849
<PackageReference Include="Microsoft.SourceLink.GitHub" Version="10.0.300" Condition="Exists('$(MSBuildThisFileDirectory)../../.git')">
4950
<PrivateAssets>All</PrivateAssets>
5051
</PackageReference>
5152
<PackageReference Include="StyleCop.Analyzers" Version="1.2.0-beta.556">
5253
<PrivateAssets>All</PrivateAssets>
5354
</PackageReference>
55+
<PackageReference Include="SonarAnalyzer.CSharp" Version="10.27.0.140913">
56+
<PrivateAssets>all</PrivateAssets>
57+
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
58+
</PackageReference>
5459
</ItemGroup>
55-
5660
<ItemGroup>
5761
<Compile Update="PoolGetActivatorResources.Designer.cs">
5862
<DesignTime>True</DesignTime>
@@ -70,7 +74,6 @@
7074
<DependentUpon>RegistrationExtensionsResources.resx</DependentUpon>
7175
</Compile>
7276
</ItemGroup>
73-
7477
<ItemGroup>
7578
<EmbeddedResource Update="PoolGetActivatorResources.resx">
7679
<Generator>ResXFileCodeGenerator</Generator>
@@ -85,5 +88,4 @@
8588
<LastGenOutput>RegistrationExtensionsResources.Designer.cs</LastGenOutput>
8689
</EmbeddedResource>
8790
</ItemGroup>
88-
8991
</Project>

‎src/Autofac.Pooling/AutofacPooledObjectPolicy.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ namespace Autofac.Pooling;
1010
/// Provides the <see cref="IPooledObjectPolicy{TLimit}"/> needed for creating/returning objects in the pool in an Autofac way.
1111
/// </summary>
1212
/// <typeparam name="TPooledObject">The type of object being pooled.</typeparam>
13-
internal class AutofacPooledObjectPolicy<TPooledObject> : IPooledObjectPolicy<TPooledObject>
13+
internal sealed class AutofacPooledObjectPolicy<TPooledObject> : IPooledObjectPolicy<TPooledObject>
1414
where TPooledObject : class
1515
{
1616
private readonly Service _poolInstanceService;

‎src/Autofac.Pooling/PoolService.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ namespace Autofac.Pooling;
1111
/// <summary>
1212
/// Defines a service used to access an <see cref="ObjectPool{T}"/>, ensuring that each pooled item registration gets a different pool.
1313
/// </summary>
14-
internal class PoolService : Service, IEquatable<PoolService>
14+
internal sealed class PoolService : Service, IEquatable<PoolService>
1515
{
1616
private readonly IComponentRegistration _pooledItemRegistration;
1717

‎src/Autofac.Pooling/PooledLifetime.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ namespace Autofac.Pooling;
88
/// <summary>
99
/// Lifetime wrapper to help us detect if we finished registering in a pooled configuration.
1010
/// </summary>
11+
[SuppressMessage("S2094", "S2094", Justification = "Lifetimes are classes, not interfaces. Changing this would be a breaking change.")]
1112
public class PooledLifetime : CurrentScopeLifetime
1213
{
1314
}

0 commit comments

Comments
 (0)