SonarCloud quality/security remediation tracking #641

Track progress on SonarCloud findings for this project.

Addressed in commits af11135 and 37ca7b9 (2026-06-21), and #679 (2026-11-08):

✅ 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)
• S3881 — Seal ReusableStringWriter (IO/PolledStringWriter.cs); its only constructor is private, so sealing is not an API break. Dispose(bool) intentionally returns the instance to the pool rather than calling base.Dispose (refactor: address pending SonarCloud S3881/S3776 items in lexer and ReusableStringWriter #679)
• 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)
• S3776 — Tokenizer.Parse: extract ParseExpressionToken, ReadExtraBraces, ReadExpressionOpening, ReadStaticCharacter; read/peek order unchanged (refactor: address pending SonarCloud S3881/S3776 items in lexer and ReusableStringWriter #679)
• S3776 — LiteralParser: extract AppendEscapeSequence and IsEndOfLiteral (refactor: address pending SonarCloud S3881/S3776 items in lexer and ReusableStringWriter #679)
• 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
• Characterization tests HelperLiteralArgumentsAreParsed covering literal escapes and non-delimited literal termination, verified against both pre- and post-refactor lexer (refactor: address pending SonarCloud S3881/S3776 items in lexer and ReusableStringWriter #679)

❌ 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.

Medium effort

• 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.

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).