Skip to content

Commit ae10daa

Browse files
author
RandomCrocodile
committed
feature: add Confuser.Analyzers project with CX004 analyzer (#49)
First increment of the Tier 3 custom analyzers (#49, parent #46). Establishes the Confuser.Analyzers project (netstandard2.0 Roslyn analyzer) and wires it into every project via ConfuserEx.Common.targets as an analyzer reference. CX004 — flags Assembly.GetTypes()/Module.GetTypes() that are not guarded against ReflectionTypeLoadException (the cause of packer/plugin startup crashes). The analyzer resolves the invocation symbol and only fires for System.Reflection types, so dnlib's ModuleDef.GetTypes() (which returns TypeDef and never throws) is ignored. - Fixed the two real CX004 violations: PluginDiscovery.AddPlugins and ComponentDiscovery.LoadComponents now catch ReflectionTypeLoadException and keep the types that loaded. - Analyzer wired with SkipGetTargetFrameworkProperties + UndefineProperties so the netstandard2.0 analyzer attaches to all targets including net20. - 4 analyzer unit tests (fires on unguarded; silent when guarded by RTLE/Exception or on unrelated GetTypes()). Full solution builds clean with zero CX004 warnings. Remaining under #49 (follow-up): CX002 (unguarded Resolve*Def), CX003 (ResolveThrow audit). CX001's premise is stale — the listed call sites were refactored to a context-aware Import helper; the only host-reflection path left is that helper's fallback (Confuser.Protections/Utils.cs).
1 parent 2d4c539 commit ae10daa

9 files changed

Lines changed: 302 additions & 5 deletions
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
<Project Sdk="Microsoft.NET.Sdk">
2+
3+
<!-- Roslyn analyzer assembly. Deliberately standalone (does NOT import
4+
ConfuserEx.Common.props) — analyzers must be a single netstandard2.0 assembly
5+
with no runtime output and no strong-name/multi-target concerns. -->
6+
<PropertyGroup>
7+
<TargetFramework>netstandard2.0</TargetFramework>
8+
<IncludeBuildOutput>false</IncludeBuildOutput>
9+
<Nullable>enable</Nullable>
10+
<LangVersion>latest</LangVersion>
11+
<EnforceExtendedAnalyzerRules>true</EnforceExtendedAnalyzerRules>
12+
<IsPackable>false</IsPackable>
13+
<!-- These analyzers ship inside this repo only; they are never consumed as a NuGet package. -->
14+
<GeneratePackageOnBuild>false</GeneratePackageOnBuild>
15+
<!-- RS2008 (analyzer release tracking) only matters for analyzers shipped as packages. -->
16+
<NoWarn>$(NoWarn);RS2008</NoWarn>
17+
</PropertyGroup>
18+
19+
<ItemGroup>
20+
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="4.8.0" PrivateAssets="all" />
21+
<PackageReference Include="Microsoft.CodeAnalysis.Analyzers" Version="3.3.4" PrivateAssets="all">
22+
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
23+
</PackageReference>
24+
</ItemGroup>
25+
26+
</Project>
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
namespace Confuser.Analyzers {
2+
/// <summary>
3+
/// Diagnostic IDs for the ConfuserEx-specific analyzers. Each catches a bug class that has
4+
/// actually been fixed in the codebase, to prevent regressions at compile time.
5+
/// </summary>
6+
internal static class DiagnosticIds {
7+
/// <summary>Imports a member from the host runtime instead of the target's corlib.</summary>
8+
public const string HostRuntimeImport = "CX001";
9+
10+
/// <summary><c>ResolveTypeDef()</c>/<c>ResolveMethodDef()</c> used without a null check.</summary>
11+
public const string UnguardedResolve = "CX002";
12+
13+
/// <summary>Usage of a <c>...Throw</c> resolve helper (audit / awareness).</summary>
14+
public const string ResolveThrowAudit = "CX003";
15+
16+
/// <summary><c>GetTypes()</c> without handling <c>ReflectionTypeLoadException</c>.</summary>
17+
public const string UnhandledReflectionTypeLoad = "CX004";
18+
19+
public const string Category = "ConfuserEx";
20+
}
21+
}
Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
using System.Collections.Immutable;
2+
using Microsoft.CodeAnalysis;
3+
using Microsoft.CodeAnalysis.CSharp;
4+
using Microsoft.CodeAnalysis.CSharp.Syntax;
5+
using Microsoft.CodeAnalysis.Diagnostics;
6+
7+
namespace Confuser.Analyzers {
8+
/// <summary>
9+
/// CX004 — flags <c>Assembly.GetTypes()</c> / <c>Module.GetTypes()</c> that are not guarded
10+
/// against <see cref="System.Reflection.ReflectionTypeLoadException" />. That exception is
11+
/// thrown whenever a contained type cannot be loaded (common for plugin assemblies with
12+
/// unresolved dependencies) and caused packer/plugin startup crashes.
13+
/// </summary>
14+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
15+
public sealed class UnhandledReflectionTypeLoadAnalyzer : DiagnosticAnalyzer {
16+
static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor(
17+
DiagnosticIds.UnhandledReflectionTypeLoad,
18+
"GetTypes() without ReflectionTypeLoadException handling",
19+
"'{0}.GetTypes()' can throw ReflectionTypeLoadException when a contained type is unresolvable; wrap it in a try/catch",
20+
DiagnosticIds.Category,
21+
DiagnosticSeverity.Warning,
22+
isEnabledByDefault: true,
23+
description: "Assembly.GetTypes() and Module.GetTypes() throw ReflectionTypeLoadException when any " +
24+
"contained type cannot be loaded. Guard the call (and prefer ex.Types on the exception) to avoid " +
25+
"startup crashes when loading plugin or external assemblies.");
26+
27+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
28+
29+
public override void Initialize(AnalysisContext context) {
30+
context.EnableConcurrentExecution();
31+
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
32+
context.RegisterSyntaxNodeAction(Analyze, SyntaxKind.InvocationExpression);
33+
}
34+
35+
static void Analyze(SyntaxNodeAnalysisContext context) {
36+
var invocation = (InvocationExpressionSyntax)context.Node;
37+
if (invocation.Expression is not MemberAccessExpressionSyntax member ||
38+
member.Name.Identifier.ValueText != "GetTypes")
39+
return;
40+
41+
if (context.SemanticModel.GetSymbolInfo(invocation, context.CancellationToken).Symbol is not IMethodSymbol method ||
42+
method.Name != "GetTypes" || !method.Parameters.IsEmpty)
43+
return;
44+
45+
var containingType = method.ContainingType?.ToDisplayString();
46+
if (containingType != "System.Reflection.Assembly" && containingType != "System.Reflection.Module")
47+
return;
48+
49+
var rtle = context.Compilation.GetTypeByMetadataName("System.Reflection.ReflectionTypeLoadException");
50+
if (IsGuarded(invocation, context.SemanticModel, rtle, context.CancellationToken))
51+
return;
52+
53+
context.ReportDiagnostic(Diagnostic.Create(Rule, member.Name.GetLocation(), containingType));
54+
}
55+
56+
static bool IsGuarded(SyntaxNode node, SemanticModel model, INamedTypeSymbol? rtle, System.Threading.CancellationToken ct) {
57+
for (var current = node.Parent; current != null; current = current.Parent) {
58+
if (current is TryStatementSyntax tryStmt &&
59+
tryStmt.Block.Span.Contains(node.Span) &&
60+
CatchesReflectionTypeLoad(tryStmt, model, rtle, ct))
61+
return true;
62+
63+
// Do not walk past the enclosing method / lambda / local-function boundary.
64+
if (current is BaseMethodDeclarationSyntax ||
65+
current is AnonymousFunctionExpressionSyntax ||
66+
current is LocalFunctionStatementSyntax)
67+
break;
68+
}
69+
70+
return false;
71+
}
72+
73+
static bool CatchesReflectionTypeLoad(TryStatementSyntax tryStmt, SemanticModel model, INamedTypeSymbol? rtle,
74+
System.Threading.CancellationToken ct) {
75+
foreach (var clause in tryStmt.Catches) {
76+
// A general 'catch { }' (no declared type) catches everything.
77+
if (clause.Declaration is null)
78+
return true;
79+
80+
var caught = model.GetTypeInfo(clause.Declaration.Type, ct).Type as INamedTypeSymbol;
81+
if (caught is null)
82+
continue;
83+
84+
// The catch guards the call if ReflectionTypeLoadException is assignable to the caught
85+
// type (i.e. the caught type is RTLE or one of its base types such as SystemException /
86+
// Exception). If we cannot resolve RTLE, fall back to name matching on the base chain.
87+
if (rtle is not null) {
88+
for (var t = rtle; t is not null; t = t.BaseType) {
89+
if (SymbolEqualityComparer.Default.Equals(t, caught))
90+
return true;
91+
}
92+
}
93+
else {
94+
var name = caught.ToDisplayString();
95+
if (name == "System.Exception" || name == "System.SystemException" ||
96+
name == "System.Reflection.ReflectionTypeLoadException")
97+
return true;
98+
}
99+
}
100+
101+
return false;
102+
}
103+
}
104+
}

Confuser.Core/PluginDiscovery.cs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,18 @@ public static bool HasAccessibleDefConstructor(Type type) {
5555
protected static void AddPlugins(
5656
ConfuserContext context, IList<Protection> protections, IList<Packer> packers,
5757
IList<ConfuserComponent> components, Assembly asm) {
58-
foreach (var module in asm.GetLoadedModules())
59-
foreach (var i in module.GetTypes()) {
58+
foreach (var module in asm.GetLoadedModules()) {
59+
Type[] moduleTypes;
60+
try {
61+
moduleTypes = module.GetTypes();
62+
}
63+
catch (ReflectionTypeLoadException ex) {
64+
// A plugin assembly may reference dependencies that are not present; keep the
65+
// types that did load instead of crashing plugin discovery.
66+
moduleTypes = Array.FindAll(ex.Types, t => t != null);
67+
}
68+
69+
foreach (var i in moduleTypes) {
6070
if (i.IsAbstract || !HasAccessibleDefConstructor(i))
6171
continue;
6272

@@ -85,6 +95,7 @@ protected static void AddPlugins(
8595
}
8696
}
8797
}
98+
}
8899
context.CheckCancellation();
89100
}
90101

Confuser2.sln

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,10 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "CrossFramework.WPF.Net8", "
205205
EndProject
206206
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "CrossFramework.Library.Net10", "Tests\CrossFramework.Library.Net10\CrossFramework.Library.Net10.csproj", "{4458415A-0F5E-4136-B723-7A67955D6047}"
207207
EndProject
208+
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Confuser.Analyzers", "Confuser.Analyzers\Confuser.Analyzers.csproj", "{886E5FF3-DF94-4C6B-A5C3-E97038B5C275}"
209+
EndProject
210+
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Confuser.Analyzers.Test", "Tests\Confuser.Analyzers.Test\Confuser.Analyzers.Test.csproj", "{1B22CAAE-FC4A-478D-BD68-D29A3081F938}"
211+
EndProject
208212
Global
209213
GlobalSection(SolutionConfigurationPlatforms) = preSolution
210214
Debug|Any CPU = Debug|Any CPU
@@ -1343,6 +1347,30 @@ Global
13431347
{4458415A-0F5E-4136-B723-7A67955D6047}.Release|x64.Build.0 = Release|Any CPU
13441348
{4458415A-0F5E-4136-B723-7A67955D6047}.Release|x86.ActiveCfg = Release|Any CPU
13451349
{4458415A-0F5E-4136-B723-7A67955D6047}.Release|x86.Build.0 = Release|Any CPU
1350+
{886E5FF3-DF94-4C6B-A5C3-E97038B5C275}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
1351+
{886E5FF3-DF94-4C6B-A5C3-E97038B5C275}.Debug|Any CPU.Build.0 = Debug|Any CPU
1352+
{886E5FF3-DF94-4C6B-A5C3-E97038B5C275}.Debug|x64.ActiveCfg = Debug|Any CPU
1353+
{886E5FF3-DF94-4C6B-A5C3-E97038B5C275}.Debug|x64.Build.0 = Debug|Any CPU
1354+
{886E5FF3-DF94-4C6B-A5C3-E97038B5C275}.Debug|x86.ActiveCfg = Debug|Any CPU
1355+
{886E5FF3-DF94-4C6B-A5C3-E97038B5C275}.Debug|x86.Build.0 = Debug|Any CPU
1356+
{886E5FF3-DF94-4C6B-A5C3-E97038B5C275}.Release|Any CPU.ActiveCfg = Release|Any CPU
1357+
{886E5FF3-DF94-4C6B-A5C3-E97038B5C275}.Release|Any CPU.Build.0 = Release|Any CPU
1358+
{886E5FF3-DF94-4C6B-A5C3-E97038B5C275}.Release|x64.ActiveCfg = Release|Any CPU
1359+
{886E5FF3-DF94-4C6B-A5C3-E97038B5C275}.Release|x64.Build.0 = Release|Any CPU
1360+
{886E5FF3-DF94-4C6B-A5C3-E97038B5C275}.Release|x86.ActiveCfg = Release|Any CPU
1361+
{886E5FF3-DF94-4C6B-A5C3-E97038B5C275}.Release|x86.Build.0 = Release|Any CPU
1362+
{1B22CAAE-FC4A-478D-BD68-D29A3081F938}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
1363+
{1B22CAAE-FC4A-478D-BD68-D29A3081F938}.Debug|Any CPU.Build.0 = Debug|Any CPU
1364+
{1B22CAAE-FC4A-478D-BD68-D29A3081F938}.Debug|x64.ActiveCfg = Debug|Any CPU
1365+
{1B22CAAE-FC4A-478D-BD68-D29A3081F938}.Debug|x64.Build.0 = Debug|Any CPU
1366+
{1B22CAAE-FC4A-478D-BD68-D29A3081F938}.Debug|x86.ActiveCfg = Debug|Any CPU
1367+
{1B22CAAE-FC4A-478D-BD68-D29A3081F938}.Debug|x86.Build.0 = Debug|Any CPU
1368+
{1B22CAAE-FC4A-478D-BD68-D29A3081F938}.Release|Any CPU.ActiveCfg = Release|Any CPU
1369+
{1B22CAAE-FC4A-478D-BD68-D29A3081F938}.Release|Any CPU.Build.0 = Release|Any CPU
1370+
{1B22CAAE-FC4A-478D-BD68-D29A3081F938}.Release|x64.ActiveCfg = Release|Any CPU
1371+
{1B22CAAE-FC4A-478D-BD68-D29A3081F938}.Release|x64.Build.0 = Release|Any CPU
1372+
{1B22CAAE-FC4A-478D-BD68-D29A3081F938}.Release|x86.ActiveCfg = Release|Any CPU
1373+
{1B22CAAE-FC4A-478D-BD68-D29A3081F938}.Release|x86.Build.0 = Release|Any CPU
13461374
EndGlobalSection
13471375
GlobalSection(SolutionProperties) = preSolution
13481376
HideSolutionNode = FALSE
@@ -1434,6 +1462,7 @@ Global
14341462
{3942E3FD-06BC-470C-A1ED-BB18F2332B94} = {356BDB31-853E-43BB-8F9A-D8AC08F69EBB}
14351463
{30DC9F52-E08A-4EF0-B041-04AED6135C3D} = {356BDB31-853E-43BB-8F9A-D8AC08F69EBB}
14361464
{4458415A-0F5E-4136-B723-7A67955D6047} = {356BDB31-853E-43BB-8F9A-D8AC08F69EBB}
1465+
{1B22CAAE-FC4A-478D-BD68-D29A3081F938} = {356BDB31-853E-43BB-8F9A-D8AC08F69EBB}
14371466
EndGlobalSection
14381467
GlobalSection(ExtensibilityGlobals) = postSolution
14391468
SolutionGuid = {0D937D9E-E04B-4A68-B639-D4260473A388}

ConfuserEx.Common.targets

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,15 @@
77
<PackageReference Include="Roslynator.Analyzers" Version="4.*" PrivateAssets="all"
88
Condition="'$(EnableNETAnalyzers)' != 'false'" />
99
</ItemGroup>
10-
10+
11+
<!-- Tier 3: ConfuserEx-specific analyzers (issue #49). Referenced as an analyzer, not a
12+
runtime dependency. Excludes the analyzer project itself to avoid a self-reference. -->
13+
<ItemGroup Condition="'$(EnableNETAnalyzers)' != 'false' and '$(MSBuildProjectName)' != 'Confuser.Analyzers'">
14+
<!-- SkipGetTargetFrameworkProperties bypasses TFM-compatibility negotiation so the
15+
netstandard2.0 analyzer can be attached to any target (including net20). -->
16+
<ProjectReference Include="$(MSBuildThisFileDirectory)Confuser.Analyzers\Confuser.Analyzers.csproj"
17+
OutputItemType="Analyzer" ReferenceOutputAssembly="false" PrivateAssets="all"
18+
SkipGetTargetFrameworkProperties="true" UndefineProperties="TargetFramework" />
19+
</ItemGroup>
20+
1121
</Project>

ConfuserEx/ComponentDiscovery.cs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,18 @@ public static void LoadComponents(IList<ConfuserComponent> protections, IList<Co
1111
var alc = new PluginLoadContext(pluginPath);
1212
try {
1313
Assembly assembly = alc.LoadFromAssemblyPath(pluginPath);
14-
foreach (var module in assembly.GetLoadedModules())
15-
foreach (var i in module.GetTypes()) {
14+
foreach (var module in assembly.GetLoadedModules()) {
15+
Type[] moduleTypes;
16+
try {
17+
moduleTypes = module.GetTypes();
18+
}
19+
catch (ReflectionTypeLoadException ex) {
20+
// A plugin may reference dependencies that are not present; keep the types
21+
// that did load instead of crashing component discovery.
22+
moduleTypes = Array.FindAll(ex.Types, t => t != null);
23+
}
24+
25+
foreach (var i in moduleTypes) {
1626
if (i.IsAbstract || !PluginDiscovery.HasAccessibleDefConstructor(i))
1727
continue;
1828

@@ -25,6 +35,7 @@ public static void LoadComponents(IList<ConfuserComponent> protections, IList<Co
2535
AddPacker(packers, Info.FromComponent(packer, pluginPath));
2636
}
2737
}
38+
}
2839
}
2940
finally {
3041
alc.Unload();
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<Project Sdk="Microsoft.NET.Sdk">
2+
3+
<PropertyGroup>
4+
<TargetFramework>net10.0</TargetFramework>
5+
<IsPackable>false</IsPackable>
6+
<Nullable>enable</Nullable>
7+
</PropertyGroup>
8+
9+
<ItemGroup>
10+
<!-- Match the Roslyn version the analyzer is built against to avoid CS1705. -->
11+
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="4.8.0" />
12+
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Analyzer.Testing" Version="1.1.2" />
13+
</ItemGroup>
14+
15+
<ItemGroup>
16+
<ProjectReference Include="..\..\Confuser.Analyzers\Confuser.Analyzers.csproj" />
17+
</ItemGroup>
18+
19+
</Project>
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
using System.Threading.Tasks;
2+
using Microsoft.CodeAnalysis.Testing;
3+
using Xunit;
4+
using Verify = Microsoft.CodeAnalysis.CSharp.Testing.CSharpAnalyzerVerifier<
5+
Confuser.Analyzers.UnhandledReflectionTypeLoadAnalyzer,
6+
Microsoft.CodeAnalysis.Testing.DefaultVerifier>;
7+
8+
namespace Confuser.Analyzers.Test {
9+
public class UnhandledReflectionTypeLoadAnalyzerTest {
10+
[Fact]
11+
public async Task Flags_UnguardedAssemblyGetTypes() {
12+
const string source = @"
13+
using System.Reflection;
14+
class C {
15+
void M(Assembly asm) {
16+
var t = asm.{|#0:GetTypes|}();
17+
}
18+
}";
19+
var expected = Verify.Diagnostic("CX004")
20+
.WithLocation(0)
21+
.WithArguments("System.Reflection.Assembly");
22+
await Verify.VerifyAnalyzerAsync(source, expected);
23+
}
24+
25+
[Fact]
26+
public async Task DoesNotFlag_WhenGuardedByReflectionTypeLoadException() {
27+
const string source = @"
28+
using System;
29+
using System.Reflection;
30+
class C {
31+
void M(Assembly asm) {
32+
try { var t = asm.GetTypes(); }
33+
catch (ReflectionTypeLoadException) { }
34+
}
35+
}";
36+
await Verify.VerifyAnalyzerAsync(source);
37+
}
38+
39+
[Fact]
40+
public async Task DoesNotFlag_WhenGuardedByBaseException() {
41+
const string source = @"
42+
using System;
43+
using System.Reflection;
44+
class C {
45+
void M(Assembly asm) {
46+
try { var t = asm.GetTypes(); }
47+
catch (Exception) { }
48+
}
49+
}";
50+
await Verify.VerifyAnalyzerAsync(source);
51+
}
52+
53+
[Fact]
54+
public async Task DoesNotFlag_UnrelatedGetTypesMethod() {
55+
// A GetTypes() on a non-reflection type must not be flagged.
56+
const string source = @"
57+
class Other { public int[] GetTypes() => new int[0]; }
58+
class C {
59+
void M(Other o) {
60+
var t = o.GetTypes();
61+
}
62+
}";
63+
await Verify.VerifyAnalyzerAsync(source);
64+
}
65+
}
66+
}

0 commit comments

Comments
 (0)