ci: add static analysis enforcement to build pipeline

[newmanifold]

Changes

• Add Code Lint CI workflow that runs dotnet build with EnforceCodeStyleInBuild=true and fails on any IDE* or CA* analyzer warnings
• Enforce the following Roslyn rules via .editorconfig:
  • IDE0005 — remove unnecessary using directives
  • IDE0008 — use explicit type instead of var when type is not apparent
  • IDE0041 — use is null instead of ReferenceEquals
  • IDE0100 — remove redundant equality expressions
  • CA1507 — use nameof instead of string literal
  • CA1510 — use ArgumentNullException.ThrowIfNull
  • CA1825 — use Array.Empty<T>() for zero-length array allocations
  • CA1829 — use Count property instead of Enumerable.Count()
  • CA1834 — use StringBuilder.Append(char) for single-character appends
• Fix all pre-existing violations across the codebase

Types of changes

What types of changes does your code introduce?

• Bugfix (a non-breaking change that fixes an issue)
• New feature (a non-breaking change that adds functionality)
• Breaking change (a change that causes existing functionality not to work as expected)
• Optimization
• Refactoring
• Documentation update
• Build-related changes
• Other: Description

Testing

Requires testing

• Yes
• No

Documentation

Requires documentation update

• Yes
• No

Requires explanation in Release Notes

• Yes
• No

[newmanifold]

@claude review please

[claude]

Claude finished @newmanifold's task in 1m 55s —— View job

---

PR Review: ci: add Roslyn code style enforcement to build pipeline

• Gather context and read relevant files
• Review CI workflow file
• Review .editorconfig changes
• Review source code fixes
• Final assessment

---

Summary

This is a well-scoped CI quality PR. The workflow design, editorconfig changes, and bulk source fixes are all reasonable. One functional bug in the workflow step and a few minor nits below.

---

Issues

🔴 Bug: Build exit code is silently swallowed — .github/workflows/code-lint.yml:40

dotnet build src/Nethermind/Nethermind.slnx \
  ... 2>&1 | tee /tmp/build.log
! grep -qP "warning (IDE|CA)\d+" /tmp/build.log

In bash, cmd | tee file returns the exit code of tee, not cmd. If dotnet build exits non-zero for a reason unrelated to lint (e.g., a compilation error, missing package, infrastructure failure), the step still passes — only the grep result matters. This means broken builds can silently pass the lint check.

Fix: add set -o pipefail at the top of the run block, or capture the build exit code explicitly. Fix this →

🟡 TreatWarningsAsErrors=false + grep is fragile — .github/workflows/code-lint.yml:40

The check relies on grep pattern warning (IDE|CA)\d+ to detect failures rather than letting MSBuild fail on warnings. This means:

• Any new Roslyn rule that uses a different naming format (e.g., CS9*, SYSLIB*) would be silently missed
• The workflow description says it "fails on any IDE* or CA* analyzer warnings" — but only through log scanning, not hard enforcement

Consider whether -warnaserror:IDE0005,IDE0008,IDE0041,... (only for the enforced rules) would be cleaner and more reliable than -p:TreatWarningsAsErrors=false + grep.

🟡 Minor: double blank lines after removed usings

A few files have a double blank line left where a using was removed:

• src/Nethermind/Nethermind.Core.Test/Modules/PseudoNetworkModule.cs
• src/Nethermind/Nethermind.Evm.Test/Eip8037RegressionTests.cs

These are cosmetic but would normally be cleaned up by dotnet format whitespace.

---

Observations (informational)

#if ZK_EVM wrapping is correct. The using Nethermind.Core.Collections; in MemDb.cs and using System.Collections.Generic; in Metrics.cs are wrapped because the symbols are only referenced inside #if ZK_EVM blocks. The Roslyn analyzer correctly flags these as unused in the default build, and the fix is appropriate.

actions/checkout@v6 is consistent with other workflows in the repo.

Concurrency group (${{ github.workflow }}-${{ github.ref }}) with cancel-in-progress: true is correct and matches repo conventions.

Bulk source fixes are clean:

• == false → ! — consistent with coding style
• ArgumentNullException.ThrowIfNull — correct CA1510 fix
• StringBuilder.Append('[') char overloads — correct CA1834 fix
• readFromFile.Count instead of .Count() — correct CA1829 fix

---

Verdict

The intent and scope of this PR are good. The critical fix needed before merging is the pipefail issue — without it, a broken build in CI could silently report as a clean lint check. The TreatWarningsAsErrors approach is worth reconsidering for robustness, but it's not blocking.

[newmanifold]

@claude review again

[claude]

Claude finished @newmanifold's task in 2m 3s —— View job

---

PR Review: ci: add Roslyn code style enforcement to build pipeline

• Gather context
• Review CI workflow file
• Review .editorconfig changes
• Review source code fixes
• Check if previous review issues were addressed
• Final assessment

---

Previous Issues — Now Resolved

set -o pipefail added — the critical bug from the last review is fixed. dotnet build exit codes now propagate correctly through the pipe.

Double blank lines — cleaned up in PseudoNetworkModule.cs and Eip8037RegressionTests.cs.

---

New Observations

🟡 set -o pipefail is redundant — .github/workflows/code-lint.yml:41

GitHub Actions' default shell already runs bash with -e -o pipefail: bash --noprofile --norc -e -o pipefail {0}. The explicit set -o pipefail is harmless and fine as defensive documentation, but it's not doing additional work.

🟡 IDE0041 is error while all others are warning — .editorconfig:97

dotnet_diagnostic.IDE0041.severity = error   # ← error
dotnet_diagnostic.IDE0005.severity = warning
dotnet_diagnostic.CA1510.severity = warning  # ← all others are warnings

This asymmetry is probably intentional (IDE0041 / ReferenceEquals is a more serious correctness issue), but it has a subtle effect: error IDE0041 diagnostics would fail the build via exit code directly, while warning IDE*/CA* violations are caught by grep. As long as that's deliberate, it's fine.

---

Source Fixes — All Correct

• == false → !: consistent with coding style, all changes look correct
• ArgumentNullException.ThrowIfNull: cleaner than manual throws, .NET 6+ API used appropriately
• StringBuilder.Append('[') char overloads: correct CA1834 fix, verified in JsonRpcProcessorTests.cs
• SlicedReadOnlyList.cs and FallbackToFieldFromApi.cs: null guard modernisation is clean

---

Verdict

The PR is in good shape. Both blocking issues from the last review are resolved. The remaining points above are observations, not blockers. Ready to merge.

[github-actions]

EVM Opcode Benchmark Diff

Aggregated runs: base=1, pr=1

No significant regressions or improvements detected.

[LukaszRozmej]

More rules please!

[LukaszRozmej]

maybe remove it and make the Dictionaries fully qualified when used.

[newmanifold]

It would be more ugly, type of _spanDb is conditional based on what whether ZK_EVM is defined, so after this we will have to use #if ZK_EVM sites where extension methods are used.

[LukaszRozmej]

maybe remove it and make the Dictionaries fully qualified when used.

[newmanifold]

More rules please!

Any suggestions are welcomed, but we should probably do that in next PRs