Skip to content

Commit d4d9f85

Browse files
marinasundstrommarinasundstrom
authored
Avoid resolving SemanticModel inside analyzer helpers (#289)
1 parent f0fec81 commit d4d9f85

8 files changed

+85
-189
lines changed

‎AGENTS.md‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,15 @@
1515
- Restore and compile the solution with
1616
`dotnet build CheckedExceptions.sln`.
1717

18+
## Local Package Source
19+
- `NuGet.config` includes `./artifacts/package/debug` for local packages, but it is currently unused.
20+
- If a restore or build fails because this folder is missing, either create it (for example, `mkdir -p artifacts/package/debug`) or ignore the source.
21+
1822
## Testing
1923
- Execute the unit tests with
2024
`dotnet test CheckedExceptions.sln`.
2125

2226
## Formatting
2327
- Format each changed file using
24-
`dotnet format <path to dir of solution or project file> --include <comma separated list with file paths>`
25-
to respect `.editorconfig` rules.
28+
`dotnet format <path to dir of solution or project file> --no-restore --include <comma separated list with file paths>`
29+
to respect `.editorconfig` rules without triggering a restore.

‎CheckedExceptions/CheckedExceptionsAnalyzer.Analysis.cs‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ private static void AnalyzeExceptionsInTryBlock(SyntaxNodeAnalysisContext contex
1717
var thrownExceptions = CollectUnhandledExceptions(context, tryStatement.Block, settings);
1818

1919
// Collect exception types handled by preceding catch clauses
20-
var handledExceptions = new HashSet<INamedTypeSymbol>(SymbolEqualityComparer.Default);
20+
HashSet<INamedTypeSymbol>? handledExceptions = new(SymbolEqualityComparer.Default);
2121
foreach (var catchClause in tryStatement.Catches)
2222
{
2323
if (catchClause == generalCatchClause)
@@ -369,9 +369,9 @@ private static void CollectExceptionsFromExpression(SyntaxNode expression, Compi
369369
}
370370
}
371371

372-
private static HashSet<INamedTypeSymbol> GetCaughtExceptions(SyntaxList<CatchClauseSyntax> catchClauses, SemanticModel semanticModel)
372+
private static HashSet<INamedTypeSymbol>? GetCaughtExceptions(SyntaxList<CatchClauseSyntax> catchClauses, SemanticModel semanticModel)
373373
{
374-
var caughtExceptions = new HashSet<INamedTypeSymbol>(SymbolEqualityComparer.Default);
374+
HashSet<INamedTypeSymbol>? caughtExceptions = new(SymbolEqualityComparer.Default);
375375

376376
foreach (var catchClause in catchClauses)
377377
{
@@ -518,12 +518,18 @@ private static void AnalyzeFunctionAttributes(SyntaxNode node, IEnumerable<Attri
518518
if (throwsAttributes.Count is 0)
519519
return;
520520

521-
CheckForGeneralExceptionThrows(context, throwsAttributes);
521+
var throwsContext = new ThrowsContext(
522+
context.SemanticModel,
523+
context.Options,
524+
context.ReportDiagnostic,
525+
context.CancellationToken);
526+
527+
CheckForGeneralExceptionThrows(throwsAttributes, throwsContext);
522528

523529
if (throwsAttributes.Any())
524530
{
525-
CheckForDuplicateThrowsDeclarations(throwsAttributes, context);
526-
CheckForRedundantThrowsHandledByDeclaredSuperClass(throwsAttributes, context);
531+
CheckForDuplicateThrowsDeclarations(throwsAttributes, throwsContext);
532+
CheckForRedundantThrowsHandledByDeclaredSuperClass(throwsAttributes, throwsContext);
527533
}
528534
}
529535

‎CheckedExceptions/CheckedExceptionsAnalyzer.DeclaredSuperClassDetection.cs‎

Lines changed: 4 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -1,80 +1,16 @@
1-
using System.Collections.Immutable;
1+
using System;
2+
using System.Threading;
23

34
using Microsoft.CodeAnalysis;
45
using Microsoft.CodeAnalysis.CSharp.Syntax;
5-
using Microsoft.CodeAnalysis.Diagnostics;
66

77
namespace Sundstrom.CheckedExceptions;
88

99
partial class CheckedExceptionsAnalyzer
1010
{
11-
private static void CheckForRedundantThrowsDeclarationsHandledByDeclaredSuperClass(
12-
SymbolAnalysisContext context,
13-
ImmutableArray<AttributeData> throwsAttributes)
14-
{
15-
var declaredTypes = new HashSet<INamedTypeSymbol>(SymbolEqualityComparer.Default);
16-
var typeToArgMap = new Dictionary<INamedTypeSymbol, TypeOfExpressionSyntax>(SymbolEqualityComparer.Default);
17-
18-
foreach (var attrData in throwsAttributes)
19-
{
20-
var syntaxRef = attrData.ApplicationSyntaxReference;
21-
if (syntaxRef?.GetSyntax(context.CancellationToken) is not AttributeSyntax attrSyntax)
22-
continue;
23-
24-
var semanticModel = context.Compilation.GetSemanticModel(attrSyntax.SyntaxTree);
25-
26-
foreach (var arg in attrSyntax.ArgumentList?.Arguments ?? default)
27-
{
28-
if (arg.Expression is TypeOfExpressionSyntax typeOfExpr)
29-
{
30-
var typeInfo = semanticModel.GetTypeInfo(typeOfExpr.Type, context.CancellationToken);
31-
var typeSymbol = typeInfo.Type as INamedTypeSymbol;
32-
if (typeSymbol is null)
33-
continue;
34-
35-
declaredTypes.Add(typeSymbol);
36-
typeToArgMap[typeSymbol] = typeOfExpr; // more precise location
37-
}
38-
}
39-
}
40-
41-
foreach (var type in declaredTypes)
42-
{
43-
foreach (var otherType in declaredTypes)
44-
{
45-
if (type.Equals(otherType, SymbolEqualityComparer.Default))
46-
continue;
47-
48-
if (IsSubclassOf(type, otherType))
49-
{
50-
if (typeToArgMap.TryGetValue(type, out var expression))
51-
{
52-
context.ReportDiagnostic(Diagnostic.Create(
53-
RuleDuplicateThrowsByHierarchy,
54-
expression.Type.GetLocation(), // ✅ precise location
55-
otherType.Name));
56-
}
57-
break;
58-
}
59-
}
60-
}
61-
62-
static bool IsSubclassOf(INamedTypeSymbol derived, INamedTypeSymbol baseType)
63-
{
64-
var current = derived.BaseType;
65-
while (current is not null)
66-
{
67-
if (current.Equals(baseType, SymbolEqualityComparer.Default))
68-
return true;
69-
current = current.BaseType;
70-
}
71-
return false;
72-
}
73-
}
74-
7511
private static void CheckForRedundantThrowsHandledByDeclaredSuperClass(
76-
IEnumerable<AttributeSyntax> throwsAttributes,
77-
SyntaxNodeAnalysisContext context)
12+
IEnumerable<AttributeSyntax> throwsAttributes,
13+
ThrowsContext context)
7814
{
7915
var semanticModel = context.SemanticModel;
8016
var declaredTypes = new HashSet<INamedTypeSymbol>(SymbolEqualityComparer.Default);

‎CheckedExceptions/CheckedExceptionsAnalyzer.DuplicateDetection.cs‎

Lines changed: 3 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,54 +1,13 @@
1-
using System.Collections.Immutable;
1+
using System;
2+
using System.Threading;
23

34
using Microsoft.CodeAnalysis;
45
using Microsoft.CodeAnalysis.CSharp.Syntax;
5-
using Microsoft.CodeAnalysis.Diagnostics;
66

77
namespace Sundstrom.CheckedExceptions;
88

99
partial class CheckedExceptionsAnalyzer
1010
{
11-
#region Method
12-
13-
private static void CheckForDuplicateThrowsDeclarations(
14-
SymbolAnalysisContext context,
15-
ImmutableArray<AttributeData> throwsAttributes)
16-
{
17-
var reportedTypes = new HashSet<INamedTypeSymbol>(SymbolEqualityComparer.Default);
18-
19-
foreach (var attrData in throwsAttributes)
20-
{
21-
var syntaxRef = attrData.ApplicationSyntaxReference;
22-
if (syntaxRef?.GetSyntax(context.CancellationToken) is not AttributeSyntax attrSyntax)
23-
continue;
24-
25-
var semanticModel = context.Compilation.GetSemanticModel(attrSyntax.SyntaxTree);
26-
27-
foreach (var arg in attrSyntax.ArgumentList?.Arguments ?? default)
28-
{
29-
if (arg.Expression is TypeOfExpressionSyntax typeOfExpr)
30-
{
31-
var typeInfo = semanticModel.GetTypeInfo(typeOfExpr.Type, context.CancellationToken);
32-
var exceptionType = typeInfo.Type as INamedTypeSymbol;
33-
if (exceptionType is null)
34-
continue;
35-
36-
if (reportedTypes.Contains(exceptionType))
37-
{
38-
context.ReportDiagnostic(Diagnostic.Create(
39-
RuleDuplicateDeclarations,
40-
typeOfExpr.Type.GetLocation(), // ✅ precise location
41-
exceptionType.Name));
42-
}
43-
44-
reportedTypes.Add(exceptionType);
45-
}
46-
}
47-
}
48-
}
49-
50-
#endregion
51-
5211
#region Lambda expression & Local function
5312

5413
/// <summary>
@@ -58,7 +17,7 @@ private static void CheckForDuplicateThrowsDeclarations(
5817
/// <param name="context">The analysis context.</param>
5918
private static void CheckForDuplicateThrowsDeclarations(
6019
IEnumerable<AttributeSyntax> throwsAttributes,
61-
SyntaxNodeAnalysisContext context)
20+
ThrowsContext context)
6221
{
6322
var semanticModel = context.SemanticModel;
6423
var seen = new HashSet<INamedTypeSymbol>(SymbolEqualityComparer.Default);

‎CheckedExceptions/CheckedExceptionsAnalyzer.GeneralThrows.cs‎

Lines changed: 12 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
using System.Collections.Immutable;
1+
using System;
2+
using System.Threading;
23

34
using Microsoft.CodeAnalysis;
45
using Microsoft.CodeAnalysis.CSharp.Syntax;
@@ -8,60 +9,17 @@ namespace Sundstrom.CheckedExceptions;
89

910
partial class CheckedExceptionsAnalyzer
1011
{
11-
#region Methods
12-
13-
private static void CheckForGeneralExceptionThrowDeclarations(
14-
ImmutableArray<AttributeData> throwsAttributes,
15-
SymbolAnalysisContext context)
16-
{
17-
const string exceptionName = "Exception";
18-
19-
foreach (var attribute in throwsAttributes)
20-
{
21-
var syntaxRef = attribute.ApplicationSyntaxReference;
22-
if (syntaxRef?.GetSyntax(context.CancellationToken) is not AttributeSyntax attrSyntax)
23-
continue;
24-
25-
var semanticModel = context.Compilation.GetSemanticModel(attrSyntax.SyntaxTree);
26-
27-
foreach (var arg in attrSyntax.ArgumentList?.Arguments ?? [])
28-
{
29-
if (arg.Expression is TypeOfExpressionSyntax typeOfExpr)
30-
{
31-
var typeInfo = semanticModel.GetTypeInfo(typeOfExpr.Type, context.CancellationToken);
32-
var type = typeInfo.Type as INamedTypeSymbol;
33-
if (type is null)
34-
continue;
35-
36-
var settings = GetAnalyzerSettings(context.Options);
37-
38-
if (settings.BaseExceptionDeclaredDiagnosticEnabled)
39-
{
40-
if (type.Name == exceptionName && type.ContainingNamespace?.ToDisplayString() == "System")
41-
{
42-
context.ReportDiagnostic(Diagnostic.Create(
43-
RuleGeneralThrowDeclared,
44-
typeOfExpr.Type.GetLocation(), // ✅ precise location
45-
type.Name));
46-
}
47-
}
48-
}
49-
}
50-
}
51-
}
52-
53-
#endregion
54-
5512
#region Lambda expression and Local functions
5613

5714
private static void CheckForGeneralExceptionThrows(
58-
SyntaxNodeAnalysisContext context,
59-
List<AttributeSyntax> throwsAttributes)
15+
IEnumerable<AttributeSyntax> throwsAttributes,
16+
ThrowsContext context)
6017
{
6118
const string generalExceptionName = "Exception";
6219
const string generalExceptionNamespace = "System";
6320

6421
var semanticModel = context.SemanticModel;
22+
var settings = GetAnalyzerSettings(context.Options);
6523

6624
foreach (var attribute in throwsAttributes)
6725
{
@@ -75,18 +33,14 @@ private static void CheckForGeneralExceptionThrows(
7533
if (type is null)
7634
continue;
7735

78-
var settings = GetAnalyzerSettings(context.Options);
79-
80-
if (settings.BaseExceptionDeclaredDiagnosticEnabled)
81-
{
82-
if (type.Name == generalExceptionName &&
36+
if (settings.BaseExceptionDeclaredDiagnosticEnabled &&
37+
type.Name == generalExceptionName &&
8338
type.ContainingNamespace?.ToDisplayString() == generalExceptionNamespace)
84-
{
85-
context.ReportDiagnostic(Diagnostic.Create(
86-
RuleGeneralThrowDeclared,
87-
typeOfExpr.Type.GetLocation(), // ✅ report precisely on typeof(Exception)
88-
type.Name));
89-
}
39+
{
40+
context.ReportDiagnostic(Diagnostic.Create(
41+
RuleGeneralThrowDeclared,
42+
typeOfExpr.Type.GetLocation(), // ✅ report precisely on typeof(Exception)
43+
type.Name));
9044
}
9145
}
9246
}

‎CheckedExceptions/CheckedExceptionsAnalyzer.Linq.cs‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ private static void CollectDeferredChainExceptions(
135135

136136
// 2) add intrinsic deferred-op exceptions (e.g., Cast<T>)
137137
if (LinqKnowledge.DeferredBuiltIns.TryGetValue(name, out var defFactory))
138-
foreach (var t in defFactory(compilation, inv))
138+
foreach (var t in defFactory(compilation, semanticModel, inv))
139139
if (t is not null) exceptionTypes.Add(t);
140140

141141
current = GetLinqSourceOperation(inv);
@@ -331,7 +331,7 @@ private static void CollectDeferredChainExceptions_ForEnumeration(
331331
CollectThrowsFromFunctionalArguments(inv, exceptionTypes, compilation, semanticModel, settings, ct);
332332

333333
if (LinqKnowledge.DeferredBuiltIns.TryGetValue(name, out var defFactory))
334-
foreach (var t in defFactory(compilation, inv))
334+
foreach (var t in defFactory(compilation, semanticModel, inv))
335335
if (t is not null) exceptionTypes.Add(t);
336336

337337
current = GetLinqSourceOperation(inv);
@@ -613,21 +613,19 @@ public static ImmutableDictionary<string, Func<Compilation, IMethodSymbol, IEnum
613613
["ToList"] = (c, m) => [],
614614
}.ToImmutableDictionary();
615615

616-
public static ImmutableDictionary<string, Func<Compilation, IInvocationOperation, IEnumerable<INamedTypeSymbol>>> DeferredBuiltIns
617-
= new Dictionary<string, Func<Compilation, IInvocationOperation, IEnumerable<INamedTypeSymbol>>>(StringComparer.Ordinal)
616+
public static ImmutableDictionary<string, Func<Compilation, SemanticModel, IInvocationOperation, IEnumerable<INamedTypeSymbol>>> DeferredBuiltIns
617+
= new Dictionary<string, Func<Compilation, SemanticModel, IInvocationOperation, IEnumerable<INamedTypeSymbol>>>(StringComparer.Ordinal)
618618
{
619619
// Cast<T>() will throw InvalidCastException during enumeration if an element can't be cast
620-
["Cast"] = (comp, inv) =>
620+
["Cast"] = (comp, semanticModel, inv) =>
621621
{
622622
// T in Cast<T>()
623623
if (inv.TargetMethod.TypeArguments.Length is not 1)
624624
return Array.Empty<INamedTypeSymbol>();
625625
var targetT = inv.TargetMethod.TypeArguments[0];
626626

627-
var semanticModel = comp.GetSemanticModel(inv.Syntax.SyntaxTree);
628-
629627
// try to recover S
630-
var srcElem = ResolveSourceElementType(inv, /* you have it in the caller */ semanticModel, default);
628+
var srcElem = ResolveSourceElementType(inv, semanticModel, default);
631629

632630
// unknown => be pessimistic (may throw during enumeration)
633631
if (srcElem is null)

‎CheckedExceptions/CheckedExceptionsAnalyzer.ThrowsContext.cs‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
using System;
2+
using System.Threading;
3+
4+
using Microsoft.CodeAnalysis;
5+
using Microsoft.CodeAnalysis.Diagnostics;
6+
7+
namespace Sundstrom.CheckedExceptions;
8+
9+
partial class CheckedExceptionsAnalyzer
10+
{
11+
private readonly struct ThrowsContext
12+
{
13+
public SemanticModel SemanticModel { get; }
14+
public AnalyzerOptions Options { get; }
15+
public Action<Diagnostic> ReportDiagnostic { get; }
16+
public CancellationToken CancellationToken { get; }
17+
18+
public ThrowsContext(
19+
SemanticModel semanticModel,
20+
AnalyzerOptions options,
21+
Action<Diagnostic> reportDiagnostic,
22+
CancellationToken cancellationToken)
23+
{
24+
SemanticModel = semanticModel;
25+
Options = options;
26+
ReportDiagnostic = reportDiagnostic;
27+
CancellationToken = cancellationToken;
28+
}
29+
}
30+
}

0 commit comments

Comments
 (0)