Skip to content

Commit 4b52f79

Browse files
tilligtillig
authored
Merge pull request #22 from autofac/feature/tighten-analyzers
Add SonarAnalyzer.CSharp and tighten Roslyn analyzer rules
2 parents 139f554 + 2be0238 commit 4b52f79

6 files changed

Lines changed: 171 additions & 70 deletions

File tree

‎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.Extras.AggregateService/AggregateServiceGenerator.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public static object CreateInstance(Type interfaceType, IComponentContext contex
4747

4848
if (!interfaceType.GetTypeInfo().IsInterface)
4949
{
50-
throw new ArgumentException(AggregateServicesResources.TypeMustBeInterface, nameof(interfaceType));
50+
throw new ArgumentException(AggregateServicesResources.TypeMustBeInterface, paramName: nameof(interfaceType));
5151
}
5252

5353
var resolverInterceptor = new ResolvingInterceptor(interfaceType, context);

0 commit comments

Comments
 (0)