feat(sdk): demote 25 style rules from warning to suggestion#147
Conversation
|
@coderabbitai autofix |
📝 WalkthroughWalkthroughDemote multiple analyzer diagnostics from 🚥 Pre-merge checks | ✅ 8✅ Passed checks (8 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
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. |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
Pull request overview
This PR updates the SDK’s analyzer configuration policy to be more practical for test code and to reduce style-rule false positives, while keeping “bug-class” diagnostics at higher severities. It also adjusts the config generator to avoid test-/generated-path overrides from accidentally removing global default rule settings.
Changes:
- Add an SDK-injected, path-scoped
TestProjects.editorconfigto relax selected diagnostics under common test/example folder patterns (with an opt-out MSBuild property). - Demote various NetAnalyzers + ANcpLua analyzer rules from
warningtosuggestionglobally (with inline rationale). - Update
ConfigFilesGeneratorto ignore path-scoped editorconfigs when computing “configured elsewhere” rule IDs, and ship the new config via all nuspec packs.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/ConfigFilesGenerator/Program.cs | Skips path-scoped editorconfigs when determining which rule IDs are configured outside the generated global config. |
| tests/ANcpLua.Sdk.Tests/SdkTests.cs | Adds tests to validate test-path IDE1006 relaxation and the opt-out flag behavior. |
| src/Config/TestProjects.editorconfig | New path-scoped editorconfig that relaxes/demotes selected rules for test- and example-path globs. |
| src/Config/Analyzer.Microsoft.CodeAnalysis.NetAnalyzers.editorconfig | Demotes selected NetAnalyzers rules from warning to suggestion with rationale comments. |
| src/Config/Analyzer.ANcpLua.Analyzers.editorconfig | Demotes selected ANcpLua analyzer rules from warning to suggestion with rationale comments. |
| src/Build/Common/Common.props | Conditionally injects TestProjects.editorconfig and adds opt-out via DisableSdkTestEditorConfig. |
| src/ANcpLua.NET.Sdk.Web.nuspec | Includes Config/TestProjects.editorconfig in the Web SDK package. |
| src/ANcpLua.NET.Sdk.Test.nuspec | Includes Config/TestProjects.editorconfig in the Test SDK package. |
| src/ANcpLua.NET.Sdk.nuspec | Includes Config/TestProjects.editorconfig in the base SDK package. |
Comments suppressed due to low confidence (1)
tests/ANcpLua.Sdk.Tests/SdkTests.cs:888
- These new tests don't assert the build succeeded (e.g., ExitCode == 0). Without that, the assertions can pass even if Release builds fail for unrelated warnings/errors, which would make the tests a false positive for the editorconfig behavior being validated.
.BuildAsync(["--configuration", "Release"]);
// IDE1006 must not be promoted to error or warning under tests/.
Assert.False(result.HasError("IDE1006"));
Assert.False(result.HasWarning("IDE1006"));
}
| .AddSource("tests/SampleTests.cs", """ | ||
| _ = ""; | ||
|
|
||
| class Sample | ||
| { | ||
| private readonly int field; |
| # repeat the same severity table across explicit [glob] sections. Editing rules: | ||
| # update ALL sections together. Keep them byte-identical. |
| // Path-scoped editorconfigs (no `is_global = true`) override rules only inside | ||
| // their [glob] sections, not project-wide. Skipping them keeps the global | ||
| // analyzer files authoritative for default severities; otherwise rules | ||
| // relaxed only in tests/generated paths would disappear from the global file. | ||
| var pathScopedConfigs = new HashSet<string>(StringComparer.OrdinalIgnoreCase) | ||
| { | ||
| "TestProjects.editorconfig", | ||
| "GeneratedFiles.editorconfig", | ||
| }; | ||
|
|
||
| foreach (var editorconfig in Directory.EnumerateFiles(configRoot, "*.editorconfig", SearchOption.AllDirectories)) | ||
| { | ||
| if (string.Equals(Path.GetFullPath(editorconfig), Path.GetFullPath(configurationFilePath.Value), | ||
| StringComparison.OrdinalIgnoreCase)) | ||
| continue; | ||
|
|
||
| if (pathScopedConfigs.Contains(Path.GetFileName(editorconfig))) | ||
| continue; | ||
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Config/Analyzer.Microsoft.CodeAnalysis.NetAnalyzers.editorconfig`:
- Around line 1576-1579: The file currently demotes the CA5394 rule globally
(dotnet_diagnostic.CA5394.severity = suggestion); revert or change that setting
so CA5394 remains at warning in the global analyzer policy (i.e., remove the
global demotion or set severity back to warning) and, if you must demote for
tests, add a separate scoped rule (e.g., an editorconfig section targeting test
folders) to lower CA5394 only for test paths; locate the CA5394 setting and
adjust accordingly.
In `@tests/ANcpLua.Sdk.Tests/SdkTests.cs`:
- Around line 886-887: Replace xUnit Assert.True/Assert.False calls with
FluentAssertions on the result object: for examples where the diff uses
Assert.False(result.HasError("IDE1006")) and
Assert.False(result.HasWarning("IDE1006")), call the FluentAssertions extension
on the boolean expression (e.g., result.HasError("IDE1006").Should().BeFalse()
and result.HasWarning("IDE1006").Should().BeFalse()); do the same for the
analogous checks at the other locations (around lines 915-916) so all new
assertions in SdkTests.cs use FluentAssertions.Should().BeTrue()/BeFalse()
instead of Assert.True/Assert.False.
In `@tools/ConfigFilesGenerator/Program.cs`:
- Around line 152-156: The hardcoded pathScopedConfigs HashSet is brittle;
replace it with content-based detection by implementing a helper like
IsPathScopedEditorConfig(string filePath) that reads the editorconfig and
determines scope from its content (e.g., check for explicit scope markers such
as an "is_global" or "root" flag, presence/absence of global sections, or
section semantics instead of filename), then build the pathScopedConfigs set by
scanning candidate editorconfig files and calling that helper where the current
code referenced the pathScopedConfigs variable; update usages to consult this
computed set (symbols: pathScopedConfigs, IsPathScopedEditorConfig) so new
path-scoped editorconfigs are detected automatically.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: df089dfa-0468-4976-a8cf-fd1e49b064ee
📒 Files selected for processing (9)
src/ANcpLua.NET.Sdk.Test.nuspecsrc/ANcpLua.NET.Sdk.Web.nuspecsrc/ANcpLua.NET.Sdk.nuspecsrc/Build/Common/Common.propssrc/Config/Analyzer.ANcpLua.Analyzers.editorconfigsrc/Config/Analyzer.Microsoft.CodeAnalysis.NetAnalyzers.editorconfigsrc/Config/TestProjects.editorconfigtests/ANcpLua.Sdk.Tests/SdkTests.cstools/ConfigFilesGenerator/Program.cs
📜 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). (3)
- GitHub Check: Agent
- GitHub Check: create_nuget
- GitHub Check: claude-review
🧰 Additional context used
📓 Path-based instructions (13)
**/*.nuspec
📄 CodeRabbit inference engine (AGENTS.md)
Enumerate
<file>entries in .nuspec files instead of using glob patterns to ensure cross-platform compatibility
Files:
src/ANcpLua.NET.Sdk.Web.nuspecsrc/ANcpLua.NET.Sdk.Test.nuspecsrc/ANcpLua.NET.Sdk.nuspec
⚙️ CodeRabbit configuration file
**/*.nuspec: NuGet package manifest. Review for: correct package id, correct
dependency version ranges (no float-to-latest in stable releases),
proper file globs (no .pdb in release packages unless symbol package),
and that contentFiles paths match what's in src/.
Files:
src/ANcpLua.NET.Sdk.Web.nuspecsrc/ANcpLua.NET.Sdk.Test.nuspecsrc/ANcpLua.NET.Sdk.nuspec
tools/**/*.{cs,csproj}
📄 CodeRabbit inference engine (tools/CLAUDE.md)
All tools in
tools/directory must auto-detect repo root via.gitfolder traversal and useMeziantou.Framework.FullPathfor 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(useGuard.NotNullfromANcpLua.Roslyn.Utilities),File/Directory.GetCreationTime(useUtcvariants),StringComparison.InvariantCulture(useOrdinal),System.Tuple(useValueTuple)
Files:
tools/ConfigFilesGenerator/Program.cstests/ANcpLua.Sdk.Tests/SdkTests.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. Useawaitinstead.
No DateTime.Now/UtcNow in C#: Do not useDateTime.NoworDateTime.UtcNow. UseTimeProvider.Systeminstead. 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 disableor[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.cstests/ANcpLua.Sdk.Tests/SdkTests.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
src/Build/**/*.props
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/Build/**/*.props: Props files in src/Build/Common/ and src/Build/Enforcement/ should set properties and defaults using guard patterns likeCondition="'$(Property)' == ''"to allow consumer override
Use GlobalPackageReference in analyzer injection configuration for immutable package references when Central Package Management (CPM) is enabled
Files:
src/Build/Common/Common.props
**/*.props
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not import
Common.propsoutside theSdk.propswrapper and do not change the import order:Sdk.props→Microsoft.NET.Sdk→Common.props→Enforcement.props— the_MustImportMicrosoftNETSdkguard enforces this
Files:
src/Build/Common/Common.props
⚙️ CodeRabbit configuration file
**/*.props: MSBuild property files. Review for: condition guards on every override
(consumer must be able to opt out), no<RemoveAttributes>on items
that consumers may have set, and that defaults are sensible without
forcing a downstream cascade.
Files:
src/Build/Common/Common.props
**/*.{props,targets}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Never use hardcoded paths in
.props/.targetsfiles — always use$(MSBuildThisFileDirectory)instead
Files:
src/Build/Common/Common.props
**/Common.props
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In
Common.props, do not reorder the probe that reads the consumer.csprojto detect explicitTargetFrameworkbefore defaulting it
Files:
src/Build/Common/Common.props
src/Build/**
⚙️ CodeRabbit configuration file
src/Build/**: Build targets shared across .NET.Sdk, .Test, .Web variants. Review for:
target-name collisions with consumer csproj (use unique prefixes), correct
BeforeTargets/AfterTargets ordering, and idempotent execution (target
should be safe to run twice).
Files:
src/Build/Common/Common.props
tests/**/*.cs
⚙️ CodeRabbit configuration file
tests/**/*.cs: xUnit + FluentAssertions. AAA pattern. NSubstitute for mocks.
Flag: blocking-async patterns (Task.Run(() => x.Result)), missing
async Task(sync-void),Thread.Sleepinstead ofawait Task.Delay,
tests asserting on private state via reflection.
Files:
tests/ANcpLua.Sdk.Tests/SdkTests.cs
src/Config/**/*.editorconfig
📄 CodeRabbit inference engine (src/CLAUDE.md)
Configuration files in src/Config/ should use flat layout with specific naming conventions: Analyzer..editorconfig for per-analyzer rules, Global.editorconfig as authoritative, and per-variant ANcpLua.NET.Sdk..editorconfig files
Files:
src/Config/Analyzer.ANcpLua.Analyzers.editorconfigsrc/Config/Analyzer.Microsoft.CodeAnalysis.NetAnalyzers.editorconfigsrc/Config/TestProjects.editorconfig
src/Config/Analyzer.*.editorconfig
📄 CodeRabbit inference engine (AGENTS.md)
Analyzer configuration rules should be tuned per-analyzer using analyzer-specific editorconfig files in the Config directory
Files:
src/Config/Analyzer.ANcpLua.Analyzers.editorconfigsrc/Config/Analyzer.Microsoft.CodeAnalysis.NetAnalyzers.editorconfig
🧠 Learnings (1)
📚 Learning: 2026-05-12T11:36:39.749Z
Learnt from: ANcpLua
Repo: ANcpLua/ANcpLua.NET.Sdk PR: 143
File: src/Config/ANcpLua.NET.Sdk.BitNet.editorconfig:5-9
Timestamp: 2026-05-12T11:36:39.749Z
Learning: In this repository’s per-variant editorconfig overlays under `src/Config/`, keep variant settings mirrored between the web-based overlays (e.g., `ANcpLua.NET.Sdk.Web.editorconfig` and `ANcpLua.NET.Sdk.BitNet.editorconfig`). Specifically, for CA1002 (`CA1002` / `dotnet_diagnostic.CA1002.severity`), any policy change must be made first in the Web overlay and then mirrored to the BitNet overlay—do not apply CA1002 changes asymmetrically to only one variant.
Applied to files:
src/Config/Analyzer.ANcpLua.Analyzers.editorconfigsrc/Config/Analyzer.Microsoft.CodeAnalysis.NetAnalyzers.editorconfigsrc/Config/TestProjects.editorconfig
🔇 Additional comments (5)
src/ANcpLua.NET.Sdk.Test.nuspec (1)
50-50: LGTM: test-path editorconfig is explicitly packaged.src/ANcpLua.NET.Sdk.Web.nuspec (1)
50-50: LGTM: explicit nuspec inclusion is correct.src/ANcpLua.NET.Sdk.nuspec (1)
50-50: LGTM: package manifest update is clean and portable.src/Config/TestProjects.editorconfig (1)
1-227: LGTM: path-scoped test relaxations are clearly structured and consistent across globs.src/Build/Common/Common.props (1)
83-93: Analyzer injection wiring looks correct and consumer-opt-out is preserved.The exclude + conditional re-include pattern is clean, keeps the path-scoped config explicit, and maintains override control through
DisableSdkTestEditorConfig.
|
@coderabbitai autofix |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 3 file(s) based on 3 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f35325c2f2
ℹ️ 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".
| <EditorConfigFiles Include="$(MSBuildThisFileDirectory)..\..\Config\TestProjects.editorconfig" | ||
| Condition="'$(DisableSdkTestEditorConfig)' != 'true'" |
There was a problem hiding this comment.
Common.props now unconditionally adds Config/TestProjects.editorconfig, but ANcpLua.NET.Sdk.BitNet.nuspec was not updated to ship that file (it still only packs GeneratedFiles.editorconfig plus globals). For BitNet consumers, this leaves EditorConfigFiles pointing at a file that does not exist in the installed SDK package, so builds can break when the compiler is given a missing analyzerconfig path. Please either pack TestProjects.editorconfig in the BitNet nuspec too, or add an Exists(...) guard like the one used for $(ANcpLuaSdkName).editorconfig.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tools/ConfigFilesGenerator/Program.cs`:
- Around line 152-166: The code currently keys pathScopedConfigs by filename
causing collisions across directories; change pathScopedConfigs to store
normalized full paths (use Meziantou.Framework.FullPath or Path.GetFullPath)
when adding (e.g., replace Path.GetFileName(editorconfig) with
FullPath.Create(editorconfig) or Path.GetFullPath(editorconfig)) and update the
subsequent checks to compare against the same normalized full path (replace
Path.GetFileName(editorconfig) and Path.GetFullPath(configurationFilePath.Value)
comparisons with FullPath-based comparisons). Ensure
StringComparer.OrdinalIgnoreCase is used for the HashSet, apply the same
full-path normalization to the other block referenced (lines around 175-197),
and keep using IsPathScopedEditorConfig(editorconfig) to determine membership.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d19f28b5-7fe2-49b9-bcd3-b79da20e8151
📒 Files selected for processing (3)
src/Config/Analyzer.Microsoft.CodeAnalysis.NetAnalyzers.editorconfigtests/ANcpLua.Sdk.Tests/SdkTests.cstools/ConfigFilesGenerator/Program.cs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (7)
tools/**/*.{cs,csproj}
📄 CodeRabbit inference engine (tools/CLAUDE.md)
All tools in
tools/directory must auto-detect repo root via.gitfolder traversal and useMeziantou.Framework.FullPathfor 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(useGuard.NotNullfromANcpLua.Roslyn.Utilities),File/Directory.GetCreationTime(useUtcvariants),StringComparison.InvariantCulture(useOrdinal),System.Tuple(useValueTuple)
Files:
tools/ConfigFilesGenerator/Program.cstests/ANcpLua.Sdk.Tests/SdkTests.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. Useawaitinstead.
No DateTime.Now/UtcNow in C#: Do not useDateTime.NoworDateTime.UtcNow. UseTimeProvider.Systeminstead. 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 disableor[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.cstests/ANcpLua.Sdk.Tests/SdkTests.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
src/Config/**/*.editorconfig
📄 CodeRabbit inference engine (src/CLAUDE.md)
Configuration files in src/Config/ should use flat layout with specific naming conventions: Analyzer..editorconfig for per-analyzer rules, Global.editorconfig as authoritative, and per-variant ANcpLua.NET.Sdk..editorconfig files
Files:
src/Config/Analyzer.Microsoft.CodeAnalysis.NetAnalyzers.editorconfig
src/Config/Analyzer.*.editorconfig
📄 CodeRabbit inference engine (AGENTS.md)
Analyzer configuration rules should be tuned per-analyzer using analyzer-specific editorconfig files in the Config directory
Files:
src/Config/Analyzer.Microsoft.CodeAnalysis.NetAnalyzers.editorconfig
tests/**/*.cs
⚙️ CodeRabbit configuration file
tests/**/*.cs: xUnit + FluentAssertions. AAA pattern. NSubstitute for mocks.
Flag: blocking-async patterns (Task.Run(() => x.Result)), missing
async Task(sync-void),Thread.Sleepinstead ofawait Task.Delay,
tests asserting on private state via reflection.
Files:
tests/ANcpLua.Sdk.Tests/SdkTests.cs
🧠 Learnings (1)
📚 Learning: 2026-05-12T11:36:39.749Z
Learnt from: ANcpLua
Repo: ANcpLua/ANcpLua.NET.Sdk PR: 143
File: src/Config/ANcpLua.NET.Sdk.BitNet.editorconfig:5-9
Timestamp: 2026-05-12T11:36:39.749Z
Learning: In this repository’s per-variant editorconfig overlays under `src/Config/`, keep variant settings mirrored between the web-based overlays (e.g., `ANcpLua.NET.Sdk.Web.editorconfig` and `ANcpLua.NET.Sdk.BitNet.editorconfig`). Specifically, for CA1002 (`CA1002` / `dotnet_diagnostic.CA1002.severity`), any policy change must be made first in the Web overlay and then mirrored to the BitNet overlay—do not apply CA1002 changes asymmetrically to only one variant.
Applied to files:
src/Config/Analyzer.Microsoft.CodeAnalysis.NetAnalyzers.editorconfig
🔇 Additional comments (2)
src/Config/Analyzer.Microsoft.CodeAnalysis.NetAnalyzers.editorconfig (1)
21-23: Policy split is consistent and keeps security/bug guardrails intact.These demotions are scoped to style/perf-heavy rules, and the file still preserves warning-level enforcement for bug/security rules (including CA1310, CA1820, CA2000, CA2012, CA2201, and CA53xx such as CA5394). This matches the analyzer-injection contract in the PR.
Also applies to: 108-110, 120-123, 177-199, 284-287, 481-483, 539-567, 711-751
tests/ANcpLua.Sdk.Tests/SdkTests.cs (1)
861-916: Good coverage for test-path IDE1006 behavior and opt-out.Both new tests validate the intended contract clearly and follow the test conventions (xUnit + FluentAssertions + async flow).
ANcpLua
dismissed stale reviews from coderabbitai[bot] and coderabbitai[bot]
Dismissed by github-settings-automation: CodeRabbit is advisory-only under the new posture (request_changes_workflow: false); see https://github.com/ANcpLua/github-settings-automation#auto-merge-posture
|
@coderabbitai autofix |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. The branch was updated while autofix was in progress. Please try again. |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
ANcpLua
changed the title
…gestion Splits the analyzer policy into two tiers so SDK consumers get a usable default for tests + style-rule false positives: 1. New path-scoped Config/TestProjects.editorconfig auto-injected by Common.props. Covers tests/, test/, **/*.Tests/, **/*.UnitTests/, **/*.IntegrationTests/, **/examples/. Drops CA1707/CA1816/CA2000/CA2007/ CA2201/CA1031/CA1812/CA1822/CA1859/CA1852/CA5394/VSTHRD111/VSTHRD200/ IDE0051/IDE0052/IDE1006/AL0025/AL0057/AL0105/AL0112/AL0137/RS0030/ xUnit1004 to none-or-suggestion in test paths only. Opt out via <DisableSdkTestEditorConfig>true</DisableSdkTestEditorConfig>. 2. Demotes 14 NetAnalyzers + 11 AL rules from warning to suggestion in the global analyzer files. Bug-class rules (CA2000/2012/2201/1310/1820, all crypto CA53xx, AL0026 TimeProvider migration) stay at warning. ConfigFilesGenerator now skips path-scoped editorconfigs when computing GetRuleIdsConfiguredOutside so the global analyzer files stay authoritative. Adds two SdkTests: TestProjects_EditorConfig_RelaxesIDE1006InTestPath and TestProjects_EditorConfig_DisableOptOutWorks. All three .nuspec files updated to ship Config/TestProjects.editorconfig. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nfigFiles
CS8700 ("Multiple analyzer config files cannot be in the same directory")
fires because src/Config/ now has two non-global (path-scoped) editorconfigs:
the pre-existing GeneratedFiles.editorconfig and the new TestProjects.editorconfig.
Common.props already excludes TestProjects.editorconfig from the consumer-side
wildcard and conditionally re-includes it (gated on DisableSdkTestEditorConfig).
The dogfood pattern in this repo's Directory.Build.props was missed in the
prior commit, so both the lint_config job (tools/ConfigFilesGenerator) and the
SDK's own test build hit CS8700. Mirror the same exclude here.
The dogfood doesn't need TestProjects.editorconfig active: tools/ isn't a test
project and the SDK's own tests use Microsoft.NET.Sdk directly without the
test-folder relaxations.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fixed 3 file(s) based on 3 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
The path-scoped editorconfig at Config/TestProjects.editorconfig cannot
reach consumer source files. Roslyn evaluates non-global editorconfig
[glob] sections relative to the editorconfig file's containing directory,
and consumer source paths under e.g. /Users/.../Paperless/PaperlessServices.Tests/
are not descendants of ~/.nuget/packages/ancplua.net.sdk/X.Y.Z/Config/.
The two new tests appeared to pass locally only because CS8700 ("Multiple
analyzer config files cannot be in the same directory") aborted the build
before IDE1006 could fire — a false positive.
Keep the global analyzer demotions (14 CA + 11 AL rules going warning →
suggestion) and the ConfigFilesGenerator path-scoped editorconfig
detection: those work as intended and apply project-wide. Test-folder
relaxations need a different mechanism (e.g. NoWarn property injection in
ANcpLua.NET.Sdk.Test targets, conditional on $(MSBuildProjectName.EndsWith('Tests'))
patterns) and belong in a follow-up PR with its own dedicated design.
Removes:
- src/Config/TestProjects.editorconfig (the file)
- TestProjects.editorconfig references in 3 .nuspec files
- Conditional EditorConfigFiles include in Common.props
- TestProjects.editorconfig exclude in dogfood Directory.Build.props
- SdkTests.TestProjects_EditorConfig_RelaxesIDE1006InTestPath
- SdkTests.TestProjects_EditorConfig_DisableOptOutWorks
235/235 tests pass locally against packed 99.99.99.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ANcpLua
force-pushed
the
feat/test-folder-editorconfig-relaxations
branch
from
dd462d2 to
3cb6fbf
Compare
|
@coderabbitai autofix |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
| // Path-scoped editorconfigs (no `is_global = true`) override rules only inside | ||
| // their [glob] sections, not project-wide. Skipping them keeps the global | ||
| // analyzer files authoritative for default severities; otherwise rules | ||
| // relaxed only in tests/generated paths would disappear from the global file. | ||
| var pathScopedConfigs = new HashSet<string>(StringComparer.OrdinalIgnoreCase); | ||
| foreach (var editorconfig in Directory.EnumerateFiles(configRoot, "*.editorconfig", SearchOption.AllDirectories)) | ||
| { | ||
| if (IsPathScopedEditorConfig(editorconfig)) | ||
| pathScopedConfigs.Add(Path.GetFileName(editorconfig)); |
| dotnet_diagnostic.CA1823.severity = warning | ||
| # Demoted: high false-positive rate on DI-injected fields read via reflection, | ||
| # source-generator pattern fields, and incremental-build scaffolding. Real | ||
| # dead-code detection is better served by IDE0052 (kept at warning in CodingStyle). |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tools/ConfigFilesGenerator/Program.cs (1)
152-166:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse
FullPathkeys for scoped-config exclusion; filename keys can silently drop global rules.
Line 152,Line 156, andLine 165still keypathScopedConfigsby filename. If two configs in different folders share a basename, a global config can be skipped and rule-id generation drifts.Proposed fix
- var pathScopedConfigs = new HashSet<string>(StringComparer.OrdinalIgnoreCase); + var pathScopedConfigs = new HashSet<FullPath>(); foreach (var editorconfig in Directory.EnumerateFiles(configRoot, "*.editorconfig", SearchOption.AllDirectories)) { - if (IsPathScopedEditorConfig(editorconfig)) - pathScopedConfigs.Add(Path.GetFileName(editorconfig)); + var editorconfigPath = FullPath.FromPath(editorconfig); + if (IsPathScopedEditorConfig(editorconfigPath)) + pathScopedConfigs.Add(editorconfigPath); } foreach (var editorconfig in Directory.EnumerateFiles(configRoot, "*.editorconfig", SearchOption.AllDirectories)) { - if (string.Equals(Path.GetFullPath(editorconfig), Path.GetFullPath(configurationFilePath.Value), - StringComparison.OrdinalIgnoreCase)) + var editorconfigPath = FullPath.FromPath(editorconfig); + if (editorconfigPath == configurationFilePath) continue; - if (pathScopedConfigs.Contains(Path.GetFileName(editorconfig))) + if (pathScopedConfigs.Contains(editorconfigPath)) continue; - foreach (var rule in GetConfiguration(FullPath.FromPath(editorconfig)).Rules) + foreach (var rule in GetConfiguration(editorconfigPath).Rules) configuredRuleIds.Add(rule.Id); } } -static bool IsPathScopedEditorConfig(string filePath) +static bool IsPathScopedEditorConfig(FullPath filePath) { if (!File.Exists(filePath)) return false;#!/bin/bash set -euo pipefail echo "Duplicate .editorconfig basenames under src/Config (collision candidates):" fd -e editorconfig . src/Config | awk -F/ '{print $NF}' | sort | uniq -d echo echo "Current filename-based keying sites:" rg -n -C2 'pathScopedConfigs|Path.GetFileName\(editorconfig\)|IsPathScopedEditorConfig\(' tools/ConfigFilesGenerator/Program.csAs per coding guidelines: “All tools in
tools/directory must auto-detect repo root via.gitfolder traversal and useMeziantou.Framework.FullPathfor path handling.”Also applies to: 175-197
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/ConfigFilesGenerator/Program.cs` around lines 152 - 166, The code currently keys path-scoped editorconfigs by filename (pathScopedConfigs.Add(Path.GetFileName(editorconfig))) causing collisions; change pathScopedConfigs to store full paths (use Meziantou.Framework.FullPath or Path.GetFullPath) and update all lookups to compare against the full-path key (replace Path.GetFileName(editorconfig) usages and the pathScopedConfigs.Contains(...) check). Specifically, when building pathScopedConfigs in the IsPathScopedEditorConfig loop, add the editorconfig's full path (or FullPath wrapper) instead of the basename, and when checking skip conditions (the comparison with configurationFilePath.Value and the pathScopedConfigs.Contains call) normalize both sides to FullPath/Path.GetFullPath before comparing so different directories with the same filename don't collide.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@tools/ConfigFilesGenerator/Program.cs`:
- Around line 152-166: The code currently keys path-scoped editorconfigs by
filename (pathScopedConfigs.Add(Path.GetFileName(editorconfig))) causing
collisions; change pathScopedConfigs to store full paths (use
Meziantou.Framework.FullPath or Path.GetFullPath) and update all lookups to
compare against the full-path key (replace Path.GetFileName(editorconfig) usages
and the pathScopedConfigs.Contains(...) check). Specifically, when building
pathScopedConfigs in the IsPathScopedEditorConfig loop, add the editorconfig's
full path (or FullPath wrapper) instead of the basename, and when checking skip
conditions (the comparison with configurationFilePath.Value and the
pathScopedConfigs.Contains call) normalize both sides to
FullPath/Path.GetFullPath before comparing so different directories with the
same filename don't collide.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9ee863f8-356d-4b18-ad5f-049ba8f3c6c2
📒 Files selected for processing (3)
src/Config/Analyzer.ANcpLua.Analyzers.editorconfigsrc/Config/Analyzer.Microsoft.CodeAnalysis.NetAnalyzers.editorconfigtools/ConfigFilesGenerator/Program.cs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
tools/**/*.{cs,csproj}
📄 CodeRabbit inference engine (tools/CLAUDE.md)
All tools in
tools/directory must auto-detect repo root via.gitfolder traversal and useMeziantou.Framework.FullPathfor 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(useGuard.NotNullfromANcpLua.Roslyn.Utilities),File/Directory.GetCreationTime(useUtcvariants),StringComparison.InvariantCulture(useOrdinal),System.Tuple(useValueTuple)
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. Useawaitinstead.
No DateTime.Now/UtcNow in C#: Do not useDateTime.NoworDateTime.UtcNow. UseTimeProvider.Systeminstead. 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 disableor[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
src/Config/**/*.editorconfig
📄 CodeRabbit inference engine (src/CLAUDE.md)
Configuration files in src/Config/ should use flat layout with specific naming conventions: Analyzer..editorconfig for per-analyzer rules, Global.editorconfig as authoritative, and per-variant ANcpLua.NET.Sdk..editorconfig files
Files:
src/Config/Analyzer.ANcpLua.Analyzers.editorconfigsrc/Config/Analyzer.Microsoft.CodeAnalysis.NetAnalyzers.editorconfig
src/Config/Analyzer.*.editorconfig
📄 CodeRabbit inference engine (AGENTS.md)
Analyzer configuration rules should be tuned per-analyzer using analyzer-specific editorconfig files in the Config directory
Files:
src/Config/Analyzer.ANcpLua.Analyzers.editorconfigsrc/Config/Analyzer.Microsoft.CodeAnalysis.NetAnalyzers.editorconfig
🧠 Learnings (1)
📚 Learning: 2026-05-12T11:36:39.749Z
Learnt from: ANcpLua
Repo: ANcpLua/ANcpLua.NET.Sdk PR: 143
File: src/Config/ANcpLua.NET.Sdk.BitNet.editorconfig:5-9
Timestamp: 2026-05-12T11:36:39.749Z
Learning: In this repository’s per-variant editorconfig overlays under `src/Config/`, keep variant settings mirrored between the web-based overlays (e.g., `ANcpLua.NET.Sdk.Web.editorconfig` and `ANcpLua.NET.Sdk.BitNet.editorconfig`). Specifically, for CA1002 (`CA1002` / `dotnet_diagnostic.CA1002.severity`), any policy change must be made first in the Web overlay and then mirrored to the BitNet overlay—do not apply CA1002 changes asymmetrically to only one variant.
Applied to files:
src/Config/Analyzer.ANcpLua.Analyzers.editorconfigsrc/Config/Analyzer.Microsoft.CodeAnalysis.NetAnalyzers.editorconfig
🔇 Additional comments (2)
src/Config/Analyzer.ANcpLua.Analyzers.editorconfig (1)
129-132: Demotions are consistent with the stated style-vs-correctness policy.No blocking concerns in these changes.
Also applies to: 197-200, 357-360, 416-420, 486-516, 575-578, 692-697
src/Config/Analyzer.Microsoft.CodeAnalysis.NetAnalyzers.editorconfig (1)
21-24: CA1002 parity confirmed in variant overlays.Both
src/Config/ANcpLua.NET.Sdk.Web.editorconfig(line 6) andsrc/Config/ANcpLua.NET.Sdk.BitNet.editorconfig(line 9) correctly mirrordotnet_diagnostic.CA1002.severity = none, maintaining structural parity after the global demote tosuggestionin line 23.
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tools/ConfigFilesGenerator/Program.cs`:
- Around line 187-193: The current StartsWith("is_global") check is too
permissive and can match keys like "is_globalized" or section headers; change
the logic in the block around variable trimmed (used in the
GetRuleIdsConfiguredOutside flow) to first ignore section headers (skip lines
that start with '[') then split on '=' and compare the left-hand key exactly to
"is_global" using String.Equals with OrdinalIgnoreCase (e.g., parse parts[0] as
the key and check key.Equals("is_global", StringComparison.OrdinalIgnoreCase));
only if the exact key matches and the right-hand value equals "true" should you
return false.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 24bed5a4-d679-475b-aa07-3b5e2bcc0330
📒 Files selected for processing (1)
tools/ConfigFilesGenerator/Program.cs
📜 Review details
🧰 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.gitfolder traversal and useMeziantou.Framework.FullPathfor 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(useGuard.NotNullfromANcpLua.Roslyn.Utilities),File/Directory.GetCreationTime(useUtcvariants),StringComparison.InvariantCulture(useOrdinal),System.Tuple(useValueTuple)
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. Useawaitinstead.
No DateTime.Now/UtcNow in C#: Do not useDateTime.NoworDateTime.UtcNow. UseTimeProvider.Systeminstead. 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 disableor[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)
152-166: Still unresolved: keep scoped-config path filtering typed asFullPath.This block still uses
Path.GetFullPath+stringset membership instead ofMeziantou.Framework.FullPath, which is the repo’s required path abstraction for tooling code.As per coding guidelines
tools/**/*.{cs,csproj}: "All tools intools/directory must auto-detect repo root via.gitfolder traversal and useMeziantou.Framework.FullPathfor path handling".
Per tools/**/*.cs coding rule (Meziantou.Framework.FullPath for path handling), replace the string-keyed Path.GetFullPath HashSet with a FullPath-keyed set. Also folds the two enumerations into a single materialized array; equality is built-in (FullPath is comparable). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@coderabbitai autofix |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
| dotnet_diagnostic.CA1823.severity = warning | ||
| # Demoted: high false-positive rate on DI-injected fields read via reflection, | ||
| # source-generator pattern fields, and incremental-build scaffolding. Real | ||
| # dead-code detection is better served by IDE0052 (kept at warning in CodingStyle). |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tools/ConfigFilesGenerator/Program.cs (1)
184-199:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStop parsing after first section header when detecting
is_global.Line 185 currently
continues on section headers, so the parser still inspects section-local keys. That can misclassify a path-scoped file as global if any section containsis_global = true, causing silent policy drift in generated defaults.Suggested fix
static bool IsPathScopedEditorConfig(FullPath filePath) { if (!File.Exists(filePath)) return false; @@ - foreach (var line in File.ReadLines(filePath)) + foreach (var line in File.ReadLines(filePath)) { var trimmed = line.Trim(); - // Ignore section headers + if (trimmed.Length == 0 || trimmed.StartsWith('#') || trimmed.StartsWith(';')) + continue; + + // `is_global` is a preamble key; stop once sections begin. if (trimmed.StartsWith("[", StringComparison.Ordinal)) - continue; + break; var parts = trimmed.Split('=', 2, StringSplitOptions.TrimEntries); if (parts.Length != 2) continue; @@ if (string.Equals(parts[1], "true", StringComparison.OrdinalIgnoreCase)) return false; }As per coding guidelines
tools/**: “Lower bar than production src/, but still flag obvious bugs (missing null check that crashes the build, hardcoded paths, swallowed exceptions).”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/ConfigFilesGenerator/Program.cs` around lines 184 - 199, The parser currently skips section headers with "if (trimmed.StartsWith("[", StringComparison.Ordinal)) continue;" which lets later section-local "is_global" keys be treated as global; change that to stop parsing further lines when the first section header is encountered (use break instead of continue) so only top-level global keys are considered, and leave the rest of the logic that splits parts and checks parts[0] == "is_global" intact (references: the trimmed variable, the StartsWith check, the parts split, and the "is_global" comparison).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@tools/ConfigFilesGenerator/Program.cs`:
- Around line 184-199: The parser currently skips section headers with "if
(trimmed.StartsWith("[", StringComparison.Ordinal)) continue;" which lets later
section-local "is_global" keys be treated as global; change that to stop parsing
further lines when the first section header is encountered (use break instead of
continue) so only top-level global keys are considered, and leave the rest of
the logic that splits parts and checks parts[0] == "is_global" intact
(references: the trimmed variable, the StartsWith check, the parts split, and
the "is_global" comparison).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0ea7117c-3e71-4502-bb93-5f6070d96c92
📒 Files selected for processing (1)
tools/ConfigFilesGenerator/Program.cs
📜 Review details
🧰 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.gitfolder traversal and useMeziantou.Framework.FullPathfor 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(useGuard.NotNullfromANcpLua.Roslyn.Utilities),File/Directory.GetCreationTime(useUtcvariants),StringComparison.InvariantCulture(useOrdinal),System.Tuple(useValueTuple)
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. Useawaitinstead.
No DateTime.Now/UtcNow in C#: Do not useDateTime.NoworDateTime.UtcNow. UseTimeProvider.Systeminstead. 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 disableor[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)
152-166: Good refactor: FullPath-based config enumeration and exclusion is solid.This removes filename-collision risk and keeps path handling consistent across the generator.
Summary
Lower the analyzer policy floor so the SDK ships with a defensible default for style-rule false positives. Each demotion has an inline rationale comment.
warning→suggestion(CA1002/1032/1034/1052/1054/1055/1056/1307/1725/1819/1822/1823/1852/1859). Bug-class rules stay atwarning(CA2000/2012/2201/1310/1820, all crypto CA53xx including CA5394 after CodeRabbit revert).warning→suggestion(AL0025/0039/0070/0081/0094-0096/0101/0102/0114/0137).AL0026(TimeProvider migration) stays atwarningsince it flags real correctness issues.tools/ConfigFilesGenerator/Program.cs): detect path-scoped editorconfigs dynamically by parsing foris_global = true, replacing the original hardcoded list. Keeps the global analyzer files authoritative for project-wide defaults.Path not taken
The first iteration of this PR added a path-scoped
Config/TestProjects.editorconfigto relax 23 rules undertests/,**/*.Tests/, etc. — and two SDK tests that appeared to verify it. The mechanism is fundamentally broken for SDK consumers: Roslyn evaluates non-global editorconfig[glob]sections relative to the editorconfig file's directory, and consumer source paths (e.g.~/Paperless/PaperlessServices.Tests/) are not descendants of~/.nuget/packages/ancplua.net.sdk/X.Y.Z/Config/. The tests only appeared to pass locally because CS8700 (Multiple analyzer config files cannot be in the same directory, fired by the co-locatedGeneratedFiles.editorconfig+TestProjects.editorconfig) aborted the build before IDE1006 could surface — a false positive.Test-folder relaxations need a different mechanism (NoWarn property injection in
ANcpLua.NET.Sdk.Testtargets, conditional on test-project naming patterns) and belong in a follow-up PR with a dedicated design.Test plan
dotnet test tests/ANcpLua.Sdk.Testsagainst packed 99.99.99 — 235/235 passlint_configjob regenerates analyzer configs and produces no diffWarningsNotAsErrorscarve-out (style rules covered; bug-class rules remain in residual carve-out)🤖 Generated with Claude Code
Summary by CodeRabbit