Skip to content

Fix null warnings and clean up code in CheckedExceptions #86

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sibber5 wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Fix null warnings and clean up code in CheckedExceptions #86

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a66f6dc
Fix null warnings clean up code
sibber5 Jul 10, 2025
e4390f1
Fix rest of nullable warnings in CheckedExceptions proj
sibber5 Jul 11, 2025
93c89bf
Reuse original line endings for cleaner diff
sibber5 Jul 11, 2025
fd1a591
Minor change to analyzer settings
sibber5 Jul 11, 2025
25b7abb
Merge branch 'main' into main
sibber5 Jul 28, 2025
e8609b5
Merge branch 'main' into main
sibber5 Jul 28, 2025
0e36eb6
Fix merge conflict
sibber5 Jul 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions CheckedExceptions/AnalyzerSettings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ namespace Sundstrom.CheckedExceptions;

using System.Text.Json.Serialization;

public partial class AnalyzerSettings
public class AnalyzerSettings(IReadOnlyList<string> ignoredExceptions, IReadOnlyDictionary<string, ExceptionMode> informationalExceptions)
{
[JsonPropertyName("ignoredExceptions")]
public IEnumerable<string> IgnoredExceptions { get; set; } = new List<string>();
public IReadOnlyList<string> IgnoredExceptions { get; } = ignoredExceptions;

[JsonPropertyName("informationalExceptions")]
public IDictionary<string, ExceptionMode> InformationalExceptions { get; set; } = new Dictionary<string, ExceptionMode>();
public IReadOnlyDictionary<string, ExceptionMode> InformationalExceptions { get; } = informationalExceptions;

public static AnalyzerSettings CreateWithDefaults() => new(new List<string>(), new Dictionary<string, ExceptionMode>());
}

[JsonConverter(typeof(JsonStringEnumConverter))]
Expand Down
7 changes: 5 additions & 2 deletions CheckedExceptions/AttributeHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,20 @@
namespace Sundstrom.CheckedExceptions;

using System.Diagnostics;

using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;

