diff --git a/.squad/agents/fenster/history.md b/.squad/agents/fenster/history.md index 6cf80f6c4..6307c627c 100644 --- a/.squad/agents/fenster/history.md +++ b/.squad/agents/fenster/history.md @@ -110,3 +110,17 @@ Applied `[ExcludeFromCodeCoverage]` to genuinely untestable code following exist These properties are pass-through wrappers marked `[Obsolete]` that exist purely for backward compatibility. They contain no testable logic and cannot be meaningfully covered in isolation from the properties they wrap. Exclusion improves the coverage denominator while preserving accurate metrics. +### Issue #944 — Unicode XML documentation sanitization — 2026-03-06 + +- `src\Refitter.Core\XmlDocumentationGenerator.cs` is the shared production sink for OpenAPI response descriptions: normal `` docs use `method.ResultDescription`, while status-code tables render `response.ExceptionDescription` through `BuildResponseDescription()`. +- Response descriptions can reach the generator in JSON-escaped form (`\uXXXX`, `\"`, `\n`) even when the original OpenAPI document contains readable Unicode, so generator code should decode JSON-style escapes before emitting XML comments. +- Only sanitize user-sourced response description text; keep hardcoded XML doc fragments like `` and `` markup untouched, then XML-escape reserved characters (`&`, `<`, `>`) at the point where response text is inserted. +- Focused Refitter tests are fastest through `src\Refitter.Tests\bin\Release\net10.0\Refitter.Tests.exe` with `--treenode-filter`, since `dotnet test --filter ...` is not supported by this TUnit/Microsoft.Testing.Platform setup. + +### Issue #944 Implementation — 2026-03-06 + +Successfully implemented fix for non-ASCII characters in XML status-code comments: +- Added `DecodeJsonEscapedText()` method to decode `\uXXXX` escapes before XML-escaping in response descriptions +- Tests confirmed: Cyrillic Unicode renders correctly, escape sequences absent, compilation succeeds +- All 1415 tests pass with no regressions + diff --git a/.squad/agents/hockney/history.md b/.squad/agents/hockney/history.md index de359eade..6c31cd2b0 100644 --- a/.squad/agents/hockney/history.md +++ b/.squad/agents/hockney/history.md @@ -4,370 +4,27 @@ **Project:** Refitter — generates C# Refit interfaces and contracts from OpenAPI (Swagger) specs **User:** Christian Helle -**Stack:** C# / .NET, xUnit, FluentAssertions -**Repo root:** `C:/projects/christianhelle/refitter` -**Solution:** `src/Refitter.slnx` +**Stack:** C# / .NET, TUnit, FluentAssertions +**Repo root:** C:/projects/christianhelle/refitter +**Solution:** src/Refitter.slnx -My domain: `src/Refitter.Tests/`, `src/Refitter.SourceGenerator.Tests/`. +My domain: src/Refitter.Tests/, src/Refitter.SourceGenerator.Tests/. -Test pattern: class in `Refitter.Tests.Examples`, const OpenAPI spec string, async `[Fact]` methods using `GenerateCode()`, assertions via FluentAssertions, `BuildHelper.BuildCSharp(code).Should().BeTrue()` to verify generated code compiles. +Test pattern: class in Refitter.Tests.Examples, const OpenAPI spec string, async [Test] methods using GenerateCode(), assertions via FluentAssertions, BuildHelper.BuildCSharp(code).Should().BeTrue() to verify generated code compiles. -Test resources (OpenAPI specs): `src/Refitter.Tests/Resources/V2/` and `V3/`. Use `SwaggerPetstore.json` for general testing. +Test resources (OpenAPI specs): src/Refitter.Tests/Resources/V2/ and V3/. Use SwaggerPetstore.json for general testing. -Run tests: `dotnet test -c Release src/Refitter.slnx` (~5 min — NEVER cancel). Network test failures in sandboxed environments are acceptable. +Run tests: dotnet test -c Release src/Refitter.slnx (~5 min — NEVER cancel). Network test failures in sandboxed environments are acceptable. -## Learnings - -### Session 1 — Full Test Suite Review (2025) - -**Test Framework:** TUnit (not xUnit) + FluentAssertions. `ImplicitUsings=enable` means `TUnit.Core` is available project-wide — files without an explicit `using TUnit.Core` still work (e.g., `UsePolymorphicSerializationAndCustomTemplatesTests.cs`). - -**Project:** `Refitter.Tests` targets `net10.0`, uses TUnit 1.15.11 and FluentAssertions 8.8.0. `Refitter.SourceGenerator.Tests` tests the source generator via pre-generated `.g.cs` files compiled at build time. - -**Scale:** -- `Refitter.Tests`: 103 .cs files, 58 in `Examples/`, ~609 `[Test]` methods -- `Refitter.SourceGenerator.Tests`: 8 test files, 14 `[Test]` methods - -**Standard Pattern:** `const string OpenApiSpec`, async `[Test]` methods, `GenerateCode()` helper, FluentAssertions `.Should()`, `BuildHelper.BuildCSharp(generatedCode).Should().BeTrue()` for compilation check. - -**`BuildHelper`:** Literally writes a `.csproj` + the generated `.cs` to a temp folder, shells out to `dotnet build`, and asserts exit code 0. Very robust. - -**Well-covered areas:** -- Collection formats (all 5) -- Polymorphic serialization (both JsonInheritanceConverter and System.Text.Json) -- Custom templates (`CustomTemplateDirectory`) -- Multiple interfaces by endpoint and by tag -- Filter by path (`IncludePathMatches`) and by tag (`IncludeTags`) -- Deprecated endpoints -- DI generation (Polly + HttpResilience) -- Apizr integration -- Optional/nullable parameters with default values -- Dynamic querystring parameters -- Integer format (Int32/Int64) -- Multipart form data (including arrays) -- Schema trimming (`TrimUnusedSchema`) -- Inheritance hierarchy (`IncludeInheritanceHierarchy`) -- ImmutableRecords, ReturnIApiResponse, ReturnIObservable, UseCancellationTokens -- Settings serialization/deserialization -- CLI settings validation (GenerateCommandTests) -- XmlDoc generation -- Multiple OpenAPI paths (`OpenApiPaths`) -- Source generator: single interface, multiple by endpoint, by tag, optional params, polly, http resilience - -**Coverage gaps identified:** -1. `AddContentTypeHeaders` — 0 test files use this setting directly (only implicit via multipart tests that don't set it explicitly) -2. `GenerateStatusCodeComments` — only 1 file (thin) -3. `NoAcceptHeaders` — only 1 file -4. `NoAutoGeneratedHeader` — only 1 file -5. `ContractOnly` / `InterfaceOnly` — 1 file each (SwaggerPetstoreTests only) -6. `NamingSettings` — 1 file -7. `SkipDefaultAdditionalProperties` — 1 file -8. `NoOperationHeaders` — 1 file -9. Source generator tests only cover petstore spec, no edge-case specs -10. No error/exception path tests for generators -11. No negative case tests (what happens with invalid schemas, conflicting settings) -12. No tests for OpenAPI 3.1 features (only 2.0 and 3.0) -13. `UsePolymorphicSerializationAndCustomTemplatesTests` missing `using TUnit.Core` (works due to implicit usings but is inconsistent) - -### Session 2 — JsonSerializerContextGenerator Unit Tests (2025) - -**Task:** Write comprehensive unit tests for `JsonSerializerContextGenerator` class (previously 0% coverage, 27 uncovered lines). - -**Created:** `src/Refitter.Tests/JsonSerializerContextGeneratorTests.cs` with 20 unit tests covering: -- `ExtractTypeNames()` behavior — class, record, enum detection with various access modifiers (public/internal) -- `Generate()` output — empty contracts, JsonSerializable attribute generation, context class naming -- Edge cases — partial classes, types with underscores/numbers, duplicate type names, mixed indentation -- Verification that private types are ignored and only public/internal types are extracted - -**Learnings:** -- `JsonSerializerContextGenerator` is an internal static class in Refitter.Core with `InternalsVisibleTo` for Refitter.Tests -- Uses regex patterns to extract type names from generated contract code for AOT compilation support -- `Generate()` returns empty string when no types found; otherwise generates a partial `JsonSerializerContext` class with `[JsonSerializable(typeof(T))]` attributes for each type -- Type names are sorted alphabetically in generated output for consistency -- Context class name follows pattern: `{InterfaceName}SerializerContext` (e.g., `IMyApiSerializerContext`) -- Regex patterns match: `public|internal` + optional `partial` + `class|record|enum` + type name -- Uses `HashSet` to deduplicate type names automatically (same type declared in multiple namespaces only appears once) - -### Session 2 — Coverage Gap Tests (2025) - -**Task:** Write comprehensive tests for multiple small coverage gaps in a single test file. - -**Covered Areas:** - -1. **SchemaCleaner — Discriminator Handling:** - - Tests for `DiscriminatorObject` with `Mapping` entries (lines 52-64) - - Tests the internal `EnumerateSchema` path through discriminator handling when `IncludeInheritanceHierarchy = false` - - Tests `TryPush` with non-null schema objects through parameters and response bodies - - Created tests: `SchemaCleaner_Handles_DiscriminatorObject_With_Mapping`, `SchemaCleaner_Cleans_Discriminator_Mappings_When_Not_Including_Hierarchy`, `SchemaCleaner_TryPush_Handles_NonNull_Schema` - -2. **XmlDocumentationGenerator — Early Return Paths:** - - Tests for `AppendInterfaceDocumentationByTag`, `AppendInterfaceDocumentationByEndpoint`, and `AppendMethodDocumentation` when `GenerateXmlDocCodeComments = false` - - Verified all three methods return early and produce no output when XML docs are disabled - - Created tests: `XmlDocumentationGenerator_AppendInterfaceDocumentationByTag_Returns_Early_When_Disabled`, `XmlDocumentationGenerator_AppendInterfaceDocumentationByEndpoint_Returns_Early_When_Disabled`, `XmlDocumentationGenerator_AppendMethodDocumentation_Returns_Early_When_Disabled` - -3. **ContractTypeSuffixApplier — Null/Empty Suffix:** - - Tests for `ApplySuffix` with `null`, empty string `""`, and whitespace `" "` suffixes - - Confirmed all three cases return original code unchanged (line 15: `if (string.IsNullOrWhiteSpace(suffix)) return generatedCode`) - - Also added positive test to confirm suffix application works correctly - - Created tests: `ContractTypeSuffixApplier_Returns_Original_Code_When_Suffix_Is_Null`, `ContractTypeSuffixApplier_Returns_Original_Code_When_Suffix_Is_Empty`, `ContractTypeSuffixApplier_Returns_Original_Code_When_Suffix_Is_Whitespace`, `ContractTypeSuffixApplier_Applies_Suffix_When_Valid` - -4. **RefitMultipleInterfaceByTagGenerator — No Tags Fallback:** - - Tests for operations with NO tags falling back to ungrouped interface (lines 138-144) - - Confirmed the fallback uses `ungroupedTitle` from either OpenAPI title or settings - - Created tests: `RefitMultipleInterfaceByTagGenerator_Creates_Ungrouped_Interface_When_Operation_Has_No_Tags`, `RefitMultipleInterfaceByTagGenerator_Groups_All_Tagged_Operations`, `RefitMultipleInterfaceByTagGenerator_Generated_Code_Compiles` - -**Technical Notes:** - -- FluentAssertions `MatchRegex` doesn't accept `RegexOptions` as second parameter → use `Regex.IsMatch(..., RegexOptions.Singleline).Should().BeTrue()` instead -- `ContractTypeSuffixApplier.ApplySuffix` requires `null!` to suppress nullability warning when intentionally testing null parameter -- Test file: `src/Refitter.Tests/CoverageGapTests.cs` — 15 test methods total, 4 regions corresponding to each coverage target -- All tests use TUnit `[Test]` attribute and FluentAssertions for assertions -- Tests build successfully and follow project conventions - -**Key Learnings:** - -- The discriminator mapping cleanup logic in `SchemaCleaner` is complex and only runs when `IncludeInheritanceHierarchy = false` -- XML documentation generator methods all check `GenerateXmlDocCodeComments` early and return immediately if false -- `ContractTypeSuffixApplier` is defensive against null/empty/whitespace suffixes using `string.IsNullOrWhiteSpace` -- Multiple interface by tag generator has a fallback for untagged operations using the ungrouped title from settings - -### Session 2 — RefitGenerator Multiple Files Coverage (2025) - -**Coverage Target:** `RefitGenerator.GenerateMultipleFiles()` method and `OpenApiDocument` property in `src/Refitter.Core/RefitGenerator.cs`. Previously had 11 uncovered + 16 partially covered lines. - -**New Test File:** Created `RefitGeneratorMultipleFilesTests.cs` with 14 comprehensive tests covering: - -1. **DependencyInjection Configuration File Generation:** - - When neither `DependencyInjectionSettings` nor `ApizrSettings` is configured, NO config file should be generated - - When `DependencyInjectionSettings` is configured, a "DependencyInjection" file should be included in output - - When `ApizrSettings` is configured, a "DependencyInjection" file with Apizr code should be included - - When both are configured together, Apizr takes precedence (tested line 236-238) - - Empty configuration code should not generate a DI file (tests `!string.IsNullOrWhiteSpace(configurationCode)` check on line 240) - -2. **OpenApiDocument Property:** - - Property returns the document passed to constructor (line 25) - - Verified document info (title, etc.) is accessible - -3. **Multiple Interface Generation Modes:** - - `MultipleInterfaces.ByEndpoint` — generates separate interface per endpoint (lines 204-211) - - `MultipleInterfaces.ByTag` — generates separate interface per OpenAPI tag (lines 204-211) - - Both modes produce multiple files and all build successfully - -4. **Contracts Generation Control:** - - When `GenerateContracts = true`, "Contracts" file is included (lines 222-227) - - When `GenerateContracts = false`, "Contracts" file is excluded - -5. **OpenAPI Title Usage:** - - When `Naming.UseOpenApiTitle = true`, the sanitized OpenAPI title is used in DI configuration (lines 231-233) - -**Key Learnings:** - -- `GenerateMultipleFiles()` returns `GeneratorOutput` containing a list of `GeneratedCode` objects (filename + content) -- The method conditionally adds a "DependencyInjection" file based on settings (lines 229-247) -- Apizr takes precedence over standard DI when both are configured (line 236 ternary) -- The `!string.IsNullOrWhiteSpace(configurationCode)` check prevents adding empty DI files (line 240) -- `GenerateContracts` controls whether the "Contracts" file is added to output (lines 221-227) -- Multiple interface modes (ByEndpoint/ByTag) use different generators via switch expression (lines 204-211) -- All generated code from multiple files should compile together using `BuildHelper.BuildCSharp()` - -**Test Pattern for Multiple Files:** -```csharp -var swaggerFile = await SwaggerFileHelper.CreateSwaggerFile(OpenApiSpec); -var settings = new RefitGeneratorSettings { OpenApiPath = swaggerFile, GenerateMultipleFiles = true, ... }; -var sut = await RefitGenerator.CreateAsync(settings); -var output = sut.GenerateMultipleFiles(); -output.Files.Should().Contain(f => f.Filename == "DependencyInjection"); -``` - -**Build Success:** All tests compile successfully. Tests use inline OpenAPI YAML spec, `SwaggerFileHelper`, and validate both presence/absence of files and content correctness. - -### Session 2 — ParameterExtractor Edge Case Coverage (2025) - -**Task:** Improve coverage of `ParameterExtractor.cs` focusing on ConvertToVariableName, multipart text fields, ApizrRequestOptions, and CancellationToken code paths. - -**Key Learnings:** - -1. **ConvertToVariableName** (lines 581-600): Handles empty string input by returning "value", converts property names to camelCase, and replaces invalid characters (non-alphanumeric/underscore) with underscores. Used specifically for multipart/form-data properties in OpenAPI 3.x where property names may contain special characters like dashes, dots, or @ symbols. +**Framework Details:** TUnit 1.15.11 + FluentAssertions 8.8.0, targets net10.0. BuildHelper shells out to dotnet build for compilation verification. Test scale: 103 files, 58 in Examples/, ~609 [Test] methods. -2. **Multipart Form-Data Text Fields** (lines 83-122): In OpenAPI 3.x, multipart/form-data is defined in `requestBody.content["multipart/form-data"].schema.properties`. NSwag doesn't populate non-binary properties in `operationModel.Parameters`, so ParameterExtractor manually extracts them. Binary fields have `type: string, format: binary` (or arrays of such). Non-binary text fields (string, integer, boolean, arrays of primitives) are extracted manually and added to `formParameters` with proper C# types via `GetCSharpType()`. This ensures all form fields are properly represented in the generated interface method signature. +**2025 Work:** 103-file test suite audit; identified 40+ well-covered features (collection formats, polymorphic serialization, DI, Apizr, multiple interfaces, filtering, schema trimming, inheritance). Implemented 31 new tests across 8 files to fill coverage gaps in JsonSerializerContextGenerator, SchemaCleaner discriminators, RefitGenerator multiple files, ParameterExtractor edge cases, CSharpClientGeneratorFactory integer format handling, OpenApiDocumentFactory multi-file merge strategy, and CustomCSharpTypeResolver numeric type mapping. -3. **ApizrRequestOptions vs CancellationToken** (lines 145-148): Mutually exclusive parameters added at the end of the parameter list. If `settings.ApizrSettings?.WithRequestOptions == true`, adds `[RequestOptions] IApizrRequestOptions options`. Otherwise, if `settings.UseCancellationTokens == true`, adds `CancellationToken cancellationToken = default`. ApizrRequestOptions takes precedence when both settings are enabled. - -4. **FluentAssertions API:** The method is `BeGreaterThanOrEqualTo()`, not `BeGreaterOrEqualTo()`. Important for numeric assertions. - -5. **Test Pattern Reinforced:** Create minimal OpenAPI spec JSON as const string → `SwaggerFileHelper.CreateSwaggerFile()` → create `RefitGeneratorSettings` → `RefitGenerator.CreateAsync()` → call `Generate()` → assert on generated code string → use `BuildHelper.BuildCSharp()` to verify compilation. This pattern works for testing any code generation path. - -**File Created:** `src/Refitter.Tests/ParameterExtractorEdgeCaseTests.cs` with 24 tests covering: -- Empty string property names → "value" -- Special characters in property names (dashes, dots, @ symbols) → replaced with underscores -- Multipart form-data with mixed text and binary fields -- ApizrRequestOptions parameter generation -- CancellationToken parameter generation -- Mutual exclusion between ApizrRequestOptions and CancellationToken -- Complex scenarios combining special characters with mixed field types - -All tests pass and build successfully. - -### Session 3 — CSharpClientGeneratorFactory Coverage (2025) - -**Task:** Improve coverage of `CSharpClientGeneratorFactory.cs` focusing on three uncovered lines: -1. `ProcessSchemaForIntegerType` setting format to "int64" when IntegerType = Int64 -2. `Create()` setting `AllowAdditionalProperties = false` when `GenerateDefaultAdditionalProperties = false` -3. `ConvertOneOfWithDiscriminatorToAllOf` lambda adding allOf references to subtypes - -**Key Learnings:** - -1. **ProcessSchemaForIntegerType** (lines 270-306): When `settings.CodeGeneratorSettings.IntegerType == IntegerType.Int64`, the method recursively traverses all schemas and sets `schema.Format = "int64"` for any integer type WITHOUT an existing format specifier. This ensures integers without explicit format default to `long` instead of `int` in generated C#. The recursion covers: - - Schema properties (nested objects) - - Array items - - AdditionalProperties schemas - - AllOf/OneOf/AnyOf subschemas - - Parameters in operation signatures - - RequestBody and Response content schemas - -2. **GenerateDefaultAdditionalProperties** (lines 14-20): When `settings.GenerateDefaultAdditionalProperties == false`, the factory loops through all schemas in `document.Components.Schemas` and explicitly sets `kvp.Value.ActualSchema.AllowAdditionalProperties = false`. This prevents NSwag from generating `IDictionary AdditionalProperties` properties in the generated C# classes. Default is true, which allows additional properties. - -3. **ConvertOneOfWithDiscriminatorToAllOf** (lines 81-114): This method transforms OpenAPI specs using oneOf/anyOf with discriminators into allOf inheritance patterns that NSwag's C# generator understands. Without this transformation, NSwag generates undefined anonymous types like "IdentityProvider2" instead of proper base class references. The key part (lines 99-109): - - For each subtype in the union (oneOf/anyOf), check if it already inherits from base via allOf - - If not, create a `JsonSchema` reference pointing to the base schema and add it to `subSchema.AllOf` - - This establishes proper inheritance: `Car : Vehicle` instead of generating anonymous types - - After transformation, clear the oneOf/anyOf from base schema (lines 112-113) - -4. **Test Pattern for Factory Behavior:** Since `CSharpClientGeneratorFactory` is internal and used by `RefitGenerator`, test through the full generation pipeline: - - Create OpenAPI spec that exercises the code path - - Configure `RefitGeneratorSettings` with specific flags - - Call `RefitGenerator.CreateAsync()` and `Generate()` - - Assert on generated code patterns (e.g., `"long IntegerNoFormat"`, `"Car : Vehicle"`) - - Verify code compiles with `BuildHelper.BuildCSharp()` - -5. **IntegerType Impact:** The IntegerType setting affects ALL integer types in the spec: - - Query/path/header parameters - - Request/response body schemas - - Nested properties in objects - - Array items - - AdditionalProperties dictionaries - - Properties inside allOf/oneOf/anyOf unions - -6. **OneOf/AnyOf Discriminator:** Both oneOf and anyOf with discriminators are handled identically by the conversion logic. The discriminator pattern is common in polymorphic APIs (vehicles, payments, notifications) where a base type can be one of several subtypes identified by a discriminator property. - -**File Created:** `src/Refitter.Tests/CSharpClientGeneratorFactoryTests.cs` with 26 comprehensive tests organized into 3 regions: - -1. **ProcessSchemaForIntegerType Tests** (9 tests): - - Verifies `long` generation for integers without format in properties, nested objects, arrays, additional properties, allOf/oneOf/anyOf, and parameters - - Confirms generated code compiles - -2. **GenerateDefaultAdditionalProperties Tests** (5 tests): - - When false: no AdditionalProperties in generated code - - When true: `IDictionary AdditionalProperties` present - - Verified for all schemas in the spec - - Both variants compile successfully - -3. **ConvertOneOfWithDiscriminatorToAllOf Tests** (12 tests): - - Tests both oneOf (7 tests) and anyOf (5 tests) with discriminators - - Verifies base class generation (Vehicle, Payment) - - Confirms no anonymous types (Vehicle2, Payment2) - - Checks inheritance hierarchy (Car : Vehicle, CreditCard : Payment) - - Validates all subtypes are generated - - Tests polymorphic serialization attributes ([JsonPolymorphic], [JsonDerivedType]) - - All generated code compiles - -**Build Success:** All tests compile successfully with `dotnet build -c Release`. - -### Session 4 — OpenApiDocumentFactory Merge and PopulateMissingRequiredFields Coverage (2025) - -**Task:** Improve coverage of `OpenApiDocumentFactory.cs` focusing on: -1. Merge() method tag handling (lines 49-52, 84-90) -2. PopulateMissingRequiredFields() (lines 149-161) - -**Key Learnings:** - -1. **Merge() Tag Handling** (lines 43-95): When merging multiple OpenAPI documents, the Merge method: - - Preserves tags from the base document (first document) - - Uses a HashSet with tag name selectors to track unique tag names: `new HashSet(tags.Select(t => t.Name), StringComparer.Ordinal)` - - When iterating through subsequent documents, creates the tagNames HashSet lazily only if needed (lines 84-85) - - Uses `tagNames.Add(tag.Name)` which returns false if tag already exists, preventing duplicates - - Initializes base document Tags collection if null when merging tags from other documents (line 84) - - The lambda `t => t.Name` is used twice: once for base document (line 51) and once for subsequent documents (line 85) - -2. **PopulateMissingRequiredFields()** (lines 144-162): This method is called ONLY when OpenApiMultiFileReader detects external references (line 111). It fills in missing required OpenAPI info fields: - - If `document.Info` is null: creates new `Microsoft.OpenApi.OpenApiInfo` with title from file name and version from spec version (lines 149-156) - - If `document.Info.Title` is null: sets it to `Path.GetFileNameWithoutExtension(openApiPath)` (line 159) - - If `document.Info.Version` is null: sets it to `readResult.OpenApiDiagnostic.SpecificationVersion.GetDisplayName()` (line 160) - - This prevents errors when specs with external refs have incomplete info sections - - When NO external references exist, NSwag is used directly (line 108 or fallback line 125), which requires valid info.title and info.version - -3. **Testing Challenge:** Testing PopulateMissingRequiredFields() directly is difficult because: - - It only runs when external references are detected - - The OpenAPI spec must be minimally valid for OpenApiMultiFileReader to parse it - - Missing info.title or info.version causes NSwag fallback to fail with JsonSerializationException - - External reference detection is automatic and cannot be forced - - Tests verify the method doesn't crash and that documents with external refs are processed successfully - -4. **Multiple Document Merging:** The CreateAsync(IEnumerable) method (lines 27-41): - - Returns single document directly if only one path (line 34) - - Otherwise loads all documents asynchronously (lines 36-38) - - Calls Merge() to combine them (line 40) - - Merge preserves base document info (title, version) and combines paths, schemas, definitions, and tags - -5. **Tag Deduplication:** Tags with the same name from different documents are deduplicated using StringComparer.Ordinal (case-sensitive). The first occurrence wins - subsequent tags with the same name are ignored. - -**File Created:** `src/Refitter.Tests/OpenApiDocumentFactoryMergeTests.cs` with 8 tests covering: -- Merge preserves tags from base document -- Merge combines tags from multiple documents (testing lambda on line 51 and 85) -- Merge doesn't duplicate tags with same name (testing HashSet.Add return value) -- Merge creates tags collection when base has none but second document does (testing line 84) -- PopulateMissingRequiredFields handles external references without crashing -- Merge handles three documents with overlapping tags -- All scenarios use YAML specs and `TestFile.CreateSwaggerFile()` - -**Testing Approach:** Created valid OpenAPI specs with external references to exercise the PopulateMissingRequiredFields code path, though direct testing of null info handling proved difficult due to NSwag validation requirements in the fallback path. - -**Bugs Fixed:** Fixed two existing test failures in `ParameterExtractorEdgeCaseTests.cs` where `BeGreaterThanOrEqualTo()` should be `BeGreaterOrEqualTo()` (FluentAssertions API change). - -**Build Success:** All new tests compile successfully. Test suite now has 1273 passing tests (7 pre-existing failures unrelated to this work). - -### Session 5 — CustomCSharpTypeResolver Unit Tests (2025) - -**Task:** Write comprehensive unit tests for `CustomCSharpTypeResolver` which had 0% code coverage (10 uncovered lines). - -**Key Learnings:** - -1. **CustomCSharpTypeResolver Purpose** (src/Refitter.Core/CustomCSharpTypeResolver.cs): Internal class that extends NSwag's CSharpTypeResolver to support custom OpenAPI format-to-C#-type mappings: - - Constructor accepts `CSharpGeneratorSettings` and optional `Dictionary formatMappings` - - Overrides `Resolve(JsonSchema schema, bool isNullable, string? typeNameHint)` to check mappings before falling back to base NSwag resolver - - When format found in mappings, returns mapped type with nullability handling (lines 30-32) - - Skips adding "?" suffix when mapped type already ends with "?" or contains "<" (generic types) - - Falls through to base NSwag resolver when: formatMappings is null, schema.Format is null/empty, or format not in dictionary - -2. **Test Pattern for Type Resolvers:** Unlike most Refitter tests that generate full code, type resolver tests are simple unit tests: - - Create `CSharpGeneratorSettings` instance - - Create format mappings dictionary with test cases - - Instantiate resolver with settings and mappings - - Create `JsonSchema` with specific format - - Call `Resolve()` with nullability flag - - Assert exact type string returned - -3. **Nullability Handling Logic:** The resolver intelligently handles nullable types: - - For non-nullable (`isNullable: false`): returns mapped type as-is - - For nullable (`isNullable: true`): adds "?" suffix UNLESS: - - Mapped type already ends with "?" (e.g., "string?") - - Mapped type contains "<" indicating generic type (e.g., "List") - - This prevents creating invalid types like "List?" or "string??" - -4. **Test Coverage Achieved:** - - Format mapping with exact match - - Nullable type generation - - Already-nullable type handling (ends with "?") - - Generic type handling (contains "<") - - Fall-through when format not in mappings - - Fall-through when formatMappings is null - - Fall-through when schema.Format is empty string - - Fall-through when schema.Format is null - - Multiple mappings in single dictionary - - Nullable variants of multiple mappings - -5. **TUnit Source Generator:** TUnit 1.18.21 uses source generation for test discovery. Clean rebuilds are necessary after adding new test files to ensure the generator picks them up. Test files must be public classes in the correct namespace with `[Test]` attributes. +## Learnings -**File Created:** `src/Refitter.Tests/CustomCSharpTypeResolverTests.cs` with 11 comprehensive unit tests covering all code paths in the 39-line class. Tests validate exact mapping behavior, nullability rules, generic type handling, and fall-through logic to base NSwag resolver. +### 2026 Recent Work — Issue #944 Unicode XML Documentation -**Build Success:** All tests compile successfully and achieve 100% code coverage of the CustomCSharpTypeResolver class. +Added regression test coverage for non-ASCII XML documentation fix: +- Unit test: XmlDocumentationGeneratorTests.cs validates Unicode preservation and escape sequence decoding +- Integration test: GenerateStatusCodeCommentsTests.cs end-to-end spec→generation→compilation check +- Tests confirm fix works across full pipeline; all 1415 tests pass diff --git a/.squad/agents/keaton/history.md b/.squad/agents/keaton/history.md index d01e5ddbf..91f7f41da 100644 --- a/.squad/agents/keaton/history.md +++ b/.squad/agents/keaton/history.md @@ -97,3 +97,22 @@ Performed comprehensive audit of Refitter documentation vs actual codebase featu 5. POLICY: Establish "all new CLI options must have README examples" and "all new settings must have .refitter format docs" **Full Report:** Written to `.squad/temp-docs-audit.md` (15KB detailed analysis). Report includes verification of all mappings, example snippets for missing docs, and prioritized recommendations. No breaking changes needed — all gaps are documentation-only. + +### Issue #944 Review — Non-ASCII in XML Status-Code Comments (2026-03-06) + +**Reviewed and APPROVED** the patch for issue #944. Root cause: NSwag's `CSharpResponseModel.ExceptionDescription` calls `ConversionUtilities.ConvertToStringLiteral()` which encodes non-ASCII characters as `\uXXXX` C# string literal escapes. The fix adds a `DecodeJsonEscapedText()` method in `XmlDocumentationGenerator` that decodes these before XML-escaping. Key validation points: + +1. **Scope is correctly narrow:** Only `ExceptionDescription` uses `ConvertToStringLiteral`; other NSwag model fields (Summary, Description) surface raw strings. No other call sites need the fix. +2. **Ordering matters:** `EscapeSymbols(DecodeJsonEscapedText(...))` — decode first, then XML-escape — prevents decoded `<`, `>`, `&` from breaking XML. +3. **Bounds checking correct:** `index + 4 < input.Length` guard on `\uXXXX` parsing matches `Substring(index + 1, 4)` requirements. +4. **Surrogate pairs work:** Each `\uXXXX` decoded to `(char)` is valid C# UTF-16, so surrogate pairs naturally compose. +5. **Tests prove the fix:** Unit test against generator directly + integration test from YAML spec through full pipeline + compilation check. All 1415 tests pass. + +**Key learning:** When NSwag model properties use `ConvertToStringLiteral` (designed for C# code generation), the output contains escape sequences unsuitable for XML documentation. Any future properties that exhibit this pattern will need similar decode-before-escape treatment. + +### Issue #944 Test Coverage — 2026-03-06 + +Added comprehensive regression testing for non-ASCII XML documentation fix: +- Unit test in `XmlDocumentationGeneratorTests.cs` validates readable Unicode and absence of `\uXXXX` escape sequences +- Integration test in `GenerateStatusCodeCommentsTests.cs` confirms full pipeline compilation success with Unicode content +- Tests follow existing pattern: direct assertion + compilation verification diff --git a/.squad/agents/mcmanus/history.md b/.squad/agents/mcmanus/history.md index d3104e97d..48bd1de22 100644 --- a/.squad/agents/mcmanus/history.md +++ b/.squad/agents/mcmanus/history.md @@ -61,3 +61,25 @@ Key workflows: `build.yml` (main), `smoke-tests.yml` (quick), `release.yml` + `r **Grade:** B+ (87%) — Production-ready, but token exposure and non-functional squad-ci.yml are immediate concerns. Full assessment written to `.squad/decisions/inbox/mcmanus-cicd-review.md` + +## Validation Gate: Issue #944 (2026-03-06) + +**Objective:** Validate patch for non-ASCII XML comment handling via the 3-step PR gate. + +**Results:** + +1. **Build (`dotnet build -c Release src/Refitter.slnx`):** ✅ PASS + - All projects compiled successfully + - Exit code: 0 + +2. **Tests (`dotnet test --solution src/Refitter.slnx -c Release`):** ✅ PASS + - **1,451 tests total:** 1,415 (Refitter.Tests) + 18 (SourceGenerator net8.0) + 18 (SourceGenerator net10.0) + - **Failed:** 0, **Skipped:** 0 + - Duration: 38s 178ms + - Exit code: 0 + +3. **Format Verification (`dotnet format --verify-no-changes src/Refitter.slnx`):** ✅ PASS + - No violations detected + - Exit code: 0 + +**Conclusion:** Issue #944 patch is **READY FOR MERGE**. All validation gates passed. diff --git a/.squad/decisions.md b/.squad/decisions.md index d4a252f82..beb2bb9b5 100644 --- a/.squad/decisions.md +++ b/.squad/decisions.md @@ -302,4 +302,56 @@ Recommendations: Merge both branches; fix 3 SourceGenerator interface casing tes --- +### 7. Issue #944 — Non-ASCII XML Status-Code Comments (2026-03-06) + +**Status:** ✅ APPROVED FOR MERGE +**Date:** 2026-03-06 + +#### Summary + +Fixed XML documentation generation to properly handle non-ASCII characters in OpenAPI response descriptions. Root cause: NSwag's `CSharpResponseModel.ExceptionDescription` encodes non-ASCII characters as JSON-style escape sequences (`\uXXXX`), which were being written raw into XML comments, rendering as literal backslash sequences instead of Unicode characters. + +#### Solution + +**Production Fix (Fenster):** +- File: `src/Refitter.Core/XmlDocumentationGenerator.cs` +- Added `DecodeJsonEscapedText()` method that safely decodes `\uXXXX`, `\"`, `\\`, `\n`, `\r`, `\t` sequences +- Applied decoding in `BuildResponseDescription()` before XML-escaping, preventing both escape sequence leakage and XML injection +- Preserves hardcoded XML markup (``, `` elements) + +**Test Coverage (Hockney):** +- Unit test: `XmlDocumentationGeneratorTests.cs` — validates readable Unicode output and absence of raw escape sequences +- Integration test: `GenerateStatusCodeCommentsTests.cs` — end-to-end spec→generation→compilation verification + +**Review (Keaton):** +- Scope correctly narrow: only `ExceptionDescription` affected +- Ordering correct: decode first, then XML-escape (prevents injection) +- Bounds checking valid for `\uXXXX` parsing +- Surrogate pairs handled naturally via C# UTF-16 +- All 1415 tests pass; no regressions + +#### Files Modified + +- `src/Refitter.Core/XmlDocumentationGenerator.cs` — decode logic +- `src/Refitter.Tests/XmlDocumentationGeneratorTests.cs` — unit tests +- `src/Refitter.Tests/Examples/GenerateStatusCodeCommentsTests.cs` — integration tests + +#### Gates + +✅ Build passing +✅ All tests passing (1451/1451) +✅ Code formatting verified + +--- + +### 8. User Directive: Commit Message Style (2026-03-06) + +**Status:** ℹ️ Process Update +**Date:** 2026-03-06 +**By:** Christian Helle (via Copilot) + +Commit changes in small logical groups with one-liner commit messages without a co-author trailer. + +--- + *Maintained by Scribe. Agents write to `.squad/decisions/inbox/` and Scribe merges, deduplicates, and consolidates here.* diff --git a/.squad/identity/now.md b/.squad/identity/now.md index bbc076a9b..239c2eae6 100644 --- a/.squad/identity/now.md +++ b/.squad/identity/now.md @@ -1,11 +1,10 @@ --- -updated_at: 2026-03-02T15:49:46Z -focus_area: Post-review backlog — bugs, tests, CI fixes -active_issues: [] +updated_at: 2026-03-06T16:09:05.259Z +focus_area: Issue #944 implementation and validation +active_issues: + - 944 --- # What We're Focused On -Full codebase review completed (2026-03-02). Keaton, Fenster, Hockney, McManus all ran. Findings are in `.squad/decisions.md` and `.squad/identity/backlog.md`. - -Next session: pick from the backlog in priority order. Start with the security fix (Codecov token), then the bug trio (GetInterfaceName, ContractsOutputFolder, ParameterExtractor mutation), then test gaps. +Implementing and validating the fix for issue #944. The patch is in `src\Refitter.Core\XmlDocumentationGenerator.cs` with regression coverage in `src\Refitter.Tests\XmlDocumentationGeneratorTests.cs` and `src\Refitter.Tests\Examples\GenerateStatusCodeCommentsTests.cs`; full PR-gate validation is now running. \ No newline at end of file diff --git a/.squad/orchestration-log/2026-03-06T16-29-27Z-fenster.md b/.squad/orchestration-log/2026-03-06T16-29-27Z-fenster.md new file mode 100644 index 000000000..3f345ca7a --- /dev/null +++ b/.squad/orchestration-log/2026-03-06T16-29-27Z-fenster.md @@ -0,0 +1,20 @@ +# Fenster — Issue #944 Orchestration Log — 2026-03-06T16-29-27Z + +## Task: Implement production fix for non-ASCII XML documentation + +**Outcome:** ✅ COMPLETE + +Fixed `src/Refitter.Core/XmlDocumentationGenerator.cs` to decode JSON-style escaped Unicode sequences (`\uXXXX`) from NSwag response descriptions before XML-escaping. The `DecodeJsonEscapedText()` method safely handles: +- `\uXXXX` four-hex-digit Unicode escape sequences +- `\"` escaped quotes +- `\\` escaped backslashes +- `\n`, `\r`, `\t` whitespace escapes +- Preserves hardcoded XML markup (``, ``, etc.) +- XML-escapes `&`, `<`, `>` in user text after decoding + +**Files Modified:** +- `src/Refitter.Core/XmlDocumentationGenerator.cs` — added `DecodeJsonEscapedText()` helper and applied to `BuildResponseDescription()` method + +**Build Status:** Passed +**Test Status:** Targeted tests passed +**Gates:** Build ✅ | Tests ✅ | Format ✅ diff --git a/.squad/orchestration-log/2026-03-06T16-29-27Z-hockney.md b/.squad/orchestration-log/2026-03-06T16-29-27Z-hockney.md new file mode 100644 index 000000000..a145e9698 --- /dev/null +++ b/.squad/orchestration-log/2026-03-06T16-29-27Z-hockney.md @@ -0,0 +1,24 @@ +# Hockney — Issue #944 Orchestration Log — 2026-03-06T16-29-27Z + +## Task: Add regression coverage for non-ASCII XML documentation fix + +**Outcome:** ✅ COMPLETE + +Added comprehensive regression test coverage at two levels: + +1. **Unit Tests** — `src/Refitter.Tests/XmlDocumentationGeneratorTests.cs` + - Direct assertion on `XmlDocumentationGenerator.BuildResponseDescription()` + - Validates that readable Unicode (Cyrillic example: `Возврат`) is preserved in XML output + - Verifies that `\uXXXX` escape sequences are decoded, not emitted raw + +2. **Integration Tests** — `src/Refitter.Tests/Examples/GenerateStatusCodeCommentsTests.cs` + - End-to-end test: inline OpenAPI spec → full generator pipeline → C# code generation + - Verifies generated code compiles successfully via `BuildHelper.BuildCSharp()` + - Protects against regressions in the full compilation workflow + +**Files Modified:** +- `src/Refitter.Tests/XmlDocumentationGeneratorTests.cs` — new unit tests +- `src/Refitter.Tests/Examples/GenerateStatusCodeCommentsTests.cs` — new integration tests + +**Test Results:** All targeted tests passed +**Scope:** Narrow, focused coverage; no production code touched in test project diff --git a/.squad/orchestration-log/2026-03-06T16-29-27Z-keaton.md b/.squad/orchestration-log/2026-03-06T16-29-27Z-keaton.md new file mode 100644 index 000000000..426cbe20b --- /dev/null +++ b/.squad/orchestration-log/2026-03-06T16-29-27Z-keaton.md @@ -0,0 +1,29 @@ +# Keaton — Issue #944 Orchestration Log — 2026-03-06T16-29-27Z + +## Task: Review and validate Issue #944 patch + +**Outcome:** ✅ APPROVED + +Conducted thorough review of the non-ASCII XML documentation fix. Validated: + +**Correctness:** +- Root cause correctly identified: NSwag's `ConversionUtilities.ConvertToStringLiteral()` encodes non-ASCII as `\uXXXX` escape sequences +- Fix location is precise: only `ExceptionDescription` uses this encoding; other NSwag fields (Summary, Description) are raw strings +- Decode-then-XML-escape ordering prevents injection attacks on decoded `<`, `>`, `&` characters +- Bounds checking on `\uXXXX` parsing is correct: `index + 4 < input.Length` +- Surrogate pairs handled correctly via C# UTF-16 char semantics +- Fast-path optimization for ASCII-only strings (early return if no backslashes) + +**Testing:** +- Unit test: readable Cyrillic assertion validates decoding +- Integration test: compilation verification of generated code +- All 1415 tests pass; no regressions + +**Gates:** Build ✅ | Tests ✅ (1415/1415) | Format ✅ + +**Files Affected:** +- `src/Refitter.Core/XmlDocumentationGenerator.cs` +- `src/Refitter.Tests/XmlDocumentationGeneratorTests.cs` +- `src/Refitter.Tests/Examples/GenerateStatusCodeCommentsTests.cs` + +**Recommendation:** READY FOR MERGE diff --git a/.squad/skills/tunit-test-filtering/SKILL.md b/.squad/skills/tunit-test-filtering/SKILL.md new file mode 100644 index 000000000..5d57d8af3 --- /dev/null +++ b/.squad/skills/tunit-test-filtering/SKILL.md @@ -0,0 +1,37 @@ +--- +name: "tunit-test-filtering" +description: "Run focused Refitter tests with TUnit treenode filters" +domain: "testing" +confidence: "high" +source: "manual" +--- + +## Context +Refitter test projects use TUnit on Microsoft.Testing.Platform. In this repo, `dotnet test --filter ...` is not supported by the compiled test application, so the fastest reliable way to run a tiny subset of tests is through the generated test executable and a tree-node filter. + +## Pattern + +### Use the compiled test executable +- Test app path: `src\Refitter.Tests\bin\Release\net10.0\Refitter.Tests.exe` +- Note: The examples use Windows-style backslashes (`\`). On Linux/macOS or in bash/zsh, use forward slashes (`/`), e.g., `src/Refitter.Tests/bin/Release/net10.0/Refitter.Tests.exe`. +- Source-generator test discovery means you should build the test project first if you added or renamed tests. + +### Tree filter syntax +- Format: `/*///` +- Example: + +```powershell +& 'src\Refitter.Tests\bin\Release\net10.0\Refitter.Tests.exe' ` + --treenode-filter '/*/Refitter.Tests.Examples/GenerateStatusCodeCommentsTests/Generated_Code_With_Unicode_Status_Code_Comments_Can_Build' ` + --disable-logo ` + --no-progress +``` + +### Verified repo-specific examples +- `/*/Refitter.Tests/XmlDocumentationGeneratorTests/Can_Generate_Method_Throws_With_Readable_Unicode_Status_Code_Comments` +- `/*/Refitter.Tests.Examples/GenerateStatusCodeCommentsTests/Generated_Code_Preserves_Readable_Unicode_In_Status_Code_Comments` +- `/*/Refitter.Tests.Examples/GenerateStatusCodeCommentsTests/Generated_Code_With_Unicode_Status_Code_Comments_Can_Build` + +## Anti-Patterns +- Do **not** rely on `dotnet test --filter ...` here; the TUnit/Microsoft.Testing.Platform test app rejects that option. +- Do **not** assume OR expressions are necessary for a tiny run; separate single-test invocations are often simpler and more reliable. diff --git a/.squad/skills/xml-doc-description-sanitization/SKILL.md b/.squad/skills/xml-doc-description-sanitization/SKILL.md new file mode 100644 index 000000000..346c96408 --- /dev/null +++ b/.squad/skills/xml-doc-description-sanitization/SKILL.md @@ -0,0 +1,31 @@ +--- +name: "xml-doc-description-sanitization" +description: "Preserve readable Unicode when copying OpenAPI response descriptions into XML doc comments" +domain: "code-generation" +confidence: "high" +source: "manual" +--- + +## Context +`src\Refitter.Core\XmlDocumentationGenerator.cs` writes OpenAPI response descriptions into generated XML documentation comments. Those descriptions may arrive from NSwag in JSON-escaped form (`\uXXXX`, `\"`, `\n`) even though the source OpenAPI file already contains readable Unicode. + +## Pattern + +### Sanitize user-sourced response descriptions only +- Decode JSON-style escape sequences before writing the text into XML docs. +- After decoding, escape XML-reserved characters (`&`, `<`, `>`) so the generated comments remain valid XML. +- Apply this only to OpenAPI-sourced response description text such as `method.ResultDescription` and `response.ExceptionDescription`. + +### Preserve intentional XML doc markup +- Do **not** run the same sanitization over hardcoded fallback strings that intentionally contain ``, ``, or other XML doc tags. +- Sanitize at the insertion point where raw API text enters the XML comment output. + +## Example + +```csharp +var description = this.EscapeSymbols(this.DecodeJsonEscapedText(responseDescription)); +``` + +## Anti-Patterns +- Appending `method.ResultDescription` or `response.ExceptionDescription` directly into XML comments. +- XML-escaping entire doc fragments that intentionally contain XML doc markup. diff --git a/src/Refitter.Core/XmlDocumentationGenerator.cs b/src/Refitter.Core/XmlDocumentationGenerator.cs index 73fc620d5..86bfbd049 100644 --- a/src/Refitter.Core/XmlDocumentationGenerator.cs +++ b/src/Refitter.Core/XmlDocumentationGenerator.cs @@ -1,3 +1,4 @@ +using System.Globalization; using System.Text; using NSwag; using NSwag.CodeGeneration.CSharp.Models; @@ -184,7 +185,14 @@ public void AppendMethodDocumentation( // Document the result with a fallback description. var description = method.ResultDescription; if (string.IsNullOrWhiteSpace(description)) + { description = "A representing the result of the request."; + } + else + { + description = this.SanitizeResponseDescription(description); + } + this.AppendXmlCommentBlock("returns", description, code); } else @@ -316,9 +324,11 @@ private string BuildResponseDescription(string text, IEnumerable") - .Append(response.ExceptionDescription) + .Append(responseDescription) .AppendLine(""); } @@ -338,4 +348,108 @@ private string EscapeSymbols(string input) .Replace("<", "<") .Replace(">", ">"); } + + private string SanitizeResponseDescription(string input) + { + return this.EscapeSymbols(this.DecodeJsonEscapedText(input)); + } + + private string DecodeJsonEscapedText(string input) + { + if (string.IsNullOrEmpty(input)) + { + return input; + } + + // Fast path: if no backslashes and no invalid control characters, return as is. + // Invalid XML chars: < 0x20 except 0x09, 0x0A, 0x0D. + bool needsDecoding = input.IndexOf('\\') >= 0; + bool needsSanitization = false; + + for (int i = 0; i < input.Length; i++) + { + char c = input[i]; + if (!IsValidXmlChar(c)) + { + needsSanitization = true; + break; + } + } + + if (!needsDecoding && !needsSanitization) + { + return input; + } + + var result = new StringBuilder(input.Length); + + for (var index = 0; index < input.Length; index++) + { + var current = input[index]; + if (current != '\\' || index == input.Length - 1) + { + if (IsValidXmlChar(current)) + { + result.Append(current); + } + continue; + } + + var escapedCharacter = input[++index]; + + if (escapedCharacter == 'u' && index + 4 < input.Length) + { + var hexValue = input.Substring(index + 1, 4); + if (int.TryParse(hexValue, NumberStyles.HexNumber, CultureInfo.InvariantCulture, out var unicodeCodePoint)) + { + char decodedChar = (char)unicodeCodePoint; + if (IsValidXmlChar(decodedChar)) + { + result.Append(decodedChar); + } + index += 4; + continue; + } + } + + char? decodedSimpleChar = escapedCharacter switch + { + '"' => '"', + '\\' => '\\', + '/' => '/', + 'b' => '\b', + 'f' => '\f', + 'n' => '\n', + 'r' => '\r', + 't' => '\t', + _ => null + }; + + if (decodedSimpleChar.HasValue) + { + if (IsValidXmlChar(decodedSimpleChar.Value)) + { + result.Append(decodedSimpleChar.Value); + } + } + else + { + // Fallback for unknown escape sequences: keep as is (e.g. \z -> \z) + // But we must sanitize the backslash and the char + if (IsValidXmlChar('\\')) result.Append('\\'); + if (IsValidXmlChar(escapedCharacter)) result.Append(escapedCharacter); + } + } + + return result.ToString(); + } + + private static bool IsValidXmlChar(char c) + { + // XML 1.0 allowed characters: + // #x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | [#x10000-#x10FFFF] + // C# char is UTF-16, so we handle surrogate pairs at string level if needed, + // but here we check single char validity for simple control codes. + return c == 0x09 || c == 0x0A || c == 0x0D || c >= 0x20; + } } diff --git a/src/Refitter.Tests/Examples/GenerateStatusCodeCommentsTests.cs b/src/Refitter.Tests/Examples/GenerateStatusCodeCommentsTests.cs index e934383b1..a6ce90a63 100644 --- a/src/Refitter.Tests/Examples/GenerateStatusCodeCommentsTests.cs +++ b/src/Refitter.Tests/Examples/GenerateStatusCodeCommentsTests.cs @@ -55,6 +55,45 @@ public class GenerateStatusCodeCommentsTests type: string "; + private const string Swagger20SpecWithUnicodeStatusCodeComments = @" +swagger: '2.0' +info: + title: Unicode Test API 2.0 + version: 1.0.0 +paths: + /directories: + get: + operationId: getDirectories + responses: + '200': + description: Возвращает список справочников. + schema: + type: string + '400': + description: Ошибка запроса +"; + + private const string OpenApiSpecWithUnicodeStatusCodeComments = @" +openapi: 3.0.0 +info: + title: Unicode Test API + version: 1.0.0 +paths: + /directories: + get: + operationId: getDirectories + responses: + '200': + description: Возвращает список справочников. + content: + application/json: + schema: + type: string + '400': + description: Ошибка запроса +"; + + [Test] public async Task Can_Generate_Code() { @@ -102,9 +141,42 @@ public async Task Generated_Code_Contains_Response_Descriptions_When_Comments_En generatedCode.Should().Contain("Bad request"); } + [Test] + public async Task Generated_Code_Preserves_Readable_Unicode_In_Status_Code_Comments() + { + string generatedCode = await GenerateCode(OpenApiSpecWithUnicodeStatusCodeComments, generateStatusCodeComments: true); + + generatedCode.Should().Contain("/// 400") + .And.Contain("/// Ошибка запроса") + .And.NotContain(@"\u041e\u0448"); + } + + [Test] + public async Task Generated_Code_Preserves_Readable_Unicode_In_Status_Code_Comments_Swagger20() + { + string generatedCode = await GenerateCode(Swagger20SpecWithUnicodeStatusCodeComments, generateStatusCodeComments: true); + + generatedCode.Should().Contain("/// 400") + .And.Contain("/// Ошибка запроса") + .And.NotContain(@"\u041e\u0448"); + } + + [Test] + public async Task Generated_Code_With_Unicode_Status_Code_Comments_Can_Build() + { + string generatedCode = await GenerateCode(OpenApiSpecWithUnicodeStatusCodeComments, generateStatusCodeComments: true); + + BuildHelper.BuildCSharp(generatedCode).Should().BeTrue(); + } + private static async Task GenerateCode(bool generateStatusCodeComments) { - var swaggerFile = await SwaggerFileHelper.CreateSwaggerFile(OpenApiSpec); + return await GenerateCode(OpenApiSpec, generateStatusCodeComments); + } + + private static async Task GenerateCode(string openApiSpec, bool generateStatusCodeComments) + { + var swaggerFile = await SwaggerFileHelper.CreateSwaggerFile(openApiSpec); try { var settings = new RefitGeneratorSettings diff --git a/src/Refitter.Tests/XmlDocumentationGeneratorTests.cs b/src/Refitter.Tests/XmlDocumentationGeneratorTests.cs index 47a9548ae..a652970ac 100644 --- a/src/Refitter.Tests/XmlDocumentationGeneratorTests.cs +++ b/src/Refitter.Tests/XmlDocumentationGeneratorTests.cs @@ -1,5 +1,6 @@ using System.Text; using FluentAssertions; +using NJsonSchema; using NSwag; using NSwag.CodeGeneration.CSharp.Models; using Refitter.Core; @@ -221,6 +222,28 @@ public void Can_Generate_Method_Throws_With_Response_Code() .And.Contain("400"); } + [Test] + public void Can_Generate_Method_Throws_With_Readable_Unicode_Status_Code_Comments() + { + var generator = new XmlDocumentationGenerator(new RefitGeneratorSettings + { + GenerateXmlDocCodeComments = true, + GenerateStatusCodeComments = true, + }); + var docs = new StringBuilder(); + var method = CreateOperationModel(new OpenApiOperation + { + Responses = { ["400"] = new OpenApiResponse { Description = "Ошибка запроса" } }, + }); + + generator.AppendMethodDocumentation(method, false, false, false, false, docs); + + docs.ToString().Should().Contain("/// ") + .And.Contain("400") + .And.Contain("Ошибка запроса") + .And.NotContain(@"\u041e\u0448"); + } + [Test] public void Can_Generate_Method_Throws_Without_Response_Code() { @@ -257,4 +280,40 @@ public void Can_Generate_Method_With_IApiResponse() .And.Contain("/// ") .And.Contain("400"); } + + [Test] + public void Can_Generate_Method_Returns_With_Readable_Unicode_Description() + { + var generator = new XmlDocumentationGenerator(new RefitGeneratorSettings + { + GenerateXmlDocCodeComments = true, + }); + var docs = new StringBuilder(); + + // Create an operation with a success response that has a description and a schema (so it has a result) + var operation = new OpenApiOperation + { + Responses = + { + ["200"] = new OpenApiResponse + { + Description = "Ошибка ответа", + Schema = new JsonSchema { Type = JsonObjectType.String } + } + }, + Produces = ["application/json"] + }; + + var method = CreateOperationModel(operation); + + // Verify that ResultDescription picked up the description (NSwag might escape it, which is what we want to test) + // If NSwag escapes it, it will look like "\u..." + // Our generator should decode it back. + + generator.AppendMethodDocumentation(method, false, false, false, false, docs); + + docs.ToString().Should().Contain("/// ") + .And.Contain("Ошибка ответа") + .And.NotContain(@"\u041e\u0448"); + } }