Skip to content

chore(tools): clean up ConfigFilesGenerator inspection warnings#156

Merged
github-actions[bot] merged 1 commit into
mainfrom
chore/configgen-inspection-cleanup
May 13, 2026
Merged

chore(tools): clean up ConfigFilesGenerator inspection warnings#156
github-actions[bot] merged 1 commit into
mainfrom
chore/configgen-inspection-cleanup

Conversation

@ANcpLua
Copy link
Copy Markdown
Owner

@ANcpLua ANcpLua commented May 13, 2026

Summary

Surgical cleanup of Rider inspection findings in tools/ConfigFilesGenerator/Program.cs. Net diff: +4 / −11, no behavior change.

  • Merge two adjacent if (X) return current; branches in ResolveMsBuildProperty (visited.Add + properties.TryGetValue) into a single ||-guarded check — fixes the Rider Duplicated 'if' branches error.
  • Drop the always-true ternary at the tail of GetRootFolderPath. The surrounding while (!path.IsEmpty) loop only exits when IsEmpty is true, so the false-branch was dead.
  • Collapse ContainsKey + indexer + redundant trailing continue in LoadMsBuildProperties into properties.TryAdd(name, value) — addresses both the Dictionary lookup → TryAdd and redundant control-flow jump suggestions.
  • Strip the redundant System.Text. qualifier on Encoding.UTF8 (using System.Text; is already in scope).

The wrapper-induced indent warning the user saw ("expected indent 7 spaces") cleared up implicitly: a local uncommitted refactor had wrapped the top-level statements in internal class Program { ... }, re-indenting every line. That was reverted before applying these edits, so the file remains a top-level-statements program as committed on main.

Rider's analysis on the resulting file: zero errors.

Test plan

  • CI builds against the pinned SDK 10.0.300 (the only blocker locally was the SDK gap — 10.0.203 installed, global.json pins 10.0.300 with latestFeature).
  • dotnet run --project tools/ConfigFilesGenerator regenerates src/Config/Analyzer.*.editorconfig byte-identically.
  • Rider inspection on tools/ConfigFilesGenerator/Program.cs reports no findings on the seven previously-flagged categories.

🤖 Generated with Claude Code


View in Codesmith
Need help on this PR? Tag @codesmith with what you need.

  • Let Codesmith autofix CI failures and bot reviews

Summary by CodeRabbit

  • Chores
    • Internal tooling improvements for configuration file generation, including enhanced error handling and code consolidation to improve reliability.

Resolves Rider inspection findings in `tools/ConfigFilesGenerator/Program.cs`:

- merge duplicated `if (X) return current;` branches in `ResolveMsBuildProperty`
  (visited.Add + properties.TryGetValue) into a single `||`-guarded check
- drop the always-true ternary at the tail of `GetRootFolderPath`; the
  `while (!path.IsEmpty)` loop only exits when `IsEmpty` is true, so the
  false-branch was dead
- collapse `ContainsKey` + indexer + redundant `continue` in
  `LoadMsBuildProperties` into `properties.TryAdd(name, value)`
- drop the redundant `System.Text.` qualifier on `Encoding.UTF8`
  (`using System.Text;` is already in scope)

Behavior preserved; this is purely a quality-of-life cleanup so the file
stays inspection-clean as Version.props churns.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 13, 2026 15:11
@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 13, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: a65791cf-f7e4-4eac-9bc0-208cea068ec6

📥 Commits

Reviewing files that changed from the base of the PR and between 3e67cb1 and 1dd0228.

📒 Files selected for processing (1)
  • tools/ConfigFilesGenerator/Program.cs
📜 Recent review details
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: GitGuardian Security Checks
  • GitHub Check: create_nuget
  • GitHub Check: Agent
  • GitHub Check: lint_config
🧰 Additional context used
📓 Path-based instructions (4)
tools/**/*.{cs,csproj}

📄 CodeRabbit inference engine (tools/CLAUDE.md)

All tools in tools/ directory must auto-detect repo root via .git folder traversal and use Meziantou.Framework.FullPath for path handling

Files:

  • tools/ConfigFilesGenerator/Program.cs
**/*.{cs,csx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Do not use banned APIs enforced via BannedSymbols.txt: DateTime.Now/UtcNow, DateTimeOffset.Now/UtcNow, ArgumentNullException.ThrowIfNull (use Guard.NotNull from ANcpLua.Roslyn.Utilities), File/Directory.GetCreationTime (use Utc variants), StringComparison.InvariantCulture (use Ordinal), System.Tuple (use ValueTuple)

Files:

  • tools/ConfigFilesGenerator/Program.cs
**/*.cs

