SonarCloud quality/security remediation tracking

[rexm]

Track progress on SonarCloud findings for this project.

Addressed in commits af11135 and 37ca7b9 (2026-06-21):

✅ Completed

Security

• S7637 — Pin timheuer/base64-to-file CI action to full commit SHA (release.yml)
• S7636 — Move NUGET_ORG_PUSH_KEY out of inline run: block into env: section
• ReDoS hotspots — Add 1-second timeout to all 7 Regex instances in WhitespaceRemover.cs

Reliability (real bugs)

• S2955 — Replace == null on unconstrained generic type parameters with EqualityComparer<T>.Default.Equals(value, default) in HtmlEncoder, HtmlEncoderLegacy, DictionarySlim, FixedSizeDictionary, GenericDictionaryAccessor, ReadOnlyGenericDictionaryAccessor

False positives (suppressed with justification)

• S7133 — ReaderWriterLockSlimExtensions: lock IS released via DisposableContainer in using; not a bug
• S2234 — IteratorBinder: swapping template/ifEmpty for the Inverse case is intentional
• S1944 — PartialBinder: cast is a C# explicit operator on ExpressionContainer<T>, not a runtime hierarchy cast

Design / API quality

• S3881 — Seal TextEncoderWrapper and ClosureBuilder (IDisposable pool types with private constructors)
• S3218 — Use explicit interface implementation for IInternalObjectPoolPolicy<T>.Create() in all six pool policy types to eliminate method-name shadowing
• S112 — Replace IndexOutOfRangeException with ArgumentOutOfRangeException in Arguments.cs and Substring.cs
• S1168 — Return Array.Empty<Expression>() instead of null in HandlebarsExpressionVisitor
• S3993 — Add [AttributeUsage(AttributeTargets.Class)] to FeatureOrderAttribute
• S2743 — NOSONAR with justification on ImmutableStack<T> pool field (intentional per-T instance)
• S3246 — Add out covariance to ValueTypeGetterDelegate<T, TValue>
• S3011 — NOSONAR with justification on ReflectionMemberAccessor private method lookup
• S2589 — Remove always-false dead branch in MissingBlockHelperDescriptor
• S2933 — Make _enumerator field readonly in ExtendedEnumerator
• S1066 — Merge nested if statements in DelegatedMemberAliasProvider
• S3358 — Extract nested ternaries in EndExpressionToken, StartExpressionToken, PartialBlockAccumulatorContext
• S3928 — Add meaningful message to ArgumentOutOfRangeException in BlockHelperFunctionBinder
• S927 — Rename parameter exp → node in HandlebarsExpressionVisitor.Visit() to match base class signature

Maintainability

• S3776 — Extract ProcessStatement from WhitespaceRemover.ProcessTokens (cognitive complexity reduced; partial indentation logic preserved)
• S3776 — Extract HandleRawStart from RawHelperAccumulator.ConvertTokens (cognitive complexity reduced)
• CS0618 — Replace all uses of obsolete Compatibility.RelaxedHelperNaming in PathBinder, HelperConverter, HelperFunctionBinder, HandlebarsConfigurationAdapter

Test quality

• S3415 / xUnit2000 — Fix Assert.Equal(actual, expected) argument order in CustomConfigurationTests
• CS0108 — Add new modifier to hiding method in FixedSizeDictionaryTests

---

❌ Pending

Breaking / high effort

• S2326 — IDescriptor<TOptions>: type parameter TOptions is declared but never used in the interface body. Removing it is a breaking API change requiring a major version bump.
• S3776 — Tokenizer.cs cognitive complexity = 76 (threshold: 15). Significant refactor; needs careful incremental extraction to avoid destabilizing the lexer.

Medium effort

• S3776 — LiteralParser.cs: complexity above threshold; no known constraints, medium refactor.
• S3776 — BlockHelperFunctionBinder.BindByRef: the nested switch/if structure is tied to expression-tree construction — naive extraction causes runtime NullReferenceException. Needs a careful approach (see ExpressionShortcuts scope constraint note below).
• S3776 — PartialBinder.VisitPartialExpression: complexity climbed with the partial-indentation feature. Any extraction must keep Arg(...)/Cast<T>(...) variables scoped to the Call(() => ...) that uses them.
• S3881 — ReusableStringWriter (IO/PolledStringWriter.cs) is not sealed; Sonar flags the IDisposable pattern on non-sealed classes that don't expose a protected virtual Dispose(bool) to subclasses consistently. Evaluate whether sealing is appropriate or the pattern needs a sealed override.

Remaining SonarCloud findings (not yet triaged)

• The original Sonar report listed 190 code smells total; the work above addressed the ~53 flagged as critical or major. The remaining ~137 lower-priority smells have not been reviewed. A full triage pass against the current SonarCloud dashboard is needed to identify any worth fixing.

---

Notes

ExpressionShortcuts scope constraint

When refactoring code in files that use the Expressions.Shortcuts library (PartialBinder, BlockHelperFunctionBinder, etc.), variables created with Arg(...), Cast<T>(...), or New(...) must be declared in the same lexical block as the Call(() => ...) lambda that references them. Hoisting them to an outer scope causes runtime InvalidCastException or NullReferenceException because ExpressionShortcuts rewrites the lambda's expression tree at build time and out-of-scope references are not rewritten. Safe to hoist: CompilationContext.Args.BindingContext and CompilationContext.Args.EncodedWriter (these are parameter expressions, not new constant nodes).