Skugga v1.3.0

[Digvijay]

[1.3.0] - 2026-01-07

Added

• Project Governance & Support:
  • Added CODE_OF_CONDUCT.md (Contributor Covenant v2.1)
  • Added SUPPORT.md with clear support channels
  • Added .editorconfig enforcing high-quality coding standards (aligned with Sannr)
  • Added docs/AOT_COMPATIBILITY_ANALYSIS.md deeply analyzing AOT constraints and solutions
• CI/CD & Security Enhancements:
  • Integrated GitHub CodeQL for automated security and quality analysis
  • Integrated CycloneDX SBOM generation into release pipeline for supply chain security
  • Added IsAotCompatible metadata to NuGet packages for SDK-level AOT verification
  • Added automated Source Formatting Validation in CI pipeline
  • Added Code Coverage Artifacts upload in CI for better visibility

In general, to fix a constant condition where the condition is redundantly checked, you remove the redundant check and keep a single, meaningful null/boolean check. This preserves behavior while eliminating dead or misleading code.

Here, the best fix is to remove the second if (symbol == null) return; in GenerateMockClass and leave a single null-guard immediately after var symbol = target.Symbol;. This does not alter functionality: when target.Symbol is null, the method returns; when it is non-null, execution proceeds exactly as before, and the extra check had no effect.

Concretely, in src/Skugga.Generator/Generator/SkuggaGenerator.cs, in the body of GenerateMockClass(SourceProductionContext spc, TargetInfo target), replace the line:

var symbol = target.Symbol; if (symbol == null) return; if (symbol == null) return;

with:

var symbol = target.Symbol;
if (symbol == null) return;

No new methods, imports, or additional definitions are required.

---

In general, to fix a constant condition caused by a proven non-null value, remove redundant null checks and rely on the previous validation. Here, string.IsNullOrEmpty(source) followed by a return guarantees that source is non-null and non-empty beyond that point. The condition on line 157 currently uses source?.EndsWith(... ) == true to handle a possibly null source, but that’s unnecessary and flagged as a constant condition. The best minimal fix is to replace the ?. usage with direct EndsWith calls on source, eliminating the redundant null-protection and making the logic clearer without changing behavior.

Concretely, in src/Skugga.OpenApi.Generator/Core/SkuggaOpenApiGenerator.cs, update the computation of isYaml at lines 157–158 to use source.EndsWith(...) instead of source?.EndsWith(...) == true. No additional methods, imports, or definitions are needed, and the rest of the logic (including the later heuristic based on specContent) remains unchanged.

In general, to fix a “useless assignment to local variable” you either remove the unused variable/assignment or use the value in a meaningful way (e.g., return it, log it, or pass it somewhere), provided that matches the intended logic. Here, the lambda passed to AssertAllocations.Zero is only meant to perform some non-allocating work; it does not need to expose any result. The simplest and safest fix is to remove the unnecessary variable c and just perform the addition if you still want a tiny bit of computation, or even remove the addition entirely.

The best minimal fix without changing functionality is to eliminate int c = a + b; from line 106, leaving a simple, clearly side-effect-free, non-allocating body such as { int a = 10; int b = 20; var _ = a + b; } (if we want to keep the addition but avoid an unused variable warning) or just { int a = 10; int b = 20; }. Since the goal is to avoid allocations, neither form introduces allocations, and the behavior from the caller’s perspective (no observable result; only used for allocation checking) remains unchanged. No new methods, imports, or definitions are required; the change is local to the lambda body in RunZeroAllocDemo in tests/Skugga.Benchmarks/Program.cs.

In general, to fix a "Missed 'readonly' opportunity" issue, you add the readonly modifier to any field that is assigned only at its declaration or within constructors of the same class. This guarantees that the field cannot be changed after the object has been fully constructed, preventing accidental reassignment.

For this specific case, the field Symbol in the inner class TargetInfo is only assigned in the constructor, so we can safely mark it as readonly. The same pattern clearly holds for all the other fields in TargetInfo (Location, Type, LambdaExpression, InvocationSyntax, MethodName, Arguments, IsProperty): each is declared as a public field and assigned solely in the constructor. To keep the design consistent and maximize immutability, we should make all these fields readonly. This does not change existing functionality as long as no other part of the code writes to these fields after construction, which matches what we see in the provided snippet.

The change is localized to src/Skugga.Generator/Generator/SkuggaGenerator.cs inside the TargetInfo class definition (lines 1408–1432). No new methods or imports are required; we only add the readonly modifier to the field declarations.