📄 CodeRabbit inference engine (Custom checks)

**/*.cs: No .Result/.Wait() blocking async in C#: Do not use .Result, .Wait(), or .GetAwaiter().GetResult() on async operations. Use await instead.
No DateTime.Now/UtcNow in C#: Do not use DateTime.Now or DateTime.UtcNow. Use TimeProvider.System instead. Exception: test fixtures with explicit reason commented inline.
No null-forgiving operator without justification in C#: Do not use the ! (null-forgiving) operator unless an inline comment explains why the code cannot be rewritten to avoid it. Per repo policy: rewrite, don't suppress.
No suppression of fixable diagnostics in C#: Do not add #pragma warning disable or [SuppressMessage] attributes without an inline comment matching the acceptable-suppressions allowlist in CLAUDE.md. Acceptable reasons: preview API markers (MEAI001/OPENAI002), source-gen contexts, DevUI, upstream framework requirements, AOT/trimming verified safe, netstandard2.0 analyzer projects.

Files:

  • tools/ConfigFilesGenerator/Program.cs
tools/**

⚙️ CodeRabbit configuration file

tools/**: Internal tooling — config file generators, build helpers. Lower bar than
production src/, but still flag obvious bugs (missing null check that
crashes the build, hardcoded paths, swallowed exceptions).

Files:

  • tools/ConfigFilesGenerator/Program.cs
🔇 Additional comments (1)
tools/ConfigFilesGenerator/Program.cs (1)

521-521: LGTM!

Also applies to: 568-568, 749-750, 772-772


📝 Walkthrough

Walkthrough

The Program.cs file underwent targeted refinements in configuration-generation logic. The FNV-1a hash function now uses the shorter Encoding.UTF8 reference. Property-initialization now leverages TryAdd to enforce set-only-if-missing semantics directly. Property-resolution combines cycle detection and existence checks into a single guard clause. Root-folder discovery replaces conditional-expression failure handling with an explicit InvalidOperationException throw. Changes total 4 lines added, 11 removed, yielding net reduction in cyclomatic complexity.

🚥 Pre-merge checks | ✅ 7 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
No Suppression Of Fixable Diagnostics ⚠️ Warning Test files contain [SuppressMessage("Reliability", "CA2000")] attributes. These CA2000 suppressions don't match the documented allowlist exceptions. Remove CA2000 suppressions or document test disposal patterns as acceptable in CLAUDE.md allowlist.
✅ Passed checks (7 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Title follows conventional commits format (chore prefix), is under 72 chars (63), includes scope (tools), and accurately describes the cleanup work in ConfigFilesGenerator.
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 Datetime.Now/Utcnow ✅ Passed No DateTime.Now or DateTime.UtcNow usage detected in the modified C# file (tools/ConfigFilesGenerator/Program.cs). The PR contains only inspection cleanup refactorings with no temporal dependencies.
No .Result/.Wait() Blocking Async ✅ Passed No blocking async patterns (.Result, .Wait(), .GetAwaiter().GetResult()) detected in the modified C# file (tools/ConfigFilesGenerator/Program.cs). Code correctly uses await for all async operations.
No Null-Forgiving Operator Without Justification ✅ Passed Scanned tools/ConfigFilesGenerator/Program.cs for null-forgiving operators. Found zero instances of !. Code uses proper patterns without suppressions.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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

@github-actions github-actions Bot enabled auto-merge (squash) May 13, 2026 15:11
@github-actions
Copy link
Copy Markdown

@coderabbitai autofix

@claude
Copy link
Copy Markdown

claude Bot commented May 13, 2026

Claude encountered an error —— View job


I'll analyze this and get back to you.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 13, 2026

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.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR performs a small, behavior-preserving cleanup to eliminate Rider inspection warnings in tools/ConfigFilesGenerator/Program.cs, keeping the ConfigFilesGenerator tool’s logic unchanged while simplifying a few code paths.

Changes:

  • Simplifies ResolveMsBuildProperty by combining two early-return guards into a single || condition.
  • Removes dead code in GetRootFolderPath by replacing an always-true conditional return with a direct throw at loop exit.
  • Replaces a ContainsKey + indexer assignment pattern with Dictionary.TryAdd in LoadMsBuildProperties, and removes redundant control flow.
  • Drops a redundant System.Text. qualifier now that using System.Text; is in scope.

@github-actions github-actions Bot merged commit bfc4185 into main May 13, 2026
16 checks passed
@ANcpLua ANcpLua deleted the chore/configgen-inspection-cleanup branch May 13, 2026 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants