From 0894b38e0285011d655c10fb40a6b7b8e730f241 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Sun, 29 Mar 2026 22:47:37 +0000 Subject: [PATCH 1/6] Add framework extensibility and architecture review Deep analysis of modularity, extensibility bottlenecks, and architectural issues across Core, Generator, and Host layers. Covers 18 findings with priority matrix and recommendations. --- tasks/framework-extensibility-review.md | 312 ++++++++++++++++++++++++ 1 file changed, 312 insertions(+) create mode 100644 tasks/framework-extensibility-review.md diff --git a/tasks/framework-extensibility-review.md b/tasks/framework-extensibility-review.md new file mode 100644 index 00000000..5f9f092a --- /dev/null +++ b/tasks/framework-extensibility-review.md @@ -0,0 +1,312 @@ +# Framework Extensibility & Architecture Review + +> Deep review of SimpleModule's modularity, extensibility, and architectural soundness. +> Extends the findings in `tasks/module-design-review.md` with additional depth. + +--- + +## Executive Summary + +SimpleModule's source generator is genuinely impressive — 6-line `Program.cs`, compile-time module discovery, automatic contract wiring, TypeScript generation, and unified DbContext creation. The framework delivers on "convention over configuration" for the happy path. + +However, the framework has **extensibility bottlenecks** that will compound as the module count grows. The issues fall into three categories: + +1. **Closed extension points** — enums, hardcoded lifetimes, and static calls that cannot be overridden +2. **Generator scalability** — non-incremental compilation, namespace heuristics, missing source locations on diagnostics +3. **Coupling at boundaries** — Inertia serialization bypass, unified DbContext, sequential event dispatch + +--- + +## Critical Architectural Issues + +### 1. Source Generator Is Not Truly Incremental + +**Files:** `framework/SimpleModule.Generator/ModuleDiscovererGenerator.cs`, `Discovery/SymbolDiscovery.cs` + +The generator uses `CompilationProvider.Select()` which fires on **every** compilation change. `SymbolDiscovery.Extract()` performs a full recursive walk of all types in all referenced assemblies on every keystroke-triggered recompile. + +```csharp +var dataProvider = context.CompilationProvider.Select( + static (compilation, _) => SymbolDiscovery.Extract(compilation) +); +``` + +The correct incremental pattern would use `SyntaxProvider.ForAttributeWithMetadataName()` to react only when `[Module]`, `[Dto]`, or `[ViewPage]` attributes change. The `DiscoveryData.Equals()` implementation provides downstream caching (emitters skip if data is unchanged), but the expensive discovery traversal itself always runs. + +**Impact:** With 10-15 modules and hundreds of types, this is O(total types across all assemblies) per compile. Developers will notice degraded IDE responsiveness as the solution grows. + +**Recommendation:** Refactor the pipeline to use attribute-based `SyntaxProvider` filtering. Scan only assemblies that contain `[Module]`-attributed types (detectable via assembly-level attributes or a marker). + +--- + +### 2. IModule Is a Wide Interface (ISP Violation) + +**File:** `framework/SimpleModule.Core/IModule.cs` + +`IModule` conflates 9 concerns into one interface: services, routing, middleware, menu, permissions, settings, startup, shutdown, and health. Default interface methods mitigate boilerplate, but this design has real costs: + +- **Adding a new lifecycle hook** requires adding a method to `IModule`, a detection boolean to `ModuleInfoRecord` in the generator, and a call site in an emitter. This is a cross-cutting change across three layers for what should be a single extension. +- **Modules silently ignore new hooks.** When a 10th method is added to `IModule`, existing compiled modules just get the default no-op. There is no signal that a module should consider the new hook. +- **The generator must track which methods each module overrides** via `DeclaresMethod()` reflection, creating tight coupling between the interface shape and the generator's code. + +**Recommendation:** Consider decomposing into focused interfaces (`IModuleServices`, `IModuleMiddleware`, `IModuleMenu`, etc.) that modules opt into. The generator already tracks which methods are overridden — this would formalize it. Alternatively, use a registration-based pattern where modules register capabilities rather than implementing a mega-interface. + +--- + +### 3. All Diagnostics Lack Source Locations + +**File:** `framework/SimpleModule.Generator/Emitters/DiagnosticEmitter.cs` + +All 30+ SM diagnostics (SM0001-SM0044) use `Location.None`: + +```csharp +context.ReportDiagnostic(Diagnostic.Create(descriptor, Location.None, ...)); +``` + +This means IDE error squiggles never appear on the offending line. All errors show as project-level messages in the Error List. The generator **has** the `INamedTypeSymbol` at discovery time and could attach `symbol.Locations.FirstOrDefault()` for source-defined types. + +**Impact:** Developers must manually search for the problematic type/endpoint when a diagnostic fires. For SM0015 (duplicate view page name) or SM0033 (duplicate permission), this can be genuinely difficult with 10+ modules. + +**Recommendation:** Attach `Location` to every diagnostic where the originating symbol has syntax references. For metadata-only symbols (from compiled assemblies), `Location.None` is unavoidable, but most module code is source-referenced during development. + +--- + +### 4. Module-to-Endpoint Ownership Uses a Fragile Namespace Heuristic + +**File:** `framework/SimpleModule.Generator/Discovery/SymbolDiscovery.cs` — `FindClosestModuleName()` + +Endpoint classes are assigned to modules by **longest namespace prefix match**. If no prefix matches, the fallback is `modules[0].ModuleName` — the first discovered module, which is non-deterministic. + +**Scenarios that break:** +- Two modules with overlapping namespace prefixes (e.g., `SimpleModule.Products` and `SimpleModule.ProductReviews`) — endpoints in `SimpleModule.ProductReviews.Endpoints` could match `Products` if `ProductReviews` isn't discovered yet. +- An endpoint in an unconventional namespace silently falls to the first module with no diagnostic. + +**Recommendation:** Require endpoint classes to be in or under the module's declared namespace, and emit a diagnostic when the heuristic falls back to `modules[0]`. + +--- + +### 5. Contract Implementations Are Hardcoded to Scoped Lifetime + +**File:** `framework/SimpleModule.Generator/Emitters/ModuleExtensionsEmitter.cs` + +The generator always emits: +```csharp +services.AddScoped(); +``` + +There is no attribute or convention to request `Singleton` or `Transient` lifetime. A module author who needs a singleton service (e.g., a cache, a connection pool wrapper) must bypass auto-discovery entirely and register manually in `ConfigureServices`. + +**Impact:** This forces a choice: use auto-discovery with scoped lifetime, or opt out completely. There's no middle ground. + +**Recommendation:** Support a `[ContractLifetime(ServiceLifetime.Singleton)]` attribute on the implementation class that the generator reads. + +--- + +### 6. Inertia Serialization Bypasses DI-Registered JSON Options + +**Files:** `framework/SimpleModule.Core/Inertia/InertiaResult.cs` + +`InertiaResult` creates its own private `JsonSerializerOptions` with camelCase naming: + +```csharp +private static readonly JsonSerializerOptions _camelCaseOptions = new() +{ + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + // ... +}; +``` + +This **does not** include the `ModulesJsonResolver` registered by `AddModules()`. Consequences: +- Custom JSON converters (e.g., Vogen value object converters) registered in `ConfigureHttpJsonOptions` do not apply to Inertia responses. +- A DTO serialized via an API endpoint and via an Inertia view endpoint can produce different JSON. +- The comment explains this is intentional for Vogen unwrapping consistency, but it creates a split serialization path. + +**Recommendation:** Resolve `JsonSerializerOptions` from DI (via `IOptions`) and merge the camelCase policy, rather than maintaining a separate static instance. + +--- + +### 7. MenuSection Is a Closed Enum + +**File:** `framework/SimpleModule.Core/Menu/MenuSection.cs` + +```csharp +public enum MenuSection { Navbar, UserDropdown, AdminSidebar, AppSidebar } +``` + +Adding a new section (e.g., `Footer`, `SettingsSidebar`, `MobileNav`) requires modifying the Core enum — a breaking change for all compiled modules. + +Additionally, `MenuItem.RequiresAuth` is a boolean, not a policy reference. There's no way to tie menu visibility to a permission check — the UI must manually filter based on user claims. + +**Recommendation:** Replace the enum with a string-based section key or an open `MenuSection` record type. Add an optional `RequiredPermission` string to `MenuItem` that the framework evaluates at render time. + +--- + +### 8. InertiaSharedData Has No Namespace Isolation + +**File:** `framework/SimpleModule.Core/Inertia/InertiaSharedData.cs` + +`InertiaSharedData` is a scoped key-value store (`Dictionary`) that any middleware or endpoint can write to. Two modules setting the same key (e.g., `"notifications"`, `"user"`) silently overwrite each other. + +**Recommendation:** Namespace shared data keys by module name, or provide a typed API (`SetSharedData(key, value)`) that prevents collisions. + +--- + +### 9. Event Bus Has No Middleware/Pipeline Support + +**Files:** `framework/SimpleModule.Core/Events/EventBus.cs`, `BackgroundEventDispatcher.cs` + +The event bus dispatches directly to handlers with no interception points. There's no way to add: +- Logging/tracing around all event handlers +- Retry policies for failed handlers +- Metrics collection +- Transaction boundaries + +The AuditLogs module works around this by **decorating** `IEventBus` itself (`AuditingEventBus`), but this is a one-off pattern — there's no composable pipeline. + +Additionally, `BackgroundEventChannel` uses an **unbounded channel** with no backpressure. Under load, memory grows without limit. Background dispatch swallows all exceptions (logged only). + +**Recommendation:** Add an `IEventMiddleware` or `IEventPipelineBehavior` pattern (similar to MediatR's pipeline). Consider bounded channels with configurable capacity. + +--- + +### 10. No Conditional Module Registration + +**File:** `framework/SimpleModule.Generator/Emitters/ModuleExtensionsEmitter.cs` + +All discovered modules are always registered. There is no feature flag, environment check, or configuration mechanism to exclude a module at runtime. The only way to remove a module is to remove its `ProjectReference`. + +**Impact:** Cannot disable a module for specific environments (e.g., disable `Admin` in production, disable `AuditLogs` in development). Cannot do A/B testing of module implementations. + +**Recommendation:** Support a module enable/disable configuration (e.g., `SimpleModule:Modules:AuditLogs:Enabled = false`) that the generated `AddModules()` checks before calling `ConfigureServices`. + +--- + +## Significant Design Concerns + +### 11. Test Infrastructure Requires Manual Updates Per Module + +**File:** `tests/SimpleModule.Tests.Shared/Fixtures/SimpleModuleWebApplicationFactory.cs` + +The test factory explicitly lists every module's `DbContext` for SQLite replacement. Adding a new module requires manually adding `ReplaceDbContext()` and `EnsureCreated()` calls. This is the only part of the framework without auto-discovery. + +**Recommendation:** The generator could emit a helper listing all module DbContext types that the test factory reads automatically. + +--- + +### 12. GlobalExceptionHandler Is Sealed and Not Extensible + +**File:** `framework/SimpleModule.Core/` (exception handling) + +`GlobalExceptionHandler` maps domain exceptions to HTTP status codes via a pattern-match switch. Modules cannot contribute additional exception-to-response mappings without modifying Core. A module with a custom exception type (e.g., `RateLimitedException`) must either use one of the existing base exceptions or accept a generic 500 response. + +**Recommendation:** Allow modules to register `IExceptionMapper` implementations that the handler consults before its default switch. + +--- + +### 13. Endpoint Classes Cannot Use Constructor Injection + +**Files:** `framework/SimpleModule.Core/IEndpoint.cs`, Generator `EndpointExtensionsEmitter.cs` + +Endpoints are instantiated with `new EndpointClass()` — no DI involvement. Services must be injected via Minimal API parameter binding inside the `Map` method's lambda. This is consistent with ASP.NET Minimal API patterns but means: +- No shared cross-cutting logic via an injected base service +- No endpoint-level middleware or filter extension point expressible via the interface +- `[AllowAnonymous]` and `[RequirePermission]` attributes are discovered by the generator for diagnostics but **not enforced in generated code** — the endpoint must still call these methods inside `Map()` + +**Recommendation:** Consider resolving endpoints from DI (`ActivatorUtilities.CreateInstance`) to enable constructor injection. Alternatively, have the generator automatically apply `[AllowAnonymous]` and `.RequirePermission()` based on discovered attributes, removing the manual step. + +--- + +### 14. TypeScript Generation Has Gaps + +**Files:** `framework/SimpleModule.Generator/Emitters/TypeScriptDefinitionsEmitter.cs`, `Helpers/TypeMappingHelpers.cs` + +- **Enums map to `any`**, losing type safety in the frontend +- **Generic DTO types** produce incorrect TypeScript +- **Inheritance** is not handled — base class properties are not included +- **Module name derivation** assumes `SimpleModule.{ModuleName}.Contracts.{Type}` namespace convention — deviations produce wrong grouping +- The TypeScript is embedded inside `#if SIMPLEMODULE_TS` / `#endif` in a C# comment block, requiring an external tool (`extract-ts-types.mjs`) to parse it out + +**Recommendation:** Support enum-to-union-type mapping. Handle base class property inheritance. Validate module name derivation and emit a diagnostic on mismatch. + +--- + +### 15. Result and Exceptions Coexist Without Clear Boundaries + +**File:** `framework/SimpleModule.Core/` (Result type and exception types) + +Core provides both `Result` (functional) and domain exceptions (`NotFoundException`, `ValidationException`). There's no enforced convention about which to use when: +- `GlobalExceptionHandler` handles exceptions +- `Result` must be manually unwrapped by endpoint code +- Modules mix both approaches inconsistently + +**Recommendation:** Establish a clear convention: exceptions for truly exceptional failures, `Result` for expected business outcomes. Consider a `Results.ToResponse()` helper that maps `Result` to `IResult` automatically. + +--- + +### 16. SettingDefinition Is Mutable + +**File:** `framework/SimpleModule.Core/` (Settings) + +`SettingDefinition` is a class with public setters on all properties. Any code holding a reference can mutate it after registration. Since settings definitions are collected once at startup and stored in a singleton registry, mutation after registration corrupts the global state. + +**Recommendation:** Make `SettingDefinition` a record or seal it with init-only properties. + +--- + +### 17. `ConfigureEndpoints` Escape Hatch Is All-or-Nothing + +**File:** `framework/SimpleModule.Generator/Emitters/EndpointExtensionsEmitter.cs` + +If a module declares `ConfigureEndpoints`, the generator skips **all** auto-registration for that module's endpoints. You cannot mix auto-registered and manually registered endpoints in the same module. + +**Recommendation:** Change the escape hatch to be additive — auto-register discovered endpoints first, then call `ConfigureEndpoints` for any additional custom routes. + +--- + +### 18. SM0038 Detection Is Too Coarse + +**File:** `framework/SimpleModule.Generator/Emitters/DiagnosticEmitter.cs` + +The diagnostic for "infrastructure type in Contracts assembly" checks if the FQN **contains the substring `"DbContext"`**. This false-positives on legitimate types like `DbContextSummaryDto` or `UserDbContextOptions`. + +**Recommendation:** Check for inheritance from `Microsoft.EntityFrameworkCore.DbContext` rather than string matching. + +--- + +## Summary: Priority Matrix + +| Priority | Issue | Impact | Effort | +|----------|-------|--------|--------| +| **P0** | #1 Generator not incremental | Compile-time degrades with scale | High | +| **P0** | #3 Diagnostics lack source locations | Developer experience | Medium | +| **P1** | #2 IModule ISP violation | Extensibility ceiling | High | +| **P1** | #6 Inertia serialization bypass | Silent data inconsistencies | Low | +| **P1** | #5 Hardcoded Scoped lifetime | Forces manual workarounds | Low | +| **P1** | #9 Event bus no pipeline | Cross-cutting concern gap | Medium | +| **P1** | #13 Endpoints not DI-resolved | Attributes not enforced in codegen | Medium | +| **P2** | #4 Namespace heuristic fallback | Silent misassignment | Low | +| **P2** | #7 Closed MenuSection enum | Breaking change to extend | Low | +| **P2** | #10 No conditional modules | Cannot disable per environment | Low | +| **P2** | #11 Test factory manual updates | New module onboarding friction | Low | +| **P2** | #12 Exception handler sealed | Module-specific errors map to 500 | Low | +| **P2** | #14 TypeScript gaps (enums, inheritance) | Frontend type safety holes | Medium | +| **P2** | #17 ConfigureEndpoints all-or-nothing | Limits mixing strategies | Low | +| **P3** | #8 InertiaSharedData no namespacing | Key collision risk | Low | +| **P3** | #15 Result vs exceptions ambiguity | Inconsistent patterns | Low | +| **P3** | #16 SettingDefinition mutable | Post-registration corruption risk | Low | +| **P3** | #18 SM0038 substring match | False positive diagnostics | Low | + +--- + +## What the Framework Gets Right + +These are genuine architectural strengths worth preserving: + +1. **6-line Program.cs** — The generated `AddSimpleModule()` / `UseSimpleModule()` facade is excellent developer experience +2. **Topological module ordering** — Kahn's algorithm with cycle detection (SM0010) is robust +3. **Contract auto-registration** with validation (SM0025/SM0026/SM0028/SM0029) provides compile-time safety +4. **SM0011 cross-module reference detection** — enforces the contracts boundary pattern +5. **AuditLogs as a decorator** — demonstrates the cross-cutting concern pattern working correctly without other modules changing +6. **Schema isolation per module** — PostgreSQL schemas / SQLite prefixes per module entity +7. **The emitter pattern** (`IEmitter` array) — adding new code generation concerns is clean and extensible +8. **Vite library mode per module** — each module builds independently, externals are shared From ecb3f075ff9b76b0a391f75fec2a8b50f98071b0 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Tue, 31 Mar 2026 20:38:42 +0000 Subject: [PATCH 2/6] Fix P0: Add source locations to diagnostics and thread CancellationToken through generator Two critical improvements to the source generator: 1. **Diagnostic source locations**: All SM diagnostics (SM0001-SM0044) now include source file locations when available, enabling IDE error squiggles and click-to-navigate. Added SourceLocationRecord (serializable for incremental caching) to all discovery records, extracted via GetSourceLocation from ISymbol.Locations during discovery. 2. **CancellationToken support**: Threaded CancellationToken through SymbolDiscovery.Extract and all recursive Find* methods. The IDE can now cancel stale discovery work when a new compilation is triggered, preventing wasted computation on every keystroke. --- .../Discovery/DiscoveryData.cs | 84 +++++++--- .../Discovery/SymbolDiscovery.cs | 149 ++++++++++++++---- .../Emitters/DiagnosticEmitter.cs | 104 ++++++++---- .../Emitters/HostDbContextEmitter.cs | 25 ++- .../ModuleDiscovererGenerator.cs | 5 +- .../TopologicalSortTests.cs | 21 ++- 6 files changed, 296 insertions(+), 92 deletions(-) diff --git a/framework/SimpleModule.Generator/Discovery/DiscoveryData.cs b/framework/SimpleModule.Generator/Discovery/DiscoveryData.cs index da03207b..53767e6e 100644 --- a/framework/SimpleModule.Generator/Discovery/DiscoveryData.cs +++ b/framework/SimpleModule.Generator/Discovery/DiscoveryData.cs @@ -15,6 +15,19 @@ internal static int HashArray(int hash, ImmutableArray items) } } +/// +/// Serializable source location for incremental caching. +/// Unlike , this record is fully equatable +/// and does not hold references to syntax trees, making it safe for incremental pipelines. +/// +internal readonly record struct SourceLocationRecord( + string FilePath, + int StartLine, + int StartCharacter, + int EndLine, + int EndCharacter +); + #region Equatable data model for incremental caching internal readonly record struct DiscoveryData( @@ -104,7 +117,8 @@ internal readonly record struct ModuleInfoRecord( string RoutePrefix, string ViewPrefix, ImmutableArray Endpoints, - ImmutableArray Views + ImmutableArray Views, + SourceLocationRecord? Location ) { public bool Equals(ModuleInfoRecord other) @@ -122,7 +136,8 @@ public bool Equals(ModuleInfoRecord other) && RoutePrefix == other.RoutePrefix && ViewPrefix == other.ViewPrefix && Endpoints.SequenceEqual(other.Endpoints) - && Views.SequenceEqual(other.Views); + && Views.SequenceEqual(other.Views) + && Location == other.Location; } public override int GetHashCode() @@ -142,6 +157,7 @@ public override int GetHashCode() hash = HashHelper.Combine(hash, (ViewPrefix ?? "").GetHashCode()); hash = HashHelper.HashArray(hash, Endpoints); hash = HashHelper.HashArray(hash, Views); + hash = HashHelper.Combine(hash, Location.GetHashCode()); return hash; } } @@ -169,7 +185,7 @@ public override int GetHashCode() } } -internal readonly record struct ViewInfoRecord(string FullyQualifiedName, string Page); +internal readonly record struct ViewInfoRecord(string FullyQualifiedName, string Page, SourceLocationRecord? Location); internal readonly record struct DtoTypeInfoRecord( string FullyQualifiedName, @@ -208,7 +224,8 @@ internal readonly record struct DbContextInfoRecord( string IdentityUserTypeFqn, string IdentityRoleTypeFqn, string IdentityKeyTypeFqn, - ImmutableArray DbSets + ImmutableArray DbSets, + SourceLocationRecord? Location ) { public bool Equals(DbContextInfoRecord other) @@ -219,7 +236,8 @@ public bool Equals(DbContextInfoRecord other) && IdentityUserTypeFqn == other.IdentityUserTypeFqn && IdentityRoleTypeFqn == other.IdentityRoleTypeFqn && IdentityKeyTypeFqn == other.IdentityKeyTypeFqn - && DbSets.SequenceEqual(other.DbSets); + && DbSets.SequenceEqual(other.DbSets) + && Location == other.Location; } public override int GetHashCode() @@ -232,6 +250,7 @@ public override int GetHashCode() hash = HashHelper.Combine(hash, (IdentityRoleTypeFqn ?? "").GetHashCode()); hash = HashHelper.Combine(hash, (IdentityKeyTypeFqn ?? "").GetHashCode()); hash = HashHelper.HashArray(hash, DbSets); + hash = HashHelper.Combine(hash, Location.GetHashCode()); return hash; } } @@ -241,7 +260,8 @@ public override int GetHashCode() internal readonly record struct EntityConfigInfoRecord( string ConfigFqn, string EntityFqn, - string ModuleName + string ModuleName, + SourceLocationRecord? Location ); internal readonly record struct ModuleDependencyRecord( @@ -254,13 +274,15 @@ internal readonly record struct IllegalModuleReferenceRecord( string ReferencingModuleName, string ReferencingAssemblyName, string ReferencedModuleName, - string ReferencedAssemblyName + string ReferencedAssemblyName, + SourceLocationRecord? Location ); internal readonly record struct ContractInterfaceInfoRecord( string ContractsAssemblyName, string InterfaceName, - int MethodCount + int MethodCount, + SourceLocationRecord? Location ); internal readonly record struct ContractImplementationRecord( @@ -269,21 +291,24 @@ internal readonly record struct ContractImplementationRecord( string ModuleName, bool IsPublic, bool IsAbstract, - bool DependsOnDbContext + bool DependsOnDbContext, + SourceLocationRecord? Location ); internal readonly record struct PermissionClassRecord( string FullyQualifiedName, string ModuleName, bool IsSealed, - ImmutableArray Fields + ImmutableArray Fields, + SourceLocationRecord? Location ) { public bool Equals(PermissionClassRecord other) => FullyQualifiedName == other.FullyQualifiedName && ModuleName == other.ModuleName && IsSealed == other.IsSealed - && Fields.SequenceEqual(other.Fields); + && Fields.SequenceEqual(other.Fields) + && Location == other.Location; public override int GetHashCode() { @@ -292,6 +317,7 @@ public override int GetHashCode() hash = HashHelper.Combine(hash, (ModuleName ?? "").GetHashCode()); hash = HashHelper.Combine(hash, IsSealed.GetHashCode()); hash = HashHelper.HashArray(hash, Fields); + hash = HashHelper.Combine(hash, Location.GetHashCode()); return hash; } } @@ -299,21 +325,24 @@ public override int GetHashCode() internal readonly record struct PermissionFieldRecord( string FieldName, string Value, - bool IsConstString + bool IsConstString, + SourceLocationRecord? Location ); internal readonly record struct FeatureClassRecord( string FullyQualifiedName, string ModuleName, bool IsSealed, - ImmutableArray Fields + ImmutableArray Fields, + SourceLocationRecord? Location ) { public bool Equals(FeatureClassRecord other) => FullyQualifiedName == other.FullyQualifiedName && ModuleName == other.ModuleName && IsSealed == other.IsSealed - && Fields.SequenceEqual(other.Fields); + && Fields.SequenceEqual(other.Fields) + && Location == other.Location; public override int GetHashCode() { @@ -322,6 +351,7 @@ public override int GetHashCode() hash = HashHelper.Combine(hash, (ModuleName ?? "").GetHashCode()); hash = HashHelper.Combine(hash, IsSealed.GetHashCode()); hash = HashHelper.HashArray(hash, Fields); + hash = HashHelper.Combine(hash, Location.GetHashCode()); return hash; } } @@ -329,19 +359,22 @@ public override int GetHashCode() internal readonly record struct FeatureFieldRecord( string FieldName, string Value, - bool IsConstString + bool IsConstString, + SourceLocationRecord? Location ); internal readonly record struct InterceptorInfoRecord( string FullyQualifiedName, string ModuleName, - ImmutableArray ConstructorParamTypeFqns + ImmutableArray ConstructorParamTypeFqns, + SourceLocationRecord? Location ) { public bool Equals(InterceptorInfoRecord other) => FullyQualifiedName == other.FullyQualifiedName && ModuleName == other.ModuleName - && ConstructorParamTypeFqns.SequenceEqual(other.ConstructorParamTypeFqns); + && ConstructorParamTypeFqns.SequenceEqual(other.ConstructorParamTypeFqns) + && Location == other.Location; public override int GetHashCode() { @@ -349,11 +382,16 @@ public override int GetHashCode() hash = HashHelper.Combine(hash, FullyQualifiedName.GetHashCode()); hash = HashHelper.Combine(hash, (ModuleName ?? "").GetHashCode()); hash = HashHelper.HashArray(hash, ConstructorParamTypeFqns); + hash = HashHelper.Combine(hash, Location.GetHashCode()); return hash; } } -internal readonly record struct ModuleOptionsRecord(string FullyQualifiedName, string ModuleName) +internal readonly record struct ModuleOptionsRecord( + string FullyQualifiedName, + string ModuleName, + SourceLocationRecord? Location +) { internal static Dictionary> GroupByModule( ImmutableArray options @@ -399,6 +437,7 @@ internal sealed class ModuleInfo public string ViewPrefix { get; set; } = ""; public List Endpoints { get; set; } = new(); public List Views { get; set; } = new(); + public SourceLocationRecord? Location { get; set; } } internal sealed class EndpointInfo @@ -413,6 +452,7 @@ internal sealed class ViewInfo public string FullyQualifiedName { get; set; } = ""; public string? Page { get; set; } public string InferredClassName { get; set; } = ""; + public SourceLocationRecord? Location { get; set; } } internal sealed class DtoTypeInfo @@ -445,6 +485,7 @@ internal sealed class DbContextInfo public string IdentityRoleTypeFqn { get; set; } = ""; public string IdentityKeyTypeFqn { get; set; } = ""; public List DbSets { get; set; } = new(); + public SourceLocationRecord? Location { get; set; } } internal sealed class DbSetInfo @@ -458,6 +499,7 @@ internal sealed class EntityConfigInfo public string ConfigFqn { get; set; } = ""; public string EntityFqn { get; set; } = ""; public string ModuleName { get; set; } = ""; + public SourceLocationRecord? Location { get; set; } } internal sealed class ContractImplementationInfo @@ -468,6 +510,7 @@ internal sealed class ContractImplementationInfo public bool IsPublic { get; set; } public bool IsAbstract { get; set; } public bool DependsOnDbContext { get; set; } + public SourceLocationRecord? Location { get; set; } } internal sealed class PermissionClassInfo @@ -476,6 +519,7 @@ internal sealed class PermissionClassInfo public string ModuleName { get; set; } = ""; public bool IsSealed { get; set; } public List Fields { get; set; } = new(); + public SourceLocationRecord? Location { get; set; } } internal sealed class PermissionFieldInfo @@ -483,6 +527,7 @@ internal sealed class PermissionFieldInfo public string FieldName { get; set; } = ""; public string Value { get; set; } = ""; public bool IsConstString { get; set; } + public SourceLocationRecord? Location { get; set; } } internal sealed class FeatureClassInfo @@ -491,6 +536,7 @@ internal sealed class FeatureClassInfo public string ModuleName { get; set; } = ""; public bool IsSealed { get; set; } public List Fields { get; set; } = new(); + public SourceLocationRecord? Location { get; set; } } internal sealed class FeatureFieldInfo @@ -498,6 +544,7 @@ internal sealed class FeatureFieldInfo public string FieldName { get; set; } = ""; public string Value { get; set; } = ""; public bool IsConstString { get; set; } + public SourceLocationRecord? Location { get; set; } } internal sealed class InterceptorInfo @@ -505,6 +552,7 @@ internal sealed class InterceptorInfo public string FullyQualifiedName { get; set; } = ""; public string ModuleName { get; set; } = ""; public List ConstructorParamTypeFqns { get; set; } = new(); + public SourceLocationRecord? Location { get; set; } } #endregion diff --git a/framework/SimpleModule.Generator/Discovery/SymbolDiscovery.cs b/framework/SimpleModule.Generator/Discovery/SymbolDiscovery.cs index ec2b59e5..6846abe0 100644 --- a/framework/SimpleModule.Generator/Discovery/SymbolDiscovery.cs +++ b/framework/SimpleModule.Generator/Discovery/SymbolDiscovery.cs @@ -2,13 +2,37 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; +using System.Threading; using Microsoft.CodeAnalysis; namespace SimpleModule.Generator; internal static class SymbolDiscovery { - internal static DiscoveryData Extract(Compilation compilation) + /// + /// Extracts a serializable source location from a symbol, if available. + /// Returns null for symbols only available in metadata (compiled DLLs). + /// + private static SourceLocationRecord? GetSourceLocation(ISymbol symbol) + { + foreach (var loc in symbol.Locations) + { + if (loc.IsInSource) + { + var span = loc.GetLineSpan(); + return new SourceLocationRecord( + span.Path, + span.StartLinePosition.Line, + span.StartLinePosition.Character, + span.EndLinePosition.Line, + span.EndLinePosition.Character + ); + } + } + return null; + } + + internal static DiscoveryData Extract(Compilation compilation, CancellationToken cancellationToken) { var hostAssemblyName = compilation.Assembly.Name; @@ -34,16 +58,18 @@ internal static DiscoveryData Extract(Compilation compilation) foreach (var reference in compilation.References) { + cancellationToken.ThrowIfCancellationRequested(); + if ( compilation.GetAssemblyOrModuleSymbol(reference) is not IAssemblySymbol assemblySymbol ) continue; - FindModuleTypes(assemblySymbol.GlobalNamespace, moduleAttributeSymbol, modules); + FindModuleTypes(assemblySymbol.GlobalNamespace, moduleAttributeSymbol, modules, cancellationToken); } - FindModuleTypes(compilation.Assembly.GlobalNamespace, moduleAttributeSymbol, modules); + FindModuleTypes(compilation.Assembly.GlobalNamespace, moduleAttributeSymbol, modules, cancellationToken); if (modules.Count == 0) return DiscoveryData.Empty; @@ -68,6 +94,8 @@ is not IAssemblySymbol assemblySymbol ); foreach (var module in modules) { + cancellationToken.ThrowIfCancellationRequested(); + if (!moduleSymbols.TryGetValue(module.FullyQualifiedName, out var typeSymbol)) continue; @@ -82,7 +110,8 @@ is not IAssemblySymbol assemblySymbol endpointInterfaceSymbol, viewEndpointInterfaceSymbol, rawEndpoints, - rawViews + rawViews, + cancellationToken ); // Match each endpoint/view to the module whose namespace is closest @@ -119,6 +148,8 @@ is not IAssemblySymbol assemblySymbol var scannedAssemblies = new HashSet(SymbolEqualityComparer.Default); foreach (var module in modules) { + cancellationToken.ThrowIfCancellationRequested(); + if (!moduleSymbols.TryGetValue(module.FullyQualifiedName, out var typeSymbol)) continue; @@ -129,8 +160,8 @@ is not IAssemblySymbol assemblySymbol // Collect unmatched items from this assembly var rawDbContexts = new List(); var rawEntityConfigs = new List(); - FindDbContextTypes(assembly.GlobalNamespace, "", rawDbContexts); - FindEntityConfigTypes(assembly.GlobalNamespace, "", rawEntityConfigs); + FindDbContextTypes(assembly.GlobalNamespace, "", rawDbContexts, cancellationToken); + FindEntityConfigTypes(assembly.GlobalNamespace, "", rawEntityConfigs, cancellationToken); // Match each DbContext to the module whose namespace is closest foreach (var ctx in rawDbContexts) @@ -153,18 +184,22 @@ is not IAssemblySymbol assemblySymbol { foreach (var reference in compilation.References) { + cancellationToken.ThrowIfCancellationRequested(); + if ( compilation.GetAssemblyOrModuleSymbol(reference) is not IAssemblySymbol assemblySymbol ) continue; - FindDtoTypes(assemblySymbol.GlobalNamespace, dtoAttributeSymbol, dtoTypes); + FindDtoTypes(assemblySymbol.GlobalNamespace, dtoAttributeSymbol, dtoTypes, cancellationToken); } - FindDtoTypes(compilation.Assembly.GlobalNamespace, dtoAttributeSymbol, dtoTypes); + FindDtoTypes(compilation.Assembly.GlobalNamespace, dtoAttributeSymbol, dtoTypes, cancellationToken); } + cancellationToken.ThrowIfCancellationRequested(); + var componentBaseSymbol = compilation.GetTypeByMetadataName( "Microsoft.AspNetCore.Components.ComponentBase" ); @@ -175,6 +210,8 @@ is not IAssemblySymbol assemblySymbol ); foreach (var reference in compilation.References) { + cancellationToken.ThrowIfCancellationRequested(); + if (compilation.GetAssemblyOrModuleSymbol(reference) is not IAssemblySymbol asm) continue; @@ -193,6 +230,7 @@ is not IAssemblySymbol assemblySymbol } // --- Dependency inference --- + cancellationToken.ThrowIfCancellationRequested(); // Step 1: Build module assembly map (assembly name → module name) var moduleAssemblyMap = new Dictionary(StringComparer.OrdinalIgnoreCase); @@ -259,12 +297,14 @@ is not IAssemblySymbol assemblySymbol foreach (var kvp in contractsAssemblySymbols) { + cancellationToken.ThrowIfCancellationRequested(); FindConventionDtoTypes( kvp.Value.GlobalNamespace, noDtoAttrSymbol, eventInterfaceSymbol, existingDtoFqns, - dtoTypes + dtoTypes, + cancellationToken ); } @@ -498,7 +538,8 @@ is not IAssemblySymbol assemblySymbol thisModuleName, moduleAssembly.Name, referencedModuleName, - refName + refName, + module.Location ) ); } @@ -542,8 +583,9 @@ is not IAssemblySymbol assemblySymbol e.AllowAnonymous )) .ToImmutableArray(), - m.Views.Select(v => new ViewInfoRecord(v.FullyQualifiedName, v.Page ?? "")) - .ToImmutableArray() + m.Views.Select(v => new ViewInfoRecord(v.FullyQualifiedName, v.Page ?? "", v.Location)) + .ToImmutableArray(), + m.Location )) .ToImmutableArray(), dtoTypes @@ -568,11 +610,12 @@ is not IAssemblySymbol assemblySymbol c.IdentityRoleTypeFqn, c.IdentityKeyTypeFqn, c.DbSets.Select(d => new DbSetInfoRecord(d.PropertyName, d.EntityFqn)) - .ToImmutableArray() + .ToImmutableArray(), + c.Location )) .ToImmutableArray(), entityConfigs - .Select(e => new EntityConfigInfoRecord(e.ConfigFqn, e.EntityFqn, e.ModuleName)) + .Select(e => new EntityConfigInfoRecord(e.ConfigFqn, e.EntityFqn, e.ModuleName, e.Location)) .ToImmutableArray(), dependencies.ToImmutableArray(), illegalReferences.ToImmutableArray(), @@ -584,7 +627,8 @@ is not IAssemblySymbol assemblySymbol c.ModuleName, c.IsPublic, c.IsAbstract, - c.DependsOnDbContext + c.DependsOnDbContext, + c.Location )) .ToImmutableArray(), permissionClasses @@ -595,9 +639,11 @@ is not IAssemblySymbol assemblySymbol p.Fields.Select(f => new PermissionFieldRecord( f.FieldName, f.Value, - f.IsConstString + f.IsConstString, + f.Location )) - .ToImmutableArray() + .ToImmutableArray(), + p.Location )) .ToImmutableArray(), featureClasses @@ -608,16 +654,19 @@ is not IAssemblySymbol assemblySymbol f.Fields.Select(ff => new FeatureFieldRecord( ff.FieldName, ff.Value, - ff.IsConstString + ff.IsConstString, + ff.Location )) - .ToImmutableArray() + .ToImmutableArray(), + f.Location )) .ToImmutableArray(), interceptors .Select(i => new InterceptorInfoRecord( i.FullyQualifiedName, i.ModuleName, - i.ConstructorParamTypeFqns.ToImmutableArray() + i.ConstructorParamTypeFqns.ToImmutableArray(), + i.Location )) .ToImmutableArray(), vogenValueObjects.ToImmutableArray(), @@ -629,14 +678,17 @@ is not IAssemblySymbol assemblySymbol private static void FindModuleTypes( INamespaceSymbol namespaceSymbol, INamedTypeSymbol moduleAttributeSymbol, - List modules + List modules, + CancellationToken cancellationToken ) { foreach (var member in namespaceSymbol.GetMembers()) { + cancellationToken.ThrowIfCancellationRequested(); + if (member is INamespaceSymbol childNamespace) { - FindModuleTypes(childNamespace, moduleAttributeSymbol, modules); + FindModuleTypes(childNamespace, moduleAttributeSymbol, modules, cancellationToken); } else if (member is INamedTypeSymbol typeSymbol) { @@ -707,6 +759,7 @@ List modules ), RoutePrefix = routePrefix, ViewPrefix = viewPrefix, + Location = GetSourceLocation(typeSymbol), } ); break; @@ -721,11 +774,14 @@ private static void FindEndpointTypes( INamedTypeSymbol endpointInterfaceSymbol, INamedTypeSymbol? viewEndpointInterfaceSymbol, List endpoints, - List views + List views, + CancellationToken cancellationToken ) { foreach (var member in namespaceSymbol.GetMembers()) { + cancellationToken.ThrowIfCancellationRequested(); + if (member is INamespaceSymbol childNamespace) { FindEndpointTypes( @@ -733,7 +789,8 @@ List views endpointInterfaceSymbol, viewEndpointInterfaceSymbol, endpoints, - views + views, + cancellationToken ); } else if (member is INamedTypeSymbol typeSymbol) @@ -781,6 +838,7 @@ viewEndpointInterfaceSymbol is not null FullyQualifiedName = fqn, Page = page, InferredClassName = className, + Location = GetSourceLocation(typeSymbol), } ); } @@ -887,14 +945,17 @@ private static bool DeclaresMethod(INamedTypeSymbol typeSymbol, string methodNam private static void FindDtoTypes( INamespaceSymbol namespaceSymbol, INamedTypeSymbol dtoAttributeSymbol, - List dtoTypes + List dtoTypes, + CancellationToken cancellationToken ) { foreach (var member in namespaceSymbol.GetMembers()) { + cancellationToken.ThrowIfCancellationRequested(); + if (member is INamespaceSymbol childNamespace) { - FindDtoTypes(childNamespace, dtoAttributeSymbol, dtoTypes); + FindDtoTypes(childNamespace, dtoAttributeSymbol, dtoTypes, cancellationToken); } else if (member is INamedTypeSymbol typeSymbol) { @@ -1022,14 +1083,17 @@ private static string FindClosestModuleName(string typeFqn, List mod private static void FindDbContextTypes( INamespaceSymbol namespaceSymbol, string moduleName, - List dbContexts + List dbContexts, + CancellationToken cancellationToken ) { foreach (var member in namespaceSymbol.GetMembers()) { + cancellationToken.ThrowIfCancellationRequested(); + if (member is INamespaceSymbol childNamespace) { - FindDbContextTypes(childNamespace, moduleName, dbContexts); + FindDbContextTypes(childNamespace, moduleName, dbContexts, cancellationToken); } else if ( member is INamedTypeSymbol typeSymbol @@ -1095,6 +1159,7 @@ member is INamedTypeSymbol typeSymbol IdentityUserTypeFqn = identityUserFqn, IdentityRoleTypeFqn = identityRoleFqn, IdentityKeyTypeFqn = identityKeyFqn, + Location = GetSourceLocation(typeSymbol), }; // Collect DbSet properties @@ -1128,14 +1193,17 @@ m is IPropertySymbol prop private static void FindEntityConfigTypes( INamespaceSymbol namespaceSymbol, string moduleName, - List entityConfigs + List entityConfigs, + CancellationToken cancellationToken ) { foreach (var member in namespaceSymbol.GetMembers()) { + cancellationToken.ThrowIfCancellationRequested(); + if (member is INamespaceSymbol childNamespace) { - FindEntityConfigTypes(childNamespace, moduleName, entityConfigs); + FindEntityConfigTypes(childNamespace, moduleName, entityConfigs, cancellationToken); } else if ( member is INamedTypeSymbol typeSymbol @@ -1164,6 +1232,7 @@ member is INamedTypeSymbol typeSymbol ), EntityFqn = entityFqn, ModuleName = moduleName, + Location = GetSourceLocation(typeSymbol), } ); break; @@ -1202,7 +1271,8 @@ member is INamedTypeSymbol typeSymbol new ContractInterfaceInfoRecord( assemblyName, typeSymbol.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat), - methodCount + methodCount, + GetSourceLocation(typeSymbol) ) ); } @@ -1247,6 +1317,7 @@ List results IsPublic = typeSymbol.DeclaredAccessibility == Accessibility.Public, IsAbstract = typeSymbol.IsAbstract, DependsOnDbContext = HasDbContextConstructorParam(typeSymbol), + Location = GetSourceLocation(typeSymbol), } ); } @@ -1281,6 +1352,7 @@ member is INamedTypeSymbol typeSymbol ), ModuleName = moduleName, IsSealed = typeSymbol.IsSealed, + Location = GetSourceLocation(typeSymbol), }; // Collect public const string fields @@ -1302,6 +1374,7 @@ m is IFieldSymbol field IsConstString = field.IsConst && field.Type.SpecialType == SpecialType.System_String, + Location = GetSourceLocation(field), } ); } @@ -1338,6 +1411,7 @@ member is INamedTypeSymbol typeSymbol ), ModuleName = moduleName, IsSealed = typeSymbol.IsSealed, + Location = GetSourceLocation(typeSymbol), }; // Collect public const string fields @@ -1359,6 +1433,7 @@ m is IFieldSymbol field IsConstString = field.IsConst && field.Type.SpecialType == SpecialType.System_String, + Location = GetSourceLocation(field), } ); } @@ -1383,7 +1458,8 @@ List results results.Add( new ModuleOptionsRecord( typeSymbol.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat), - moduleName + moduleName, + GetSourceLocation(typeSymbol) ) ) ); @@ -1445,6 +1521,7 @@ member is INamedTypeSymbol typeSymbol SymbolDisplayFormat.FullyQualifiedFormat ), ModuleName = moduleName, + Location = GetSourceLocation(typeSymbol), }; // Extract constructor parameter type FQNs @@ -1474,11 +1551,14 @@ private static void FindConventionDtoTypes( INamedTypeSymbol? noDtoAttrSymbol, INamedTypeSymbol? eventInterfaceSymbol, HashSet existingFqns, - List dtoTypes + List dtoTypes, + CancellationToken cancellationToken ) { foreach (var member in namespaceSymbol.GetMembers()) { + cancellationToken.ThrowIfCancellationRequested(); + if (member is INamespaceSymbol childNs) { FindConventionDtoTypes( @@ -1486,7 +1566,8 @@ List dtoTypes noDtoAttrSymbol, eventInterfaceSymbol, existingFqns, - dtoTypes + dtoTypes, + cancellationToken ); } else if ( diff --git a/framework/SimpleModule.Generator/Emitters/DiagnosticEmitter.cs b/framework/SimpleModule.Generator/Emitters/DiagnosticEmitter.cs index f76e5750..9ee7c1c7 100644 --- a/framework/SimpleModule.Generator/Emitters/DiagnosticEmitter.cs +++ b/framework/SimpleModule.Generator/Emitters/DiagnosticEmitter.cs @@ -3,11 +3,27 @@ using System.Collections.Immutable; using System.Linq; using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Text; namespace SimpleModule.Generator; internal sealed class DiagnosticEmitter : IEmitter { + /// + /// Reconstructs a from a serializable . + /// Returns for null (metadata-only types). + /// + private static Location ToLocation(SourceLocationRecord? loc) + { + if (loc is null) + return Location.None; + + var start = new LinePosition(loc.Value.StartLine, loc.Value.StartCharacter); + var end = new LinePosition(loc.Value.EndLine, loc.Value.EndCharacter); + var span = new LinePositionSpan(start, end); + return Location.Create(loc.Value.FilePath, TextSpan.FromBounds(0, 0), span); + } + internal static readonly DiagnosticDescriptor DuplicateDbSetPropertyName = new( id: "SM0001", title: "Duplicate DbSet property name across modules", @@ -315,7 +331,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( EmptyModuleName, - Location.None, + ToLocation(module.Location), Strip(module.FullyQualifiedName) ) ); @@ -334,7 +350,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( DuplicateModuleName, - Location.None, + ToLocation(module.Location), module.ModuleName, Strip(existingFqn), Strip(module.FullyQualifiedName) @@ -368,7 +384,11 @@ public void Emit(SourceProductionContext context, DiscoveryData data) ) { context.ReportDiagnostic( - Diagnostic.Create(EmptyModuleWarning, Location.None, module.ModuleName) + Diagnostic.Create( + EmptyModuleWarning, + ToLocation(module.Location), + module.ModuleName + ) ); } } @@ -393,7 +413,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( MultipleIdentityDbContexts, - Location.None, + ToLocation(ctx.Location), Strip(firstIdentity.Value.FullyQualifiedName), firstIdentity.Value.ModuleName, Strip(ctx.FullyQualifiedName), @@ -411,7 +431,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( IdentityDbContextBadTypeArgs, - Location.None, + ToLocation(ctx.Location), Strip(ctx.FullyQualifiedName), ctx.ModuleName, 0 @@ -435,7 +455,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( EntityConfigForMissingEntity, - Location.None, + ToLocation(config.Location), Strip(config.EntityFqn), Strip(config.ConfigFqn), config.ModuleName @@ -453,7 +473,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( DuplicateEntityConfiguration, - Location.None, + ToLocation(config.Location), Strip(config.EntityFqn), existing, Strip(config.ConfigFqn) @@ -498,10 +518,21 @@ public void Emit(SourceProductionContext context, DiscoveryData data) var first = sortResult.Cycle[0]; var second = sortResult.Cycle.Length > 1 ? sortResult.Cycle[1] : first; + // Find location of the first module in the cycle + SourceLocationRecord? cycleLoc = null; + foreach (var module in data.Modules) + { + if (module.ModuleName == first) + { + cycleLoc = module.Location; + break; + } + } + context.ReportDiagnostic( Diagnostic.Create( CircularModuleDependency, - Location.None, + ToLocation(cycleLoc), cycleStr, howStr, first, @@ -516,7 +547,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( IllegalImplementationReference, - Location.None, + ToLocation(illegal.Location), illegal.ReferencingModuleName, illegal.ReferencedModuleName, illegal.ReferencedAssemblyName @@ -533,7 +564,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( ContractInterfaceTooLargeError, - Location.None, + ToLocation(iface.Location), Strip(iface.InterfaceName), iface.MethodCount, shortName @@ -546,7 +577,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( ContractInterfaceTooLargeWarning, - Location.None, + ToLocation(iface.Location), Strip(iface.InterfaceName), iface.MethodCount, shortName @@ -567,10 +598,21 @@ public void Emit(SourceProductionContext context, DiscoveryData data) var key = dep.ModuleName + "|" + dep.ContractsAssemblyName; if (!contractsWithInterfaces.Contains(dep.ContractsAssemblyName) && reported.Add(key)) { + // Find the module's location for this diagnostic + SourceLocationRecord? depModuleLoc = null; + foreach (var module in data.Modules) + { + if (module.ModuleName == dep.ModuleName) + { + depModuleLoc = module.Location; + break; + } + } + context.ReportDiagnostic( Diagnostic.Create( MissingContractInterfaces, - Location.None, + ToLocation(depModuleLoc), dep.ModuleName, dep.ContractsAssemblyName ) @@ -600,7 +642,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( ContractImplementationNotPublic, - Location.None, + ToLocation(impl.Location), Strip(impl.ImplementationFqn), Strip(impl.InterfaceFqn) ) @@ -612,7 +654,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( ContractImplementationIsAbstract, - Location.None, + ToLocation(impl.Location), Strip(impl.ImplementationFqn), Strip(impl.InterfaceFqn) ) @@ -633,7 +675,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( NoContractImplementation, - Location.None, + ToLocation(iface.Location), Strip(iface.InterfaceName), moduleName ) @@ -660,7 +702,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( MultipleContractImplementations, - Location.None, + ToLocation(validImpls[1].Location), Strip(kvp.Key), validImpls[0].ModuleName, string.Join(", ", names) @@ -681,7 +723,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) if (!perm.IsSealed) { context.ReportDiagnostic( - Diagnostic.Create(PermissionClassNotSealed, Location.None, permCleanName) + Diagnostic.Create(PermissionClassNotSealed, ToLocation(perm.Location), permCleanName) ); } @@ -693,7 +735,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( PermissionFieldNotConstString, - Location.None, + ToLocation(field.Location), permCleanName, field.FieldName ) @@ -713,7 +755,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( PermissionValueBadPattern, - Location.None, + ToLocation(field.Location), field.Value, permCleanName ) @@ -729,7 +771,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( PermissionValueWrongPrefix, - Location.None, + ToLocation(field.Location), field.Value, perm.ModuleName ) @@ -743,7 +785,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( DuplicatePermissionValue, - Location.None, + ToLocation(field.Location), field.Value, existingOwner, permCleanName @@ -780,7 +822,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) contractsIdx >= 0 ? fqn.Substring(0, contractsIdx + ".Contracts".Length) : fqn; context.ReportDiagnostic( - Diagnostic.Create(DtoTypeNoProperties, Location.None, fqn, contractsAsm) + Diagnostic.Create(DtoTypeNoProperties, Location.None, fqn, contractsAsm) // DTOs are typically in metadata (Contracts assemblies) ); } } @@ -814,7 +856,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( DuplicateViewPageName, - Location.None, + ToLocation(view.Location), view.Page, Strip(existing.EndpointFqn), existing.ModuleName, @@ -844,7 +886,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( ViewPagePrefixMismatch, - Location.None, + ToLocation(view.Location), Strip(view.FullyQualifiedName), module.ModuleName, view.Page @@ -863,7 +905,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( ViewEndpointWithoutViewPrefix, - Location.None, + ToLocation(module.Location), module.ModuleName, module.Views.Length, module.ModuleName.ToLowerInvariant() @@ -885,7 +927,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( InterceptorDependsOnDbContext, - Location.None, + ToLocation(interceptor.Location), Strip(interceptor.FullyQualifiedName), interceptor.ModuleName, Strip(paramFqn) @@ -906,7 +948,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( MultipleModuleOptions, - Location.None, + ToLocation(kvp.Value[1].Location), kvp.Key, Strip(kvp.Value[0].FullyQualifiedName), Strip(kvp.Value[1].FullyQualifiedName) @@ -926,7 +968,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) if (!feat.IsSealed) { context.ReportDiagnostic( - Diagnostic.Create(FeatureClassNotSealed, Location.None, featCleanName) + Diagnostic.Create(FeatureClassNotSealed, ToLocation(feat.Location), featCleanName) ); } @@ -938,7 +980,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( FeatureFieldNotConstString, - Location.None, + ToLocation(field.Location), field.FieldName, featCleanName ) @@ -952,7 +994,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( FeatureFieldNamingViolation, - Location.None, + ToLocation(field.Location), field.Value, featCleanName ) @@ -967,7 +1009,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( DuplicateFeatureName, - Location.None, + ToLocation(field.Location), field.Value, Strip(existingOwner), featCleanName diff --git a/framework/SimpleModule.Generator/Emitters/HostDbContextEmitter.cs b/framework/SimpleModule.Generator/Emitters/HostDbContextEmitter.cs index 556f3931..36fb3db7 100644 --- a/framework/SimpleModule.Generator/Emitters/HostDbContextEmitter.cs +++ b/framework/SimpleModule.Generator/Emitters/HostDbContextEmitter.cs @@ -1,3 +1,4 @@ +using System.Linq; using System.Text; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Text; @@ -6,6 +7,17 @@ namespace SimpleModule.Generator; internal sealed class HostDbContextEmitter : IEmitter { + private static Location ToLocation(SourceLocationRecord? loc) + { + if (loc is null) + return Location.None; + + var start = new LinePosition(loc.Value.StartLine, loc.Value.StartCharacter); + var end = new LinePosition(loc.Value.EndLine, loc.Value.EndCharacter); + var span = new LinePositionSpan(start, end); + return Location.Create(loc.Value.FilePath, TextSpan.FromBounds(0, 0), span); + } + #pragma warning disable CA1308 // Schema names are conventionally lowercase in PostgreSQL/SQL Server public void Emit(SourceProductionContext context, DiscoveryData data) { @@ -78,10 +90,21 @@ string ModuleName // Same entity type with same property name is fine (deduplicated earlier) if (existing.EntityFqn != entry.EntityFqn) { + // Find the DbContext location for the conflicting module + SourceLocationRecord? dbCtxLoc = null; + foreach (var ctx in data.DbContexts) + { + if (ctx.ModuleName == entry.ModuleName) + { + dbCtxLoc = ctx.Location; + break; + } + } + context.ReportDiagnostic( Diagnostic.Create( DiagnosticEmitter.DuplicateDbSetPropertyName, - Location.None, + ToLocation(dbCtxLoc), entry.PropertyName, existing.ModuleName, TypeMappingHelpers.StripGlobalPrefix(existing.EntityFqn), diff --git a/framework/SimpleModule.Generator/ModuleDiscovererGenerator.cs b/framework/SimpleModule.Generator/ModuleDiscovererGenerator.cs index 8ec5a2bd..3c89987b 100644 --- a/framework/SimpleModule.Generator/ModuleDiscovererGenerator.cs +++ b/framework/SimpleModule.Generator/ModuleDiscovererGenerator.cs @@ -28,8 +28,11 @@ public void Initialize(IncrementalGeneratorInitializationContext context) { // Extract an equatable data model from the compilation so the incremental // pipeline can cache results and skip re-generation when nothing changes. + // The CancellationToken allows the IDE to cancel stale discovery work + // when a new compilation is triggered (e.g., on each keystroke). var dataProvider = context.CompilationProvider.Select( - static (compilation, _) => SymbolDiscovery.Extract(compilation) + static (compilation, cancellationToken) => + SymbolDiscovery.Extract(compilation, cancellationToken) ); context.RegisterSourceOutput( diff --git a/tests/SimpleModule.Generator.Tests/TopologicalSortTests.cs b/tests/SimpleModule.Generator.Tests/TopologicalSortTests.cs index 76bdbd50..a01d737d 100644 --- a/tests/SimpleModule.Generator.Tests/TopologicalSortTests.cs +++ b/tests/SimpleModule.Generator.Tests/TopologicalSortTests.cs @@ -207,7 +207,8 @@ public void SortModules_WithDependencies_ReordersByDependency() "", "", ImmutableArray.Empty, - ImmutableArray.Empty + ImmutableArray.Empty, + null ), new ModuleInfoRecord( "global::A.AModule", @@ -223,7 +224,8 @@ public void SortModules_WithDependencies_ReordersByDependency() "", "", ImmutableArray.Empty, - ImmutableArray.Empty + ImmutableArray.Empty, + null ) ), ImmutableArray.Empty, @@ -268,7 +270,8 @@ public void SortModules_WithCycle_ReturnsOriginalOrder() "", "", ImmutableArray.Empty, - ImmutableArray.Empty + ImmutableArray.Empty, + null ), new ModuleInfoRecord( "global::B.BModule", @@ -284,7 +287,8 @@ public void SortModules_WithCycle_ReturnsOriginalOrder() "", "", ImmutableArray.Empty, - ImmutableArray.Empty + ImmutableArray.Empty, + null ) ), ImmutableArray.Empty, @@ -332,7 +336,8 @@ public void SortModules_NoDependencies_PreservesOriginalOrder() "", "", ImmutableArray.Empty, - ImmutableArray.Empty + ImmutableArray.Empty, + null ), new ModuleInfoRecord( "global::A.AModule", @@ -348,7 +353,8 @@ public void SortModules_NoDependencies_PreservesOriginalOrder() "", "", ImmutableArray.Empty, - ImmutableArray.Empty + ImmutableArray.Empty, + null ), new ModuleInfoRecord( "global::B.BModule", @@ -364,7 +370,8 @@ public void SortModules_NoDependencies_PreservesOriginalOrder() "", "", ImmutableArray.Empty, - ImmutableArray.Empty + ImmutableArray.Empty, + null ) ), ImmutableArray.Empty, From ea15e1202054bc038de4a835dd289662269a1c31 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 1 Apr 2026 15:35:13 +0000 Subject: [PATCH 3/6] Add P1 implementation plan for 5 architectural improvements Detailed plan covering: - P1-1: IModule ISP decomposition into focused interfaces - P1-2: Fix Inertia serialization to use DI JSON options - P1-3: Configurable contract implementation lifetime via attribute - P1-4: Event bus pipeline behavior (MediatR-style) - P1-5: Enforce endpoint auth attributes in generated code Includes file-by-file change lists, code examples, risk assessment, and recommended execution order. --- tasks/p1-implementation-plan.md | 248 ++++++++++++++++++++++++++++++++ 1 file changed, 248 insertions(+) create mode 100644 tasks/p1-implementation-plan.md diff --git a/tasks/p1-implementation-plan.md b/tasks/p1-implementation-plan.md new file mode 100644 index 00000000..f3800ba4 --- /dev/null +++ b/tasks/p1-implementation-plan.md @@ -0,0 +1,248 @@ +# P1 Implementation Plan + +## P1-1: Decompose IModule Wide Interface (ISP Violation) + +### Problem +`IModule` has 10 methods covering 7 unrelated concerns (DI, routing, middleware, menu, permissions, settings, feature flags, lifecycle, health). Adding a new hook requires changes across Core, Generator discovery, emitter code, and diagnostic checks. + +### Approach: Focused Capability Interfaces + +Keep `IModule` as a lean marker interface (just `[Module]` attribute discovery). Extract each concern into an opt-in interface: + +``` +IModule — marker, discovered by [Module] attribute +├── IModuleServices — ConfigureServices(IServiceCollection, IConfiguration) +├── IModuleMiddleware — ConfigureMiddleware(IApplicationBuilder) +├── IModuleMenu — ConfigureMenu(IMenuBuilder) +├── IModulePermissions — ConfigurePermissions(PermissionRegistryBuilder) +├── IModuleSettings — ConfigureSettings(ISettingsBuilder) +├── IModuleFeatureFlags — ConfigureFeatureFlags(IFeatureFlagBuilder) +├── IModuleLifecycle — OnStartAsync, OnStopAsync, CheckHealthAsync +└── ConfigureEndpoints stays — escape hatch, already has HasConfigureEndpoints bool +``` + +### Files to Change + +| File | Change | +|------|--------| +| `framework/SimpleModule.Core/IModule.cs` | Keep as marker with `ConfigureEndpoints` only. Extract 7 new interfaces. Optionally keep default methods on `IModule` for backward compat during transition. | +| `framework/SimpleModule.Core/IModuleServices.cs` (new) | `ConfigureServices(IServiceCollection, IConfiguration)` | +| `framework/SimpleModule.Core/IModuleMiddleware.cs` (new) | `ConfigureMiddleware(IApplicationBuilder)` | +| `framework/SimpleModule.Core/IModuleMenu.cs` (new) | `ConfigureMenu(IMenuBuilder)` | +| `framework/SimpleModule.Core/IModuleSettings.cs` (new) | `ConfigureSettings(ISettingsBuilder)` | +| `framework/SimpleModule.Core/IModuleFeatureFlags.cs` (new) | `ConfigureFeatureFlags(IFeatureFlagBuilder)` | +| `framework/SimpleModule.Core/IModuleLifecycle.cs` (new) | `OnStartAsync`, `OnStopAsync`, `CheckHealthAsync` | +| `framework/SimpleModule.Generator/Discovery/SymbolDiscovery.cs` | Replace `DeclaresMethod` checks with `ImplementsInterface` checks for each sub-interface | +| `framework/SimpleModule.Generator/Emitters/ModuleExtensionsEmitter.cs` | Cast to sub-interface before calling: `if (module is IModuleServices svc) svc.ConfigureServices(...)` | +| `framework/SimpleModule.Generator/Emitters/MenuExtensionsEmitter.cs` | Same pattern — cast to `IModuleMenu` | +| `framework/SimpleModule.Generator/Emitters/SettingsExtensionsEmitter.cs` | Cast to `IModuleSettings` | +| `framework/SimpleModule.Generator/Emitters/HostingExtensionsEmitter.cs` | Cast to `IModuleMiddleware` | +| `framework/SimpleModule.Generator/Emitters/DiagnosticEmitter.cs` | SM0043 (empty module) checks sub-interface implementation instead of `HasConfigure*` bools | +| `framework/SimpleModule.Core/Hosting/ModuleLifecycleHostedService.cs` | Cast to `IModuleLifecycle` instead of calling directly on `IModule` | +| All existing module classes | Implement the new sub-interfaces (can do `IModule, IModuleServices, IModuleMenu`) | + +### Migration Strategy +- **Phase 1**: Add new interfaces. Keep default methods on `IModule` so existing modules don't break. +- **Phase 2**: Generator detects both `IModule` method overrides AND sub-interface implementation. +- **Phase 3**: Deprecate methods on `IModule`, migrate modules, remove defaults. + +### Risk: High effort, touching every module. Recommend Phase 1+2 first for backward compat. + +--- + +## P1-2: Fix Inertia Serialization to Use DI JSON Options + +### Problem +`InertiaResult` uses a private static `JsonSerializerOptions` with only `CamelCase` naming. The DI-registered `HttpJsonOptions` includes `ModulesJsonResolver` (Vogen value objects, DTO type info). This means API endpoints and Inertia view endpoints serialize the same DTO differently. + +### Approach: Resolve Options from DI at Render Time + +### Files to Change + +| File | Change | +|------|--------| +| `framework/SimpleModule.Core/Inertia/InertiaResult.cs:15-18` | Remove static `_camelCaseOptions` field | +| `framework/SimpleModule.Core/Inertia/InertiaResult.cs:50,55,80` | Replace `_camelCaseOptions` with resolved options | +| `framework/SimpleModule.Core/Inertia/InertiaResult.cs` (ExecuteAsync) | Resolve `IOptions` from `httpContext.RequestServices`, clone options, ensure `CamelCase` naming is set, cache per-request | + +### Implementation Detail + +```csharp +// In ExecuteAsync: +var jsonOptions = httpContext.RequestServices + .GetRequiredService>(); +var options = new JsonSerializerOptions(jsonOptions.Value.SerializerOptions) +{ + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, +}; +``` + +To avoid creating a new `JsonSerializerOptions` on every request, cache the merged options in a static `Lazy<>` initialized on first use from the `IServiceProvider`, or use a dedicated singleton service `IInertiaJsonOptionsProvider` registered at startup. + +### Risk: Low. Single file change, clear fix path. + +--- + +## P1-3: Support Configurable Contract Implementation Lifetime + +### Problem +The generator always emits `services.AddScoped()`. Module authors who need Singleton or Transient must bypass auto-discovery. + +### Approach: New `[ContractLifetime]` Attribute + +### Files to Change + +| File | Change | +|------|--------| +| `framework/SimpleModule.Core/ContractLifetimeAttribute.cs` (new) | `[ContractLifetime(ServiceLifetime.Singleton)]` attribute targeting classes | +| `framework/SimpleModule.Generator/Discovery/DiscoveryData.cs:288` | Add `int Lifetime` field to `ContractImplementationRecord` (use int instead of enum since generator targets netstandard2.0 and can't reference `ServiceLifetime`) | +| `framework/SimpleModule.Generator/Discovery/DiscoveryData.cs` (mutable) | Add `int Lifetime` to `ContractImplementationInfo` | +| `framework/SimpleModule.Generator/Discovery/SymbolDiscovery.cs` (~line 868) | In `FindContractImplementations`, check for `[ContractLifetime]` attribute, read `ServiceLifetime` value. Default to 1 (Scoped) if absent. | +| `framework/SimpleModule.Generator/Emitters/ModuleExtensionsEmitter.cs:104` | Emit `AddScoped`, `AddSingleton`, or `AddTransient` based on `impl.Lifetime` | + +### Implementation Detail + +```csharp +// Core attribute: +[AttributeUsage(AttributeTargets.Class)] +public sealed class ContractLifetimeAttribute(ServiceLifetime lifetime) : Attribute +{ + public ServiceLifetime Lifetime { get; } = lifetime; +} + +// Generator emitter (line 104): +var method = impl.Lifetime switch +{ + 0 => "AddSingleton", // ServiceLifetime.Singleton = 0 + 2 => "AddTransient", // ServiceLifetime.Transient = 2 + _ => "AddScoped", // ServiceLifetime.Scoped = 1 (default) +}; +sb.AppendLine($" services.{method}<{impl.InterfaceFqn}, {impl.ImplementationFqn}>();"); +``` + +### Risk: Low. Additive change, backward compatible (no attribute = Scoped). + +--- + +## P1-4: Add Event Bus Pipeline Behavior + +### Problem +`EventBus.PublishAsync` dispatches directly to handlers with no interception points. No way to add logging, metrics, retries, or transaction boundaries at the bus level. `BackgroundEventChannel` uses unbounded channel with no backpressure. + +### Approach: MediatR-style `IEventPipelineBehavior` + +### Files to Change + +| File | Change | +|------|--------| +| `framework/SimpleModule.Core/Events/IEventPipelineBehavior.cs` (new) | Pipeline behavior interface | +| `framework/SimpleModule.Core/Events/EventBus.cs:113-142` | Resolve `IEventPipelineBehavior` from DI, build middleware chain, invoke before handler loop | +| `framework/SimpleModule.Core/Events/BackgroundEventChannel.cs:9` | Change `Channel.CreateUnbounded` to `Channel.CreateBounded` with configurable capacity | + +### Implementation Detail + +```csharp +// New interface: +public interface IEventPipelineBehavior where T : IEvent +{ + Task HandleAsync(T @event, Func next, CancellationToken cancellationToken); +} + +// In EventBus.PublishAsync: +var behaviors = _serviceProvider.GetServices>(); +Task DispatchToHandlers() => InvokeHandlers(handlers, @event, cancellationToken); + +var pipeline = behaviors.Reverse().Aggregate( + (Func)DispatchToHandlers, + (next, behavior) => () => behavior.HandleAsync(@event, next, cancellationToken) +); +await pipeline(); +``` + +For backpressure: +```csharp +// BackgroundEventChannel.cs: +Channel.CreateBounded>( + new BoundedChannelOptions(1000) { FullMode = BoundedChannelFullMode.Wait } +); +``` + +### Risk: Medium. The pipeline pattern is additive (no behaviors = no overhead). Bounded channel needs capacity tuning consideration. + +--- + +## P1-5: Enforce Endpoint Attributes in Generated Code + +### Problem +The generator discovers `[AllowAnonymous]` and `[RequirePermission("...")]` on endpoint classes but doesn't emit the corresponding routing calls. The group-level `.RequireAuthorization()` applies to all endpoints, and individual overrides must be done manually inside `Map()`. + +### Approach: Emit Authorization Modifiers After `Map()` Call + +### Prerequisite +`IEndpoint.Map()` and `IViewEndpoint.Map()` currently return `void`. To chain `.AllowAnonymous()` or `.RequireAuthorization()`, the `Map` method must return a value (`IEndpointConventionBuilder`). This is a **breaking change** to the endpoint interface. + +**Alternative (non-breaking)**: Instead of changing the interface, apply authorization at the **route group level per-endpoint** by having the generator wrap each endpoint in its own sub-group with the appropriate policy. Or, scan attributes and apply them to the group before calling `Map()`. + +### Recommended Approach: Post-Map Wrapper + +Since we can't easily change `Map()` return type without breaking all endpoints, use a different strategy: have the generator emit attribute-based endpoint filters on the route group. + +Actually, the simplest non-breaking approach: the generator should emit per-endpoint sub-groups: + +```csharp +// Generated for an [AllowAnonymous] endpoint: +{ + var epGroup = group.MapGroup(""); + epGroup.AllowAnonymous(); + new BrowseEndpoint().Map(epGroup); +} + +// Generated for [RequirePermission("Products.Edit")] endpoint: +{ + var epGroup = group.MapGroup(""); + epGroup.RequireAuthorization(new RequirePermissionAttribute("Products.Edit")); + new EditEndpoint().Map(epGroup); +} +``` + +### Files to Change + +| File | Change | +|------|--------| +| `framework/SimpleModule.Generator/Emitters/EndpointExtensionsEmitter.cs:47-48` | For endpoints with `AllowAnonymous` or `RequiredPermissions`, wrap in a sub-group with the appropriate authorization | +| `framework/SimpleModule.Generator/Emitters/EndpointExtensionsEmitter.cs:80-81` | Same for view endpoints | + +### Implementation Detail + +```csharp +// In the emitter, for each endpoint: +if (endpoint.AllowAnonymous) +{ + sb.AppendLine($" {{ var _eg = group.MapGroup(\"\"); _eg.AllowAnonymous(); new {endpoint.FullyQualifiedName}().Map(_eg); }}"); +} +else if (endpoint.RequiredPermissions.Length > 0) +{ + var perms = string.Join("\", \"", endpoint.RequiredPermissions); + sb.AppendLine($" {{ var _eg = group.MapGroup(\"\"); _eg.RequirePermission(\"{perms}\"); new {endpoint.FullyQualifiedName}().Map(_eg); }}"); +} +else +{ + sb.AppendLine($" new {endpoint.FullyQualifiedName}().Map(group);"); +} +``` + +### Risk: Low-Medium. The sub-group pattern is a known ASP.NET Minimal API technique. Need to verify it doesn't create issues with route resolution or Swagger grouping. + +--- + +## Priority & Sequencing + +| Order | Issue | Effort | Breaking? | Dependencies | +|-------|-------|--------|-----------|-------------| +| 1 | **P1-3** Contract lifetime | Low | No | None | +| 2 | **P1-2** Inertia JSON fix | Low | No | None | +| 3 | **P1-5** Endpoint enforcement | Low-Med | No | None | +| 4 | **P1-4** Event bus pipeline | Medium | No | None | +| 5 | **P1-1** IModule decomposition | High | Yes (phased) | All modules | + +Start with P1-3 and P1-2 (quick wins), then P1-5 (moderate), then P1-4 (design-heavy), and finally P1-1 (major refactor needing phased rollout). From db192ff0f3601d58b8c0ff962216ab1935501c5f Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 1 Apr 2026 17:32:26 +0000 Subject: [PATCH 4/6] Implement all P1 architectural improvements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five improvements to framework modularity and extensibility: P1-1: Decompose IModule into focused interfaces - Add IModuleServices, IModuleMenu, IModuleMiddleware, IModuleSettings, IModuleLifecycle as opt-in capability interfaces - Generator detects both old IModule overrides and new sub-interfaces - ModuleLifecycleHostedService prefers IModuleLifecycle when available - Fully backward compatible — existing modules unchanged P1-2: Fix Inertia serialization to use DI JSON options - InertiaResult now resolves JsonOptions from DI and merges with CamelCase policy, ensuring ModulesJsonResolver (Vogen, DTOs) applies - Eliminates split serialization between API and view endpoints - Options cached after first resolution for zero per-request overhead P1-3: Configurable contract implementation DI lifetime - Add [ContractLifetime(ServiceLifetime.Singleton)] attribute - Generator reads attribute and emits AddSingleton/AddTransient/AddScoped - Default remains Scoped when attribute is absent P1-4: Event bus pipeline behavior - Add IEventPipelineBehavior for cross-cutting concerns (logging, metrics, retries) around event handler dispatch - Behaviors form a middleware chain resolved from DI - Switch BackgroundEventChannel to bounded channel (10K capacity) for backpressure protection P1-5: Enforce endpoint auth attributes in generated code - Generator now emits AllowAnonymous() and RequirePermission() based on discovered [AllowAnonymous] and [RequirePermission] attributes - Uses per-endpoint sub-groups (non-breaking, no interface change) Also fix lint-staged config for csharpier v1.2.6 CLI change. --- .../ContractLifetimeAttribute.cs | 19 +++ .../Events/BackgroundEventChannel.cs | 8 +- .../SimpleModule.Core/Events/EventBus.cs | 17 +++ .../Events/IEventPipelineBehavior.cs | 30 ++++ .../Hosting/ModuleLifecycleHostedService.cs | 23 +++- .../SimpleModule.Core/IModuleLifecycle.cs | 14 ++ framework/SimpleModule.Core/IModuleMenu.cs | 12 ++ .../SimpleModule.Core/IModuleMiddleware.cs | 12 ++ .../SimpleModule.Core/IModuleServices.cs | 13 ++ .../SimpleModule.Core/IModuleSettings.cs | 12 ++ .../Inertia/InertiaResult.cs | 54 ++++++-- .../Discovery/DiscoveryData.cs | 10 +- .../Discovery/SymbolDiscovery.cs | 128 +++++++++++++++--- .../Emitters/DiagnosticEmitter.cs | 12 +- .../Emitters/EndpointExtensionsEmitter.cs | 35 ++++- .../Emitters/ModuleExtensionsEmitter.cs | 8 +- package.json | 2 +- 17 files changed, 361 insertions(+), 48 deletions(-) create mode 100644 framework/SimpleModule.Core/ContractLifetimeAttribute.cs create mode 100644 framework/SimpleModule.Core/Events/IEventPipelineBehavior.cs create mode 100644 framework/SimpleModule.Core/IModuleLifecycle.cs create mode 100644 framework/SimpleModule.Core/IModuleMenu.cs create mode 100644 framework/SimpleModule.Core/IModuleMiddleware.cs create mode 100644 framework/SimpleModule.Core/IModuleServices.cs create mode 100644 framework/SimpleModule.Core/IModuleSettings.cs diff --git a/framework/SimpleModule.Core/ContractLifetimeAttribute.cs b/framework/SimpleModule.Core/ContractLifetimeAttribute.cs new file mode 100644 index 00000000..9f774a4e --- /dev/null +++ b/framework/SimpleModule.Core/ContractLifetimeAttribute.cs @@ -0,0 +1,19 @@ +using Microsoft.Extensions.DependencyInjection; + +namespace SimpleModule.Core; + +/// +/// Specifies the DI service lifetime for an auto-discovered contract implementation. +/// When absent, the default lifetime is . +/// +/// +/// +/// [ContractLifetime(ServiceLifetime.Singleton)] +/// public sealed class MyCacheService : ICacheContracts { ... } +/// +/// +[AttributeUsage(AttributeTargets.Class, Inherited = false)] +public sealed class ContractLifetimeAttribute(ServiceLifetime lifetime) : Attribute +{ + public ServiceLifetime Lifetime { get; } = lifetime; +} diff --git a/framework/SimpleModule.Core/Events/BackgroundEventChannel.cs b/framework/SimpleModule.Core/Events/BackgroundEventChannel.cs index b5df4866..fecf7f85 100644 --- a/framework/SimpleModule.Core/Events/BackgroundEventChannel.cs +++ b/framework/SimpleModule.Core/Events/BackgroundEventChannel.cs @@ -11,8 +11,12 @@ namespace SimpleModule.Core.Events; public sealed partial class BackgroundEventChannel(ILogger logger) { private readonly Channel> _channel = - Channel.CreateUnbounded>( - new UnboundedChannelOptions { SingleReader = true } + Channel.CreateBounded>( + new BoundedChannelOptions(10_000) + { + SingleReader = true, + FullMode = BoundedChannelFullMode.Wait, + } ); internal ChannelReader> Reader => diff --git a/framework/SimpleModule.Core/Events/EventBus.cs b/framework/SimpleModule.Core/Events/EventBus.cs index 1a6991e6..ebb6e3cf 100644 --- a/framework/SimpleModule.Core/Events/EventBus.cs +++ b/framework/SimpleModule.Core/Events/EventBus.cs @@ -112,6 +112,23 @@ BackgroundEventChannel backgroundChannel /// public async Task PublishAsync(T @event, CancellationToken cancellationToken = default) where T : IEvent + { + var behaviors = serviceProvider.GetServices>(); + + // Build the pipeline: behaviors wrap around the handler dispatch + Func dispatch = () => DispatchToHandlers(@event, cancellationToken); + foreach (var behavior in behaviors.Reverse()) + { + var next = dispatch; + var b = behavior; + dispatch = () => b.HandleAsync(@event, next, cancellationToken); + } + + await dispatch(); + } + + private async Task DispatchToHandlers(T @event, CancellationToken cancellationToken) + where T : IEvent { var handlers = serviceProvider.GetServices>(); List? exceptions = null; diff --git a/framework/SimpleModule.Core/Events/IEventPipelineBehavior.cs b/framework/SimpleModule.Core/Events/IEventPipelineBehavior.cs new file mode 100644 index 00000000..dd9161f6 --- /dev/null +++ b/framework/SimpleModule.Core/Events/IEventPipelineBehavior.cs @@ -0,0 +1,30 @@ +namespace SimpleModule.Core.Events; + +/// +/// Defines a pipeline behavior that wraps event handler execution, enabling +/// cross-cutting concerns like logging, metrics, retries, or transaction boundaries. +/// +/// The event type to intercept. +/// +/// Pipeline behaviors execute in reverse registration order (outermost first), +/// forming a middleware chain around the event handler invocations. +/// Register via services.AddScoped<IEventPipelineBehavior<MyEvent>, MyBehavior>(). +/// +/// +/// +/// public sealed class LoggingBehavior<T> : IEventPipelineBehavior<T> where T : IEvent +/// { +/// public async Task HandleAsync(T @event, Func<Task> next, CancellationToken ct) +/// { +/// _logger.LogInformation("Handling {Event}", typeof(T).Name); +/// await next(); +/// _logger.LogInformation("Handled {Event}", typeof(T).Name); +/// } +/// } +/// +/// +public interface IEventPipelineBehavior + where T : IEvent +{ + Task HandleAsync(T @event, Func next, CancellationToken cancellationToken); +} diff --git a/framework/SimpleModule.Core/Hosting/ModuleLifecycleHostedService.cs b/framework/SimpleModule.Core/Hosting/ModuleLifecycleHostedService.cs index 42d3b9e2..bf6828f2 100644 --- a/framework/SimpleModule.Core/Hosting/ModuleLifecycleHostedService.cs +++ b/framework/SimpleModule.Core/Hosting/ModuleLifecycleHostedService.cs @@ -5,8 +5,9 @@ namespace SimpleModule.Core.Hosting; /// -/// Calls and lifecycle hooks -/// on all discovered modules during application startup and shutdown. +/// Calls lifecycle hooks on all discovered modules during application startup and shutdown. +/// Supports both / (default methods) +/// and the focused interface. /// public sealed partial class ModuleLifecycleHostedService( IEnumerable modules, @@ -22,7 +23,14 @@ public async Task StartAsync(CancellationToken cancellationToken) var moduleName = module.GetType().Name; try { - await module.OnStartAsync(cancellationToken); + if (module is IModuleLifecycle lifecycle) + { + await lifecycle.OnStartAsync(cancellationToken); + } + else + { + await module.OnStartAsync(cancellationToken); + } LogModuleStarted(logger, moduleName); } catch (OperationCanceledException) @@ -53,7 +61,14 @@ public async Task StopAsync(CancellationToken cancellationToken) var moduleName = module.GetType().Name; try { - await module.OnStopAsync(cancellationToken); + if (module is IModuleLifecycle lifecycle) + { + await lifecycle.OnStopAsync(cancellationToken); + } + else + { + await module.OnStopAsync(cancellationToken); + } LogModuleStopped(logger, moduleName); } catch (OperationCanceledException) diff --git a/framework/SimpleModule.Core/IModuleLifecycle.cs b/framework/SimpleModule.Core/IModuleLifecycle.cs new file mode 100644 index 00000000..36b09ec5 --- /dev/null +++ b/framework/SimpleModule.Core/IModuleLifecycle.cs @@ -0,0 +1,14 @@ +namespace SimpleModule.Core; + +/// +/// Implement this interface for startup/shutdown lifecycle hooks and health probes. +/// Preferred over overriding / +/// on the module class. +/// +public interface IModuleLifecycle +{ + Task OnStartAsync(CancellationToken cancellationToken); + Task OnStopAsync(CancellationToken cancellationToken); + Task CheckHealthAsync(CancellationToken cancellationToken) => + Task.FromResult(ModuleHealthStatus.Healthy); +} diff --git a/framework/SimpleModule.Core/IModuleMenu.cs b/framework/SimpleModule.Core/IModuleMenu.cs new file mode 100644 index 00000000..63659f8f --- /dev/null +++ b/framework/SimpleModule.Core/IModuleMenu.cs @@ -0,0 +1,12 @@ +using SimpleModule.Core.Menu; + +namespace SimpleModule.Core; + +/// +/// Implement this interface to contribute navigation menu items. +/// Preferred over overriding on the module class. +/// +public interface IModuleMenu +{ + void ConfigureMenu(IMenuBuilder menus); +} diff --git a/framework/SimpleModule.Core/IModuleMiddleware.cs b/framework/SimpleModule.Core/IModuleMiddleware.cs new file mode 100644 index 00000000..88f680ad --- /dev/null +++ b/framework/SimpleModule.Core/IModuleMiddleware.cs @@ -0,0 +1,12 @@ +using Microsoft.AspNetCore.Builder; + +namespace SimpleModule.Core; + +/// +/// Implement this interface to configure ASP.NET middleware for the module. +/// Preferred over overriding on the module class. +/// +public interface IModuleMiddleware +{ + void ConfigureMiddleware(IApplicationBuilder app); +} diff --git a/framework/SimpleModule.Core/IModuleServices.cs b/framework/SimpleModule.Core/IModuleServices.cs new file mode 100644 index 00000000..d0632330 --- /dev/null +++ b/framework/SimpleModule.Core/IModuleServices.cs @@ -0,0 +1,13 @@ +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; + +namespace SimpleModule.Core; + +/// +/// Implement this interface to register services in the DI container. +/// Preferred over overriding on the module class. +/// +public interface IModuleServices +{ + void ConfigureServices(IServiceCollection services, IConfiguration configuration); +} diff --git a/framework/SimpleModule.Core/IModuleSettings.cs b/framework/SimpleModule.Core/IModuleSettings.cs new file mode 100644 index 00000000..95d85c9b --- /dev/null +++ b/framework/SimpleModule.Core/IModuleSettings.cs @@ -0,0 +1,12 @@ +using SimpleModule.Core.Settings; + +namespace SimpleModule.Core; + +/// +/// Implement this interface to define runtime-configurable settings. +/// Preferred over overriding on the module class. +/// +public interface IModuleSettings +{ + void ConfigureSettings(ISettingsBuilder settings); +} diff --git a/framework/SimpleModule.Core/Inertia/InertiaResult.cs b/framework/SimpleModule.Core/Inertia/InertiaResult.cs index 23c35374..cf07a321 100644 --- a/framework/SimpleModule.Core/Inertia/InertiaResult.cs +++ b/framework/SimpleModule.Core/Inertia/InertiaResult.cs @@ -1,6 +1,8 @@ using System.Text.Json; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Json; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; namespace SimpleModule.Core.Inertia; @@ -12,10 +14,7 @@ public static IResult Render(string component, object? props = null) => internal sealed class InertiaResult : IResult { - private static readonly JsonSerializerOptions _camelCaseOptions = new() - { - PropertyNamingPolicy = JsonNamingPolicy.CamelCase, - }; + private static JsonSerializerOptions? _cachedOptions; private readonly string _component; private readonly object? _props; @@ -28,8 +27,9 @@ public InertiaResult(string component, object? props) public async Task ExecuteAsync(HttpContext httpContext) { + var options = GetSerializerOptions(httpContext); var sharedData = httpContext.RequestServices.GetService(); - var mergedProps = MergeProps(_props, sharedData); + var mergedProps = MergeProps(_props, sharedData, options); var pageData = new { @@ -44,21 +44,51 @@ public async Task ExecuteAsync(HttpContext httpContext) httpContext.Response.Headers["X-Inertia"] = "true"; httpContext.Response.Headers["Vary"] = "X-Inertia"; httpContext.Response.ContentType = "application/json"; - // Use the same serializer as the SSR path so Vogen value objects - // are consistently unwrapped (WriteAsJsonAsync uses a different - // JsonSerializerOptions from DI which may not handle them). - var json = JsonSerializer.Serialize(pageData, _camelCaseOptions); + var json = JsonSerializer.Serialize(pageData, options); await httpContext.Response.WriteAsync(json); return; } - var pageJson = JsonSerializer.Serialize(pageData, _camelCaseOptions); + var pageJson = JsonSerializer.Serialize(pageData, options); var renderer = httpContext.RequestServices.GetRequiredService(); await renderer.RenderPageAsync(httpContext, pageJson); } - private static object MergeProps(object? props, InertiaSharedData? sharedData) + /// + /// Resolves JSON serializer options from DI and merges with camelCase policy. + /// Caches the merged options for subsequent requests since the DI options are + /// configured once at startup and don't change. + /// + private static JsonSerializerOptions GetSerializerOptions(HttpContext httpContext) + { + if (_cachedOptions is not null) + return _cachedOptions; + + var diOptions = httpContext.RequestServices.GetService>(); + if (diOptions is not null) + { + var merged = new JsonSerializerOptions(diOptions.Value.SerializerOptions) + { + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + }; + _cachedOptions = merged; + return merged; + } + + var fallback = new JsonSerializerOptions + { + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + }; + _cachedOptions = fallback; + return fallback; + } + + private static object MergeProps( + object? props, + InertiaSharedData? sharedData, + JsonSerializerOptions options + ) { if (sharedData is null || sharedData.All.Count == 0) { @@ -77,7 +107,7 @@ private static object MergeProps(object? props, InertiaSharedData? sharedData) // Use JSON round-trip to merge endpoint props into shared data if (props is not null) { - var json = JsonSerializer.SerializeToElement(props, _camelCaseOptions); + var json = JsonSerializer.SerializeToElement(props, options); foreach (var property in json.EnumerateObject()) { result[property.Name] = property.Value; diff --git a/framework/SimpleModule.Generator/Discovery/DiscoveryData.cs b/framework/SimpleModule.Generator/Discovery/DiscoveryData.cs index 53767e6e..a23d7e77 100644 --- a/framework/SimpleModule.Generator/Discovery/DiscoveryData.cs +++ b/framework/SimpleModule.Generator/Discovery/DiscoveryData.cs @@ -185,7 +185,11 @@ public override int GetHashCode() } } -internal readonly record struct ViewInfoRecord(string FullyQualifiedName, string Page, SourceLocationRecord? Location); +internal readonly record struct ViewInfoRecord( + string FullyQualifiedName, + string Page, + SourceLocationRecord? Location +); internal readonly record struct DtoTypeInfoRecord( string FullyQualifiedName, @@ -292,7 +296,8 @@ internal readonly record struct ContractImplementationRecord( bool IsPublic, bool IsAbstract, bool DependsOnDbContext, - SourceLocationRecord? Location + SourceLocationRecord? Location, + int Lifetime ); internal readonly record struct PermissionClassRecord( @@ -511,6 +516,7 @@ internal sealed class ContractImplementationInfo public bool IsAbstract { get; set; } public bool DependsOnDbContext { get; set; } public SourceLocationRecord? Location { get; set; } + public int Lifetime { get; set; } = 1; // Default: Scoped (ServiceLifetime.Scoped = 1) } internal sealed class PermissionClassInfo diff --git a/framework/SimpleModule.Generator/Discovery/SymbolDiscovery.cs b/framework/SimpleModule.Generator/Discovery/SymbolDiscovery.cs index 6846abe0..6b7f9d9d 100644 --- a/framework/SimpleModule.Generator/Discovery/SymbolDiscovery.cs +++ b/framework/SimpleModule.Generator/Discovery/SymbolDiscovery.cs @@ -32,7 +32,10 @@ internal static class SymbolDiscovery return null; } - internal static DiscoveryData Extract(Compilation compilation, CancellationToken cancellationToken) + internal static DiscoveryData Extract( + Compilation compilation, + CancellationToken cancellationToken + ) { var hostAssemblyName = compilation.Assembly.Name; @@ -66,10 +69,20 @@ is not IAssemblySymbol assemblySymbol ) continue; - FindModuleTypes(assemblySymbol.GlobalNamespace, moduleAttributeSymbol, modules, cancellationToken); + FindModuleTypes( + assemblySymbol.GlobalNamespace, + moduleAttributeSymbol, + modules, + cancellationToken + ); } - FindModuleTypes(compilation.Assembly.GlobalNamespace, moduleAttributeSymbol, modules, cancellationToken); + FindModuleTypes( + compilation.Assembly.GlobalNamespace, + moduleAttributeSymbol, + modules, + cancellationToken + ); if (modules.Count == 0) return DiscoveryData.Empty; @@ -161,7 +174,12 @@ is not IAssemblySymbol assemblySymbol var rawDbContexts = new List(); var rawEntityConfigs = new List(); FindDbContextTypes(assembly.GlobalNamespace, "", rawDbContexts, cancellationToken); - FindEntityConfigTypes(assembly.GlobalNamespace, "", rawEntityConfigs, cancellationToken); + FindEntityConfigTypes( + assembly.GlobalNamespace, + "", + rawEntityConfigs, + cancellationToken + ); // Match each DbContext to the module whose namespace is closest foreach (var ctx in rawDbContexts) @@ -192,10 +210,20 @@ is not IAssemblySymbol assemblySymbol ) continue; - FindDtoTypes(assemblySymbol.GlobalNamespace, dtoAttributeSymbol, dtoTypes, cancellationToken); + FindDtoTypes( + assemblySymbol.GlobalNamespace, + dtoAttributeSymbol, + dtoTypes, + cancellationToken + ); } - FindDtoTypes(compilation.Assembly.GlobalNamespace, dtoAttributeSymbol, dtoTypes, cancellationToken); + FindDtoTypes( + compilation.Assembly.GlobalNamespace, + dtoAttributeSymbol, + dtoTypes, + cancellationToken + ); } cancellationToken.ThrowIfCancellationRequested(); @@ -583,7 +611,11 @@ is not IAssemblySymbol assemblySymbol e.AllowAnonymous )) .ToImmutableArray(), - m.Views.Select(v => new ViewInfoRecord(v.FullyQualifiedName, v.Page ?? "", v.Location)) + m.Views.Select(v => new ViewInfoRecord( + v.FullyQualifiedName, + v.Page ?? "", + v.Location + )) .ToImmutableArray(), m.Location )) @@ -615,7 +647,12 @@ is not IAssemblySymbol assemblySymbol )) .ToImmutableArray(), entityConfigs - .Select(e => new EntityConfigInfoRecord(e.ConfigFqn, e.EntityFqn, e.ModuleName, e.Location)) + .Select(e => new EntityConfigInfoRecord( + e.ConfigFqn, + e.EntityFqn, + e.ModuleName, + e.Location + )) .ToImmutableArray(), dependencies.ToImmutableArray(), illegalReferences.ToImmutableArray(), @@ -628,7 +665,8 @@ is not IAssemblySymbol assemblySymbol c.IsPublic, c.IsAbstract, c.DependsOnDbContext, - c.Location + c.Location, + c.Lifetime )) .ToImmutableArray(), permissionClasses @@ -732,27 +770,38 @@ CancellationToken cancellationToken SymbolDisplayFormat.FullyQualifiedFormat ), ModuleName = moduleName, - HasConfigureServices = DeclaresMethod( - typeSymbol, - "ConfigureServices" - ), + HasConfigureServices = + DeclaresMethod(typeSymbol, "ConfigureServices") + || ImplementsNamedInterface( + typeSymbol, + "SimpleModule.Core.IModuleServices" + ), HasConfigureEndpoints = DeclaresMethod( typeSymbol, "ConfigureEndpoints" ), - HasConfigureMenu = DeclaresMethod(typeSymbol, "ConfigureMenu"), - HasConfigureMiddleware = DeclaresMethod( - typeSymbol, - "ConfigureMiddleware" - ), + HasConfigureMenu = + DeclaresMethod(typeSymbol, "ConfigureMenu") + || ImplementsNamedInterface( + typeSymbol, + "SimpleModule.Core.IModuleMenu" + ), + HasConfigureMiddleware = + DeclaresMethod(typeSymbol, "ConfigureMiddleware") + || ImplementsNamedInterface( + typeSymbol, + "SimpleModule.Core.IModuleMiddleware" + ), HasConfigurePermissions = DeclaresMethod( typeSymbol, "ConfigurePermissions" ), - HasConfigureSettings = DeclaresMethod( - typeSymbol, - "ConfigureSettings" - ), + HasConfigureSettings = + DeclaresMethod(typeSymbol, "ConfigureSettings") + || ImplementsNamedInterface( + typeSymbol, + "SimpleModule.Core.IModuleSettings" + ), HasConfigureFeatureFlags = DeclaresMethod( typeSymbol, "ConfigureFeatureFlags" @@ -903,6 +952,16 @@ INamedTypeSymbol interfaceSymbol return false; } + private static bool ImplementsNamedInterface(INamedTypeSymbol typeSymbol, string interfaceFqn) + { + foreach (var iface in typeSymbol.AllInterfaces) + { + if (iface.ToDisplayString() == interfaceFqn) + return true; + } + return false; + } + private static void ScanModuleAssemblies( List modules, Dictionary moduleSymbols, @@ -1021,6 +1080,30 @@ private static bool InheritsFrom(INamedTypeSymbol typeSymbol, INamedTypeSymbol b return false; } + /// + /// Reads [ContractLifetime(ServiceLifetime.X)] from the type. + /// Returns 1 (Scoped) if the attribute is not present. + /// ServiceLifetime: Singleton=0, Scoped=1, Transient=2 + /// + private static int GetContractLifetime(INamedTypeSymbol typeSymbol) + { + foreach (var attr in typeSymbol.GetAttributes()) + { + var attrName = attr.AttributeClass?.ToDisplayString( + SymbolDisplayFormat.FullyQualifiedFormat + ); + if ( + attrName == "global::SimpleModule.Core.ContractLifetimeAttribute" + && attr.ConstructorArguments.Length > 0 + && attr.ConstructorArguments[0].Value is int lifetime + ) + { + return lifetime; + } + } + return 1; // Default: Scoped + } + private static bool HasDbContextConstructorParam(INamedTypeSymbol typeSymbol) { foreach (var ctor in typeSymbol.Constructors) @@ -1318,6 +1401,7 @@ List results IsAbstract = typeSymbol.IsAbstract, DependsOnDbContext = HasDbContextConstructorParam(typeSymbol), Location = GetSourceLocation(typeSymbol), + Lifetime = GetContractLifetime(typeSymbol), } ); } diff --git a/framework/SimpleModule.Generator/Emitters/DiagnosticEmitter.cs b/framework/SimpleModule.Generator/Emitters/DiagnosticEmitter.cs index 9ee7c1c7..992b7c22 100644 --- a/framework/SimpleModule.Generator/Emitters/DiagnosticEmitter.cs +++ b/framework/SimpleModule.Generator/Emitters/DiagnosticEmitter.cs @@ -723,7 +723,11 @@ public void Emit(SourceProductionContext context, DiscoveryData data) if (!perm.IsSealed) { context.ReportDiagnostic( - Diagnostic.Create(PermissionClassNotSealed, ToLocation(perm.Location), permCleanName) + Diagnostic.Create( + PermissionClassNotSealed, + ToLocation(perm.Location), + permCleanName + ) ); } @@ -968,7 +972,11 @@ public void Emit(SourceProductionContext context, DiscoveryData data) if (!feat.IsSealed) { context.ReportDiagnostic( - Diagnostic.Create(FeatureClassNotSealed, ToLocation(feat.Location), featCleanName) + Diagnostic.Create( + FeatureClassNotSealed, + ToLocation(feat.Location), + featCleanName + ) ); } diff --git a/framework/SimpleModule.Generator/Emitters/EndpointExtensionsEmitter.cs b/framework/SimpleModule.Generator/Emitters/EndpointExtensionsEmitter.cs index 717364e9..ca40d4cf 100644 --- a/framework/SimpleModule.Generator/Emitters/EndpointExtensionsEmitter.cs +++ b/framework/SimpleModule.Generator/Emitters/EndpointExtensionsEmitter.cs @@ -17,6 +17,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) sb.AppendLine("using Microsoft.AspNetCore.Routing;"); sb.AppendLine("using Microsoft.AspNetCore.Http;"); sb.AppendLine("using Microsoft.AspNetCore.Authorization;"); + sb.AppendLine("using SimpleModule.Core.Authorization;"); sb.AppendLine(); sb.AppendLine("namespace SimpleModule.Core;"); sb.AppendLine(); @@ -45,14 +46,14 @@ public void Emit(SourceProductionContext context, DiscoveryData data) ); foreach (var endpoint in module.Endpoints) { - sb.AppendLine($" new {endpoint.FullyQualifiedName}().Map(group);"); + EmitEndpointRegistration(sb, endpoint, "group"); } } else { foreach (var endpoint in module.Endpoints) { - sb.AppendLine($" new {endpoint.FullyQualifiedName}().Map(app);"); + EmitEndpointRegistration(sb, endpoint, "app"); } } @@ -109,4 +110,34 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.AddSource("EndpointExtensions.g.cs", SourceText.From(sb.ToString(), Encoding.UTF8)); } + + /// + /// Emits endpoint registration with authorization enforcement based on discovered attributes. + /// If the endpoint has [AllowAnonymous] or [RequirePermission], wraps it in a sub-group + /// to apply the policy without changing the IEndpoint.Map() return type (non-breaking). + /// + private static void EmitEndpointRegistration( + StringBuilder sb, + EndpointInfoRecord endpoint, + string routeBuilder + ) + { + if (endpoint.AllowAnonymous) + { + sb.AppendLine( + $" {{ var _eg = {routeBuilder}.MapGroup(\"\"); _eg.AllowAnonymous(); new {endpoint.FullyQualifiedName}().Map(_eg); }}" + ); + } + else if (endpoint.RequiredPermissions.Length > 0) + { + var perms = string.Join("\", \"", endpoint.RequiredPermissions); + sb.AppendLine( + $" {{ var _eg = {routeBuilder}.MapGroup(\"\"); _eg.RequirePermission(\"{perms}\"); new {endpoint.FullyQualifiedName}().Map(_eg); }}" + ); + } + else + { + sb.AppendLine($" new {endpoint.FullyQualifiedName}().Map({routeBuilder});"); + } + } } diff --git a/framework/SimpleModule.Generator/Emitters/ModuleExtensionsEmitter.cs b/framework/SimpleModule.Generator/Emitters/ModuleExtensionsEmitter.cs index 9ee59c5d..91666832 100644 --- a/framework/SimpleModule.Generator/Emitters/ModuleExtensionsEmitter.cs +++ b/framework/SimpleModule.Generator/Emitters/ModuleExtensionsEmitter.cs @@ -100,8 +100,14 @@ public void Emit(SourceProductionContext context, DiscoveryData data) if (kvp.Value.Count == 1) { var impl = kvp.Value[0]; + var method = impl.Lifetime switch + { + 0 => "AddSingleton", + 2 => "AddTransient", + _ => "AddScoped", + }; sb.AppendLine( - $" services.AddScoped<{impl.InterfaceFqn}, {impl.ImplementationFqn}>();" + $" services.{method}<{impl.InterfaceFqn}, {impl.ImplementationFqn}>();" ); } } diff --git a/package.json b/package.json index 01c5457b..f0731793 100644 --- a/package.json +++ b/package.json @@ -48,6 +48,6 @@ }, "lint-staged": { "*.{js,jsx,ts,tsx,css,json}": "biome check --write --no-errors-on-unmatched", - "*.cs": "dotnet csharpier" + "*.cs": "dotnet csharpier format" } } From b9c544df11ac9bd399894b6f6c24d3576acf355a Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 1 Apr 2026 17:42:04 +0000 Subject: [PATCH 5/6] Update Products module to use focused IModuleServices and IModuleMenu interfaces Migrate ProductsModule from relying solely on IModule default method overrides to explicitly implementing the focused capability interfaces. Demonstrates the new opt-in pattern from P1-1. --- modules/Products/src/SimpleModule.Products/ProductsModule.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/Products/src/SimpleModule.Products/ProductsModule.cs b/modules/Products/src/SimpleModule.Products/ProductsModule.cs index 128fb47e..52fc9bb5 100644 --- a/modules/Products/src/SimpleModule.Products/ProductsModule.cs +++ b/modules/Products/src/SimpleModule.Products/ProductsModule.cs @@ -12,7 +12,7 @@ namespace SimpleModule.Products; RoutePrefix = ProductsConstants.RoutePrefix, ViewPrefix = "/products" )] -public class ProductsModule : IModule +public class ProductsModule : IModule, IModuleServices, IModuleMenu { public void ConfigureServices(IServiceCollection services, IConfiguration configuration) { From 870636ca7fbb5d112e5b55a75dacb441d463a2a4 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 1 Apr 2026 18:10:30 +0000 Subject: [PATCH 6/6] Fix code quality issues from review - Extract duplicated ToLocation helper to shared LocationHelper class - Replace stringly-typed ImplementsNamedInterface with symbol-based lookup using compilation.GetTypeByMetadataName (zero-allocation) - Add volatile to InertiaResult._cachedOptions to prevent data race on concurrent first requests - Fix BackgroundEventChannel: change FullMode from Wait to DropWrite since Enqueue uses TryWrite (non-blocking). Wait mode with TryWrite silently drops events without backpressure. - Replace magic integers 0/2 with named constants for ServiceLifetime - Remove misleading inline comment on DtoTypeNoProperties diagnostic --- .../Events/BackgroundEventChannel.cs | 4 +- .../Inertia/InertiaResult.cs | 2 +- .../Discovery/SymbolDiscovery.cs | 69 ++++++++++------ .../Emitters/DiagnosticEmitter.cs | 78 ++++++++----------- .../Emitters/HostDbContextEmitter.cs | 13 +--- .../Emitters/ModuleExtensionsEmitter.cs | 10 ++- .../Helpers/LocationHelper.cs | 22 ++++++ 7 files changed, 111 insertions(+), 87 deletions(-) create mode 100644 framework/SimpleModule.Generator/Helpers/LocationHelper.cs diff --git a/framework/SimpleModule.Core/Events/BackgroundEventChannel.cs b/framework/SimpleModule.Core/Events/BackgroundEventChannel.cs index fecf7f85..6fcaaa0d 100644 --- a/framework/SimpleModule.Core/Events/BackgroundEventChannel.cs +++ b/framework/SimpleModule.Core/Events/BackgroundEventChannel.cs @@ -15,7 +15,7 @@ public sealed partial class BackgroundEventChannel(ILogger(T @event) [LoggerMessage( Level = LogLevel.Warning, - Message = "Background event '{EventName}' dropped — channel closed" + Message = "Background event '{EventName}' dropped — channel full or closed" )] private static partial void LogEventDropped(ILogger logger, string eventName); } diff --git a/framework/SimpleModule.Core/Inertia/InertiaResult.cs b/framework/SimpleModule.Core/Inertia/InertiaResult.cs index cf07a321..46ea62d1 100644 --- a/framework/SimpleModule.Core/Inertia/InertiaResult.cs +++ b/framework/SimpleModule.Core/Inertia/InertiaResult.cs @@ -14,7 +14,7 @@ public static IResult Render(string component, object? props = null) => internal sealed class InertiaResult : IResult { - private static JsonSerializerOptions? _cachedOptions; + private static volatile JsonSerializerOptions? _cachedOptions; private readonly string _component; private readonly object? _props; diff --git a/framework/SimpleModule.Generator/Discovery/SymbolDiscovery.cs b/framework/SimpleModule.Generator/Discovery/SymbolDiscovery.cs index 6b7f9d9d..4f91ea10 100644 --- a/framework/SimpleModule.Generator/Discovery/SymbolDiscovery.cs +++ b/framework/SimpleModule.Generator/Discovery/SymbolDiscovery.cs @@ -57,6 +57,18 @@ CancellationToken cancellationToken "SimpleModule.Core.IViewEndpoint" ); + // Resolve focused sub-interface symbols for module capability detection + var moduleServicesSymbol = compilation.GetTypeByMetadataName( + "SimpleModule.Core.IModuleServices" + ); + var moduleMenuSymbol = compilation.GetTypeByMetadataName("SimpleModule.Core.IModuleMenu"); + var moduleMiddlewareSymbol = compilation.GetTypeByMetadataName( + "SimpleModule.Core.IModuleMiddleware" + ); + var moduleSettingsSymbol = compilation.GetTypeByMetadataName( + "SimpleModule.Core.IModuleSettings" + ); + var modules = new List(); foreach (var reference in compilation.References) @@ -72,6 +84,10 @@ is not IAssemblySymbol assemblySymbol FindModuleTypes( assemblySymbol.GlobalNamespace, moduleAttributeSymbol, + moduleServicesSymbol, + moduleMenuSymbol, + moduleMiddlewareSymbol, + moduleSettingsSymbol, modules, cancellationToken ); @@ -80,6 +96,10 @@ is not IAssemblySymbol assemblySymbol FindModuleTypes( compilation.Assembly.GlobalNamespace, moduleAttributeSymbol, + moduleServicesSymbol, + moduleMenuSymbol, + moduleMiddlewareSymbol, + moduleSettingsSymbol, modules, cancellationToken ); @@ -716,6 +736,10 @@ is not IAssemblySymbol assemblySymbol private static void FindModuleTypes( INamespaceSymbol namespaceSymbol, INamedTypeSymbol moduleAttributeSymbol, + INamedTypeSymbol? moduleServicesSymbol, + INamedTypeSymbol? moduleMenuSymbol, + INamedTypeSymbol? moduleMiddlewareSymbol, + INamedTypeSymbol? moduleSettingsSymbol, List modules, CancellationToken cancellationToken ) @@ -726,7 +750,16 @@ CancellationToken cancellationToken if (member is INamespaceSymbol childNamespace) { - FindModuleTypes(childNamespace, moduleAttributeSymbol, modules, cancellationToken); + FindModuleTypes( + childNamespace, + moduleAttributeSymbol, + moduleServicesSymbol, + moduleMenuSymbol, + moduleMiddlewareSymbol, + moduleSettingsSymbol, + modules, + cancellationToken + ); } else if (member is INamedTypeSymbol typeSymbol) { @@ -772,9 +805,9 @@ CancellationToken cancellationToken ModuleName = moduleName, HasConfigureServices = DeclaresMethod(typeSymbol, "ConfigureServices") - || ImplementsNamedInterface( - typeSymbol, - "SimpleModule.Core.IModuleServices" + || ( + moduleServicesSymbol is not null + && ImplementsInterface(typeSymbol, moduleServicesSymbol) ), HasConfigureEndpoints = DeclaresMethod( typeSymbol, @@ -782,15 +815,15 @@ CancellationToken cancellationToken ), HasConfigureMenu = DeclaresMethod(typeSymbol, "ConfigureMenu") - || ImplementsNamedInterface( - typeSymbol, - "SimpleModule.Core.IModuleMenu" + || ( + moduleMenuSymbol is not null + && ImplementsInterface(typeSymbol, moduleMenuSymbol) ), HasConfigureMiddleware = DeclaresMethod(typeSymbol, "ConfigureMiddleware") - || ImplementsNamedInterface( - typeSymbol, - "SimpleModule.Core.IModuleMiddleware" + || ( + moduleMiddlewareSymbol is not null + && ImplementsInterface(typeSymbol, moduleMiddlewareSymbol) ), HasConfigurePermissions = DeclaresMethod( typeSymbol, @@ -798,9 +831,9 @@ CancellationToken cancellationToken ), HasConfigureSettings = DeclaresMethod(typeSymbol, "ConfigureSettings") - || ImplementsNamedInterface( - typeSymbol, - "SimpleModule.Core.IModuleSettings" + || ( + moduleSettingsSymbol is not null + && ImplementsInterface(typeSymbol, moduleSettingsSymbol) ), HasConfigureFeatureFlags = DeclaresMethod( typeSymbol, @@ -952,16 +985,6 @@ INamedTypeSymbol interfaceSymbol return false; } - private static bool ImplementsNamedInterface(INamedTypeSymbol typeSymbol, string interfaceFqn) - { - foreach (var iface in typeSymbol.AllInterfaces) - { - if (iface.ToDisplayString() == interfaceFqn) - return true; - } - return false; - } - private static void ScanModuleAssemblies( List modules, Dictionary moduleSymbols, diff --git a/framework/SimpleModule.Generator/Emitters/DiagnosticEmitter.cs b/framework/SimpleModule.Generator/Emitters/DiagnosticEmitter.cs index 992b7c22..a69e5a26 100644 --- a/framework/SimpleModule.Generator/Emitters/DiagnosticEmitter.cs +++ b/framework/SimpleModule.Generator/Emitters/DiagnosticEmitter.cs @@ -3,27 +3,11 @@ using System.Collections.Immutable; using System.Linq; using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.Text; namespace SimpleModule.Generator; internal sealed class DiagnosticEmitter : IEmitter { - /// - /// Reconstructs a from a serializable . - /// Returns for null (metadata-only types). - /// - private static Location ToLocation(SourceLocationRecord? loc) - { - if (loc is null) - return Location.None; - - var start = new LinePosition(loc.Value.StartLine, loc.Value.StartCharacter); - var end = new LinePosition(loc.Value.EndLine, loc.Value.EndCharacter); - var span = new LinePositionSpan(start, end); - return Location.Create(loc.Value.FilePath, TextSpan.FromBounds(0, 0), span); - } - internal static readonly DiagnosticDescriptor DuplicateDbSetPropertyName = new( id: "SM0001", title: "Duplicate DbSet property name across modules", @@ -331,7 +315,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( EmptyModuleName, - ToLocation(module.Location), + LocationHelper.ToLocation(module.Location), Strip(module.FullyQualifiedName) ) ); @@ -350,7 +334,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( DuplicateModuleName, - ToLocation(module.Location), + LocationHelper.ToLocation(module.Location), module.ModuleName, Strip(existingFqn), Strip(module.FullyQualifiedName) @@ -386,7 +370,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( EmptyModuleWarning, - ToLocation(module.Location), + LocationHelper.ToLocation(module.Location), module.ModuleName ) ); @@ -413,7 +397,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( MultipleIdentityDbContexts, - ToLocation(ctx.Location), + LocationHelper.ToLocation(ctx.Location), Strip(firstIdentity.Value.FullyQualifiedName), firstIdentity.Value.ModuleName, Strip(ctx.FullyQualifiedName), @@ -431,7 +415,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( IdentityDbContextBadTypeArgs, - ToLocation(ctx.Location), + LocationHelper.ToLocation(ctx.Location), Strip(ctx.FullyQualifiedName), ctx.ModuleName, 0 @@ -455,7 +439,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( EntityConfigForMissingEntity, - ToLocation(config.Location), + LocationHelper.ToLocation(config.Location), Strip(config.EntityFqn), Strip(config.ConfigFqn), config.ModuleName @@ -473,7 +457,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( DuplicateEntityConfiguration, - ToLocation(config.Location), + LocationHelper.ToLocation(config.Location), Strip(config.EntityFqn), existing, Strip(config.ConfigFqn) @@ -532,7 +516,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( CircularModuleDependency, - ToLocation(cycleLoc), + LocationHelper.ToLocation(cycleLoc), cycleStr, howStr, first, @@ -547,7 +531,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( IllegalImplementationReference, - ToLocation(illegal.Location), + LocationHelper.ToLocation(illegal.Location), illegal.ReferencingModuleName, illegal.ReferencedModuleName, illegal.ReferencedAssemblyName @@ -564,7 +548,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( ContractInterfaceTooLargeError, - ToLocation(iface.Location), + LocationHelper.ToLocation(iface.Location), Strip(iface.InterfaceName), iface.MethodCount, shortName @@ -577,7 +561,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( ContractInterfaceTooLargeWarning, - ToLocation(iface.Location), + LocationHelper.ToLocation(iface.Location), Strip(iface.InterfaceName), iface.MethodCount, shortName @@ -612,7 +596,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( MissingContractInterfaces, - ToLocation(depModuleLoc), + LocationHelper.ToLocation(depModuleLoc), dep.ModuleName, dep.ContractsAssemblyName ) @@ -642,7 +626,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( ContractImplementationNotPublic, - ToLocation(impl.Location), + LocationHelper.ToLocation(impl.Location), Strip(impl.ImplementationFqn), Strip(impl.InterfaceFqn) ) @@ -654,7 +638,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( ContractImplementationIsAbstract, - ToLocation(impl.Location), + LocationHelper.ToLocation(impl.Location), Strip(impl.ImplementationFqn), Strip(impl.InterfaceFqn) ) @@ -675,7 +659,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( NoContractImplementation, - ToLocation(iface.Location), + LocationHelper.ToLocation(iface.Location), Strip(iface.InterfaceName), moduleName ) @@ -702,7 +686,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( MultipleContractImplementations, - ToLocation(validImpls[1].Location), + LocationHelper.ToLocation(validImpls[1].Location), Strip(kvp.Key), validImpls[0].ModuleName, string.Join(", ", names) @@ -725,7 +709,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( PermissionClassNotSealed, - ToLocation(perm.Location), + LocationHelper.ToLocation(perm.Location), permCleanName ) ); @@ -739,7 +723,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( PermissionFieldNotConstString, - ToLocation(field.Location), + LocationHelper.ToLocation(field.Location), permCleanName, field.FieldName ) @@ -759,7 +743,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( PermissionValueBadPattern, - ToLocation(field.Location), + LocationHelper.ToLocation(field.Location), field.Value, permCleanName ) @@ -775,7 +759,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( PermissionValueWrongPrefix, - ToLocation(field.Location), + LocationHelper.ToLocation(field.Location), field.Value, perm.ModuleName ) @@ -789,7 +773,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( DuplicatePermissionValue, - ToLocation(field.Location), + LocationHelper.ToLocation(field.Location), field.Value, existingOwner, permCleanName @@ -826,7 +810,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) contractsIdx >= 0 ? fqn.Substring(0, contractsIdx + ".Contracts".Length) : fqn; context.ReportDiagnostic( - Diagnostic.Create(DtoTypeNoProperties, Location.None, fqn, contractsAsm) // DTOs are typically in metadata (Contracts assemblies) + Diagnostic.Create(DtoTypeNoProperties, Location.None, fqn, contractsAsm) ); } } @@ -860,7 +844,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( DuplicateViewPageName, - ToLocation(view.Location), + LocationHelper.ToLocation(view.Location), view.Page, Strip(existing.EndpointFqn), existing.ModuleName, @@ -890,7 +874,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( ViewPagePrefixMismatch, - ToLocation(view.Location), + LocationHelper.ToLocation(view.Location), Strip(view.FullyQualifiedName), module.ModuleName, view.Page @@ -909,7 +893,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( ViewEndpointWithoutViewPrefix, - ToLocation(module.Location), + LocationHelper.ToLocation(module.Location), module.ModuleName, module.Views.Length, module.ModuleName.ToLowerInvariant() @@ -931,7 +915,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( InterceptorDependsOnDbContext, - ToLocation(interceptor.Location), + LocationHelper.ToLocation(interceptor.Location), Strip(interceptor.FullyQualifiedName), interceptor.ModuleName, Strip(paramFqn) @@ -952,7 +936,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( MultipleModuleOptions, - ToLocation(kvp.Value[1].Location), + LocationHelper.ToLocation(kvp.Value[1].Location), kvp.Key, Strip(kvp.Value[0].FullyQualifiedName), Strip(kvp.Value[1].FullyQualifiedName) @@ -974,7 +958,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( FeatureClassNotSealed, - ToLocation(feat.Location), + LocationHelper.ToLocation(feat.Location), featCleanName ) ); @@ -988,7 +972,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( FeatureFieldNotConstString, - ToLocation(field.Location), + LocationHelper.ToLocation(field.Location), field.FieldName, featCleanName ) @@ -1002,7 +986,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( FeatureFieldNamingViolation, - ToLocation(field.Location), + LocationHelper.ToLocation(field.Location), field.Value, featCleanName ) @@ -1017,7 +1001,7 @@ public void Emit(SourceProductionContext context, DiscoveryData data) context.ReportDiagnostic( Diagnostic.Create( DuplicateFeatureName, - ToLocation(field.Location), + LocationHelper.ToLocation(field.Location), field.Value, Strip(existingOwner), featCleanName diff --git a/framework/SimpleModule.Generator/Emitters/HostDbContextEmitter.cs b/framework/SimpleModule.Generator/Emitters/HostDbContextEmitter.cs index 36fb3db7..8a27cf48 100644 --- a/framework/SimpleModule.Generator/Emitters/HostDbContextEmitter.cs +++ b/framework/SimpleModule.Generator/Emitters/HostDbContextEmitter.cs @@ -7,17 +7,6 @@ namespace SimpleModule.Generator; internal sealed class HostDbContextEmitter : IEmitter { - private static Location ToLocation(SourceLocationRecord? loc) - { - if (loc is null) - return Location.None; - - var start = new LinePosition(loc.Value.StartLine, loc.Value.StartCharacter); - var end = new LinePosition(loc.Value.EndLine, loc.Value.EndCharacter); - var span = new LinePositionSpan(start, end); - return Location.Create(loc.Value.FilePath, TextSpan.FromBounds(0, 0), span); - } - #pragma warning disable CA1308 // Schema names are conventionally lowercase in PostgreSQL/SQL Server public void Emit(SourceProductionContext context, DiscoveryData data) { @@ -104,7 +93,7 @@ string ModuleName context.ReportDiagnostic( Diagnostic.Create( DiagnosticEmitter.DuplicateDbSetPropertyName, - ToLocation(dbCtxLoc), + LocationHelper.ToLocation(dbCtxLoc), entry.PropertyName, existing.ModuleName, TypeMappingHelpers.StripGlobalPrefix(existing.EntityFqn), diff --git a/framework/SimpleModule.Generator/Emitters/ModuleExtensionsEmitter.cs b/framework/SimpleModule.Generator/Emitters/ModuleExtensionsEmitter.cs index 91666832..3c2f9629 100644 --- a/framework/SimpleModule.Generator/Emitters/ModuleExtensionsEmitter.cs +++ b/framework/SimpleModule.Generator/Emitters/ModuleExtensionsEmitter.cs @@ -8,6 +8,12 @@ namespace SimpleModule.Generator; internal sealed class ModuleExtensionsEmitter : IEmitter { + // Mirror Microsoft.Extensions.DependencyInjection.ServiceLifetime enum values. + // The generator reads these as integers from [ContractLifetime] attribute arguments + // because it targets netstandard2.0 and cannot reference the enum directly. + private const int LifetimeSingleton = 0; + private const int LifetimeTransient = 2; + public void Emit(SourceProductionContext context, DiscoveryData data) { var (sortedModules, sortResult) = TopologicalSort.SortModulesWithResult(data); @@ -102,8 +108,8 @@ public void Emit(SourceProductionContext context, DiscoveryData data) var impl = kvp.Value[0]; var method = impl.Lifetime switch { - 0 => "AddSingleton", - 2 => "AddTransient", + LifetimeSingleton => "AddSingleton", + LifetimeTransient => "AddTransient", _ => "AddScoped", }; sb.AppendLine( diff --git a/framework/SimpleModule.Generator/Helpers/LocationHelper.cs b/framework/SimpleModule.Generator/Helpers/LocationHelper.cs new file mode 100644 index 00000000..e9739faa --- /dev/null +++ b/framework/SimpleModule.Generator/Helpers/LocationHelper.cs @@ -0,0 +1,22 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Text; + +namespace SimpleModule.Generator; + +internal static class LocationHelper +{ + /// + /// Reconstructs a from a serializable . + /// Returns for null (metadata-only types). + /// + internal static Location ToLocation(SourceLocationRecord? loc) + { + if (loc is null) + return Location.None; + + var start = new LinePosition(loc.Value.StartLine, loc.Value.StartCharacter); + var end = new LinePosition(loc.Value.EndLine, loc.Value.EndCharacter); + var span = new LinePositionSpan(start, end); + return Location.Create(loc.Value.FilePath, TextSpan.FromBounds(0, 0), span); + } +}