Exempt the settings manager from the provider access rule

This commit is contained in:
Thorsten Sommer 2026-08-12 20:40:09 +02:00
parent f0527f3852
commit 494e48ddd3
Signed by untrusted user who does not match committer: tsommer
GPG Key ID: 371BBA77A02C0108
2 changed files with 40 additions and 9 deletions

View File

@ -1,4 +1,3 @@
using System.Diagnostics.CodeAnalysis;
using System.Linq.Expressions;
using System.Text.Json;
@ -434,7 +433,6 @@ public sealed class SettingsManager
return localeTag[..separatorIndex];
}
[SuppressMessage("Usage", "MWAIS0001:Direct access to `Providers` is not allowed")]
public Provider GetPreselectedProvider(Tools.Components component, string? currentProviderId = null, bool usePreselectionBeforeCurrentProvider = false)
{
var minimumLevel = this.GetMinimumConfidenceLevel(component);
@ -486,7 +484,6 @@ public sealed class SettingsManager
return this.ConfigurationData.Providers.FirstOrDefault(x => x.Id == this.ConfigurationData.App.PreselectedProvider && x.UsedLLMProvider.GetConfidence(this).Level >= minimumLevel) ?? Provider.NONE;
}
[SuppressMessage("Usage", "MWAIS0001:Direct access to `Providers` is not allowed")]
public Provider GetChatProviderForLoadedChat(string? chatProviderId = null)
{
var minimumLevel = this.GetMinimumConfidenceLevel(Tools.Components.CHAT);
@ -548,7 +545,6 @@ public sealed class SettingsManager
/// </para>
/// </remarks>
/// <returns>All configured providers, unfiltered.</returns>
[SuppressMessage("Usage", "MWAIS0001:Direct access to `Providers` is not allowed")]
public IReadOnlyList<Provider> GetAllProviders() => this.ConfigurationData.Providers;
/// <summary>
@ -563,7 +559,6 @@ public sealed class SettingsManager
/// </remarks>
/// <param name="providerId">The id of the provider to look up.</param>
/// <returns>The provider, or <see cref="Provider.NONE"/> when no provider with that id exists.</returns>
[SuppressMessage("Usage", "MWAIS0001:Direct access to `Providers` is not allowed")]
public Provider GetProviderById(string? providerId)
{
if (string.IsNullOrWhiteSpace(providerId))
@ -611,7 +606,6 @@ public sealed class SettingsManager
/// <param name="component">The component for which the providers get filtered.</param>
/// <param name="explicitMinimum">An explicit minimum level, which is applied when it is higher than the component's minimum.</param>
/// <returns>All providers the component may use.</returns>
[SuppressMessage("Usage", "MWAIS0001:Direct access to `Providers` is not allowed")]
public IEnumerable<Provider> GetConfidentProviders(Tools.Components component, ConfidenceLevel explicitMinimum = ConfidenceLevel.UNKNOWN)
{
var minimumLevel = this.GetEffectiveMinimumConfidenceLevel(component, explicitMinimum);

View File

@ -17,11 +17,16 @@ public sealed class ProviderAccessAnalyzer : DiagnosticAnalyzer
private static readonly string TITLE = "Direct access to `Providers` is not allowed";
private static readonly string MESSAGE_FORMAT = "Direct access to `SettingsManager.ConfigurationData.Providers` is not allowed. Instead, use APIs like `SettingsManager.GetPreselectedProvider`, etc.";
private static readonly string MESSAGE_FORMAT = "Direct access to `SettingsManager.ConfigurationData.Providers` is not allowed. Instead, use APIs like `SettingsManager.GetAllProviders`, `GetProviderById`, `GetConfidentProviders`, `GetPreselectedProvider`, or `GetChatProviderForLoadedChat`.";
private static readonly string DESCRIPTION = MESSAGE_FORMAT;
private const string CATEGORY = "Usage";
/// <summary>
/// The one type which owns the provider list and is therefore allowed to access it directly.
/// </summary>
private const string OWNING_TYPE = "AIStudio.Settings.SettingsManager";
private static readonly DiagnosticDescriptor RULE = new(DIAGNOSTIC_ID, TITLE, MESSAGE_FORMAT, CATEGORY, DiagnosticSeverity.Error, isEnabledByDefault: true, description: DESCRIPTION);
@ -42,8 +47,17 @@ public sealed class ProviderAccessAnalyzer : DiagnosticAnalyzer
if (memberAccess.Name.Identifier.Text != "Providers")
return;
//
// The settings manager owns the provider list: it implements the very APIs which all other
// code is meant to use, so it must access `Providers` directly. Exempting it here keeps
// those implementations free of suppression attributes, which would otherwise read as if
// suppressing this rule was a normal thing to do:
//
if (IsOwningType(context.ContainingSymbol))
return;
// Get the full path of the member access:
var fullPath = this.GetFullMemberAccessPath(memberAccess);
var fullPath = GetFullMemberAccessPath(memberAccess);
// Check for the forbidden pattern:
if (fullPath.EndsWith("ConfigurationData.Providers"))
@ -53,7 +67,30 @@ public sealed class ProviderAccessAnalyzer : DiagnosticAnalyzer
}
}
private string GetFullMemberAccessPath(ExpressionSyntax expression)
/// <summary>
/// Checks whether the analyzed node sits inside the type which owns the provider list.
/// </summary>
/// <remarks>
/// The containing symbol is the member the node belongs to, e.g. a method or a property. We walk
/// the chain of containing types so that nested types of the owning type are covered as well.
/// </remarks>
/// <param name="containingSymbol">The symbol containing the analyzed node, which may be null.</param>
/// <returns>True, when the node belongs to the owning type.</returns>
private static bool IsOwningType(ISymbol? containingSymbol)
{
var containingType = containingSymbol as INamedTypeSymbol ?? containingSymbol?.ContainingType;
while (containingType != null)
{
if (containingType.ToDisplayString() == OWNING_TYPE)
return true;
containingType = containingType.ContainingType;
}
return false;
}
private static string GetFullMemberAccessPath(ExpressionSyntax expression)
{
var parts = new List<string>();
while (expression is MemberAccessExpressionSyntax memberAccess)