From 3871001a46ad2b39a60268d5739037a114c375e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20O=2E=20S=C3=B8rensen?= Date: Sun, 2 Aug 2026 19:25:08 +0200 Subject: [PATCH 01/12] feat: Report unused usings, own the using layout, and prove the rules fire. Three things, all found by running this package's own globalconfig against probe files rather than by reading it. CS8019 never worked. It has asked for unused usings to be reported for as long as it has been here, and a file with an unused using builds clean: CS8019 is emitted hidden by the compiler and a severity in config does not raise it. IDE0005 is the rule that reports; GenerateDocumentationFile, which it needs, was already set in build/Nota.CodeAnalysis.props. One consuming repository had forty-five unused directives behind this. dotnet_separate_import_directive_groups promised more than it delivered. SA1516 uses it for the blank line after the System group and that works; the per-vendor grouping the name suggests is IDE0055's job, and IDE0055 is off, so nothing checked it. Rather than enable IDE0055 - which would also start enforcing indentation and spacing, and reflow every consumer - UsingLayoutAnalyser now owns the layout: System, then third party, then the consuming solution's own namespaces, one run per vendor. SA1210 has to go off for it, because it sorts the whole list alphabetically and wants a solution's own root above a vendor whenever it sorts earlier. SA1208, SA1209, SA1211, SA1216 and SA1217 do not conflict and stay on. first_party_prefixes cannot live here - it differs per solution - so consumers set it in their own .editorconfig, and the scheme degrades sensibly when they do not. Nothing verified any of this, and pull requests did not build. That is how a dead CS8019 line survives: the product is configuration, and configuration fails silently. Nota.CodeAnalysis.Verification is a consumer that breaks five rules on purpose, and verify.sh asserts each one reported. Its samples compile only under -p:VerifyRules=true, because some of these rules are error severity and an ordinary solution build must not fail. Confirmed to work in both directions: switching IDE0005 back off makes verify.sh fail naming it. Pull requests now build and verify; pack and push stay conditioned on main so a pull request cannot publish. Co-Authored-By: Claude Opus 5 --- DevOps/Azure-Pipelines/nuget.yml | 17 +++++- .../Nota.CodeAnalysis.Verification.csproj | 48 ++++++++++++++++ .../Samples/Broken.cs | 17 ++++++ .../Samples/Vendor.cs | 6 ++ Nota.CodeAnalysis.Verification/verify.sh | 56 +++++++++++++++++++ Nota.CodeAnalysis.sln | 26 +++++++++ Nota.CodeAnalysis/Nota.CodeAnalysis.csproj | 3 +- .../content/Nota.CodeAnalysis.globalconfig | 34 ++++++++++- 8 files changed, 204 insertions(+), 3 deletions(-) create mode 100644 Nota.CodeAnalysis.Verification/Nota.CodeAnalysis.Verification.csproj create mode 100644 Nota.CodeAnalysis.Verification/Samples/Broken.cs create mode 100644 Nota.CodeAnalysis.Verification/Samples/Vendor.cs create mode 100755 Nota.CodeAnalysis.Verification/verify.sh diff --git a/DevOps/Azure-Pipelines/nuget.yml b/DevOps/Azure-Pipelines/nuget.yml index a10d020..f1ecf0b 100644 --- a/DevOps/Azure-Pipelines/nuget.yml +++ b/DevOps/Azure-Pipelines/nuget.yml @@ -1,6 +1,11 @@ trigger: - main -pr: none + +# Pull requests build now. They did not, and the only thing this repository ships is configuration - +# so a change that silently stopped a rule from reporting could not be caught by anything before it +# merged. The verification step below is what makes building a pull request worth anything. +pr: + - main variables: - group: github-packages @@ -43,8 +48,17 @@ stages: command: 'build' arguments: '--configuration $(buildConfiguration)' projects: "$(solution)" + # The rules are asserted to actually report, against a project that breaks them on purpose. + # Assumes a bash-capable agent; swap for PowerShell@2 if these are Windows agents. + - task: Bash@3 + displayName: "Verify the rules report" + inputs: + filePath: "Nota.CodeAnalysis.Verification/verify.sh" + - task: DotNetCoreCLI@2 displayName: "Generate Nuget package" + # Only main publishes. Without this a pull request build would push a package. + condition: and(succeeded(), eq(variables['Build.SourceBranch'], 'refs/heads/main')) inputs: command: 'pack' arguments: '--configuration $(buildConfiguration)' @@ -53,6 +67,7 @@ stages: versioningScheme: 'off' - task: DotNetCoreCLI@2 displayName: Publish nuget packages + condition: and(succeeded(), eq(variables['Build.SourceBranch'], 'refs/heads/main')) inputs: command: 'custom' custom: 'nuget' diff --git a/Nota.CodeAnalysis.Verification/Nota.CodeAnalysis.Verification.csproj b/Nota.CodeAnalysis.Verification/Nota.CodeAnalysis.Verification.csproj new file mode 100644 index 0000000..c5e9abf --- /dev/null +++ b/Nota.CodeAnalysis.Verification/Nota.CodeAnalysis.Verification.csproj @@ -0,0 +1,48 @@ + + + + + net8.0 + false + + + false + + + true + true + true + disable + + + + + + + + + + + + + + + + + + + + + + diff --git a/Nota.CodeAnalysis.Verification/Samples/Broken.cs b/Nota.CodeAnalysis.Verification/Samples/Broken.cs new file mode 100644 index 0000000..c811c57 --- /dev/null +++ b/Nota.CodeAnalysis.Verification/Samples/Broken.cs @@ -0,0 +1,17 @@ +using Nota.Vendor; +using System; +using System.Xml; + +namespace Nota.Verification; + +/// Deliberately wrong. Every diagnostic here is asserted by verify.sh. +public static class Broken +{ + /// Uses var, which IDE0008 forbids. + public static string Value() + { + var text = typeof(Thing).Name + Environment.NewLine; + + return text; + } +} diff --git a/Nota.CodeAnalysis.Verification/Samples/Vendor.cs b/Nota.CodeAnalysis.Verification/Samples/Vendor.cs new file mode 100644 index 0000000..b8d93e0 --- /dev/null +++ b/Nota.CodeAnalysis.Verification/Samples/Vendor.cs @@ -0,0 +1,6 @@ +namespace Nota.Vendor; + +/// Stub, so the first using resolves. +public class Thing +{ +} diff --git a/Nota.CodeAnalysis.Verification/verify.sh b/Nota.CodeAnalysis.Verification/verify.sh new file mode 100755 index 0000000..c58c7e2 --- /dev/null +++ b/Nota.CodeAnalysis.Verification/verify.sh @@ -0,0 +1,56 @@ +#!/usr/bin/env bash +# +# Asserts that the rules this package ships actually report. +# +# The product of this repository is configuration, and configuration fails silently. CS8019 sat in +# the globalconfig asking for unused usings to be reported, and reported nothing, for as long as +# anyone can tell - because a package that ships settings has nothing that exercises them. This is +# the thing that would have said so on the first build. +# +# Run from anywhere. Exits non-zero with the missing rules named. + +set -euo pipefail + +here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +props="$here/../Nota.CodeAnalysis/build/Nota.CodeAnalysis.props" + +failures=() + +# 1. The verification project stands in for a consumer, so the properties it sets itself have to be +# the ones the props file gives a consumer. If they drift, this project proves nothing about what +# anyone actually gets. +for property in EnforceCodeStyleInBuild EnableNETAnalyzers GenerateDocumentationFile ImplicitUsings; do + if ! grep -q "<$property>" "$props"; then + failures+=("build/Nota.CodeAnalysis.props no longer sets $property, which this project assumes") + fi +done + +# 2. Broken.cs breaks each of these on purpose. A rule that stops reporting is a rule that has been +# switched off, renamed, or - as with CS8019 - never worked. +# +# IDE0005 unused using directive the one CS8019 was supposed to cover +# IDE0008 var instead of an explicit type +# UA1000 using directives out of order UsingLayoutAnalyser +# SA1208 System usings not placed first +# SA1516 no blank line after the System group +expected=(IDE0005 IDE0008 UA1000 SA1208 SA1516) + +# VerifyRules is what pulls Samples/ into the compilation. Without it the project builds empty, which +# is what every other build of this solution wants. +output="$(dotnet build "$here/Nota.CodeAnalysis.Verification.csproj" \ + --no-incremental -v:m -p:VerifyRules=true -p:TreatWarningsAsErrors=false 2>&1 || true)" + +for rule in "${expected[@]}"; do + if ! grep -qE "(warning|error) $rule[:(]" <<<"$output"; then + failures+=("$rule did not report - it is configured but not reaching consumers") + fi +done + +if [ ${#failures[@]} -ne 0 ]; then + printf 'Verification failed:\n' >&2 + printf ' - %s\n' "${failures[@]}" >&2 + printf '\nFull build output:\n%s\n' "$output" >&2 + exit 1 +fi + +printf 'All %d rules reported.\n' "${#expected[@]}" diff --git a/Nota.CodeAnalysis.sln b/Nota.CodeAnalysis.sln index b323dad..10f67ea 100644 --- a/Nota.CodeAnalysis.sln +++ b/Nota.CodeAnalysis.sln @@ -5,16 +5,42 @@ VisualStudioVersion = 17.8.34525.116 MinimumVisualStudioVersion = 10.0.40219.1 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Nota.CodeAnalysis", "Nota.CodeAnalysis\Nota.CodeAnalysis.csproj", "{C2D84812-9EAA-4DCF-A562-02A26FACE800}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Nota.CodeAnalysis.Verification", "Nota.CodeAnalysis.Verification\Nota.CodeAnalysis.Verification.csproj", "{1053892B-03AC-49E2-8E7F-4E83D05A835D}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU + Debug|x64 = Debug|x64 + Debug|x86 = Debug|x86 Release|Any CPU = Release|Any CPU + Release|x64 = Release|x64 + Release|x86 = Release|x86 EndGlobalSection GlobalSection(ProjectConfigurationPlatforms) = postSolution {C2D84812-9EAA-4DCF-A562-02A26FACE800}.Debug|Any CPU.ActiveCfg = Debug|Any CPU {C2D84812-9EAA-4DCF-A562-02A26FACE800}.Debug|Any CPU.Build.0 = Debug|Any CPU + {C2D84812-9EAA-4DCF-A562-02A26FACE800}.Debug|x64.ActiveCfg = Debug|Any CPU + {C2D84812-9EAA-4DCF-A562-02A26FACE800}.Debug|x64.Build.0 = Debug|Any CPU + {C2D84812-9EAA-4DCF-A562-02A26FACE800}.Debug|x86.ActiveCfg = Debug|Any CPU + {C2D84812-9EAA-4DCF-A562-02A26FACE800}.Debug|x86.Build.0 = Debug|Any CPU {C2D84812-9EAA-4DCF-A562-02A26FACE800}.Release|Any CPU.ActiveCfg = Release|Any CPU {C2D84812-9EAA-4DCF-A562-02A26FACE800}.Release|Any CPU.Build.0 = Release|Any CPU + {C2D84812-9EAA-4DCF-A562-02A26FACE800}.Release|x64.ActiveCfg = Release|Any CPU + {C2D84812-9EAA-4DCF-A562-02A26FACE800}.Release|x64.Build.0 = Release|Any CPU + {C2D84812-9EAA-4DCF-A562-02A26FACE800}.Release|x86.ActiveCfg = Release|Any CPU + {C2D84812-9EAA-4DCF-A562-02A26FACE800}.Release|x86.Build.0 = Release|Any CPU + {1053892B-03AC-49E2-8E7F-4E83D05A835D}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {1053892B-03AC-49E2-8E7F-4E83D05A835D}.Debug|Any CPU.Build.0 = Debug|Any CPU + {1053892B-03AC-49E2-8E7F-4E83D05A835D}.Debug|x64.ActiveCfg = Debug|Any CPU + {1053892B-03AC-49E2-8E7F-4E83D05A835D}.Debug|x64.Build.0 = Debug|Any CPU + {1053892B-03AC-49E2-8E7F-4E83D05A835D}.Debug|x86.ActiveCfg = Debug|Any CPU + {1053892B-03AC-49E2-8E7F-4E83D05A835D}.Debug|x86.Build.0 = Debug|Any CPU + {1053892B-03AC-49E2-8E7F-4E83D05A835D}.Release|Any CPU.ActiveCfg = Release|Any CPU + {1053892B-03AC-49E2-8E7F-4E83D05A835D}.Release|Any CPU.Build.0 = Release|Any CPU + {1053892B-03AC-49E2-8E7F-4E83D05A835D}.Release|x64.ActiveCfg = Release|Any CPU + {1053892B-03AC-49E2-8E7F-4E83D05A835D}.Release|x64.Build.0 = Release|Any CPU + {1053892B-03AC-49E2-8E7F-4E83D05A835D}.Release|x86.ActiveCfg = Release|Any CPU + {1053892B-03AC-49E2-8E7F-4E83D05A835D}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE diff --git a/Nota.CodeAnalysis/Nota.CodeAnalysis.csproj b/Nota.CodeAnalysis/Nota.CodeAnalysis.csproj index 1e2987d..3ac68a8 100644 --- a/Nota.CodeAnalysis/Nota.CodeAnalysis.csproj +++ b/Nota.CodeAnalysis/Nota.CodeAnalysis.csproj @@ -4,7 +4,7 @@ true https://github.com/Notalib/Nota.CodeAnalysis true - 2.1.4 + 2.2.0 content/README.md @@ -12,6 +12,7 @@ + diff --git a/Nota.CodeAnalysis/content/Nota.CodeAnalysis.globalconfig b/Nota.CodeAnalysis/content/Nota.CodeAnalysis.globalconfig index 73656d5..f4506ef 100644 --- a/Nota.CodeAnalysis/content/Nota.CodeAnalysis.globalconfig +++ b/Nota.CodeAnalysis/content/Nota.CodeAnalysis.globalconfig @@ -8,8 +8,30 @@ end_of_line = crlf # Stylecop settings ## Using Directives dotnet_sort_system_directives_first = true + +# Half of this is enforced and half is not, so do not read it as a promise. SA1516 uses it to require +# the blank line after the System group, and that part works. The per-vendor grouping the name +# suggests - Microsoft together, Serilog together - is checked by IDE0055 only, which is off here, so +# nothing reports it. UsingLayoutAnalyser below is what actually enforces the grouping; this stays +# for the System boundary, and because turning it off would silence that too. dotnet_separate_import_directive_groups = true +## Using layout - UsingLayoutAnalyser (UA1000, UA1001) +# System, then third party, then the consuming solution's own namespaces, as blocks separated by a +# blank line, each sorted alphabetically, and one run per vendor inside a block. Neither StyleCop nor +# the built-in options can express that: SA1208/SA1210 know only "System first, then alphabetical", +# and dotnet_separate_import_directive_groups groups by first-level namespace with no notion of whose +# code it is. +# +# first_party_prefixes cannot be set here - it differs per solution. A consuming repository sets it +# in its own .editorconfig: +# +# usinglayout.first_party_prefixes = Contoso +# +# Left unset the scheme degrades to System, then everything else by vendor, which is still stricter +# than anything on offer by default. +usinglayout.separate_roots = true + ## naming rules stylecop.naming.tupleElementNameCasing = camelCase @@ -497,7 +519,12 @@ dotnet_diagnostic.CA5405.severity = warning dotnet_diagnostic.CS0649.severity = warning dotnet_diagnostic.CS1574.severity = warning dotnet_diagnostic.CS1591.severity = none +# Inert, and kept only so nobody sets it again expecting an effect. CS8019 is emitted hidden by the +# compiler and a severity here does not raise it: with this line and nothing else, a file with an +# unused using builds clean. Measured, not assumed. IDE0005 below is the rule that reports, and it +# needs GenerateDocumentationFile, which build/Nota.CodeAnalysis.props already sets. dotnet_diagnostic.CS8019.severity = warning +dotnet_diagnostic.IDE0005.severity = warning dotnet_diagnostic.IDE0008.severity = error dotnet_diagnostic.IDE0016.severity = none dotnet_diagnostic.IDE0016WithoutSuggestion.severity = none @@ -520,7 +547,12 @@ dotnet_diagnostic.SA1127.severity = none dotnet_diagnostic.SA1201.severity = none dotnet_diagnostic.SA1202.severity = none dotnet_diagnostic.SA1204.severity = warning -dotnet_diagnostic.SA1210.severity = warning +# Off, and it cannot be anything else while UA1000 owns the ordering. SA1210 sorts the whole list +# alphabetically end to end, so it wants a solution's own root above a vendor whenever it sorts +# earlier - Contoso above Microsoft - which is exactly what the layout moves to the bottom. Every +# laid-out file would be an SA1210 warning. SA1208, SA1209, SA1211, SA1216 and SA1217 do not conflict +# and stay on. +dotnet_diagnostic.SA1210.severity = none dotnet_diagnostic.SA1309.severity = none dotnet_diagnostic.SA1310.severity = warning dotnet_diagnostic.SA1401.severity = warning From c93c28de7dbf51b6ad6a7640d3da538dd5234408 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20O=2E=20S=C3=B8rensen?= Date: Sun, 2 Aug 2026 20:27:07 +0200 Subject: [PATCH 02/12] fix: Stop requiring a byte order mark, and guard the encoding properly. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The BOM requirement was added to get code building in Docker containers. It never did that. A file without a BOM compiles fine - Roslyn defaults to UTF-8 when no BOM is present - and the whole of the largest solution builds and passes with the marks stripped, on macOS and in the container. Whatever the original problem was, this was not its fix. SA1412 is what required it, and editing charset does not change that: SA1412 does not read charset at all, and demands a BOM whether it says utf-8 or utf-8-bom. Off is the only setting that moves it. charset is set to utf-8 to say what the convention is, and to be correct if this content is ever used as an .editorconfig. But the rule was mislabelled rather than useless, and dropping it alone would have left things worse. It was the only thing standing between the build and a file saved as Windows-1252 - measured: such a file compiles with no warning, and the compiler writes U+FFFD into the assembly. Silent, and it reaches the binary. No analyser can report it, because by the time an analyser runs the text has already been decoded; and it applies to .resx and .json as much as to .cs. So verify-encoding.sh, running in the same pipeline stage. It accepts BOM-marked UTF-16, which is what svcutil and EF migrations emit and what the compiler reads correctly - and those files must keep their BOM, since it is the only record of their encoding. It cannot catch a wrong encoding that happens to produce valid UTF-8, the "“" case, which is indistinguishable from someone writing those characters deliberately. Verified against a tree with spaces in its paths, a UTF-16 .resx, and a Latin-1 file - the clean tree passes and the bad file is caught. That combination is deliberate: an earlier draft passed paths through an unquoted variable, and every file under "Service References" was reported as corrupt because the name has a space in it. Co-Authored-By: Claude Opus 5 --- .gitignore | 3 + DevOps/Azure-Pipelines/nuget.yml | 7 +++ .../Samples/Broken.cs | 2 +- .../Samples/Vendor.cs | 2 +- .../verify-encoding.sh | 56 +++++++++++++++++++ .../content/Nota.CodeAnalysis.globalconfig | 12 +++- 6 files changed, 78 insertions(+), 4 deletions(-) create mode 100755 Nota.CodeAnalysis.Verification/verify-encoding.sh diff --git a/.gitignore b/.gitignore index 63bc327..f5b089d 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,6 @@ .vs obj bin + +# macOS Finder metadata. +.DS_Store diff --git a/DevOps/Azure-Pipelines/nuget.yml b/DevOps/Azure-Pipelines/nuget.yml index f1ecf0b..c62db80 100644 --- a/DevOps/Azure-Pipelines/nuget.yml +++ b/DevOps/Azure-Pipelines/nuget.yml @@ -55,6 +55,13 @@ stages: inputs: filePath: "Nota.CodeAnalysis.Verification/verify.sh" + # The encoding guard that replaces SA1412. Nothing in the compiler covers this: a file saved + # as Windows-1252 builds clean and reaches the assembly as U+FFFD. + - task: Bash@3 + displayName: "Verify source encoding" + inputs: + filePath: "Nota.CodeAnalysis.Verification/verify-encoding.sh" + - task: DotNetCoreCLI@2 displayName: "Generate Nuget package" # Only main publishes. Without this a pull request build would push a package. diff --git a/Nota.CodeAnalysis.Verification/Samples/Broken.cs b/Nota.CodeAnalysis.Verification/Samples/Broken.cs index c811c57..b0b996f 100644 --- a/Nota.CodeAnalysis.Verification/Samples/Broken.cs +++ b/Nota.CodeAnalysis.Verification/Samples/Broken.cs @@ -1,4 +1,4 @@ -using Nota.Vendor; +using Nota.Vendor; using System; using System.Xml; diff --git a/Nota.CodeAnalysis.Verification/Samples/Vendor.cs b/Nota.CodeAnalysis.Verification/Samples/Vendor.cs index b8d93e0..12ece93 100644 --- a/Nota.CodeAnalysis.Verification/Samples/Vendor.cs +++ b/Nota.CodeAnalysis.Verification/Samples/Vendor.cs @@ -1,4 +1,4 @@ -namespace Nota.Vendor; +namespace Nota.Vendor; /// Stub, so the first using resolves. public class Thing diff --git a/Nota.CodeAnalysis.Verification/verify-encoding.sh b/Nota.CodeAnalysis.Verification/verify-encoding.sh new file mode 100755 index 0000000..01f0353 --- /dev/null +++ b/Nota.CodeAnalysis.Verification/verify-encoding.sh @@ -0,0 +1,56 @@ +#!/usr/bin/env sh +# +# Fails if any source file is neither valid UTF-8 nor a BOM-marked UTF-16 file. +# +# This is the guard that has to exist before SA1412 is switched off. SA1412 required a byte order +# mark, which was never the point - but it was, by accident, the only thing standing between the +# build and a file saved as Windows-1252. Measured: such a file compiles with no warning and no +# error, and the compiler writes U+FFFD replacement characters into the assembly. The corruption is +# silent, it reaches the binary, and no analyser reports it, because by the time an analyser runs the +# text has already been decoded. +# +# No analyser can cover this anyway - it is a property of bytes on disk, and it applies to .json and +# .resx as much as to .cs. Hence a script. +# +# UTF-16 is accepted when it carries a BOM. Generated output - svcutil service references, EF +# migrations - is often UTF-16, the compiler reads it correctly from the BOM, and such a file must +# keep that BOM: it is the only thing recording the encoding. +# +# What this cannot catch: a wrong encoding that happens to produce valid UTF-8 - the classic "“" +# mojibake - which is indistinguishable from someone deliberately writing those characters. What it +# does catch is any file containing a byte sequence that is not legal UTF-8, which is every ordinary +# case of an ANSI file with Danish, Cyrillic or CJK text in it. +# +# POSIX sh and BSD-safe, so it runs on a developer's Mac and in a Linux build container alike. +# +# Paths are passed to a child shell by find rather than through a variable and a for loop. That is +# not fussiness: word splitting on an unquoted expansion breaks every path containing a space, and +# reports each half as unreadable - which looks exactly like a corrupt file, in a whole directory of +# them at once, because "Service References" has a space in it. + +set -eu + +root="${1:-.}" + +found="$(find "$root" \ + \( -name '*.cs' -o -name '*.csproj' -o -name '*.json' -o -name '*.resx' -o -name '*.md' -o -name '*.props' -o -name '*.targets' \) \ + -not -path '*/obj/*' -not -path '*/bin/*' -not -path '*/.git/*' \ + -exec sh -c ' + for f do + bom=$(head -c 3 "$f" | xxd -p) + case "$bom" in + fffe*|feff*) ;; + *) iconv -f UTF-8 -t UTF-8 "$f" >/dev/null 2>&1 || printf "%s\n" "$f" ;; + esac + done + ' sh {} +)" + +if [ -n "$found" ]; then + printf 'Not valid UTF-8:\n' >&2 + printf '%s\n' "$found" | sed 's/^/ /' >&2 + printf '\nThese compile without complaint and land in the assembly as U+FFFD.\n' >&2 + printf 'Re-save them as UTF-8; check the tool or editor that last wrote them.\n' >&2 + exit 1 +fi + +printf 'All source files are valid UTF-8 or BOM-marked UTF-16.\n' diff --git a/Nota.CodeAnalysis/content/Nota.CodeAnalysis.globalconfig b/Nota.CodeAnalysis/content/Nota.CodeAnalysis.globalconfig index f4506ef..fb9ae92 100644 --- a/Nota.CodeAnalysis/content/Nota.CodeAnalysis.globalconfig +++ b/Nota.CodeAnalysis/content/Nota.CodeAnalysis.globalconfig @@ -1,5 +1,5 @@ is_global = true -charset = utf-8-bom +charset = utf-8 indent_style = space indent_size = 4 tab_width = 4 @@ -557,7 +557,15 @@ dotnet_diagnostic.SA1309.severity = none dotnet_diagnostic.SA1310.severity = warning dotnet_diagnostic.SA1401.severity = warning dotnet_diagnostic.SA1402.severity = suggestion -dotnet_diagnostic.SA1412.severity = warning +# Off: source is UTF-8 without a BOM. SA1412 is the only thing that was requiring one, and it does +# not read charset at all - it demands a BOM whether charset says utf-8 or utf-8-bom, which is why +# turning it off is the change and editing charset is not. +# +# Nothing enforces the other direction. No analyser reports a BOM that should not be there, and +# charset below reaches no editor: this file arrives through MSBuild as a global analyzer config, +# and editors read .editorconfig from disk. A repository that wants the absence of a BOM enforced +# needs its own .editorconfig, or a check outside the compiler. +dotnet_diagnostic.SA1412.severity = none dotnet_diagnostic.SA1413.severity = warning dotnet_diagnostic.SA1600.severity = none dotnet_diagnostic.SA1601.severity = none From c6bdd292aa72a8443a8da74068f415779f555320 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20O=2E=20S=C3=B8rensen?= Date: Sun, 2 Aug 2026 20:33:53 +0200 Subject: [PATCH 03/12] docs: Explain what the verification project is and is not. It reads like a test project and is not one: nothing here ships, and it exists because a rule that cannot report looks exactly like a rule being obeyed. The CS8019 story is written down, because it is the argument for the whole project and it will otherwise be forgotten the first time someone wonders why a project full of deliberately broken files is in the solution. Also records the two things easiest to get wrong later: watch a check fail before trusting it, and remember this exercises the globalconfig's content rather than the package, so a wrong PackagePath would still ship silently. Co-Authored-By: Claude Opus 5 --- Nota.CodeAnalysis.Verification/README.md | 94 ++++++++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 Nota.CodeAnalysis.Verification/README.md diff --git a/Nota.CodeAnalysis.Verification/README.md b/Nota.CodeAnalysis.Verification/README.md new file mode 100644 index 0000000..b65efae --- /dev/null +++ b/Nota.CodeAnalysis.Verification/README.md @@ -0,0 +1,94 @@ +# Nota.CodeAnalysis.Verification + +Checks that the rules this repository ships actually report. Nothing here is shipped or consumed: +`IsPackable` is false, it is in no package, and nothing depends on it. + +## Why it exists + +The product of this repository is configuration, and configuration fails silently. A rule that is +misspelled, superseded, or simply incapable of reporting looks exactly like a rule that is being +obeyed - the build is quiet either way. + +That is not hypothetical. `dotnet_diagnostic.CS8019.severity = warning` asked for unused usings to be +reported, and reported nothing, for as long as anyone can tell: CS8019 is emitted hidden by the +compiler and a severity in config does not raise it. One consuming solution had forty-five unused +directives behind it. Every build was green. + +So this project is a consumer, built on purpose against files that break the rules, and a script that +fails if any of them stayed quiet. + +## Running it + +```sh +./verify.sh # the rules report +./verify-encoding.sh # source files are valid UTF-8 +``` + +Both run in the pipeline, on pull requests as well as on `main`. Both exit non-zero on failure and +name what went wrong. + +## How it is put together + +`Samples/` is excluded from compilation unless `VerifyRules` is set: + +```xml + + +``` + +Some of the rules are error severity - `IDE0008` is - so compiling the samples would fail any +ordinary build of the solution, including the one the pipeline runs before it gets here. `verify.sh` +passes `-p:VerifyRules=true`; every other build gets an empty assembly. + +The project also declares the same properties `build/Nota.CodeAnalysis.props` gives a consumer, and +`verify.sh` fails if the props file stops setting one of them. Otherwise this project could drift +into testing a configuration nobody actually receives - and `GenerateDocumentationFile` in particular +looks redundant and is not: without it `IDE0005` silently stops reporting. + +## What `verify.sh` asserts + +`Samples/Broken.cs` breaks each of these deliberately. + +| Rule | What it catches | +|-----------|----------------------------------------------| +| `IDE0005` | an unused using - what CS8019 never did | +| `IDE0008` | `var` instead of an explicit type | +| `UA1000` | using directives out of order | +| `SA1208` | System usings not placed first | +| `SA1516` | no blank line after the System group | + +## What `verify-encoding.sh` asserts + +Every source file is valid UTF-8, or UTF-16 carrying a BOM. + +This is the guard that let `SA1412` be switched off. SA1412 demanded a byte order mark, which was +never what anyone wanted, but it was the only thing standing between the build and a file saved as +Windows-1252 - and such a file compiles with no warning at all, putting U+FFFD replacement characters +straight into the assembly. No analyser can report that: by the time an analyser runs, the text has +already been decoded. It is also a property of `.resx` and `.json` files, which no analyser reads. + +BOM-marked UTF-16 is accepted rather than flagged. svcutil and EF migrations emit it, the compiler +reads it correctly, and those files must keep their BOM - it is the only record of their encoding. + +It cannot catch a wrong encoding that happens to produce valid UTF-8, the classic `“` mojibake, +which is indistinguishable from someone writing those characters on purpose. + +## Adding a rule + +Break it in `Samples/Broken.cs`, then add its id to `expected` in `verify.sh`. + +**Watch it fail before you trust it.** Set the rule's severity to `none` in the globalconfig and run +`verify.sh`; it should fail naming that rule. A check nobody has seen fail is not a check - which is +the whole reason this project exists. + +## What it does not cover + +It exercises the globalconfig's *content*, not the *package*. It imports the globalconfig directly +and declares the analyser references itself, rather than installing the built `.nupkg`. So a wrong +`PackagePath`, a renamed `build/Nota.CodeAnalysis.props`, or a props file that never gets imported +would all ship a package that installs cleanly and does nothing - the same failure shape as CS8019, +one level up. Closing that means packing, installing from a local feed into a throwaway project, and +asserting there. + +Both scripts are POSIX `sh` and assume a bash-capable agent (`Bash@3` in the pipeline). On Windows +agents they need porting to PowerShell. From 177c57fd4218afe5c7befa8dc1f2328f65a7a387 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20O=2E=20S=C3=B8rensen?= Date: Sun, 2 Aug 2026 20:41:37 +0200 Subject: [PATCH 04/12] feat: Give consumers an encoding rule, not just this repository. Removing SA1412 left consumers with nothing. verify-encoding.sh guards this repository, but a package cannot ask every consumer to wire up a script - so the net effect of the previous commit, for anyone installing the package, was that the accidental guard went away and no replacement arrived. That is exactly the combination that leaves people worse off, and it shipped. build/Nota.CodeAnalysis.targets now validates every file being compiled, before CoreCompile, and fails with NOTA0001 naming the file. It has to happen there: a file saved as Windows-1252 compiles with no warning and reaches the assembly as U+FFFD, and an analyser cannot see it because by the time an analyser runs the text has been decoded and the bytes are gone. UTF-16 with a BOM passes - svcutil and EF migrations emit it, the compiler reads it correctly, and such a file must keep its BOM. Only @(Compile) is read, so .resx and .json are not covered; reading the whole tree on every build was not worth what it costs. NotaValidateSourceEncoding=false switches it off. Verified as a consumer sees it, not just in this solution: packed, installed from a local feed into a fresh project, clean source builds and one Windows-1252 file fails with NOTA0001. That path matters because it is the one the verification project does not cover - it imports the globalconfig directly, so a broken PackagePath would still ship silently. NOTA0001 is added to verify.sh, which needs a deliberately mis-encoded sample, so Samples/ is now excluded from verify-encoding.sh: those files are wrong on purpose and that check is for files that are wrong by accident. Co-Authored-By: Claude Opus 5 --- .../Nota.CodeAnalysis.Verification.csproj | 7 ++ .../Samples/Latin1Encoded.cs | 8 ++ .../verify-encoding.sh | 5 ++ Nota.CodeAnalysis.Verification/verify.sh | 15 ++-- Nota.CodeAnalysis/Nota.CodeAnalysis.csproj | 1 + .../build/Nota.CodeAnalysis.targets | 86 +++++++++++++++++++ 6 files changed, 116 insertions(+), 6 deletions(-) create mode 100644 Nota.CodeAnalysis.Verification/Samples/Latin1Encoded.cs create mode 100644 Nota.CodeAnalysis/build/Nota.CodeAnalysis.targets diff --git a/Nota.CodeAnalysis.Verification/Nota.CodeAnalysis.Verification.csproj b/Nota.CodeAnalysis.Verification/Nota.CodeAnalysis.Verification.csproj index c5e9abf..d95a6a5 100644 --- a/Nota.CodeAnalysis.Verification/Nota.CodeAnalysis.Verification.csproj +++ b/Nota.CodeAnalysis.Verification/Nota.CodeAnalysis.Verification.csproj @@ -25,6 +25,13 @@ + + + + + + true + + + + + + + + = 2 + && ((bytes[0] == 0xFF && bytes[1] == 0xFE) || (bytes[0] == 0xFE && bytes[1] == 0xFF))) + { + continue; + } + + try + { + new System.Text.UTF8Encoding(false, true).GetString(bytes); + } + catch (System.Text.DecoderFallbackException) + { + Log.LogError( + null, + "NOTA0001", + null, + path, + 1, + 1, + 0, + 0, + "File is not valid UTF-8. It will compile without complaint and reach the assembly as U+FFFD replacement characters. Re-save it as UTF-8, and check the tool or editor that last wrote it."); + } + } + ]]> + + + + + + + + + From 9414e0284330c0ddd86aef622733c9e6e34a3c64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20O=2E=20S=C3=B8rensen?= Date: Sun, 2 Aug 2026 20:50:05 +0200 Subject: [PATCH 05/12] docs: Make the readme usable, and delete the one nobody reads. The readme was a single sentence, and it is not only the repository's front page: the csproj packs it to content/README.md and declares it as PackageReadmeFile, so it is what anyone browsing the package sees. It now says how to install it, what switches on, the one key a consumer has to set themselves, and which rules surprise people. Four things are worth a stranger's attention before their first build, and none were written down anywhere: IDE0008 and VSTHRD100 are errors rather than warnings, NOTA0001 is a build task and not a diagnostic so it has its own opt-out, and usinglayout.first_party_prefixes cannot be shipped by the package because it differs per solution - unset, the using layout quietly stops distinguishing your code from a vendor's. GenerateDocumentationFile gets a line of its own. It looks like an unrelated documentation setting and is the switch that makes IDE0005 report at all; someone will eventually decide it is redundant. Nota.CodeAnalysis/README.md is deleted. It was a near-duplicate of the first line of this one, nothing referenced it, and it was not the file that reaches the package - two readmes saying almost the same thing is how one of them goes stale without anyone noticing. Co-Authored-By: Claude Opus 5 --- .gitignore | 3 ++ Nota.CodeAnalysis/README.md | 1 - README.md | 105 +++++++++++++++++++++++++++++++++++- 3 files changed, 107 insertions(+), 2 deletions(-) delete mode 100644 Nota.CodeAnalysis/README.md diff --git a/.gitignore b/.gitignore index f5b089d..df88999 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,6 @@ bin # macOS Finder metadata. .DS_Store + +# Rider / IntelliJ project metadata. +.idea/ diff --git a/Nota.CodeAnalysis/README.md b/Nota.CodeAnalysis/README.md deleted file mode 100644 index af6fdd9..0000000 --- a/Nota.CodeAnalysis/README.md +++ /dev/null @@ -1 +0,0 @@ -# Opinionated Installable Nuget Package with Nota Codestyle, Stylecop and Code Analysis rules. diff --git a/README.md b/README.md index 8c48bae..6a093da 100644 --- a/README.md +++ b/README.md @@ -1 +1,104 @@ -# Installable Nuget Package with Nota Codestyle, Stylecop and Code Analysis rules. +# Nota.CodeAnalysis + +Nota's code style, StyleCop and code analysis rules, as one package. Install it and a project gets +the same rules as every other project here, enforced at build time rather than agreed in a wiki. + +## Installing + +The package lives on Nota's GitHub Packages feed, so a `NuGet.config` needs the source: + +```xml + +``` + +Then reference it once per project, or once in a `Directory.Build.props` for a whole solution: + +```xml + + all + runtime; build; native; contentfiles; analyzers; buildtransitive + +``` + +`PrivateAssets` matters: without it the rules flow to anything that references your library, and +whether *your* code uses `var` is nobody else's build error. + +## What arrives with it + +Four properties are switched on, in `build/Nota.CodeAnalysis.props`: + +| Property | Why | +|----------|-----| +| `EnforceCodeStyleInBuild` | without it the IDE#### rules never run, whatever their severity says | +| `EnableNETAnalyzers` | the CA#### rules | +| `GenerateDocumentationFile` | load-bearing, and not obviously so: severity is a filter, and this is the switch that produces the diagnostics being filtered. Remove it and `IDE0005` silently stops reporting | +| `ImplicitUsings` | disabled - usings are stated, not inherited | + +Four analyser packages come as dependencies: StyleCop.Analyzers, Microsoft.VisualStudio.Threading.Analyzers, +SerilogAnalyzer, and UsingLayoutAnalyser. Around 400 rule severities are set in +`content/Nota.CodeAnalysis.globalconfig`. + +## What you have to configure yourself + +One key, and only one. UsingLayoutAnalyser sorts usings into System, then third party, then *your* +namespaces - and it cannot know what yours are called. Put this in the consuming repository's +`.editorconfig`: + +```ini +[*.cs] +usinglayout.first_party_prefixes = Nota +``` + +Comma-separated for several roots. Left unset the scheme degrades to System-then-everything-else, +which still works but stops telling your code apart from a vendor's. + +## Rules worth knowing before your first build + +Most of this is unsurprising. These are the ones that catch people out: + +- **`IDE0008` is an error.** `var` is banned outright; write the type. +- **`VSTHRD100` is an error.** No `async void`. +- **`NOTA0001`** fails the build on a source file that is not valid UTF-8. It is an MSBuild task + rather than an analyser because it has to see the bytes: a file saved as Windows-1252 compiles with + no warning at all and reaches the assembly as U+FFFD replacement characters. UTF-16 with a byte + order mark passes, since the compiler reads it correctly - and such a file must keep its BOM, which + is the only record of its encoding. +- **`UA1000` and `UA1001`** enforce the using layout: System, then third party, then yours, as blocks + separated by a blank line, one run per vendor. An existing repository is converted in one pass with + `dotnet format analyzers --diagnostics UA1000 UA1001 --severity warn`. + +## Turning things off + +Any rule can be overridden in the consuming repository's own `.editorconfig`, which takes precedence +over this package's global config: + +```ini +dotnet_diagnostic.IDE0008.severity = suggestion +``` + +The encoding check is a build task rather than a diagnostic, so it has its own switch: + +```xml +false +``` + +## Working on this repository + +The product here is configuration, and configuration fails silently: a rule that cannot report looks +exactly like a rule being obeyed, because the build is quiet either way. `dotnet_diagnostic.CS8019` +asked for unused usings to be reported and reported nothing for years, with forty-five of them +collected behind it in one consuming solution. + +`Nota.CodeAnalysis.Verification` exists to make that noisy. It is a consumer built against files that +break the rules on purpose, and a script that fails if any of them stayed quiet. Read its README +before changing rules. + +```sh +./Nota.CodeAnalysis.Verification/verify.sh # the rules report +./Nota.CodeAnalysis.Verification/verify-encoding.sh # source is valid UTF-8 +``` + +Both run on pull requests as well as on `main`. + +Releases are cut by bumping `` in `Nota.CodeAnalysis/Nota.CodeAnalysis.csproj` and merging +to `main`; the pipeline packs and pushes. Pull request builds verify but never publish. From 4ba109210a34a94a25ce0d8d38ab278fed2bf1ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20O=2E=20S=C3=B8rensen?= Date: Sun, 2 Aug 2026 20:54:09 +0200 Subject: [PATCH 06/12] feat: Default the first-party prefix to Nota. The using layout needs to know which namespaces are the consumer's own, and the previous commit left that as the one thing every repository had to remember. A setting every repository must remember is a setting most repositories will not have - and unset, the layout quietly stops distinguishing Nota code from a vendor's, which is the whole point of it. Nota is right for nearly everything here, so it is the default. A consumer whose code is called something else overrides it in their own .editorconfig, and getting it wrong is not fatal: their namespaces are sorted as one more vendor rather than last. The precedence this relies on was verified rather than assumed - the built package installed into a project whose .editorconfig disagreed with it, and the .editorconfig won. That holds for every severity in the globalconfig too, so the readme now says so as a fact rather than an expectation. The comment above the setting had said this key could not live here. It is now the line directly beneath it. Co-Authored-By: Claude Opus 5 --- .../content/Nota.CodeAnalysis.globalconfig | 17 +++++++++++----- README.md | 20 ++++++++++--------- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/Nota.CodeAnalysis/content/Nota.CodeAnalysis.globalconfig b/Nota.CodeAnalysis/content/Nota.CodeAnalysis.globalconfig index fb9ae92..3e82801 100644 --- a/Nota.CodeAnalysis/content/Nota.CodeAnalysis.globalconfig +++ b/Nota.CodeAnalysis/content/Nota.CodeAnalysis.globalconfig @@ -23,13 +23,20 @@ dotnet_separate_import_directive_groups = true # and dotnet_separate_import_directive_groups groups by first-level namespace with no notion of whose # code it is. # -# first_party_prefixes cannot be set here - it differs per solution. A consuming repository sets it -# in its own .editorconfig: +# Defaulted to Nota, because that is what nearly every consumer's own code is called, and a setting +# every repository has to remember is a setting most repositories will not have. A consumer whose +# code is called something else overrides it in their own .editorconfig: # -# usinglayout.first_party_prefixes = Contoso +# [*.cs] +# usinglayout.first_party_prefixes = Contoso, Fabrikam # -# Left unset the scheme degrades to System, then everything else by vendor, which is still stricter -# than anything on offer by default. +# That does take precedence - verified against the built package, not assumed: an .editorconfig entry +# beats a global analyzer config entry for the same key. The same is true of every severity here, so +# nothing in this file is a decision a consumer is stuck with. +# +# A repository that gets this wrong still gets a sensible layout - its own namespaces are simply +# treated as one more vendor - which is why it is worth defaulting rather than demanding. +usinglayout.first_party_prefixes = Nota usinglayout.separate_roots = true ## naming rules diff --git a/README.md b/README.md index 6a093da..4b64cd3 100644 --- a/README.md +++ b/README.md @@ -38,19 +38,20 @@ Four analyser packages come as dependencies: StyleCop.Analyzers, Microsoft.Visua SerilogAnalyzer, and UsingLayoutAnalyser. Around 400 rule severities are set in `content/Nota.CodeAnalysis.globalconfig`. -## What you have to configure yourself +## Configuring it -One key, and only one. UsingLayoutAnalyser sorts usings into System, then third party, then *your* -namespaces - and it cannot know what yours are called. Put this in the consuming repository's -`.editorconfig`: +Nothing is required. The one setting most likely to need changing is which namespaces count as +*yours*, for the using layout - it defaults to `Nota`, which is right for almost everything here. + +If your code is called something else, say so in your own `.editorconfig`: ```ini [*.cs] -usinglayout.first_party_prefixes = Nota +usinglayout.first_party_prefixes = Contoso, Fabrikam ``` -Comma-separated for several roots. Left unset the scheme degrades to System-then-everything-else, -which still works but stops telling your code apart from a vendor's. +Comma-separated for several roots. Getting it wrong is not fatal - your namespaces are simply sorted +as one more vendor rather than last. ## Rules worth knowing before your first build @@ -69,8 +70,9 @@ Most of this is unsurprising. These are the ones that catch people out: ## Turning things off -Any rule can be overridden in the consuming repository's own `.editorconfig`, which takes precedence -over this package's global config: +Any rule can be overridden in the consuming repository's own `.editorconfig`. An `.editorconfig` +entry beats a global analyzer config entry for the same key, so nothing this package sets is a +decision you are stuck with: ```ini dotnet_diagnostic.IDE0008.severity = suggestion From 0efd4ff457320b1babafdf075a4aefe42a1f828d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20O=2E=20S=C3=B8rensen?= Date: Sun, 2 Aug 2026 20:57:14 +0200 Subject: [PATCH 07/12] build: Build on .NET 10, because the using rules do not load on 8. The pipeline installed SDK 8 and would have packed a release whose using rules did nothing. An analyser cannot reference a newer Roslyn than the compiler running it: UsingLayoutAnalyser is built against 4.14, SDK 8.0.423 runs 4.11, and the compiler answers with CS9057 - a warning, not an error - then carries on without it. Verified on that exact SDK, where StyleCop reported normally and UA1000 and UA1001 were simply absent. Which is this package's own failure mode one level out: a rule that is configured, believed, and incapable of reporting. Nothing about the build looks wrong. The verification project moves to net10.0 for the same reason. On net8.0 it would still pass while proving less, because the analyser it is meant to be verifying could not be loaded at all. Worth noting the guard worked: pinned to SDK 8, verify.sh fails with "UA1000 did not report - it is configured but not reaching consumers". Had this been missed, the pipeline would have gone red rather than shipping. Consumers building on SDK 8 still get CS9057 and no using rules. That is acceptable while everything here is on .NET 10; the durable fix is to build UsingLayoutAnalyser against an older Roslyn, which costs nothing it currently uses. Co-Authored-By: Claude Opus 5 --- DevOps/Azure-Pipelines/nuget.yml | 8 ++++++-- .../Nota.CodeAnalysis.Verification.csproj | 4 +++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/DevOps/Azure-Pipelines/nuget.yml b/DevOps/Azure-Pipelines/nuget.yml index c62db80..cbd281a 100644 --- a/DevOps/Azure-Pipelines/nuget.yml +++ b/DevOps/Azure-Pipelines/nuget.yml @@ -26,10 +26,14 @@ stages: - name: buildConfiguration value: "Release" steps: + # .NET 10, not 8. An analyser cannot reference a newer Roslyn than the compiler running it: + # UsingLayoutAnalyser is built against 4.14, SDK 8 runs 4.11, and the result is CS9057 - a + # warning, not an error - with UA1000 and UA1001 silently absent. Verified on 8.0.423, where + # StyleCop still reported and the using rules did not. - task: UseDotNet@2 - displayName: 'Install .NET Core SDK' + displayName: 'Install .NET SDK' inputs: - version: 8.x + version: 10.x - task: DownloadSecureFile@1 name: "nuget" displayName: "Download NuGet.config from secure storage" diff --git a/Nota.CodeAnalysis.Verification/Nota.CodeAnalysis.Verification.csproj b/Nota.CodeAnalysis.Verification/Nota.CodeAnalysis.Verification.csproj index d95a6a5..c6a3b9f 100644 --- a/Nota.CodeAnalysis.Verification/Nota.CodeAnalysis.Verification.csproj +++ b/Nota.CodeAnalysis.Verification/Nota.CodeAnalysis.Verification.csproj @@ -7,7 +7,9 @@ years asking for unused usings to be reported, and reported nothing. Nothing would have noticed. --> - net8.0 + + net10.0 false From d5270ea94c83c87145a891789f8623483801ebd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20O=2E=20S=C3=B8rensen?= Date: Sun, 2 Aug 2026 21:08:08 +0200 Subject: [PATCH 08/12] build: Replace the Azure pipeline with a GitHub Action. The repository is on GitHub and the packages go to GitHub Packages, so the build was the only part living elsewhere - and it paid for that with a variable group holding a personal access token and a secure NuGet.config downloaded on every run. GITHUB_TOKEN replaces the first. The second turns out to have been unnecessary all along: every dependency here is public, and the solution restores from nuget.org alone, which was checked rather than assumed. It also settles a question the Azure version left open. Those verification steps were Bash@3 tasks against self-hosted agents whose operating system I never established; on ubuntu-latest the question does not arise. Same shape as before: build, verify the rules report, verify the encoding, pack, and publish only from main. Pull requests do everything except publish, and the package is uploaded as an artifact either way so a packaging mistake is visible without merging. SDK 10 rather than 8, for the reason recorded in the previous commit: on 8 the using rules answer CS9057 and then silently do not run. Actions are pinned to current majors. upload-artifact@v4 was written first and replaced with v7 - v4 runs on Node 20, which is the deprecation this pipeline would have started warning about immediately. Co-Authored-By: Claude Opus 5 --- .github/workflows/build-and-publish.yml | 73 +++++++++++++++++++++ .gitignore | 3 + DevOps/Azure-Pipelines/nuget.yml | 85 ------------------------- 3 files changed, 76 insertions(+), 85 deletions(-) create mode 100644 .github/workflows/build-and-publish.yml delete mode 100644 DevOps/Azure-Pipelines/nuget.yml diff --git a/.github/workflows/build-and-publish.yml b/.github/workflows/build-and-publish.yml new file mode 100644 index 0000000..8a55301 --- /dev/null +++ b/.github/workflows/build-and-publish.yml @@ -0,0 +1,73 @@ +# Builds, verifies, and publishes to GitHub Packages. +# +# Verification is the point rather than a formality. What this repository ships is configuration, and +# configuration fails silently: a rule that cannot report looks exactly like a rule being obeyed, +# because the build is quiet either way. Both scripts below exist to make that noisy, and they run on +# pull requests as well as on main, because a rule that stopped reporting should not be discovered +# after it has merged. +name: build and publish + +on: + push: + branches: [main] + pull_request: + branches: [main] + workflow_dispatch: + +# Only main publishes. A pull request builds and verifies and stops there. +permissions: + contents: read + packages: write + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + build: + runs-on: ubuntu-latest + timeout-minutes: 15 + + steps: + - uses: actions/checkout@v7 + + - uses: actions/setup-dotnet@v6 + with: + # 10, not 8. An analyser cannot reference a newer Roslyn than the compiler running it, and + # UsingLayoutAnalyser is the rule set most likely to be affected: on SDK 8 it answers + # CS9057 - a warning - and its rules are then silently absent. This is also what consumers + # build with, and testing on what people use beats testing on the oldest thing that works. + dotnet-version: '10.x' + + # No NuGet.config and no secure file: every dependency is public on nuget.org. The Azure + # pipeline this replaced downloaded a secure NuGet.config it did not need. + - name: Build + run: dotnet build Nota.CodeAnalysis.sln --configuration Release + + - name: Verify the rules report + run: ./Nota.CodeAnalysis.Verification/verify.sh + + - name: Verify source encoding + run: ./Nota.CodeAnalysis.Verification/verify-encoding.sh + + - name: Pack + run: dotnet pack Nota.CodeAnalysis.sln --configuration Release --no-build --output artifacts + + # Kept even on a pull request, so a packaging mistake is visible without merging. + - name: Upload the package + uses: actions/upload-artifact@v7 + with: + name: nupkg + path: artifacts/*.nupkg + + # GITHUB_TOKEN rather than a personal access token: publishing to this repository's own package + # registry needs nothing else, which is the whole reason to be here rather than in Azure. + # --skip-duplicate so a rerun is not an error - the version comes from the csproj, and pushing + # the same one twice is a mistake worth ignoring rather than failing on. + - name: Publish to GitHub Packages + if: github.event_name == 'push' && github.ref == 'refs/heads/main' + run: > + dotnet nuget push artifacts/*.nupkg + --source https://nuget.pkg.github.com/Notalib/index.json + --api-key ${{ secrets.GITHUB_TOKEN }} + --skip-duplicate diff --git a/.gitignore b/.gitignore index df88999..4a453c3 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,6 @@ bin # Rider / IntelliJ project metadata. .idea/ + +# Local pack output. +artifacts/ diff --git a/DevOps/Azure-Pipelines/nuget.yml b/DevOps/Azure-Pipelines/nuget.yml deleted file mode 100644 index cbd281a..0000000 --- a/DevOps/Azure-Pipelines/nuget.yml +++ /dev/null @@ -1,85 +0,0 @@ -trigger: - - main - -# Pull requests build now. They did not, and the only thing this repository ships is configuration - -# so a change that silently stopped a rule from reporting could not be caught by anything before it -# merged. The verification step below is what makes building a pull request worth anything. -pr: - - main - -variables: - - group: github-packages - -pool: - name: "Nota OnPremise Agents" - -stages: - - stage: Build - jobs: - - job: Build - displayName: Build - variables: - - name: solution - value: "Nota.CodeAnalysis.sln" - - name: buildPlatform - value: "Any CPU" - - name: buildConfiguration - value: "Release" - steps: - # .NET 10, not 8. An analyser cannot reference a newer Roslyn than the compiler running it: - # UsingLayoutAnalyser is built against 4.14, SDK 8 runs 4.11, and the result is CS9057 - a - # warning, not an error - with UA1000 and UA1001 silently absent. Verified on 8.0.423, where - # StyleCop still reported and the using rules did not. - - task: UseDotNet@2 - displayName: 'Install .NET SDK' - inputs: - version: 10.x - - task: DownloadSecureFile@1 - name: "nuget" - displayName: "Download NuGet.config from secure storage" - inputs: - secureFile: "NuGet.config" - - task: DotNetCoreCLI@2 - displayName: Nuget restore solution - inputs: - command: "restore" - restoreSolution: "$(solution)" - feedsToUse: "config" - nugetConfigPath: $(nuget.secureFilePath) - - task: DotNetCoreCLI@2 - displayName: 'Build' - inputs: - command: 'build' - arguments: '--configuration $(buildConfiguration)' - projects: "$(solution)" - # The rules are asserted to actually report, against a project that breaks them on purpose. - # Assumes a bash-capable agent; swap for PowerShell@2 if these are Windows agents. - - task: Bash@3 - displayName: "Verify the rules report" - inputs: - filePath: "Nota.CodeAnalysis.Verification/verify.sh" - - # The encoding guard that replaces SA1412. Nothing in the compiler covers this: a file saved - # as Windows-1252 builds clean and reaches the assembly as U+FFFD. - - task: Bash@3 - displayName: "Verify source encoding" - inputs: - filePath: "Nota.CodeAnalysis.Verification/verify-encoding.sh" - - - task: DotNetCoreCLI@2 - displayName: "Generate Nuget package" - # Only main publishes. Without this a pull request build would push a package. - condition: and(succeeded(), eq(variables['Build.SourceBranch'], 'refs/heads/main')) - inputs: - command: 'pack' - arguments: '--configuration $(buildConfiguration)' - packagesToPack: "$(solution)" - nobuild: true - versioningScheme: 'off' - - task: DotNetCoreCLI@2 - displayName: Publish nuget packages - condition: and(succeeded(), eq(variables['Build.SourceBranch'], 'refs/heads/main')) - inputs: - command: 'custom' - custom: 'nuget' - arguments: 'push "$(Build.ArtifactStagingDirectory)/**/*.nupkg" --source https://nuget.pkg.github.com/Notalib/index.json --skip-duplicate --api-key "$(github_token)"' From 5c32b6661950d35a91521733b68486ff466d2535 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20O=2E=20S=C3=B8rensen?= Date: Sun, 2 Aug 2026 21:22:11 +0200 Subject: [PATCH 09/12] fix: Take UsingLayoutAnalyser 0.2.1, and let the threading rules ship again. 0.2.1 is built against Roslyn 4.8 rather than 4.14, so the using rules load on every SDK from .NET 8 onwards. On 0.2.0 an SDK 8 consumer got CS9057 - a warning - and no UA1000 or UA1001 at all. Microsoft.VisualStudio.Threading.Analyzers loses the PrivateAssets that Rider added when its version was bumped. That is the right default for an ordinary project and wrong for this one: PrivateAssets stops a reference reaching consumers, and for an analyser that means the rules never arrive. The globalconfig went on declaring VSTHRD100 an error while no consumer could receive the analyser that reports it - configured, believed, and incapable of firing, which is the failure this package has now produced three times. Verified through the built package rather than the solution, because the solution cannot show it: the verification project references the analysers directly, so VSTHRD100 fired there throughout while consumers got nothing. Packed, installed from a local feed, and an async void method now fails the build with VSTHRD100. The nuspec declares all four analysers again. None of these references may carry PrivateAssets, and the item group now says so, because the next version bump will offer to add it back. Co-Authored-By: Claude Opus 5 --- .../Nota.CodeAnalysis.Verification.csproj | 4 ++-- Nota.CodeAnalysis/Nota.CodeAnalysis.csproj | 14 ++++++++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/Nota.CodeAnalysis.Verification/Nota.CodeAnalysis.Verification.csproj b/Nota.CodeAnalysis.Verification/Nota.CodeAnalysis.Verification.csproj index c6a3b9f..e316aaa 100644 --- a/Nota.CodeAnalysis.Verification/Nota.CodeAnalysis.Verification.csproj +++ b/Nota.CodeAnalysis.Verification/Nota.CodeAnalysis.Verification.csproj @@ -48,10 +48,10 @@ - + - + diff --git a/Nota.CodeAnalysis/Nota.CodeAnalysis.csproj b/Nota.CodeAnalysis/Nota.CodeAnalysis.csproj index f26569b..4ba9bd7 100644 --- a/Nota.CodeAnalysis/Nota.CodeAnalysis.csproj +++ b/Nota.CodeAnalysis/Nota.CodeAnalysis.csproj @@ -8,11 +8,21 @@ content/README.md + - + - + From 4d92e0e7606f19007b7cc05b444843807b35e12e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20O=2E=20S=C3=B8rensen?= Date: Sun, 2 Aug 2026 21:27:51 +0200 Subject: [PATCH 10/12] feat: Verify the package delivers its rules, not just that they are configured. verify.sh runs inside the solution, where the globalconfig is imported directly and the analysers are referenced by the project itself. A rule can pass there while no consumer receives it, and that has now happened twice: UsingLayoutAnalyser built against a newer Roslyn than the SDK running it, skipped with CS9057; and the threading analyser acquiring PrivateAssets in a version bump, so VSTHRD100 went on firing here while consumers got nothing at all. Neither was visible from inside. verify-package.sh packs, installs into a throwaway project from a local feed, and compiles a file breaking one rule per analyser - IDE0008 for the globalconfig and props, NOTA0001 for the targets, SA1208, VSTHRD100, UA1000 and Serilog003 for the four analysers reaching a consumer at all. CS9057 fails it outright, being a warning nothing else would notice. It packs under a throwaway version rather than the real one. That is the whole check, not a detail: NuGet extracts a package once per version into the global cache, so re-packing 2.2.0 and installing 2.2.0 gets whatever was extracted first and the change under test never arrives. Written the obvious way, this script passed cleanly against the exact regression it exists to catch. It was only found by putting PrivateAssets back and watching what happened - which is what the project's own readme tells everyone to do, and which I had not done. Both states are now confirmed: with the regression it fails naming VSTHRD100, without it, all rules survive. Co-Authored-By: Claude Opus 5 --- .github/workflows/build-and-publish.yml | 7 + Nota.CodeAnalysis.Verification/README.md | 39 +++-- .../verify-package.sh | 159 ++++++++++++++++++ 3 files changed, 194 insertions(+), 11 deletions(-) create mode 100755 Nota.CodeAnalysis.Verification/verify-package.sh diff --git a/.github/workflows/build-and-publish.yml b/.github/workflows/build-and-publish.yml index 8a55301..18dfd71 100644 --- a/.github/workflows/build-and-publish.yml +++ b/.github/workflows/build-and-publish.yml @@ -50,6 +50,13 @@ jobs: - name: Verify source encoding run: ./Nota.CodeAnalysis.Verification/verify-encoding.sh + # The two checks above run inside the solution and cannot see whether the package delivers + # anything. This one packs, installs into a throwaway project, and compiles a file that breaks + # a rule from each analyser. It is the only check that fails when a package installs cleanly + # and does nothing, which has happened twice. + - name: Verify the package delivers the rules + run: ./Nota.CodeAnalysis.Verification/verify-package.sh + - name: Pack run: dotnet pack Nota.CodeAnalysis.sln --configuration Release --no-build --output artifacts diff --git a/Nota.CodeAnalysis.Verification/README.md b/Nota.CodeAnalysis.Verification/README.md index b65efae..413c650 100644 --- a/Nota.CodeAnalysis.Verification/README.md +++ b/Nota.CodeAnalysis.Verification/README.md @@ -22,10 +22,11 @@ fails if any of them stayed quiet. ```sh ./verify.sh # the rules report ./verify-encoding.sh # source files are valid UTF-8 +./verify-package.sh # the rules survive being packaged ``` -Both run in the pipeline, on pull requests as well as on `main`. Both exit non-zero on failure and -name what went wrong. +All three run in the pipeline, on pull requests as well as on `main`. All exit non-zero on failure +and name what went wrong. ## How it is put together @@ -81,14 +82,30 @@ Break it in `Samples/Broken.cs`, then add its id to `expected` in `verify.sh`. `verify.sh`; it should fail naming that rule. A check nobody has seen fail is not a check - which is the whole reason this project exists. -## What it does not cover +## What `verify-package.sh` asserts -It exercises the globalconfig's *content*, not the *package*. It imports the globalconfig directly -and declares the analyser references itself, rather than installing the built `.nupkg`. So a wrong -`PackagePath`, a renamed `build/Nota.CodeAnalysis.props`, or a props file that never gets imported -would all ship a package that installs cleanly and does nothing - the same failure shape as CS8019, -one level up. Closing that means packing, installing from a local feed into a throwaway project, and -asserting there. +That the rules survive packaging, which the other two cannot see. They run inside the solution, where +the globalconfig is imported directly and the analysers are referenced by the project itself - so a +rule can be reported here while no consumer receives it. That is not theoretical; it has happened +twice: -Both scripts are POSIX `sh` and assume a bash-capable agent (`Bash@3` in the pipeline). On Windows -agents they need porting to PowerShell. +- **UsingLayoutAnalyser was built against a newer Roslyn than the SDK running it.** The compiler + answered `CS9057`, a warning, and skipped the analyser. Green build, no using rules. +- **The threading analyser acquired `PrivateAssets` during a version bump**, which stops a reference + reaching consumers. `VSTHRD100` went on firing inside this solution while consumers got nothing. + +So it packs, installs into a throwaway project from a local feed, and compiles a file that breaks one +rule per analyser - plus a deliberately mis-encoded file for `NOTA0001`, which proves +`build/Nota.CodeAnalysis.targets` was packed and imported. It fails on `CS9057` too, since that is a +warning nothing else would notice. + +It packs under a throwaway version like `0.0.0-verify.20260802143000`. That is not cosmetic: NuGet +extracts a package once per version into the global cache, so re-packing `2.2.0` and installing +`2.2.0` gets whatever was extracted first, and the change under test never reaches the consumer. This +script passed cleanly against a regression it was written to catch until the version was made unique. + +## What none of them cover + +The scripts are POSIX `sh` and run on `ubuntu-latest`. On a Windows agent they would need porting. + +`verify-package.sh` needs network on a cold cache, for the throwaway project's own dependencies. diff --git a/Nota.CodeAnalysis.Verification/verify-package.sh b/Nota.CodeAnalysis.Verification/verify-package.sh new file mode 100755 index 0000000..f7c0c54 --- /dev/null +++ b/Nota.CodeAnalysis.Verification/verify-package.sh @@ -0,0 +1,159 @@ +#!/usr/bin/env sh +# +# Asserts that the rules survive being packaged. +# +# verify.sh proves the globalconfig's content: it imports that file directly and declares the +# analyser references itself, standing in for a consumer. That is not the same as being one, and the +# difference has now cost this repository twice. +# +# - UsingLayoutAnalyser was built against a Roslyn newer than the SDK running it. The compiler +# answered CS9057, a warning, and skipped the analyser. Green build, no rules. +# - Microsoft.VisualStudio.Threading.Analyzers acquired PrivateAssets during a version bump, which +# stops a reference reaching consumers. VSTHRD100 kept firing inside this solution, because the +# verification project references the analyser directly, while no consumer received it at all. +# +# Neither is visible from inside the solution. Both are obvious the moment something installs the +# package and compiles a file that breaks a rule, which is all this does: pack, install from a local +# feed into a throwaway project, and check each rule reported. +# +# It covers the parts verify.sh cannot reach - PackagePath, the build/ props and targets being +# imported at all, and whether a dependency actually flows - and it is the only check that fails when +# a package installs cleanly and does nothing. +# +# POSIX sh and BSD-safe. Needs network on a cold cache, for the throwaway project's own dependencies. + +set -eu + +here="$(cd "$(dirname "$0")" && pwd)" +root="$(cd "$here/.." && pwd)" + +work="$(mktemp -d)" +cleanup() { rm -rf "$work"; } +trap cleanup EXIT INT TERM + +feed="$work/feed" +app="$work/app" +mkdir -p "$feed" "$app" + +# A version nothing can already have. Without this the check silently passes: NuGet extracts a +# package once per version into the global cache, so re-packing 2.2.0 and installing 2.2.0 gets +# whatever 2.2.0 was extracted first - the change under test never reaches the consumer. That is not +# hypothetical; this script gave a clean pass against a regression it was written to catch, until the +# version was made unique. +version="0.0.0-verify.$(date +%Y%m%d%H%M%S)" + +printf 'Packing %s...\n' "$version" +dotnet pack "$root/Nota.CodeAnalysis.sln" --configuration Release --output "$feed" -p:Version="$version" >"$work/pack.log" 2>&1 || { + printf 'pack failed:\n' >&2 + cat "$work/pack.log" >&2 + exit 1 +} + +package="Nota.CodeAnalysis.$version.nupkg" +[ -f "$feed/$package" ] || { printf 'no package was produced at %s\n' "$feed/$package" >&2; exit 1; } + +printf 'Testing %s as a consumer would install it.\n' "$package" + +cat > "$app/nuget.config" < + + + + + + + +EOF + +cat > "$app/App.csproj" < + + net10.0 + + false + + + + + + + +EOF + +# Every line here breaks something on purpose. What is asserted below is that each rule survived +# packaging - not that the rule works, which verify.sh already covers. +cat > "$app/Broken.cs" <<'EOF' +using Nota.Vendor; +using System; + +using Serilog; + +namespace App; + +/// Breaks one rule per analyser, on purpose. +public static class Broken +{ + /// var is IDE0008, from the globalconfig. + public static string Value() + { + var text = typeof(Thing).Name + Environment.NewLine; + + return text; + } + + /// async void is VSTHRD100, from the threading analyser. + public static async void Fire() + { + await System.Threading.Tasks.Task.Delay(1).ConfigureAwait(false); + } + + /// An unbound property is Serilog003, from SerilogAnalyzer. + public static void LogIt() => Log.Information("Hello {Name}"); +} +EOF + +cat > "$app/Vendor.cs" <<'EOF' +namespace Nota.Vendor; + +/// Stub, so the first using resolves. +public class Thing +{ +} +EOF + +# Windows-1252, for NOTA0001 - which proves build/Nota.CodeAnalysis.targets was packed and imported. +printf 'namespace App;\n\n/// Saved in the wrong encoding on purpose.\npublic static class Mis\n{\n /// Danish text.\n public const string T = "' > "$app/MisEncoded.cs" +printf '\346\370\345' >> "$app/MisEncoded.cs" +printf '";\n}\n' >> "$app/MisEncoded.cs" + +output="$(cd "$app" && dotnet build --no-incremental -v:m 2>&1 || true)" + +# IDE0008 the globalconfig was packed, and the props file turned rule enforcement on +# NOTA0001 build/Nota.CodeAnalysis.targets was packed and imported +# SA1208 StyleCop.Analyzers reached the consumer +# VSTHRD100 Microsoft.VisualStudio.Threading.Analyzers reached the consumer +# UA1000 UsingLayoutAnalyser reached the consumer, and loaded on this Roslyn +# Serilog003 SerilogAnalyzer reached the consumer +expected="IDE0008 NOTA0001 SA1208 VSTHRD100 UA1000 Serilog003" + +missing="" +for rule in $expected; do + if ! printf '%s' "$output" | grep -qE "(warning|error) $rule[:( ]"; then + missing="$missing $rule" + fi +done + +# CS9057 is never acceptable: it means an analyser referenced a newer compiler than the one running, +# and was skipped. It is a warning, so nothing else would fail. +if printf '%s' "$output" | grep -q "CS9057"; then + missing="$missing CS9057-was-reported(an-analyser-was-skipped)" +fi + +if [ -n "$missing" ]; then + printf '\nThe package does not deliver:%s\n' "$missing" >&2 + printf 'It installed without complaint. That is what this check is for.\n\n' >&2 + printf '%s\n' "$output" >&2 + exit 1 +fi + +printf 'All rules survive packaging.\n' From 76d1792d8f1313192667f81a00b5a1bd1d2af26d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20O=2E=20S=C3=B8rensen?= Date: Sun, 2 Aug 2026 21:31:54 +0200 Subject: [PATCH 11/12] build: Publish on a version tag, not on every merge to main. Merging and releasing were the same act, which made the version number a thing someone had to remember to change before merging - and forgetting it published nothing while looking like it had, because --skip-duplicate silently declines a version already in the registry. A v* tag now publishes, and the tag is the version: nothing in the repository can disagree with what shipped. Pushes to main and pull requests still build and run all three verifications; they simply stop before publishing. in the csproj stays as a local default for anyone packing by hand, and is now labelled as one. On a tag it is overridden for both build and pack, so the assembly and the package agree. Both paths were run locally: with no tag the pack is 2.2.0 from the csproj, and with VERSION_ARG set as the workflow sets it, 2.3.0. The unset case matters - an undefined variable in a run step expands to nothing rather than erroring, which is what lets one command serve both. Co-Authored-By: Claude Opus 5 --- .github/workflows/build-and-publish.yml | 21 +++++++++++++++------ Nota.CodeAnalysis/Nota.CodeAnalysis.csproj | 2 ++ README.md | 13 +++++++++++-- 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/.github/workflows/build-and-publish.yml b/.github/workflows/build-and-publish.yml index 18dfd71..087bc42 100644 --- a/.github/workflows/build-and-publish.yml +++ b/.github/workflows/build-and-publish.yml @@ -10,11 +10,13 @@ name: build and publish on: push: branches: [main] + tags: ['v*'] pull_request: branches: [main] workflow_dispatch: -# Only main publishes. A pull request builds and verifies and stops there. +# Only a version tag publishes. Pushes to main and pull requests build and verify and stop there, +# so merging is not the same act as releasing. permissions: contents: read packages: write @@ -39,10 +41,17 @@ jobs: # build with, and testing on what people use beats testing on the oldest thing that works. dotnet-version: '10.x' + # On a tag the tag is the version, so nothing in the repository can disagree with what shipped. + # Off a tag this is unset and expands to nothing, leaving the csproj's own - which is + # then only ever a local default, never the thing that gets published. + - name: Take the version from the tag + if: github.ref_type == 'tag' + run: echo "VERSION_ARG=-p:Version=${GITHUB_REF_NAME#v}" >> "$GITHUB_ENV" + # No NuGet.config and no secure file: every dependency is public on nuget.org. The Azure # pipeline this replaced downloaded a secure NuGet.config it did not need. - name: Build - run: dotnet build Nota.CodeAnalysis.sln --configuration Release + run: dotnet build Nota.CodeAnalysis.sln --configuration Release $VERSION_ARG - name: Verify the rules report run: ./Nota.CodeAnalysis.Verification/verify.sh @@ -58,7 +67,7 @@ jobs: run: ./Nota.CodeAnalysis.Verification/verify-package.sh - name: Pack - run: dotnet pack Nota.CodeAnalysis.sln --configuration Release --no-build --output artifacts + run: dotnet pack Nota.CodeAnalysis.sln --configuration Release --no-build --output artifacts $VERSION_ARG # Kept even on a pull request, so a packaging mistake is visible without merging. - name: Upload the package @@ -69,10 +78,10 @@ jobs: # GITHUB_TOKEN rather than a personal access token: publishing to this repository's own package # registry needs nothing else, which is the whole reason to be here rather than in Azure. - # --skip-duplicate so a rerun is not an error - the version comes from the csproj, and pushing - # the same one twice is a mistake worth ignoring rather than failing on. + # --skip-duplicate so re-running a failed job is not itself an error; the registry keeps a + # version forever, and pushing the same one twice is worth ignoring rather than failing on. - name: Publish to GitHub Packages - if: github.event_name == 'push' && github.ref == 'refs/heads/main' + if: github.ref_type == 'tag' run: > dotnet nuget push artifacts/*.nupkg --source https://nuget.pkg.github.com/Notalib/index.json diff --git a/Nota.CodeAnalysis/Nota.CodeAnalysis.csproj b/Nota.CodeAnalysis/Nota.CodeAnalysis.csproj index 4ba9bd7..dc2c206 100644 --- a/Nota.CodeAnalysis/Nota.CodeAnalysis.csproj +++ b/Nota.CodeAnalysis/Nota.CodeAnalysis.csproj @@ -4,6 +4,8 @@ true https://github.com/Notalib/Nota.CodeAnalysis true + 2.2.0 content/README.md diff --git a/README.md b/README.md index 4b64cd3..93571e4 100644 --- a/README.md +++ b/README.md @@ -102,5 +102,14 @@ before changing rules. Both run on pull requests as well as on `main`. -Releases are cut by bumping `` in `Nota.CodeAnalysis/Nota.CodeAnalysis.csproj` and merging -to `main`; the pipeline packs and pushes. Pull request builds verify but never publish. +Releases are cut by tagging: + +```sh +git tag -a v2.3.0 -m "Nota.CodeAnalysis 2.3.0" && git push origin v2.3.0 +``` + +The tag is the version, so nothing in the repository can disagree with what shipped. `` in +the csproj is only a local default, for anyone packing by hand. + +Merging to `main` builds and verifies but does not publish, and neither do pull requests - releasing +is a separate act from merging. From 021a9a06a518424a5e4c23b659d0ee415bfaf813 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20O=2E=20S=C3=B8rensen?= Date: Sun, 2 Aug 2026 21:44:51 +0200 Subject: [PATCH 12/12] docs: Say when the two packaging failures actually happened. Both were described as things that had cost this repository, which reads as shipped bugs found in the package. They were not. UsingLayoutAnalyser is added by this branch and its CS9057 problem never existed anywhere else; the threading analyser's PrivateAssets was added by an editor during a version bump on this same branch. Neither reached a consumer. CS8019 is the one that was genuinely there for years, and the distinction matters to anyone reading this later to judge how much the checks are worth: two of the three failures were created and caught inside an afternoon, which is a better argument for them than a history of shipped mistakes would have been, and it happens to be the true one. Co-Authored-By: Claude Opus 5 --- Nota.CodeAnalysis.Verification/README.md | 4 ++-- Nota.CodeAnalysis.Verification/verify-package.sh | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Nota.CodeAnalysis.Verification/README.md b/Nota.CodeAnalysis.Verification/README.md index 413c650..9ecbb11 100644 --- a/Nota.CodeAnalysis.Verification/README.md +++ b/Nota.CodeAnalysis.Verification/README.md @@ -86,8 +86,8 @@ the whole reason this project exists. That the rules survive packaging, which the other two cannot see. They run inside the solution, where the globalconfig is imported directly and the analysers are referenced by the project itself - so a -rule can be reported here while no consumer receives it. That is not theoretical; it has happened -twice: +rule can be reported here while no consumer receives it. That is not theoretical - it happened twice +while this project was being written, both times caught before merging: - **UsingLayoutAnalyser was built against a newer Roslyn than the SDK running it.** The compiler answered `CS9057`, a warning, and skipped the analyser. Green build, no using rules. diff --git a/Nota.CodeAnalysis.Verification/verify-package.sh b/Nota.CodeAnalysis.Verification/verify-package.sh index f7c0c54..791be3a 100755 --- a/Nota.CodeAnalysis.Verification/verify-package.sh +++ b/Nota.CodeAnalysis.Verification/verify-package.sh @@ -4,7 +4,7 @@ # # verify.sh proves the globalconfig's content: it imports that file directly and declares the # analyser references itself, standing in for a consumer. That is not the same as being one, and the -# difference has now cost this repository twice. +# difference has already mattered twice, both times in changes that had not yet merged. # # - UsingLayoutAnalyser was built against a Roslyn newer than the SDK running it. The compiler # answered CS9057, a warning, and skipped the analyser. Green build, no rules.