chore: align analyzer conventions

[ANcpLua]

Summary

• align local analyzer/style enforcement with ANcpLua.NET.Sdk config guidance
• remove broken BitNet testing fixture remnants from this repo
• fix release-build analyzer warnings without changing documented lowercase/camelCase contracts

Local validation

• dotnet build ANcpLua.Roslyn.Utilities.slnx -c Release

[github-actions]

@coderabbitai autofix

[coderabbitai]

Note

Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.

Autofix skipped. No unresolved CodeRabbit review comments with fix instructions found.

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

Configuration & Convention Alignment

Directory.Build.props refactored to centralize analyzer/style enforcement:

• Delegates naming conventions and analyzer severity rules to ../ANcpLua.NET.Sdk/src/Config/*.editorconfig via EnableEditorConfigDogfooding property
• Unconditional TreatWarningsAsErrors replaced with CI-scoped conditions (ContinuousIntegrationBuild gate)
• Standardized language settings: LangVersion=latest, Nullable=enable, ImplicitUsings=enable, EnableNETAnalyzers=true, AnalysisLevel=latest-all
• Introduces cross-repo dependency: build configuration now sourced from sibling ANcpLua.NET.Sdk repository

.editorconfig reduced to minimal form (root = true only), with all inline C# conventions removed and delegated to SDK repository.

BitNet Testing Fixture Removal

Completely removed non-functional BitNet/llama.cpp integration infrastructure:

• Deleted BitNetFixture (85 lines): xUnit async lifecycle fixture with health-check probing, environment variable resolution (BITNET_URL, BITNET_MODEL), and IChatClient initialization via OpenAIClient
• Deleted BitNetTestGroup and BitNetAttribute (22 lines): xUnit collection fixture markers and metadata association
• No orphaned references: removal is clean; no remaining code depends on removed fixtures

Analyzer Warning Suppressions (Release-Build Fixes)

Added 11 targeted SuppressMessage attributes without behavioral changes; documented contracts remain intact:

• CA1308 ("Normalize strings to uppercase") suppressed on methods with intentional lowercase/camelCase contracts:
  • ShortId.NewPrefixedSortable: lowercase hexadecimal ID contract
  • StringExtensions.ToParameterName: camelCase parameter naming contract
  • StringExtensions.ToShortHash: lowercase parameter explicitly enables lowercase formatting
• CA2000 ("Dispose objects before losing scope") suppressed on test fixture setup methods with documented lifetime management via stored _factory field disposed in teardown:
  • IntegrationTestBase<TProgram> (NUnit)
  • KestrelTestBase<TProgram> (NUnit, TUnit)
  • IntegrationTestBase<TProgram> (TUnit)

Code Hygiene & Refactoring

• OperationExtensions.HasExpressionType: Refactored switch expression with inline type extraction to eliminate intermediate null-check; same logic flow, simplified structure
• DotNetSdkHelpers.Get: Added using declaration for ZipArchive disposal on .zip payload extraction
• ReflectionExtensions.GetMethodFromGenericDefinition: Corrected BindingFlags casing (All instead of all) in non-NET6_0_OR_GREATER fallback path
• Base64Url: Conditional StackallocByteThreshold definition scoped to NET5_0_OR_GREATER eliminating unused constant in other target frameworks
• QueryString.Build: Removed null-forgiving operator (value!), relying on existing IsNullOrWhiteSpace guard
• Test assertions: Updated DiscriminatedUnionGeneratorTests and ExtensibleEnumMirrorGeneratorTests to use const string for test inputs; removed null-forgiving operators in MatchingDslTests matcher assertions

Risk Surface

• Build configuration now externalized: Directory.Build.props assumes ../ANcpLua.NET.Sdk sibling repository presence and stability; missing or misconfigured repository breaks analyzer/style enforcement
• CI-conditional warnings-as-errors: Release warnings only enforced when ContinuousIntegrationBuild=true; local non-CI builds may not catch violations
• EditorConfig dogfooding: Depends on ANcpLua.NET.Sdk config update cadence; divergence between SDK conventions and this repository requires manual re-sync

Validation Evidence

• Local build command: dotnet build ANcpLua.Roslyn.Utilities.slnx -c Release passes without warnings
• No semantic behavioral changes in refactored methods; logic flow preserved
• Analyzer suppressions document intentional deviations from convention rules; lowercase/camelCase contracts explicitly justified
• BitNet fixture removal verified clean: zero remaining references

Walkthrough

This PR consolidates MSBuild analyzer configuration, removes distributed .editorconfig rules, integrates canonical editorconfig from a shared SDK, improves resource disposal in test helpers, and documents intentional analyzer suppressions across utilities and test infrastructure.

Changes

Build Configuration and Code-Quality Settings

Layer / File(s)	Summary
Centralized build properties and CI enforcement Directory.Build.props	Default language (LangVersion=latest, Nullable=true, ImplicitUsings=true) and analyzer settings are now unconditional; warning-as-errors and code-style enforcement are CI-scoped to ContinuousIntegrationBuild.
Editorconfig dogfooding integration Directory.Build.props	When EnableEditorConfigDogfooding=true, the build includes canonical .editorconfig files from the sibling ANcpLua.NET.Sdk source tree, excluding SDK-specific patterns.

Resource Disposal and Analyzer Suppressions

Layer / File(s)	Summary
ZipArchive disposal in SDK extraction src/ANcpLua.Roslyn.Utilities.Testing/MSBuild/DotNetSdkHelpers.cs	.zip SDK extraction now properly disposes ZipArchive via using declaration, preventing resource leaks.
CA2000 suppressions in test infrastructure src/ANcpLua.Roslyn.Utilities.Testing/WebTesting/{NUnit,TUnit}/*	Test base classes (IntegrationTestBase, KestrelTestBase) suppress CA2000 with documented justification that WebApplicationFactory is managed by test lifecycle methods.
Analyzer suppressions and null-forgiving cleanup in utilities src/ANcpLua.Roslyn.Utilities/{Encoding,Ids,Web}/*, src/ANcpLua.Roslyn.Utilities/OperationExtensions.cs, src/ANcpLua.Roslyn.Utilities/StringExtensions.*, src/ANcpLua.Roslyn.Utilities/ReflectionExtensions.cs	Utility code adds CA1308 suppressions for intentionally lowercase methods and CA2000 documentation, removes unnecessary null-forgiving operators, and refactors HasExpressionType to direct boolean-result switch expression.

Test Infrastructure Updates

Layer / File(s)	Summary
Test input const string declarations tests/ANcpLua.Roslyn.Utilities.DiscriminatedUnion.Tests/DiscriminatedUnionGeneratorTests.cs, tests/ANcpLua.Roslyn.Utilities.ExtensibleEnumMirror.Tests/ExtensibleEnumMirrorGeneratorTests.cs	Generator test inputs are standardized from var to const string declarations.
Null-forgiving cleanup in test assertions tests/ANcpLua.Roslyn.Utilities.Testing.Tests/MatchingDslTests.cs	Unnecessary ! operators removed from derived variable where null checks already validate non-nullability.

---

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

---

Possibly related PRs

• ANcpLua/ANcpLua.NET.Sdk#130: Shares overlapping Directory.Build.props logic for CI-scoped warning-as-errors and conditional editorconfig integration via EnableEditorConfigDogfooding.
• ANcpLua/ANcpLua.NET.Sdk#154: Regenerates canonical analyzer editorconfig content that this PR begins to dogfood via EnableEditorConfigDogfooding.
• ANcpLua/ANcpLua.Roslyn.Utilities#107: Introduced the ExtensibleEnumMirror generator test suite that is now being updated with const string declarations.

---

Suggested labels

area:build, area:testing, area:utilities, area:tests, breaking

---

Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

• Ignore (reviewers only)

❌ Failed checks (1 error, 4 warnings)

Check name	Status	Explanation	Resolution
Public Contract Documented	❌ Error	PR removes public types (BitNetFixture, BitNetAttribute, BitNetTestGroup) without updating README docs. README still lists these as features. No explicit breaking-change notes in PR description.	Remove BitNet from README.md (lines 20, 159). Add breaking-change documentation to PR with migration guidance.
Docstring Coverage	⚠️ Warning	Docstring coverage is 48.00% which is insufficient. The required threshold is 75.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
No Hidden Fallback Path	⚠️ Warning	ReflectionExtensions.cs has undocumented fallback paths for .NET <6.0 (InvokeUnwrapped, GetMethodFromGenericDefinition) that target netstandard2.0 but lack any test coverage—only net10.0 is tested.	Add netstandard2.0-targeted tests or conditionally compiled tests validating both .NET 6+ and pre-6.0 fallback paths produce equivalent results.
Dependency Hygiene	⚠️ Warning	Orphaned dependencies: OpenAI and Microsoft.Extensions.AI.Abstractions remain declared despite BitNetFixture removal. No code uses these packages.	Remove Microsoft.Extensions.AI.Abstractions and OpenAI from ANcpLua.Roslyn.Utilities.Testing.csproj and Directory.Packages.props. Remove OpenAIVersion and MicrosoftExtensionsAIAbstractionsVersion from Version.props.
No Null-Forgiving Operator Without Justification	⚠️ Warning	47 null-forgiving operators (!) added without inline comments. Violations include property initializations, assignments, and method returns without explaining suppression.	Add inline comments explaining null-safety suppression for each !, or remove where null-checks make them redundant.

✅ Passed checks (15 passed)

Check name	Status	Explanation
Title check	✅ Passed	Title follows conventional commits format with 'chore' scope, is under 72 characters (33 chars), has no trailing punctuation, and accurately identifies the highest-impact change: consolidating analyzer enforcement across config sources.
Description check	✅ Passed	Description clearly relates to the changeset, outlining three concrete objectives: analyzer alignment, fixture removal, and warning suppression, with validation confirmation.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
No Secrets Or Pii	✅ Passed	PR contains no secrets, tokens, credentials, API keys, or sensitive data. BitNet fixture accessing BITNET_URL/BITNET_MODEL was removed. All changes are safe configuration and code cleanup.
Generated Files Regenerated	✅ Passed	No source generators, schemas, models, or generation templates modified. Changes are configuration, fixture removal, analyzer suppressions, and test refactoring. No generated files need regeneration.
Async And Cancellation Safe	✅ Passed	No sync-over-async, fire-and-forget, missing CancellationToken propagation, sleeps, or resource disposal issues. ZipArchive disposal fixed. All async test lifecycle patterns correct.
Ci Release Safety	✅ Passed	Workflows and build config do not violate CI Release Safety criteria: no validation jobs removed, concurrency configured, permissions justified, secrets not exposed.
Tests Match Risk	✅ Passed	No changes to persistence, queues, external services, migrations, or workflows. Low-risk refactors in non-critical areas.
No Copy Paste Tables	✅ Passed	No hardcoded repetitive case lists, mapping tables, duplicate switch branches, or parallel arrays introduced. SuppressMessage attributes are standard targeted suppressions, not problematic patterns.
Observability Boundary	✅ Passed	No new observable services introduced. PR removes BitNet fixtures and updates analyzer/build config only. Observability check not applicable.
No Datetime.Now/Utcnow	✅ Passed	No DateTime.Now or DateTime.UtcNow found in any added/modified C# files. Comprehensive search across src/ and tests/ directories confirms clean compliance with the check.
No .Result/.Wait() Blocking Async	✅ Passed	All modified C# files scanned for .Result, .Wait(), and .GetAwaiter().GetResult(). No blocking async patterns found.
No Isourcegenerator	✅ Passed	No ISourceGenerator implementations found in any modified or added C# files. The PR contains only code-quality improvements and test fixture removals—no new source generators were added.
Sources Public Types Must Be Internal	✅ Passed	No C# source files exist in src/ANcpLua.Roslyn.Utilities.Sources/ and no files in this directory were added/modified in this PR. The check is not applicable.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch chore/remove-broken-bitnet-fixture

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch chore/remove-broken-bitnet-fixture

• 🛠️ architecture hardening
• 🛠️ security pass
• 🛠️ test gap closure
• 🛠️ docs and changelog alignment
• 🛠️ performance and allocation pass

Warning

Review ran into problems

🔥 Problems

Linked repositories: Your configuration references 17 linked repositories, but your current plan allows 10. Analyzed ANcpLua/ANcpLua.Agents, ANcpLua/ANcpLua.Analyzers, ANcpLua/ANcpLua.NET.Sdk, O-ANcppLua/ANcpLua.OtelConventions.Api, ANcpLua/Arqio, ANcpLua/BSc_2025_Alexander_Nachtmann, ANcpLua/C64AIToolChain, ANcpLua/ErrorOrX, O-ANcppLua/Nuke.OpenTelemetry.Conventions, ANcpLua/Paperless, skipped ANcpLua/ancplua-claude-plugins, ANcpLua/dotcov, ANcpLua/nhmw-digital-collection, O-ANcppLua/qyl, ANcpLua/safe-autoresearch, ANcpLua/typespec-otel-semconv, ANcpLua/yt-transcript.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 duplication

Metric	Results
Duplication	0

View in Codacy

AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.

_{TIP This summary will be updated as you push new changes.}

[codacy-production]

Pull Request Overview

The PR successfully aligns the codebase with updated analyzer conventions and removes deprecated testing components. Codacy analysis indicates the changes are generally up to standards.

However, there are two primary issues that should be addressed before merging:

1. Functional Gap: The QueryString utility does not URL-encode keys, which could lead to malformed URLs if keys contain reserved characters.
2. Logic Risk: In ReflectionExtensions, the omission of BindingFlags.FlattenHierarchy in the fallback implementation may lead to failures when resolving static methods from base classes.

Additionally, several core utility refactors lack accompanying unit tests in this diff, posing a regression risk.

About this PR

• The PR modifies logic in several core utility classes (Hashing, Identifiers, Reflection, QueryString) to satisfy analyzer rules, but no unit tests for these specific changes are included. Ensure that existing suites cover these paths or add new tests to prevent regressions.

Test suggestions

• Verify 'DiscriminatedUnion' generator produces correct syntax and exhaustive match switches using updated 'const' source literals.
• Verify 'ExtensibleEnumMirror' generator correctly identifies and reports diagnostics for invalid target structs.
• Ensure 'QueryString.Build' correctly filters null and whitespace values and performs URI escaping on valid inputs.
• Verify 'DotNetSdkHelpers' handles ZIP and GZIP/TAR extraction correctly, including Unix file permission updates.
• Verify 'Base64Url' encoding uses stackalloc for small buffers on supported frameworks and falls back to string conversion for larger ones.

Prompt proposal for missing tests

Consider implementing these tests if applicable:
1. Ensure 'QueryString.Build' correctly filters null and whitespace values and performs URI escaping on valid inputs.
2. Verify 'DotNetSdkHelpers' handles ZIP and GZIP/TAR extraction correctly, including Unix file permission updates.
3. Verify 'Base64Url' encoding uses stackalloc for small buffers on supported frameworks and falls back to string conversion for larger ones.

Low confidence findings

• The removal of null-forgiving operators (!) in 'QueryString.cs' and 'MatchingDslTests.cs' relies on flow analysis. Please verify these paths are covered by tests to ensure no NullReferenceExceptions were introduced.

_{TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback}

[codacy-production]

_{🟡 MEDIUM RISK}

Add BindingFlags.FlattenHierarchy to ensure static methods from base classes are included in the search.

Suggested change

	const BindingFlags All = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Static |
	BindingFlags.Instance;
	const BindingFlags All = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Static |
	BindingFlags.Instance | BindingFlags.FlattenHierarchy;

[codacy-production]

_{🟡 MEDIUM RISK}

Suggestion: URL-encode the query string key to ensure the generated string is always valid and to handle special characters correctly.

Suggested change

	sb.Append(key).Append('=').Append(Uri.EscapeDataString(value));
	sb.Append(Uri.EscapeDataString(key)).Append('=').Append(Uri.EscapeDataString(value));

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8293dd19af

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Add fallback when importing external analyzer configs

This import points to ../ANcpLua.NET.Sdk/..., but this repository’s CI checkout only pulls the current repo (see .github/workflows/nuget-publish.yml), so that sibling path is absent in normal builds. Because this commit also removes the in-repo .editorconfig rules, the glob resolves to no analyzer config files and the intended naming/style diagnostics silently stop being enforced. Please gate this include behind an Exists(...) check and keep a local fallback config so analyzer conventions still run when the sibling repo is not present.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Set CI warning promotion before it is referenced

This condition depends on $(ContinuousIntegrationBuild), but that property is only assigned later in the same file (<PropertyGroup Condition="'$(CI)' == 'true'">). MSBuild evaluates properties in order, so in CI environments that rely on this file to set ContinuousIntegrationBuild, the condition here evaluates false and warnings are not elevated to errors. Set ContinuousIntegrationBuild earlier or key this condition directly off $(CI)/$(GITHUB_ACTIONS).

Useful? React with 👍 / 👎.