In general, to fix this kind of issue you add the readonly modifier to fields that are only ever assigned at their declaration or inside constructors of the same class. This prevents later reassignment and documents the immutability of the field.

Here, in src/Skugga.Generator/Generator/SkuggaGenerator.cs, inside the nested TargetInfo class, the Location field is assigned only in the TargetInfo constructor and is never modified elsewhere in the provided code. The best fix with no behavioral change is to declare this field as public readonly Location Location;. No additional methods, imports, or other changes are needed because Location is already in scope and the constructor assignment is allowed for readonly fields.

Concretely:

• Edit the field declaration around line 1412 in TargetInfo.
• Change public Location Location; to public readonly Location Location;.
• Leave all other fields and code unchanged.

In general, to fix a “missed readonly opportunity” for a field, you add the readonly modifier to the field declaration, provided that the field is only assigned at its declaration or within constructors of the same class. This enforces immutability after construction and prevents later unintended assignments.

For this specific case, in src/Skugga.Generator/Generator/SkuggaGenerator.cs, within the nested TargetInfo class, update the declaration of the Type field on line 1413 from public TargetType Type; to public readonly TargetType Type;. No other code changes or imports are needed, since construction remains the same and no additional functionality is introduced. This is the minimal, non‑behavior‑changing fix that satisfies the analyzer.

To fix the problem, mark the LambdaExpression field as readonly since it is only assigned during construction. This prevents accidental reassignment later and aligns with the intended immutability of the TargetInfo object.

Concretely, in src/Skugga.Generator/Generator/SkuggaGenerator.cs, within the nested TargetInfo class, add the readonly modifier to the declaration of LambdaExpression. No other code changes, imports, or method additions are necessary, because the constructor already performs the only assignment and readonly fields may be assigned in constructors.

In general, the fix is to avoid catching Exception and instead catch the specific exception type(s) that represent the expected failure scenario, letting unexpected exceptions bubble up. In this code, the expected failure from AssertAllocations.Zero is likely represented by a specific assertion exception (for example, InvalidOperationException or a custom assertion exception). Since we cannot see its implementation, the most conservative improvement that narrows the catch while not breaking behavior is to catch InvalidOperationException, which is a common choice for contract/assertion-like checks, while leaving the second catch (Exception ex) (for the "expected allocation" path) as-is because that block explicitly wants to treat any thrown exception as a success signal for the test.

Concretely, in RunZeroAllocDemo():

• Change the first catch (Exception ex) (lines 104–109 block) to catch (InvalidOperationException ex), which means:
  • Allocation assertion failures are still logged as before.
  • Serious, unrelated exceptions (e.g., OutOfMemoryException) are no longer swallowed.
• Leave the second catch (lines 112–117) untouched, because that test is intentionally checking that an exception—of any type—occurs when allocations happen.

No new methods or external dependencies are required; we only adjust the catch clause in the shown file.

In general, to fix an over‑broad catch (Exception) you should identify the specific exception type(s) that the enclosed code is expected to raise under normal error conditions and catch only those. Any other exception types should be allowed to propagate so they can fail fast and be diagnosed, or be handled separately with different messaging.

Here, both tests exercise AssertAllocations.Zero(...). The “zero allocation” contract violations should be reported via a specific, expected exception type (for example, a hypothetical AllocationDetectedException or AssertionException) rather than Exception. The best fix, without changing visible behavior, is:

• Replace the generic catch (Exception ex) in [Test 2] with a catch of the specific expected exception type thrown when allocations are detected.
• Optionally, handle unexpected exceptions in a separate catch block with a different message so they are not misreported as “expected allocation”.
• Make the same change for [Test 1], so that only known assertion/allocation exceptions are caught and logged as test failures.

Because we only see this file and cannot change other parts of the codebase, we should use an exception type that is (a) standard and (b) semantically appropriate for assertion‑like failures. InvalidOperationException is a reasonable, commonly used choice for contract violations in library methods, and we can assume AssertAllocations.Zero throws that (or we’re aligning with that intent) without changing external behavior: the code still logs the message when the expected assertion‑style failure occurs, and unexpected exceptions will now surface instead of being mislabeled.

Concretely in tests/Skugga.Benchmarks/Program.cs:

• On line 109, change catch (Exception ex) to catch (InvalidOperationException ex).
• On line 117, change catch (Exception ex) to catch (InvalidOperationException ex).
• Optionally, add a final catch (Exception ex) after each specific catch that logs as an unexpected error, so unexpected failures are visible and not conflated with expected allocation detection. This does not change the happy-path output, but improves correctness and diagnosability.

No new imports are needed; InvalidOperationException is in System, which is already imported.