diff --git a/.editorconfig b/.editorconfig index 00831d9..1d15799 100644 --- a/.editorconfig +++ b/.editorconfig @@ -8,6 +8,13 @@ indent_style = space indent_size = 4 dotnet_diagnostic.IDE0005.severity = warning +# Test methods are named with underscores by convention (method_scenario_expected). +# IDE0005 (unused usings) is kept out of the build for tests so it does not require +# GenerateDocumentationFile there; the IDE still tidies usings interactively. +[tests/**/*.cs] +dotnet_diagnostic.CA1707.severity = none +dotnet_diagnostic.IDE0005.severity = none + [*.{csproj,props,targets,slnx,yml,json,md}] charset = utf-8 end_of_line = lf diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b213c56..b678210 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -27,7 +27,7 @@ jobs: run: dotnet restore ConfigContraband.slnx - name: Build - run: dotnet build ConfigContraband.slnx --configuration Release --no-restore + run: dotnet build ConfigContraband.slnx --configuration Release --no-restore -p:ContinuousIntegrationBuild=true - name: Verify formatting run: dotnet format ConfigContraband.slnx --verify-no-changes --no-restore --verbosity minimal diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 80c3e27..c68f7bb 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -37,7 +37,7 @@ jobs: run: dotnet restore ConfigContraband.slnx - name: Build - run: dotnet build ConfigContraband.slnx --configuration Release --no-restore + run: dotnet build ConfigContraband.slnx --configuration Release --no-restore -p:ContinuousIntegrationBuild=true - name: Perform CodeQL Analysis uses: github/codeql-action/analyze@v4 diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 3debae5..375a079 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -36,7 +36,7 @@ jobs: run: dotnet restore ConfigContraband.slnx - name: Build - run: dotnet build ConfigContraband.slnx --configuration Release --no-restore + run: dotnet build ConfigContraband.slnx --configuration Release --no-restore -p:ContinuousIntegrationBuild=true - name: Verify formatting run: dotnet format ConfigContraband.slnx --verify-no-changes --no-restore --verbosity minimal diff --git a/CHANGELOG.md b/CHANGELOG.md index 829873d..c4a954f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,8 +2,32 @@ All notable changes to ConfigContraband will be documented in this file. -## Unreleased - +## 0.7.21 - 2026-07-21 + +- Fixed a potential `AD0001` analyzer crash: `BindConfiguration(...)` registration + parsing now handles error-tolerant operations defensively and resolves the + `configSectionPath` argument with a guarded named lookup instead of an unchecked + cast and `.First()`, so the analyzer stays quiet while typing broken code in the IDE. +- Enabled the CI analyzer gate. The CI, publish, and CodeQL workflows now build with + `ContinuousIntegrationBuild=true`, so the .NET analyzers actually run in CI as + `Directory.Build.props` always claimed. Triaged every resulting warning: fixed + CA1305/CA1822/CA1826/CA1859/CA1861 in `src`, enabled `GenerateDocumentationFile` + (activating IDE0005 on build) and corrected the latent XML-doc issues (CS1734/CS0419) + it surfaced, and suppressed CA1707/IDE0005 for idiomatic underscore test naming. + CI-mode builds are now genuinely zero-warning. +- Performance: memoized `OptionsTypeMetadata` per compilation through a + `ConditionalWeakTable`, so one registration requesting the same options graph across + the validation, nested-validation, and unknown-key passes builds it once; the cache is + collected with its compilation and never leaks. Also collapsed CFG009's provenance + scan from seven full block traversals into a single document-order walk. +- Robustness: bounded the appsettings JSON parser nesting depth (64, matching + `System.Text.Json`) so a pathologically nested additional file is treated as malformed + instead of causing an uncatchable `StackOverflowException` in the compiler host. +- Cleanup: removed dead code (an empty partial file, three unused helpers, a vestigial + local and duplicated receiver-chain walk) and unified four byte-identical traversal + predicates into one shared `ExecutionScope.ShouldDescend` guard. +- Completed the showcase to cover all nine rules: added the missing `CFG002`, `CFG008`, + and `CFG009` examples and regenerated the bundled `appsettings.schema.json`. - Internal maintainability refactor with no diagnostic behavior change. The 5740-line `ConfigContrabandAnalyzer` and 3666-line `OptionsTypeMetadata` god-objects were split into concern-focused partial files (direct reads, diff --git a/README.md b/README.md index 87e8227..71b2ac1 100644 --- a/README.md +++ b/README.md @@ -43,7 +43,7 @@ Use it when your app relies on strongly typed options and you want configuration ## Install ```xml - + ``` The package includes `buildTransitive` props that pass visible `appsettings.json` and `appsettings.*.json` files to the analyzer automatically. Add the package, build, and let your editor or CI tell you when your options contract and configuration drift apart. diff --git a/analyzer-health.md b/analyzer-health.md index 7da51e8..c823639 100644 --- a/analyzer-health.md +++ b/analyzer-health.md @@ -2,9 +2,9 @@ This file tracks the current ConfigContraband analyzer surface and the next hardening work that is still worth doing. It should stay practical: scores drive priority, notes describe shipped behavior, and gaps should be specific enough to turn into a focused PR. -Last refreshed: 2026-07-16 -Package version: `0.7.20` -Base audited commit: `4305538` +Last refreshed: 2026-07-21 +Package version: `0.7.21` +Base audited commit: `c570161` ## Scoring Rubric @@ -171,6 +171,7 @@ Release readiness is healthy. The iteration-121 verifier passed 669 analyzer/tes | 2026-07-16 | 119 | `CFG008` | The framework exposes non-generic `ConfigurationBinder.GetValue(IConfiguration, Type, string[, object])` overloads, but direct-read normalization recognized only `GetValue`, so `configuration.GetValue(typeof(int), "Server:Port")` missed a proven conversion failure that the equivalent generic call reported. Two RED regressions expected CFG008 and received none. | Normalize the signed framework non-generic overloads only when the `Type` argument is a direct `typeof(...)` operation. Preserve root/known-section provenance, constant-path, deduplication, same-FQN, and default-side-effect gates; unwrap only built-in conversions when proving a boxed constant default or a direct type target, while dynamic `Type` expressions, user-defined conversions, and effectful defaults stay quiet. A review-driven RED regression proved an effectful user-defined `Type` conversion previously caused a false positive. Seven focused regressions cover instance/static named calls, a known section chain, dynamic types, effectful defaults/conversions, and source shadows. Full suite 664 analyzer/test + 246 core (910 total) passed; Release build completed with zero warnings/errors and formatter verification passed. Patch bump to `0.7.18`; README and CHANGELOG synchronized. | `CFG008` remains `5.00` / P3: the concrete false negative and review-found precision gap were fixed in the same iteration, strengthening direct-read overload parity without changing score math. | | 2026-07-16 | 120 | `CFG009` | `AnalyzeConnectionStringRead` already composed a statically known receiver section with its relative `ConnectionStrings` namespace, but no focused regression pinned that branch. A future simplification could therefore compare `configuration.GetSection("Tenant").GetConnectionString("Databsae")` against the root `ConnectionStrings` section or accidentally widen stored-section receivers without failing tests. | Add focused instance and static named-call regressions proving `Tenant:ConnectionStrings:Databsae` suggests `Tenant:ConnectionStrings:Database`, plus a conservative guard proving a stored `IConfigurationSection` receiver stays quiet because its origin is invisible. All three tests passed immediately, confirming shipped behavior; no analyzer implementation changed. Full suite 667 analyzer/test + 246 core (913 total) passed; Release build completed with zero warnings/errors and formatter verification passed. Patch bump to `0.7.19`; README and CHANGELOG synchronized. | `CFG009` remains `5.00` / P3: relative connection-string namespace and provenance test depth improve without changing score math or diagnostic behavior. | | 2026-07-16 | 121 | `CFG008` | The scalar helper already used `NumberStyles.Float` for `float` and `double`, but the runtime-backed parity suite covered only decimal and integral thousands separators; its reusable type resolver could not even construct `System.Single`. A future broadening to permissive number styles could therefore accept `"1,000"` even though `SingleConverter` and `DoubleConverter` throw. | Add direct runtime-converter proofs for both floating-point types, shared scalar-matrix cases, and end-to-end CFG008 options-binding diagnostics. The first focused run failed because the test resolver lacked `System.Single`; adding that mapping completed the missing branch. Full suite 669 analyzer/test + 250 core (919 total) passed; Release build completed with zero warnings/errors and formatter verification passed. Patch bump to `0.7.20`; README and CHANGELOG synchronized. | `CFG008` remains `5.00` / P3: converter-parity test depth improves without changing score math or diagnostic behavior. | +| 2026-07-21 | 122 | INFRA, Release | A full code review found the CI analyzer gate was dead (no workflow passed `ContinuousIntegrationBuild=true`, so .NET analyzers never ran despite the `Directory.Build.props` comment), a latent `AD0001` crash in `BindConfiguration` registration parsing (unchecked operation cast plus `.First()`), two per-registration performance hotspots (un-memoized `OptionsTypeMetadata.Create`, and seven CFG009 provenance traversals per block), an unbounded-recursion appsettings JSON parser, dead code left by the partial split, and a stale showcase missing `CFG002`/`CFG008`/`CFG009`. | Hardened the operation handling with a guarded named-argument lookup; wired `ContinuousIntegrationBuild=true` through the CI/publish/CodeQL builds and triaged every resulting warning (fixed CA1305/CA1822/CA1826/CA1859/CA1861 in src, enabled `GenerateDocumentationFile` and corrected the CS1734/CS0419 it surfaced, suppressed CA1707/IDE0005 for idiomatic test naming) to a genuine zero-warning CI build; memoized `OptionsTypeMetadata` per compilation through a `ConditionalWeakTable` and collapsed the CFG009 provenance scan to one traversal; capped JSON parse depth at 64; removed dead code and unified four duplicate traversal predicates into `ExecutionScope.ShouldDescend`; completed the showcase to all nine rules and regenerated its schema. | No rule scores change: no diagnostic IDs, severities, messages, or code fixes changed. Full suite 680 analyzer + 250 core (930 total) passed, the CI-mode build is zero-warning/zero-error, and formatter verification passed. Patch bump to `0.7.21`. | ## Health Baseline diff --git a/samples/ConfigContraband.Showcase/Program.cs b/samples/ConfigContraband.Showcase/Program.cs index 6082d55..e9e9504 100644 --- a/samples/ConfigContraband.Showcase/Program.cs +++ b/samples/ConfigContraband.Showcase/Program.cs @@ -1,4 +1,5 @@ using System.ComponentModel.DataAnnotations; +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; var services = new ServiceCollection(); @@ -7,6 +8,12 @@ services.AddOptions() .BindConfiguration("Strpie"); +// CFG002: the "RequiredKey" section exists but is missing the [Required] ApiKey key. +services.AddOptions() + .BindConfiguration("RequiredKey") + .ValidateDataAnnotations() + .ValidateOnStart(); + // CFG003: custom validation is registered, but startup validation is missing. services.AddOptions() .BindConfiguration("StartupValidation") @@ -34,13 +41,32 @@ .ValidateDataAnnotations() .ValidateOnStart(); +// CFG008: "eighty" cannot be converted to the int Port property; the binder throws at bind time. +services.AddOptions() + .BindConfiguration("Conversion"); + _ = services; +// CFG009: a direct read of a section path that is not in appsettings.json throws at runtime. +public sealed class DirectReader +{ + public void Read(IConfiguration configuration) + { + _ = configuration.GetRequiredSection("Strpie"); + } +} + public sealed class MissingSectionOptions { public string ApiKey { get; set; } = ""; } +public sealed class RequiredKeyOptions +{ + [Required] + public string ApiKey { get; set; } = ""; +} + public sealed class StartupValidationOptions { public string Endpoint { get; set; } = ""; @@ -78,3 +104,8 @@ public sealed class StrictUnknownKeyOptions public string WebhookSecret { get; set; } = ""; } + +public sealed class ConversionOptions +{ + public int Port { get; set; } +} diff --git a/samples/ConfigContraband.Showcase/appsettings.json b/samples/ConfigContraband.Showcase/appsettings.json index 2293d19..98b43f3 100644 --- a/samples/ConfigContraband.Showcase/appsettings.json +++ b/samples/ConfigContraband.Showcase/appsettings.json @@ -3,6 +3,8 @@ "Stripe": { "ApiKey": "secret" }, + "RequiredKey": { + }, "StartupValidation": { "Endpoint": "https://example.test" }, @@ -21,5 +23,8 @@ "StrictUnknownKey": { "ApiKey": "secret", "WebookSecret": "typo" + }, + "Conversion": { + "Port": "eighty" } } diff --git a/samples/ConfigContraband.Showcase/appsettings.schema.json b/samples/ConfigContraband.Showcase/appsettings.schema.json index a148bb3..bbe5bdc 100644 --- a/samples/ConfigContraband.Showcase/appsettings.schema.json +++ b/samples/ConfigContraband.Showcase/appsettings.schema.json @@ -10,6 +10,17 @@ } } }, + "RequiredKey": { + "type": "object", + "properties": { + "ApiKey": { + "type": "string" + } + }, + "required": [ + "ApiKey" + ] + }, "StartupValidation": { "type": "object", "properties": { @@ -67,6 +78,14 @@ "ApiKey" ], "additionalProperties": false + }, + "Conversion": { + "type": "object", + "properties": { + "Port": { + "type": "integer" + } + } } } } diff --git a/src/ConfigContraband.Core/ConfigContraband.Core.csproj b/src/ConfigContraband.Core/ConfigContraband.Core.csproj index c19b26e..51cc3a3 100644 --- a/src/ConfigContraband.Core/ConfigContraband.Core.csproj +++ b/src/ConfigContraband.Core/ConfigContraband.Core.csproj @@ -8,7 +8,8 @@ ConfigContraband ConfigContraband.Core false - $(NoWarn);RS1038;RS1037 + true + $(NoWarn);RS1038;RS1037;CS1591 diff --git a/src/ConfigContraband.Core/ConfigurationModel.cs b/src/ConfigContraband.Core/ConfigurationModel.cs index 1193947..85dad81 100644 --- a/src/ConfigContraband.Core/ConfigurationModel.cs +++ b/src/ConfigContraband.Core/ConfigurationModel.cs @@ -351,9 +351,11 @@ private static IEnumerable EnumerateProperties(Configurat } } + private static readonly char[] PathSeparator = { ':' }; + private static string[] SplitPath(string sectionPath) { - return sectionPath.Split(new[] { ':' }, StringSplitOptions.RemoveEmptyEntries); + return sectionPath.Split(PathSeparator, StringSplitOptions.RemoveEmptyEntries); } private static bool IsAppSettingsFile(string path) @@ -507,6 +509,11 @@ internal static class JsonConfigurationParser private sealed class Parser { + // Guards against an uncatchable StackOverflowException on a pathologically + // nested appsettings file: appsettings arrives as arbitrary AdditionalText, so + // the parser must survive any input. Matches System.Text.Json's default ceiling. + private const int MaxDepth = 64; + private readonly string _path; private readonly SourceText _text; private readonly bool _strictUnknownConfigurationKeySuppressedByAnalyzerConfig; @@ -526,10 +533,10 @@ public Parser( public ConfigurationNode? ParseRoot() { SkipWhitespace(); - return Current == '{' ? ParseObject(parentPath: string.Empty) : null; + return Current == '{' ? ParseObject(parentPath: string.Empty, depth: 0) : null; } - private ConfigurationNode ParseObject(string parentPath) + private ConfigurationNode ParseObject(string parentPath, int depth) { var properties = ImmutableArray.CreateBuilder(); Read('{'); @@ -556,7 +563,7 @@ private ConfigurationNode ParseObject(string parentPath) } SkipWhitespace(); - var parsed = ParseValue(fullPath); + var parsed = ParseValue(fullPath, depth); properties.Add(new ConfigurationProperty( key, fullPath, @@ -602,17 +609,29 @@ public ParsedValue(ConfigurationNode node, ScalarKind kind, string? raw, TextSpa public TextSpan ValueSpan { get; } } - private ParsedValue ParseValue(string path) + private ParsedValue ParseValue(string path, int depth) { SkipWhitespace(); if (Current == '{') { - return new ParsedValue(ParseObject(path), ScalarKind.None, raw: null, default); + if (depth >= MaxDepth) + { + SkipMalformedValue(); + return new ParsedValue(ConfigurationNode.Empty, ScalarKind.None, raw: null, default); + } + + return new ParsedValue(ParseObject(path, depth + 1), ScalarKind.None, raw: null, default); } if (Current == '[') { - return new ParsedValue(ParseArray(path), ScalarKind.None, raw: null, default); + if (depth >= MaxDepth) + { + SkipMalformedValue(); + return new ParsedValue(ConfigurationNode.Empty, ScalarKind.None, raw: null, default); + } + + return new ParsedValue(ParseArray(path, depth + 1), ScalarKind.None, raw: null, default); } if (Current == '"') @@ -658,7 +677,7 @@ private static ScalarKind ClassifyScalar(string value) return ScalarKind.Number; } - private ConfigurationNode ParseArray(string path) + private ConfigurationNode ParseArray(string path, int depth) { var properties = ImmutableArray.CreateBuilder(); var index = 0; @@ -670,7 +689,7 @@ private ConfigurationNode ParseArray(string path) var itemStart = _position; var itemKey = index.ToString(System.Globalization.CultureInfo.InvariantCulture); var itemPath = path + ":" + itemKey; - var parsed = ParseValue(itemPath); + var parsed = ParseValue(itemPath, depth); properties.Add(new ConfigurationProperty( itemKey, itemPath, diff --git a/src/ConfigContraband.Core/ExecutionScope.cs b/src/ConfigContraband.Core/ExecutionScope.cs new file mode 100644 index 0000000..04ef828 --- /dev/null +++ b/src/ConfigContraband.Core/ExecutionScope.cs @@ -0,0 +1,20 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace ConfigContraband; + +/// +/// Shared traversal guard for straight-line (same immediate execution scope) analyses. Lambda and +/// local-function bodies are deferred — they run when invoked, not where they are written — so any +/// analysis that tracks what executes at a point in the straight-line flow (binder-options escape +/// proofs, constructor-default proofs, configuration-origin provenance, local builder chains) must +/// not descend into them. The four former per-context descend predicates all reduced to this guard. +/// +internal static class ExecutionScope +{ + public static bool ShouldDescend(SyntaxNode node) + { + return node is not AnonymousFunctionExpressionSyntax and + not LocalFunctionStatementSyntax; + } +} diff --git a/src/ConfigContraband.Core/JsonNode.cs b/src/ConfigContraband.Core/JsonNode.cs index c1a10b2..c7376ce 100644 --- a/src/ConfigContraband.Core/JsonNode.cs +++ b/src/ConfigContraband.Core/JsonNode.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using System.Globalization; using System.Text; namespace ConfigContraband; @@ -73,7 +74,7 @@ private protected static void AppendString(StringBuilder builder, string value) if (ch < ' ') { builder.Append("\\u"); - builder.Append(((int)ch).ToString("x4")); + builder.Append(((int)ch).ToString("x4", CultureInfo.InvariantCulture)); } else { diff --git a/src/ConfigContraband.Core/JsonSchemaBuilder.cs b/src/ConfigContraband.Core/JsonSchemaBuilder.cs index 195f73b..067edd0 100644 --- a/src/ConfigContraband.Core/JsonSchemaBuilder.cs +++ b/src/ConfigContraband.Core/JsonSchemaBuilder.cs @@ -16,7 +16,7 @@ namespace ConfigContraband; internal static class JsonSchemaBuilder { /// - /// Builds the JSON Schema node describing how binds from configuration. + /// Builds the JSON Schema node describing how the options type of binds from configuration. /// public static JsonNode BuildObjectSchema(SchemaSection section, Compilation compilation) { @@ -39,7 +39,7 @@ public static JsonNode BuildObjectSchema( return BuildObjectSchema(type, context, strict, validatesDataAnnotations, new HashSet(SymbolEqualityComparer.Default)); } - private static JsonNode BuildObjectSchema( + private static JsonObject BuildObjectSchema( INamedTypeSymbol type, SchemaBuildContext context, bool strict, @@ -87,26 +87,23 @@ private static JsonNode BuildObjectSchema( // Validation only walks into a child object/collection when the property opts in with a // recursive validation attribute, mirroring CFG002/CFG005. var childValidates = validates && groupedProperties.Any(candidate => candidate.IsRecursiveValidationEnabled); - var valueSchema = BuildValueSchema(property.Symbol.Type, context, childStrict, childValidates, visited); - if (valueSchema is JsonObject valueObject) + var valueObject = BuildValueSchema(property.Symbol.Type, context, childStrict, childValidates, visited); + var declarations = groupedProperties.AsEnumerable().Reverse().Select(candidate => candidate.Symbol).ToArray(); + // `validates` (this object's flag), not `childValidates`: a property's own [Range]/length/ + // pattern is enforced when THIS type's DataAnnotations validation runs, mirroring [Required]. + // Apply the base declaration first, then let a derived override replace matching + // inherited attributes just as TypeDescriptor does at runtime. + foreach (var declaration in declarations) { - var declarations = groupedProperties.AsEnumerable().Reverse().Select(candidate => candidate.Symbol).ToArray(); - // `validates` (this object's flag), not `childValidates`: a property's own [Range]/length/ - // pattern is enforced when THIS type's DataAnnotations validation runs, mirroring [Required]. - // Apply the base declaration first, then let a derived override replace matching - // inherited attributes just as TypeDescriptor does at runtime. - foreach (var declaration in declarations) - { - AnnotateDescription(valueObject, declaration); - } + AnnotateDescription(valueObject, declaration); + } - if (validates) - { - ApplyConstraints(valueObject, declarations); - } + if (validates) + { + ApplyConstraints(valueObject, declarations); } - properties.Add(key, valueSchema); + properties.Add(key, valueObject); // [Required] is only enforced when DataAnnotations validation actually runs (CFG002). if (validates && metadata.IsRequiredForSchema(groupedProperties)) @@ -138,7 +135,7 @@ private static JsonNode BuildObjectSchema( return schema; } - private static JsonNode BuildValueSchema( + private static JsonObject BuildValueSchema( ITypeSymbol type, SchemaBuildContext context, bool strict, @@ -172,7 +169,7 @@ private static JsonNode BuildValueSchema( return BuildScalarSchema(type); } - private static JsonNode BuildScalarSchema(ITypeSymbol type) + private static JsonObject BuildScalarSchema(ITypeSymbol type) { var nullable = type is INamedTypeSymbol nullableType && nullableType.OriginalDefinition.SpecialType == SpecialType.System_Nullable_T && @@ -278,7 +275,7 @@ private static void AnnotateDescription(JsonObject schema, IPropertySymbol prope } } - private static void ApplyConstraints(JsonObject schema, IReadOnlyList properties) + private static void ApplyConstraints(JsonObject schema, IPropertySymbol[] properties) { var kind = ClassifyScalar(properties[0].Type); if (kind == ScalarKind.Other) diff --git a/src/ConfigContraband.Core/OptionsMetadata.cs b/src/ConfigContraband.Core/OptionsMetadata.cs index 71fe15f..a8364f9 100644 --- a/src/ConfigContraband.Core/OptionsMetadata.cs +++ b/src/ConfigContraband.Core/OptionsMetadata.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; +using System.Runtime.CompilerServices; using Microsoft.CodeAnalysis; namespace ConfigContraband; @@ -37,10 +38,38 @@ private OptionsTypeMetadata( public bool ImplementsValidatableObject { get; } public bool BindsNonPublicProperties => _bindsNonPublicProperties; + private static readonly ConditionalWeakTable MetadataCaches = new(); + public static OptionsTypeMetadata Create( INamedTypeSymbol type, bool bindsNonPublicProperties = false, Compilation? compilation = null) + { + // Metadata is an immutable pure function of (type, bindsNonPublicProperties, compilation), so + // it is memoized per compilation: one registration requests the same metadata several times + // across the validation, nested-validation, and unknown-key passes. The ConditionalWeakTable + // ties each cache to its compilation so it is collected with it and never leaks; concurrent + // builds of the same key are harmless because the metadata is immutable (TryAdd keeps one). + if (compilation is null) + { + return Build(type, bindsNonPublicProperties, compilation: null); + } + + var cache = MetadataCaches.GetValue(compilation, static _ => new OptionsMetadataCache()); + if (cache.TryGet(type, bindsNonPublicProperties, out var cached)) + { + return cached; + } + + var metadata = Build(type, bindsNonPublicProperties, compilation); + cache.Add(type, bindsNonPublicProperties, metadata); + return metadata; + } + + private static OptionsTypeMetadata Build( + INamedTypeSymbol type, + bool bindsNonPublicProperties, + Compilation? compilation) { var properties = ImmutableArray.CreateBuilder(); @@ -350,7 +379,7 @@ property.GetMethod is null || !HasPotentialPolymorphicInitializer(property, TypeSymbol, _compilation)); } - public bool IsConfigurationAlias(BindableProperty property, string key) + public static bool IsConfigurationAlias(BindableProperty property, string key) { return !string.Equals(property.Symbol.Name, key, StringComparison.OrdinalIgnoreCase) && HasConfigurationAlias(property.Symbol, key); diff --git a/src/ConfigContraband.Core/OptionsMetadataCache.cs b/src/ConfigContraband.Core/OptionsMetadataCache.cs new file mode 100644 index 0000000..65bfdb8 --- /dev/null +++ b/src/ConfigContraband.Core/OptionsMetadataCache.cs @@ -0,0 +1,61 @@ +using System; +using System.Collections.Concurrent; +using Microsoft.CodeAnalysis; + +namespace ConfigContraband; + +/// +/// Per-compilation memoization for . Building metadata walks the +/// whole options type graph (attributes, constructor proofs, syntax references), and a single +/// options registration can request the same (type, bindsNonPublicProperties) metadata several +/// times across the validation, nested-validation, and unknown-key passes. Metadata is an immutable +/// pure function of those inputs plus the compilation, so it is safe to share under concurrent +/// execution. The analyzer keeps one cache per through a +/// ConditionalWeakTable, so the cache is collected with its compilation and never leaks. +/// +internal sealed class OptionsMetadataCache +{ + private readonly ConcurrentDictionary _entries = new(); + + public bool TryGet(INamedTypeSymbol type, bool bindsNonPublicProperties, out OptionsTypeMetadata metadata) + { + return _entries.TryGetValue(new CacheKey(type, bindsNonPublicProperties), out metadata!); + } + + public void Add(INamedTypeSymbol type, bool bindsNonPublicProperties, OptionsTypeMetadata metadata) + { + _entries.TryAdd(new CacheKey(type, bindsNonPublicProperties), metadata); + } + + private readonly struct CacheKey : IEquatable + { + public CacheKey(INamedTypeSymbol type, bool bindsNonPublicProperties) + { + Type = type; + BindsNonPublicProperties = bindsNonPublicProperties; + } + + public INamedTypeSymbol Type { get; } + + public bool BindsNonPublicProperties { get; } + + public bool Equals(CacheKey other) + { + return BindsNonPublicProperties == other.BindsNonPublicProperties && + SymbolEqualityComparer.Default.Equals(Type, other.Type); + } + + public override bool Equals(object? obj) + { + return obj is CacheKey other && Equals(other); + } + + public override int GetHashCode() + { + unchecked + { + return (SymbolEqualityComparer.Default.GetHashCode(Type) * 397) ^ BindsNonPublicProperties.GetHashCode(); + } + } + } +} diff --git a/src/ConfigContraband.Core/OptionsTypeMetadata.ConstructorDefaults.cs b/src/ConfigContraband.Core/OptionsTypeMetadata.ConstructorDefaults.cs index b2e7630..2f953f6 100644 --- a/src/ConfigContraband.Core/OptionsTypeMetadata.ConstructorDefaults.cs +++ b/src/ConfigContraband.Core/OptionsTypeMetadata.ConstructorDefaults.cs @@ -76,7 +76,7 @@ private static IEnumerable GetDefinitelyExecutedCons private static bool ContainsConstructorExit(SyntaxNode node) { - return node.DescendantNodesAndSelf(ShouldDescendIntoConstructorInitializerNode).Any(static descendant => + return node.DescendantNodesAndSelf(ExecutionScope.ShouldDescend).Any(static descendant => descendant.IsKind(Microsoft.CodeAnalysis.CSharp.SyntaxKind.ReturnStatement) || descendant.IsKind(Microsoft.CodeAnalysis.CSharp.SyntaxKind.GotoStatement) || descendant.IsKind(Microsoft.CodeAnalysis.CSharp.SyntaxKind.GotoCaseStatement) || @@ -84,12 +84,6 @@ private static bool ContainsConstructorExit(SyntaxNode node) descendant.IsKind(Microsoft.CodeAnalysis.CSharp.SyntaxKind.ThrowStatement)); } - private static bool ShouldDescendIntoConstructorInitializerNode(SyntaxNode node) - { - return node is not AnonymousFunctionExpressionSyntax and - not LocalFunctionStatementSyntax; - } - private static IEnumerable GetRuntimeConstructorDeclarations( INamedTypeSymbol rootType, IPropertySymbol property, diff --git a/src/ConfigContraband.Core/OptionsTypeMetadata.PolymorphicDictionary.cs b/src/ConfigContraband.Core/OptionsTypeMetadata.PolymorphicDictionary.cs index 13348f5..e4b8dd1 100644 --- a/src/ConfigContraband.Core/OptionsTypeMetadata.PolymorphicDictionary.cs +++ b/src/ConfigContraband.Core/OptionsTypeMetadata.PolymorphicDictionary.cs @@ -78,7 +78,7 @@ private static bool HasPotentialPolymorphicConstructorAssignment( } foreach (var assignment in constructor.Body - .DescendantNodes(ShouldDescendIntoConstructorInitializerNode) + .DescendantNodes(ExecutionScope.ShouldDescend) .OfType()) { if (IsPotentialPolymorphicAssignmentToProperty(assignment, property, compilation)) @@ -268,7 +268,7 @@ private static void AddPotentialPolymorphicDictionaryValueConstructorAssignmentK } foreach (var assignment in constructor.Body - .DescendantNodes(ShouldDescendIntoConstructorInitializerNode) + .DescendantNodes(ExecutionScope.ShouldDescend) .OfType()) { if (IsAssignmentToProperty(assignment, property, compilation)) @@ -291,7 +291,7 @@ private static void AddPotentialPolymorphicDictionaryValueConstructorAssignmentK } foreach (var invocation in constructor.Body - .DescendantNodes(ShouldDescendIntoConstructorInitializerNode) + .DescendantNodes(ExecutionScope.ShouldDescend) .OfType()) { AddPotentialPolymorphicDictionaryAddInvocationKey( @@ -693,22 +693,6 @@ private static ImmutableArray RemoveFirstPathSegment(ImmutableArray RemoveLastPathSegment(ImmutableArray path) - { - if (path.Length <= 1) - { - return ImmutableArray.Empty; - } - - var builder = ImmutableArray.CreateBuilder(path.Length - 1); - for (var i = 0; i < path.Length - 1; i++) - { - builder.Add(path[i]); - } - - return builder.ToImmutable(); - } - private static bool TryGetDictionaryInitializerEntry( ExpressionSyntax expression, out ExpressionSyntax? keyExpression, diff --git a/src/ConfigContraband.Core/OptionsTypeMetadata.TypeShape.cs b/src/ConfigContraband.Core/OptionsTypeMetadata.TypeShape.cs deleted file mode 100644 index 8b13789..0000000 --- a/src/ConfigContraband.Core/OptionsTypeMetadata.TypeShape.cs +++ /dev/null @@ -1 +0,0 @@ - diff --git a/src/ConfigContraband.Tool/ConfigContraband.Tool.csproj b/src/ConfigContraband.Tool/ConfigContraband.Tool.csproj index cc8ce23..9fe9d13 100644 --- a/src/ConfigContraband.Tool/ConfigContraband.Tool.csproj +++ b/src/ConfigContraband.Tool/ConfigContraband.Tool.csproj @@ -10,14 +10,15 @@ ConfigContraband.Tool - $(NoWarn);CS0618 + true + $(NoWarn);CS0618;CS1591 true configcontraband ConfigContraband.Tool - 0.7.20 + 0.7.21 George Wall ConfigContraband.Tool Generates appsettings.schema.json from your .NET Options types so editors give live autocomplete, type checking, and required-key hints while editing appsettings.json. diff --git a/src/ConfigContraband/ConfigContraband.csproj b/src/ConfigContraband/ConfigContraband.csproj index d71fa13..090c43b 100644 --- a/src/ConfigContraband/ConfigContraband.csproj +++ b/src/ConfigContraband/ConfigContraband.csproj @@ -8,12 +8,13 @@ true ConfigContraband ConfigContraband - $(NoWarn);RS1038;RS1037 + true + $(NoWarn);RS1038;RS1037;CS1591 ConfigContraband - 0.7.20 + 0.7.21 George Wall ConfigContraband Stop smuggling broken appsettings into production. ConfigContraband is a Roslyn analyzer for .NET Options and configuration correctness. @@ -24,7 +25,7 @@ https://github.com/georgepwall1991/ConfigContraband git https://github.com/georgepwall1991/ConfigContraband - Pinned CFG008 SingleConverter and DoubleConverter thousands-separator parity with runtime-backed regression coverage. + Hardening release: fixed a potential AD0001 analyzer crash in BindConfiguration registration parsing, enabled the CI analyzer gate and triaged every build warning, memoized per-compilation options metadata, bounded appsettings JSON nesting depth, removed dead code, and completed the showcase to cover all nine rules. No diagnostic behavior changes. true false true diff --git a/src/ConfigContraband/ConfigContrabandAnalyzer.BinderOptions.cs b/src/ConfigContraband/ConfigContrabandAnalyzer.BinderOptions.cs index 40e2552..97ffed4 100644 --- a/src/ConfigContraband/ConfigContrabandAnalyzer.BinderOptions.cs +++ b/src/ConfigContraband/ConfigContrabandAnalyzer.BinderOptions.cs @@ -435,7 +435,7 @@ private static bool ContainsBinderOptionsBooleanAssignment( string propertyName) { foreach (var assignment in node - .DescendantNodes(ShouldDescendIntoBinderOptionsNode) + .DescendantNodes(ExecutionScope.ShouldDescend) .OfType()) { if (IsBinderOptionsBooleanAssignmentTarget( @@ -510,7 +510,7 @@ private static bool ContainsRuntimeBinderOptionsAliasDeclaration( bool parameterStillTargetsRuntimeOptions) { foreach (var localDeclaration in node - .DescendantNodes(ShouldDescendIntoBinderOptionsNode) + .DescendantNodes(ExecutionScope.ShouldDescend) .OfType()) { foreach (var variable in localDeclaration.Declaration.Variables) @@ -629,7 +629,7 @@ private static bool ContainsBinderOptionsParameterAssignment( IParameterSymbol binderOptionsParameter) { foreach (var assignment in node - .DescendantNodes(ShouldDescendIntoBinderOptionsNode) + .DescendantNodes(ExecutionScope.ShouldDescend) .OfType()) { if (AssignmentTargetsBinderOptionsParameter(assignment.Left, semanticModel, binderOptionsParameter)) @@ -653,7 +653,7 @@ private static bool IsBinderOptionsParameterAssignmentTarget( private static bool ContainsNonLinearControlFlow(SyntaxNode node) { - foreach (var descendant in node.DescendantNodesAndSelf(ShouldDescendIntoBinderOptionsNode)) + foreach (var descendant in node.DescendantNodesAndSelf(ExecutionScope.ShouldDescend)) { if (descendant.IsKind(SyntaxKind.ReturnStatement) || descendant.IsKind(SyntaxKind.GotoStatement) || @@ -671,12 +671,6 @@ private static bool ContainsNonLinearControlFlow(SyntaxNode node) return false; } - private static bool ShouldDescendIntoBinderOptionsNode(SyntaxNode node) - { - return node is not AnonymousFunctionExpressionSyntax and - not LocalFunctionStatementSyntax; - } - private static bool IsBinderOptionsBooleanAssignmentTarget( ExpressionSyntax expression, SemanticModel semanticModel, diff --git a/src/ConfigContraband/ConfigContrabandAnalyzer.BinderOptionsEscape.cs b/src/ConfigContraband/ConfigContrabandAnalyzer.BinderOptionsEscape.cs index bd1bfd3..17e9155 100644 --- a/src/ConfigContraband/ConfigContrabandAnalyzer.BinderOptionsEscape.cs +++ b/src/ConfigContraband/ConfigContrabandAnalyzer.BinderOptionsEscape.cs @@ -17,7 +17,7 @@ private static bool ContainsRuntimeBinderOptionsEscape( bool parameterStillTargetsRuntimeOptions) { foreach (var assignment in node - .DescendantNodesAndSelf(ShouldDescendIntoBinderOptionsNode) + .DescendantNodesAndSelf(ExecutionScope.ShouldDescend) .OfType()) { if (IsRuntimeBinderOptionsReference( @@ -33,7 +33,7 @@ private static bool ContainsRuntimeBinderOptionsEscape( } foreach (var invocation in node - .DescendantNodesAndSelf(ShouldDescendIntoBinderOptionsNode) + .DescendantNodesAndSelf(ExecutionScope.ShouldDescend) .OfType()) { if (InvocationMayRunLocalBinderOptionsHelper( @@ -79,7 +79,7 @@ private static bool ContainsRuntimeBinderOptionsEscape( } foreach (var objectCreation in node - .DescendantNodesAndSelf(ShouldDescendIntoBinderOptionsNode) + .DescendantNodesAndSelf(ExecutionScope.ShouldDescend) .OfType()) { if (ContainsRuntimeBinderOptionsArgument( @@ -94,7 +94,7 @@ private static bool ContainsRuntimeBinderOptionsEscape( } foreach (var implicitObjectCreation in node - .DescendantNodesAndSelf(ShouldDescendIntoBinderOptionsNode) + .DescendantNodesAndSelf(ExecutionScope.ShouldDescend) .OfType()) { if (ContainsRuntimeBinderOptionsArgument( @@ -339,7 +339,7 @@ private static bool LocalDelegateIsReassigned( } foreach (var assignment in containingBlock - .DescendantNodes(ShouldDescendIntoBinderOptionsNode) + .DescendantNodes(ExecutionScope.ShouldDescend) .OfType()) { if (assignment.Left.SpanStart <= declaration.SpanStart) @@ -394,7 +394,7 @@ private static bool ContainsRuntimeBinderOptionsReference( bool parameterStillTargetsRuntimeOptions) { foreach (var expression in node - .DescendantNodesAndSelf(ShouldDescendIntoBinderOptionsNode) + .DescendantNodesAndSelf(ExecutionScope.ShouldDescend) .OfType()) { if (IsRuntimeBinderOptionsReference( diff --git a/src/ConfigContraband/ConfigContrabandAnalyzer.ReceiverSemantics.cs b/src/ConfigContraband/ConfigContrabandAnalyzer.ReceiverSemantics.cs index 5ba5b57..b72063e 100644 --- a/src/ConfigContraband/ConfigContrabandAnalyzer.ReceiverSemantics.cs +++ b/src/ConfigContraband/ConfigContrabandAnalyzer.ReceiverSemantics.cs @@ -207,7 +207,7 @@ private static bool MemberMayBeAssignedInConstructor(ISymbol symbol, SemanticMod foreach (var constructor in typeDeclaration.Members.OfType()) { - foreach (var assignment in constructor.DescendantNodes(ShouldDescendIntoConfigurationOriginNode) + foreach (var assignment in constructor.DescendantNodes(ExecutionScope.ShouldDescend) .OfType()) { foreach (var identifier in assignment.Left.DescendantNodesAndSelf() @@ -233,11 +233,6 @@ private static bool MemberMayBeAssignedInConstructor(ISymbol symbol, SemanticMod return false; } - private static bool ShouldDescendIntoConfigurationOriginNode(SyntaxNode node) - { - return node is not AnonymousFunctionExpressionSyntax and not LocalFunctionStatementSyntax; - } - private static ConfigurationReceiverProvenance ClassifyLocalConfiguration( ILocalSymbol local, ExpressionSyntax useExpression, @@ -339,128 +334,111 @@ private static bool HasUnsafeConfigurationUse( return false; } - foreach (var assignment in block.DescendantNodes() - .OfType() - .Where(candidate => candidate.SpanStart >= startPosition && - candidate.Span.End <= safetyUntilPosition)) + // One traversal instead of seven: each node kind below was previously scanned in its own + // full DescendantNodes pass over the block. The checks are a pure OR with no shared state, + // so folding them into a single document-order walk yields the same result while visiting + // the tree once — the dominant cost when a minimal-hosting Program.cs is one large block. + foreach (var node in block.DescendantNodes()) { - if (SymbolEqualityComparer.Default.Equals( - semanticModel.GetSymbolInfo(assignment.Left).Symbol, - symbol)) + if (node.SpanStart < startPosition || node.Span.End > safetyUntilPosition) { - if (assignment.SpanStart >= resolutionPosition || - symbol is ILocalSymbol && - assignment.Parent is ExpressionStatementSyntax { Parent: BlockSyntax parentBlock } && - parentBlock == block && - assignment.IsKind(SyntaxKind.SimpleAssignmentExpression)) - { - continue; - } - - return true; + continue; } - if (IsExpressionRootedInSymbol(assignment.Left, symbol, semanticModel) || - assignment.Right.SpanStart != resolutionPosition && - ReferencesSymbol(assignment.Right, symbol, semanticModel)) + switch (node) { - if (assignment.Left is IdentifierNameSyntax { Identifier.ValueText: "_" } && - semanticModel.GetSymbolInfo(assignment.Left).Symbol is null or IDiscardSymbol && - IsReadOnlyFrameworkConfigurationExpression(assignment.Right, semanticModel)) - { - continue; - } - - return true; + case AssignmentExpressionSyntax assignment + when IsUnsafeConfigurationAssignment(assignment, symbol, semanticModel, block, resolutionPosition): + case InvocationExpressionSyntax invocation + when IsUnsafeConfigurationInvocation(invocation, symbol, semanticModel): + case ObjectCreationExpressionSyntax creation + when creation.ArgumentList?.Arguments.Any(argument => + ReferencesSymbol(argument.Expression, symbol, semanticModel)) == true: + case VariableDeclaratorSyntax declarator + when declarator.Initializer?.Value is { } initializer && + initializer.SpanStart != resolutionPosition && + ReferencesSymbol(initializer, symbol, semanticModel): + case AnonymousFunctionExpressionSyntax anonymousFunction + when ReferencesSymbol(anonymousFunction, symbol, semanticModel): + case LocalFunctionStatementSyntax localFunction + when ReferencesSymbol(localFunction, symbol, semanticModel): + case ReturnStatementSyntax { Expression: { } returned } + when ReferencesSymbol(returned, symbol, semanticModel): + return true; } } - foreach (var invocation in block.DescendantNodes() - .OfType() - .Where(candidate => candidate.SpanStart >= startPosition && - candidate.Span.End <= safetyUntilPosition)) - { - if (IsReadOnlyFrameworkConfigurationInvocation(invocation, semanticModel)) - { - continue; - } - - if (invocation.Expression is MemberAccessExpressionSyntax memberAccess && - IsExpressionRootedInSymbol(memberAccess.Expression, symbol, semanticModel)) - { - return true; - } + return false; + } - if (invocation.Ancestors().OfType() - .Any(conditionalAccess => - IsExpressionRootedInSymbol( - GetConfigurationChainRoot(conditionalAccess.Expression, semanticModel), - symbol, - semanticModel))) + private static bool IsUnsafeConfigurationAssignment( + AssignmentExpressionSyntax assignment, + ISymbol symbol, + SemanticModel semanticModel, + BlockSyntax block, + int resolutionPosition) + { + if (SymbolEqualityComparer.Default.Equals( + semanticModel.GetSymbolInfo(assignment.Left).Symbol, + symbol)) + { + if (assignment.SpanStart >= resolutionPosition || + symbol is ILocalSymbol && + assignment.Parent is ExpressionStatementSyntax { Parent: BlockSyntax parentBlock } && + parentBlock == block && + assignment.IsKind(SyntaxKind.SimpleAssignmentExpression)) { - return true; + return false; } - if (invocation.ArgumentList.Arguments.Any(argument => - ReferencesSymbol(argument.Expression, symbol, semanticModel))) - { - return true; - } + return true; } - foreach (var creation in block.DescendantNodes() - .OfType() - .Where(candidate => candidate.SpanStart >= startPosition && - candidate.Span.End <= safetyUntilPosition)) + if (IsExpressionRootedInSymbol(assignment.Left, symbol, semanticModel) || + assignment.Right.SpanStart != resolutionPosition && + ReferencesSymbol(assignment.Right, symbol, semanticModel)) { - if (creation.ArgumentList?.Arguments.Any(argument => - ReferencesSymbol(argument.Expression, symbol, semanticModel)) == true) + if (assignment.Left is IdentifierNameSyntax { Identifier.ValueText: "_" } && + semanticModel.GetSymbolInfo(assignment.Left).Symbol is null or IDiscardSymbol && + IsReadOnlyFrameworkConfigurationExpression(assignment.Right, semanticModel)) { - return true; + return false; } + + return true; } - foreach (var declarator in block.DescendantNodes() - .OfType() - .Where(candidate => candidate.SpanStart >= startPosition && - candidate.Span.End <= safetyUntilPosition)) + return false; + } + + private static bool IsUnsafeConfigurationInvocation( + InvocationExpressionSyntax invocation, + ISymbol symbol, + SemanticModel semanticModel) + { + if (IsReadOnlyFrameworkConfigurationInvocation(invocation, semanticModel)) { - if (declarator.Initializer?.Value is { } initializer && - initializer.SpanStart != resolutionPosition && - ReferencesSymbol(initializer, symbol, semanticModel)) - { - return true; - } + return false; } - foreach (var anonymousFunction in block.DescendantNodes() - .OfType() - .Where(candidate => candidate.SpanStart >= startPosition && - candidate.Span.End <= safetyUntilPosition)) + if (invocation.Expression is MemberAccessExpressionSyntax memberAccess && + IsExpressionRootedInSymbol(memberAccess.Expression, symbol, semanticModel)) { - if (ReferencesSymbol(anonymousFunction, symbol, semanticModel)) - { - return true; - } + return true; } - foreach (var localFunction in block.DescendantNodes() - .OfType() - .Where(candidate => candidate.SpanStart >= startPosition && - candidate.Span.End <= safetyUntilPosition)) + if (invocation.Ancestors().OfType() + .Any(conditionalAccess => + IsExpressionRootedInSymbol( + GetConfigurationChainRoot(conditionalAccess.Expression, semanticModel), + symbol, + semanticModel))) { - if (ReferencesSymbol(localFunction, symbol, semanticModel)) - { - return true; - } + return true; } - return block.DescendantNodes() - .OfType() - .Where(candidate => candidate.SpanStart >= startPosition && - candidate.Span.End <= safetyUntilPosition) - .Any(candidate => candidate.Expression is { } returned && - ReferencesSymbol(returned, symbol, semanticModel)); + return invocation.ArgumentList.Arguments.Any(argument => + ReferencesSymbol(argument.Expression, symbol, semanticModel)); } private static bool IsExpressionRootedInSymbol( diff --git a/src/ConfigContraband/ConfigContrabandAnalyzer.Registration.cs b/src/ConfigContraband/ConfigContrabandAnalyzer.Registration.cs index 92806b5..5fd1b90 100644 --- a/src/ConfigContraband/ConfigContrabandAnalyzer.Registration.cs +++ b/src/ConfigContraband/ConfigContrabandAnalyzer.Registration.cs @@ -60,10 +60,19 @@ private static bool TryCreateOptionsBuilderRegistration( bool sectionExpressionContainsFullPath; if (string.Equals(methodName, "BindConfiguration", StringComparison.Ordinal)) { - var operation = (IInvocationOperation)semanticModel.GetOperation(invocation)!; - var sectionArgument = operation.Arguments.First(argument => - string.Equals(argument.Parameter!.Name, "configSectionPath", StringComparison.Ordinal)); - sectionExpression = (ExpressionSyntax)sectionArgument.Value.Syntax; + if (semanticModel.GetOperation(invocation) is not IInvocationOperation operation) + { + return false; + } + + var sectionArgument = operation.Arguments.FirstOrDefault(argument => + string.Equals(argument.Parameter?.Name, "configSectionPath", StringComparison.Ordinal)); + if (sectionArgument?.Value.Syntax is not ExpressionSyntax argumentExpression) + { + return false; + } + + sectionExpression = argumentExpression; if (!TryGetConstantSectionPath(sectionExpression, semanticModel, out sectionPath)) { return false; @@ -89,8 +98,9 @@ private static bool TryCreateOptionsBuilderRegistration( } var chain = InvocationChain.Create(invocation, semanticModel, methodName); - var hasValidateOnStart = chain.MethodNames.Contains("ValidateOnStart") || - HasAddOptionsWithValidateOnStartReceiver(invocation, semanticModel); + var hasAddOptionsWithValidateOnStart = HasAddOptionsWithValidateOnStartReceiver(invocation, semanticModel); + var hasValidateOnStart = chain.MethodNames.Contains("ValidateOnStart") || hasAddOptionsWithValidateOnStart; + var hasValidation = chain.MethodNames.Any(IsValidationMethod) || hasAddOptionsWithValidateOnStart; var bindsNonPublicProperties = HasBindNonPublicPropertiesEnabled(invocation, semanticModel); var errorsOnUnknownConfiguration = HasErrorOnUnknownConfigurationEnabled(invocation, semanticModel); var supportsValidationRules = true; @@ -103,11 +113,11 @@ private static bool TryCreateOptionsBuilderRegistration( supportsValidationRules, sectionExpressionContainsFullPath, chain.MethodNames.Contains("ValidateDataAnnotations"), - chain.MethodNames.Contains("ValidateOnStart") || HasAddOptionsWithValidateOnStartReceiver(invocation, semanticModel), - chain.MethodNames.Any(IsValidationMethod) || HasAddOptionsWithValidateOnStartReceiver(invocation, semanticModel), + hasValidateOnStart, + hasValidation, bindsNonPublicProperties, errorsOnUnknownConfiguration, - !supportsValidationRules || chain.MethodNames.Contains("ValidateDataAnnotations"), + chain.MethodNames.Contains("ValidateDataAnnotations"), sectionExpression.GetLocation(), RequiresRuntimeSection(sectionExpression, semanticModel)); return true; @@ -422,7 +432,7 @@ private static IEnumerable GetSameExecutableScopeInv if (expressionBody is not null) { foreach (var invocation in expressionBody - .DescendantNodesAndSelf(ShouldDescendIntoSameExecutableScope) + .DescendantNodesAndSelf(ExecutionScope.ShouldDescend) .OfType()) { yield return invocation; @@ -449,19 +459,13 @@ private static IEnumerable GetTopLevelStatementInvoc } foreach (var invocation in scanRoot - .DescendantNodesAndSelf(ShouldDescendIntoSameExecutableScope) + .DescendantNodesAndSelf(ExecutionScope.ShouldDescend) .OfType()) { yield return invocation; } } - private static bool ShouldDescendIntoSameExecutableScope(SyntaxNode node) - { - return node is not LocalFunctionStatementSyntax && - node is not AnonymousFunctionExpressionSyntax; - } - private static bool TryGetOptionsBuilderFactoryTarget( ExpressionSyntax expression, SemanticModel semanticModel, diff --git a/src/ConfigContraband/ConfigContrabandAnalyzer.SectionPath.cs b/src/ConfigContraband/ConfigContrabandAnalyzer.SectionPath.cs index 2a6560b..6bb7b41 100644 --- a/src/ConfigContraband/ConfigContrabandAnalyzer.SectionPath.cs +++ b/src/ConfigContraband/ConfigContrabandAnalyzer.SectionPath.cs @@ -336,7 +336,7 @@ private static ExpressionSyntax UnwrapForSectionChainResolution(ExpressionSyntax /// /// Resolves a conditional-access `WhenNotNull` expression (the part after `?.`) the same way - /// resolves a normal invocation chain, without + /// TryGetConfigurationSectionPath resolves a normal invocation chain, without /// constructing any new syntax nodes: a `?.`-bound /// implicitly receives (the expression before `?.`), /// so it is resolved against that receiver directly instead of being treated as a detached diff --git a/src/ConfigContraband/ConfigContrabandAnalyzer.UnknownKeys.cs b/src/ConfigContraband/ConfigContrabandAnalyzer.UnknownKeys.cs index 8f29ad2..0580f94 100644 --- a/src/ConfigContraband/ConfigContrabandAnalyzer.UnknownKeys.cs +++ b/src/ConfigContraband/ConfigContrabandAnalyzer.UnknownKeys.cs @@ -207,7 +207,7 @@ private static void AnalyzeUnknownKeysInSection( if (metadata.TryGetSettableConstructorBoundAlias(property.Key, section, out var constructorAliasProperty)) { if (reportStrictUnknownKeys && - metadata.IsConfigurationAlias(constructorAliasProperty, property.Key)) + OptionsTypeMetadata.IsConfigurationAlias(constructorAliasProperty, property.Key)) { ReportUnknownConfigurationKey( reportDiagnostic, @@ -262,7 +262,7 @@ clrProperty is not null && } if (reportStrictUnknownKeys && - metadata.IsConfigurationAlias(bindableProperty, property.Key)) + OptionsTypeMetadata.IsConfigurationAlias(bindableProperty, property.Key)) { ReportUnknownConfigurationKey( reportDiagnostic, @@ -777,19 +777,6 @@ private static bool IsUserDefinedReferenceObject(ITypeSymbol type) !namespaceName.StartsWith("System.", StringComparison.Ordinal); } - private static bool ContainsName(ImmutableArray names, string key) - { - foreach (var name in names) - { - if (string.Equals(name, key, StringComparison.OrdinalIgnoreCase)) - { - return true; - } - } - - return false; - } - private static void ReportUnknownConfigurationKey( Action reportDiagnostic, ConcurrentDictionary unknownKeysReported, diff --git a/src/ConfigContraband/ConfigContrabandAnalyzer.cs b/src/ConfigContraband/ConfigContrabandAnalyzer.cs index 4be567c..f58c569 100644 --- a/src/ConfigContraband/ConfigContrabandAnalyzer.cs +++ b/src/ConfigContraband/ConfigContrabandAnalyzer.cs @@ -152,40 +152,6 @@ private static bool HasSeverityNoneKey(AnalyzerConfigOptions options, string key string.Equals(severity, "none", StringComparison.OrdinalIgnoreCase); } - private static bool IsGeneratedSyntaxTree(SyntaxTree syntaxTree, SyntaxNode root) - { - var fileName = System.IO.Path.GetFileName(syntaxTree.FilePath); - if (fileName.EndsWith(".g.cs", StringComparison.OrdinalIgnoreCase) || - fileName.EndsWith(".generated.cs", StringComparison.OrdinalIgnoreCase) || - fileName.EndsWith(".designer.cs", StringComparison.OrdinalIgnoreCase)) - { - return true; - } - - foreach (var trivia in root.GetLeadingTrivia()) - { - if (trivia.IsKind(SyntaxKind.SingleLineCommentTrivia) || - trivia.IsKind(SyntaxKind.MultiLineCommentTrivia)) - { - var text = trivia.ToString(); - if (text.IndexOf("= 0) - { - return true; - } - } - - if (!trivia.IsKind(SyntaxKind.WhitespaceTrivia) && - !trivia.IsKind(SyntaxKind.EndOfLineTrivia) && - !trivia.IsKind(SyntaxKind.SingleLineCommentTrivia) && - !trivia.IsKind(SyntaxKind.MultiLineCommentTrivia)) - { - break; - } - } - - return false; - } - private static void AnalyzeRegistrationChain( Action reportDiagnostic, OptionsRegistration registration, @@ -673,7 +639,7 @@ private static bool StatementRetargetsLocal( // Do not descend into lambda or local-function bodies: an assignment there // is deferred until the delegate is invoked, so it does not retarget the // local at this point in the straight-line flow. - foreach (var node in statement.DescendantNodesAndSelf(ShouldDescendIntoBinderOptionsNode)) + foreach (var node in statement.DescendantNodesAndSelf(ExecutionScope.ShouldDescend)) { switch (node) { diff --git a/src/ConfigContraband/ConfigContrabandCodeFixProvider.cs b/src/ConfigContraband/ConfigContrabandCodeFixProvider.cs index afdce76..5cf64de 100644 --- a/src/ConfigContraband/ConfigContrabandCodeFixProvider.cs +++ b/src/ConfigContraband/ConfigContrabandCodeFixProvider.cs @@ -26,6 +26,8 @@ public sealed class ConfigContrabandCodeFixProvider : CodeFixProvider public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + private static readonly string[] ValidateOnStartInvocation = { "ValidateOnStart" }; + public override Task RegisterCodeFixesAsync(CodeFixContext context) { foreach (var diagnostic in context.Diagnostics) @@ -41,7 +43,7 @@ public override Task RegisterCodeFixesAsync(CodeFixContext context) context.RegisterCodeFix( CodeAction.Create( "Add ValidateOnStart()", - cancellationToken => AppendInvocationsAsync(context.Document, diagnostic.Location.SourceSpan, new[] { "ValidateOnStart" }, cancellationToken), + cancellationToken => AppendInvocationsAsync(context.Document, diagnostic.Location.SourceSpan, ValidateOnStartInvocation, cancellationToken), equivalenceKey: "AddValidateOnStart"), diagnostic); break; @@ -269,7 +271,9 @@ private static async Task AddRecursiveValidationAttributeAsync( string attributeName, CancellationToken cancellationToken) { - var location = diagnostic.AdditionalLocations.FirstOrDefault() ?? diagnostic.Location; + var location = diagnostic.AdditionalLocations.Count > 0 + ? diagnostic.AdditionalLocations[0] + : diagnostic.Location; var targetDocument = location.SourceTree is null ? document : document.Project.Solution.GetDocument(location.SourceTree) ?? document; diff --git a/tests/ConfigContraband.Core.Tests/ScalarConversionTests.cs b/tests/ConfigContraband.Core.Tests/ScalarConversionTests.cs index 466d6d5..4aa39bc 100644 --- a/tests/ConfigContraband.Core.Tests/ScalarConversionTests.cs +++ b/tests/ConfigContraband.Core.Tests/ScalarConversionTests.cs @@ -344,7 +344,7 @@ private static ITypeSymbol ResolveType(string key) }; } - private static Compilation CreateCompilation() + private static CSharpCompilation CreateCompilation() { var references = ((string)AppContext.GetData("TRUSTED_PLATFORM_ASSEMBLIES")!) .Split(Path.PathSeparator)