public static class AttributeHelper
{
public static AttributeData? GetSpecificAttributeData(AttributeSyntax attributeSyntax, SemanticModel semanticModel)
public static AttributeData? GetSpecificAttributeData(AttributeSyntax? attributeSyntax, SemanticModel? semanticModel)
{
if (attributeSyntax is null || semanticModel is null)
return null;

// Get the symbol to which the attribute is applied
var declaredSymbol = semanticModel.GetDeclaredSymbol(attributeSyntax.Parent?.Parent);
Debug.Assert(attributeSyntax.Parent?.Parent is not null);
var declaredSymbol = semanticModel.GetDeclaredSymbol(attributeSyntax.Parent?.Parent!);

if (declaredSymbol is null)
return null;
Expand Down
2 changes: 1 addition & 1 deletion CheckedExceptions/CheckedExceptions.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
Expand Down
3 changes: 1 addition & 2 deletions CheckedExceptions/CheckedExceptionsAnalyzer.DeclaredSuperClassDetection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ private void CheckForRedundantThrowsHandledByDeclaredSuperClass(
if (arg.Expression is TypeOfExpressionSyntax typeOfExpr)
{
var typeInfo = semanticModel.GetTypeInfo(typeOfExpr.Type, context.CancellationToken);
var typeSymbol = typeInfo.Type as INamedTypeSymbol;
if (typeSymbol == null)
if (typeInfo.Type is not INamedTypeSymbol typeSymbol)
continue;

declaredTypes.Add(typeSymbol);
Expand Down
64 changes: 32 additions & 32 deletions CheckedExceptions/CheckedExceptionsAnalyzer.DuplicateDetection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Collections.Immutable;
using System.Diagnostics;

using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;
Expand Down Expand Up @@ -26,22 +27,19 @@ private void CheckForDuplicateThrowsAttributes(

Copy link
Author

@sibber5 sibber5 Jul 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

var semanticModel = context.Compilation.GetSemanticModel(attrSyntax.SyntaxTree);

@marinasundstrom this gives a warning, see dotnet/roslyn-analyzers#3114:

GetSemanticModel is an expensive method to invoke within a diagnostic analyzer because it creates a completely new semantic model, which does not share compilation data with the compiler or other analyzers. This incurs an additional performance cost during semantic analysis. Instead, consider registering a different analyzer action which allows used of a shared SemanticModel, such as RegisterOperationAction, RegisterSyntaxNodeAction, or RegisterSemanticModelAction.

ctrl + shift + f shows this in 3 places, you might want to fix those

Copy link
Owner

@marinasundstrom marinasundstrom Jul 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing this out. I was honestly not thinking about it. And since I have had help from ChatGPT in many cases, I have just focused on making it work.

Yes, it should be passed down the call tree then.

foreach (var arg in attrSyntax.ArgumentList?.Arguments ?? default)
{
if (arg.Expression is TypeOfExpressionSyntax typeOfExpr)
if (arg.Expression is not TypeOfExpressionSyntax typeOfExpr)
continue;

var typeInfo = semanticModel.GetTypeInfo(typeOfExpr.Type, context.CancellationToken);
if (typeInfo.Type is not INamedTypeSymbol exceptionType)
continue;

if (!reportedTypes.Add(exceptionType))
{
var typeInfo = semanticModel.GetTypeInfo(typeOfExpr.Type, context.CancellationToken);
var exceptionType = typeInfo.Type as INamedTypeSymbol;
if (exceptionType is null)
continue;

if (reportedTypes.Contains(exceptionType))
{
context.ReportDiagnostic(Diagnostic.Create(
RuleDuplicateDeclarations,
typeOfExpr.GetLocation(), // ✅ precise location
exceptionType.Name));
}

reportedTypes.Add(exceptionType);
context.ReportDiagnostic(Diagnostic.Create(
RuleDuplicateDeclarations,
typeOfExpr.GetLocation(), // ✅ precise location
exceptionType.Name));
}
}
}
Expand All @@ -61,28 +59,30 @@ private void CheckForDuplicateThrowsDeclarations(
SyntaxNodeAnalysisContext context)
{
var semanticModel = context.SemanticModel;
var seen = new HashSet<INamedTypeSymbol>(SymbolEqualityComparer.Default);

HashSet<INamedTypeSymbol>? seen = null;

foreach (var throwsAttribute in throwsAttributes)
{
foreach (var arg in throwsAttribute.ArgumentList?.Arguments ?? [])
Debug.Assert(throwsAttribute is not null);

seen ??= new HashSet<INamedTypeSymbol>(SymbolEqualityComparer.Default);

foreach (var arg in throwsAttribute!.ArgumentList?.Arguments ?? [])
{
if (arg.Expression is TypeOfExpressionSyntax typeOfExpr)
if (arg.Expression is not TypeOfExpressionSyntax typeOfExpr)
continue;

var typeInfo = semanticModel.GetTypeInfo(typeOfExpr.Type, context.CancellationToken);
if (typeInfo.Type is not INamedTypeSymbol exceptionType)
continue;

if (!seen.Add(exceptionType))
{
var typeInfo = semanticModel.GetTypeInfo(typeOfExpr.Type, context.CancellationToken);
var exceptionType = typeInfo.Type as INamedTypeSymbol;
if (exceptionType == null)
continue;

if (seen.Contains(exceptionType))
{
context.ReportDiagnostic(Diagnostic.Create(
RuleDuplicateDeclarations,
typeOfExpr.GetLocation(), // ✅ precise location
exceptionType.Name));
}

seen.Add(exceptionType);
context.ReportDiagnostic(Diagnostic.Create(
RuleDuplicateDeclarations,
typeOfExpr.GetLocation(), // ✅ precise location
exceptionType.Name));
}
}
}
Expand Down
60 changes: 26 additions & 34 deletions CheckedExceptions/CheckedExceptionsAnalyzer.GeneralThrows.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ private static void CheckForGeneralExceptionThrows(
ImmutableArray<AttributeData> throwsAttributes,
SymbolAnalysisContext context)
{
const string exceptionName = "Exception";

foreach (var attribute in throwsAttributes)
{
var syntaxRef = attribute.ApplicationSyntaxReference;
Expand All @@ -26,20 +24,19 @@ private static void CheckForGeneralExceptionThrows(

foreach (var arg in attrSyntax.ArgumentList?.Arguments ?? [])
{
if (arg.Expression is TypeOfExpressionSyntax typeOfExpr)
{
var typeInfo = semanticModel.GetTypeInfo(typeOfExpr.Type, context.CancellationToken);
var type = typeInfo.Type as INamedTypeSymbol;
if (type is null)
continue;
if (arg.Expression is not TypeOfExpressionSyntax typeOfExpr)
continue;

if (type.Name == exceptionName && type.ContainingNamespace?.ToDisplayString() == "System")
{
context.ReportDiagnostic(Diagnostic.Create(
RuleGeneralThrows,
typeOfExpr.GetLocation(), // ✅ precise location
type.Name));
}
var typeInfo = semanticModel.GetTypeInfo(typeOfExpr.Type, context.CancellationToken);
if (typeInfo.Type is not INamedTypeSymbol type)
continue;

if (nameof(Exception).Equals(type.Name, StringComparison.Ordinal) && nameof(System).Equals(type.ContainingNamespace?.ToDisplayString(), StringComparison.Ordinal))
{
context.ReportDiagnostic(Diagnostic.Create(
RuleGeneralThrows,
typeOfExpr.GetLocation(), // ✅ precise location
type.Name));
}
}
}
Expand All @@ -50,34 +47,29 @@ private static void CheckForGeneralExceptionThrows(
#region Lambda expression and Local functions

private void CheckForGeneralExceptionThrows(
SyntaxNodeAnalysisContext context,
List<AttributeSyntax> throwsAttributes)
IEnumerable<AttributeSyntax> throwsAttributes,
SyntaxNodeAnalysisContext context)
{
const string generalExceptionName = "Exception";
const string generalExceptionNamespace = "System";

var semanticModel = context.SemanticModel;

foreach (var attribute in throwsAttributes)
{
foreach (var arg in attribute.ArgumentList?.Arguments ?? [])
{
if (arg.Expression is TypeOfExpressionSyntax typeOfExpr)
{
var typeInfo = semanticModel.GetTypeInfo(typeOfExpr.Type, context.CancellationToken);
var type = typeInfo.Type as INamedTypeSymbol;
if (arg.Expression is not TypeOfExpressionSyntax typeOfExpr)
continue;

if (type is null)
continue;
var typeInfo = semanticModel.GetTypeInfo(typeOfExpr.Type, context.CancellationToken);
if (typeInfo.Type is not INamedTypeSymbol type)
continue;

if (type.Name == generalExceptionName &&
type.ContainingNamespace?.ToDisplayString() == generalExceptionNamespace)
{
context.ReportDiagnostic(Diagnostic.Create(
RuleGeneralThrows,
typeOfExpr.GetLocation(), // ✅ report precisely on typeof(Exception)
type.Name));
}
if (nameof(Exception).Equals(type.Name, StringComparison.Ordinal) &&
nameof(System).Equals(type.ContainingNamespace?.ToDisplayString(), StringComparison.Ordinal))
{
context.ReportDiagnostic(Diagnostic.Create(
RuleGeneralThrows,
typeOfExpr.GetLocation(), // ✅ report precisely on typeof(Exception)
type.Name));
}
}
}
Expand Down
43 changes: 19 additions & 24 deletions CheckedExceptions/CheckedExceptionsAnalyzer.Inheritance.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,14 @@
using System.Collections.Immutable;
using System.Diagnostics;
using System.Net.NetworkInformation;

using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;

namespace Sundstrom.CheckedExceptions;

partial class CheckedExceptionsAnalyzer
{
#region Method

private void CheckForCompatibilityWithBaseOrInterface(SymbolAnalysisContext context, ImmutableArray<AttributeData> throwsAttributes)
{
var method = (IMethodSymbol)context.Symbol;
Expand All @@ -23,7 +21,8 @@ MethodKind.EventAdd or
MethodKind.EventRemove))
return;

var declaredExceptions = GetDistictExceptionTypes(throwsAttributes).ToImmutableHashSet(SymbolEqualityComparer.Default);
ImmutableHashSet<ISymbol> declaredExceptions = GetDistinctExceptionTypes(throwsAttributes).Where(x => x is not null).ToImmutableHashSet(SymbolEqualityComparer.Default)!;
Debug.Assert(!declaredExceptions.Any(x => x is null));

if (declaredExceptions.Count == 0)
return;
Expand All @@ -42,7 +41,7 @@ MethodKind.EventAdd or
}
}

private void AnalyzeMissingThrowsFromBaseMember(SymbolAnalysisContext context, IMethodSymbol method, ImmutableHashSet<ISymbol?> declaredExceptions, IMethodSymbol baseMethod, ImmutableHashSet<ISymbol?> baseExceptions)
private void AnalyzeMissingThrowsFromBaseMember(SymbolAnalysisContext context, IMethodSymbol method, ImmutableHashSet<ISymbol> declaredExceptions, IMethodSymbol baseMethod, ImmutableHashSet<ISymbol?> baseExceptions)
{
foreach (var baseException in baseExceptions.OfType<ITypeSymbol>())
{
Expand Down Expand Up @@ -77,13 +76,13 @@ private void AnalyzeMissingThrowsFromBaseMember(SymbolAnalysisContext context, I
}
}

private static void AnalyzeMissingThrowsOnBaseMember(SymbolAnalysisContext context, IMethodSymbol method, ImmutableHashSet<ISymbol?> declaredExceptions, IMethodSymbol baseMethod, ImmutableHashSet<ISymbol?> baseExceptions)
private static void AnalyzeMissingThrowsOnBaseMember(SymbolAnalysisContext context, IMethodSymbol method, ImmutableHashSet<ISymbol> declaredExceptions, IMethodSymbol baseMethod, ImmutableHashSet<ISymbol?> baseExceptions)
{
foreach (var declared in declaredExceptions)
{
var isCompatible = baseExceptions.Any(baseEx =>
declared.Equals(baseEx, SymbolEqualityComparer.Default) ||
((INamedTypeSymbol)declared).InheritsFrom((INamedTypeSymbol)baseEx));
var isCompatible = baseExceptions.Any(baseEx => baseEx is not null &&
(declared.Equals(baseEx, SymbolEqualityComparer.Default)
|| ((INamedTypeSymbol)declared).InheritsFrom((INamedTypeSymbol)baseEx)));

if (!isCompatible)
{
Expand Down Expand Up @@ -115,13 +114,11 @@ public static string FormatMethodSignature(IMethodSymbol methodSymbol)

private bool IsTooGenericException(ITypeSymbol ex)
{
var namedType = ex as INamedTypeSymbol;
if (namedType == null)
return false;
if (ex is not INamedTypeSymbol namedTypeSymbol) return false;

var fullName = namedType.ToDisplayString();
var fullName = namedTypeSymbol.ToDisplayString();

return fullName == "System.Exception" || fullName == "System.SystemException";
return fullName.Equals(typeof(Exception).FullName, StringComparison.Ordinal) || fullName.Equals(typeof(SystemException).FullName, StringComparison.Ordinal);
}

private IEnumerable<IMethodSymbol> GetBaseOrInterfaceMethods(IMethodSymbol method)
Expand All @@ -133,18 +130,18 @@ private IEnumerable<IMethodSymbol> GetBaseOrInterfaceMethods(IMethodSymbol metho

if (method.AssociatedSymbol is IPropertySymbol prop && prop.OverriddenProperty is not null)
{
if (SymbolEqualityComparer.Default.Equals(method, prop.GetMethod))
results.Add(prop.OverriddenProperty.GetMethod);
else if (SymbolEqualityComparer.Default.Equals(method, prop.SetMethod))
results.Add(prop.OverriddenProperty.SetMethod);
if (SymbolEqualityComparer.Default.Equals(method, prop.GetMethod) && prop.OverriddenProperty.GetMethod is { } getMethodSymbol)
results.Add(getMethodSymbol);
else if (SymbolEqualityComparer.Default.Equals(method, prop.SetMethod) && prop.OverriddenProperty.SetMethod is { } setMethodSymbol)
results.Add(setMethodSymbol);
}

if (method.AssociatedSymbol is IEventSymbol ev && ev.OverriddenEvent is not null)
{
if (SymbolEqualityComparer.Default.Equals(method, ev.AddMethod))
results.Add(ev.OverriddenEvent.AddMethod);
else if (SymbolEqualityComparer.Default.Equals(method, ev.RemoveMethod))
results.Add(ev.OverriddenEvent.RemoveMethod);
if (SymbolEqualityComparer.Default.Equals(method, ev.AddMethod) && ev.OverriddenEvent.AddMethod is { } addMethodSymbol)
results.Add(addMethodSymbol);
else if (SymbolEqualityComparer.Default.Equals(method, ev.RemoveMethod) && ev.OverriddenEvent.RemoveMethod is { } removeMethodSymbol)
results.Add(removeMethodSymbol);
}

var type = method.ContainingType;
Expand All @@ -162,6 +159,4 @@ private IEnumerable<IMethodSymbol> GetBaseOrInterfaceMethods(IMethodSymbol metho

return results;
}

#endregion
}
2 changes: 1 addition & 1 deletion CheckedExceptions/CheckedExceptionsAnalyzer.Shared.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ partial class CheckedExceptionsAnalyzer
/// <summary>
/// Retrieves the name of the exception type from a ThrowsAttribute's AttributeData.
/// </summary>
private string GetExceptionTypeName(AttributeData attributeData)
private string GetExceptionTypeName(AttributeData? attributeData)
{
if (attributeData is null)
return string.Empty;
Expand Down
Loading