From c06a76fbd1c1ab339fb91bc63e160f3946affe0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20O=2E=20S=C3=B8rensen?= Date: Thu, 13 Aug 2026 16:17:39 +0200 Subject: [PATCH] fix: Drop the two import keys that wedge dotnet format. dotnet_sort_system_directives_first and dotnet_separate_import_directive_groups are gone from the globalconfig. Not set to false - absent. Their presence at any value arms the organize-imports stage of "dotnet format style", which reports as "error IMPORTS: Fix imports ordering" and sorts flat-alphabetically after System, putting Nota above the vendors. That is the inverse of what UA1000 requires. The result is a loop rather than a wrong file. The style stage reorders, the analyzers stage puts it back, the file on disk never changes - so dotnet format reports nothing to do while dotnet format --verify-no-changes exits 2 forever, and a consuming repository cannot make CI green by any means available to it. The old note here reasoned that the per-vendor grouping was "checked by IDE0055 only, which is off here, so nothing reports it". That is true and beside the point: the imports stage is not a diagnostic, has no severity, and setting dotnet_diagnostic.IDE0055.severity = none does not reach it. It checked what reports, not what enforces. Rider is unaffected, which is why this only ever showed up for whoever formats from the command line. Measured against a real consumer with both of its .editorconfig files, not reasoned about: as shipped, three consecutive runs left the file byte-identical and --verify-no-changes exited 2 every time; with the keys removed, one pass produces the canonical layout and it exits 0. verify.sh asserted SA1516 for the blank line after the System group, and that only worked because the key was set - so removing it broke the suite, which is the suite doing its job. UA1001 makes that check now, and knows first party from vendor where SA1516 only ever saw first-level namespaces. Samples/Unseparated.cs carries both faults so SA1516 stays asserted on member separation, which was always its own job. Co-Authored-By: Claude Opus 5 --- Nota.CodeAnalysis.Verification/README.md | 27 +++++++++++------ .../Samples/Unseparated.cs | 26 +++++++++++++++++ Nota.CodeAnalysis.Verification/verify.sh | 8 +++-- .../content/Nota.CodeAnalysis.globalconfig | 29 ++++++++++++++----- README.md | 9 ++++++ 5 files changed, 80 insertions(+), 19 deletions(-) create mode 100644 Nota.CodeAnalysis.Verification/Samples/Unseparated.cs diff --git a/Nota.CodeAnalysis.Verification/README.md b/Nota.CodeAnalysis.Verification/README.md index 9ecbb11..17d91ba 100644 --- a/Nota.CodeAnalysis.Verification/README.md +++ b/Nota.CodeAnalysis.Verification/README.md @@ -48,15 +48,24 @@ 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 | +`Samples/Broken.cs` and `Samples/Unseparated.cs` break each of these deliberately. + +| Rule | What it catches | Sample | +|-----------|--------------------------------------------------|------------------| +| `IDE0005` | an unused using - what CS8019 never did | `Broken.cs` | +| `IDE0008` | `var` instead of an explicit type | `Broken.cs` | +| `UA1000` | using directives out of order | `Broken.cs` | +| `SA1208` | System usings not placed first | `Broken.cs` | +| `UA1001` | no blank line between using blocks | `Unseparated.cs` | +| `SA1516` | no blank line between members | `Unseparated.cs` | + +`SA1516` used to be the one asserting the blank line after the System group, and that worked only +because `dotnet_separate_import_directive_groups` was set. The key had to go - at *any* value, +including `false`, its presence arms the organize-imports stage of `dotnet format style`, which sorts +first party above the vendors and leaves `dotnet format --verify-no-changes` failing permanently with +nothing a consumer can do about it. `UA1001` makes that check now, and distinguishes first party from +vendor where `SA1516` only ever saw first-level namespaces. `SA1516` stays asserted on member +separation, which was always its own job. ## What `verify-encoding.sh` asserts diff --git a/Nota.CodeAnalysis.Verification/Samples/Unseparated.cs b/Nota.CodeAnalysis.Verification/Samples/Unseparated.cs new file mode 100644 index 0000000..f1c4579 --- /dev/null +++ b/Nota.CodeAnalysis.Verification/Samples/Unseparated.cs @@ -0,0 +1,26 @@ +using System; +using System.Xml; +using Nota.Vendor; + +namespace Nota.Verification; + +/// +/// Two deliberate faults, both asserted by verify.sh. +/// +/// The using directives are in the right order and have no blank line between the blocks, which +/// UA1001 reports. SA1516 used to report that, but only because +/// dotnet_separate_import_directive_groups was set - and setting that key at any value arms the +/// organize-imports stage of "dotnet format style", which sorts first party above the vendors and +/// leaves "dotnet format --verify-no-changes" failing forever. The key is gone; UA1001 makes the +/// check now, and knows first party from vendor where SA1516 only ever saw first-level namespaces. +/// +/// The two members below have no blank line between them either. That is SA1516's own job and +/// nothing to do with usings, so it stays asserted here rather than leaving with the key. They carry +/// no documentation comments on purpose: a comment between them would be reported by SA1514 instead, +/// and SA1600 is off here, so their absence costs nothing. +/// +public static class Unseparated +{ + public static string First => typeof(Thing).Name; + public static string Second => typeof(XmlDocument).Name + Environment.NewLine; +} diff --git a/Nota.CodeAnalysis.Verification/verify.sh b/Nota.CodeAnalysis.Verification/verify.sh index 2f7a395..529f38b 100755 --- a/Nota.CodeAnalysis.Verification/verify.sh +++ b/Nota.CodeAnalysis.Verification/verify.sh @@ -31,12 +31,16 @@ done # 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 +# UA1001 no blank line between using blocks - see Samples/Unseparated.cs. SA1516 used to cover +# this, but only because dotnet_separate_import_directive_groups was set, and that key +# had to go: at any value it arms dotnet format's organize-imports stage, which sorts +# first party above the vendors and leaves --verify-no-changes failing permanently # SA1208 System usings not placed first -# SA1516 no blank line after the System group +# SA1516 no blank line between members - its own job, not the using one it lost # NOTA0001 a source file that is not valid UTF-8, from build/Nota.CodeAnalysis.targets - the one # rule here that is a build error rather than an analyser diagnostic, and so the only # one that cannot be confirmed by reading a severity out of the globalconfig -expected=(IDE0005 IDE0008 UA1000 SA1208 SA1516 NOTA0001) +expected=(IDE0005 IDE0008 UA1000 UA1001 SA1208 SA1516 NOTA0001) # VerifyRules is what pulls Samples/ into the compilation. Without it the project builds empty, which # is what every other build of this solution wants. diff --git a/Nota.CodeAnalysis/content/Nota.CodeAnalysis.globalconfig b/Nota.CodeAnalysis/content/Nota.CodeAnalysis.globalconfig index e604344..8ddd9a8 100644 --- a/Nota.CodeAnalysis/content/Nota.CodeAnalysis.globalconfig +++ b/Nota.CodeAnalysis/content/Nota.CodeAnalysis.globalconfig @@ -7,14 +7,27 @@ 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 +# +# dotnet_sort_system_directives_first and dotnet_separate_import_directive_groups are deliberately +# absent, and must stay absent. Not set to false - absent. Their presence at any value arms the +# organize-imports stage of "dotnet format style", which reports as "error IMPORTS: Fix imports +# ordering" and sorts flat-alphabetically after System. That puts Nota above the vendors, which is +# the inverse of the layout UA1000 enforces below. +# +# The result is a loop rather than a wrong file. The style stage reorders, the analyzers stage puts +# it back, the file on disk never changes - so "dotnet format" reports nothing to do while +# "dotnet format --verify-no-changes" exits 2 forever, and CI cannot be made green. Measured against +# a real consumer, not reasoned about. +# +# This stage is not a diagnostic. It has no ID and no severity, and setting +# dotnet_diagnostic.IDE0055.severity = none does not reach it - which is what the earlier note here +# got wrong: it checked what reports, not what enforces. Rider is unaffected, so this only ever bites +# whoever formats from the command line. +# +# Cost of their absence, measured: SA1516 no longer reports a missing blank line between using +# groups, which dotnet_separate_import_directive_groups = true did give us. UA1001 already requires +# that blank line and knows first party from vendor, where SA1516 only saw first-level namespaces, so +# what goes is the blunter duplicate of a check we still have. ## Using layout - UsingLayoutAnalyser (UA1000, UA1001) # System, then third party, then the consuming solution's own namespaces, as blocks separated by a diff --git a/README.md b/README.md index 7416aa1..280ce01 100644 --- a/README.md +++ b/README.md @@ -67,6 +67,15 @@ Most of this is unsurprising. These are the ones that catch people out: - **`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`. +- **Do not set `dotnet_sort_system_directives_first` or `dotnet_separate_import_directive_groups`.** + Not even to `false`. This package leaves both keys out on purpose, and an `.editorconfig` entry + beats a global analyzer config entry, so putting one back is the one override here that breaks + something. Their presence at any value arms the organize-imports stage of `dotnet format style`, + which sorts flat-alphabetically after System and so puts your namespaces above the vendors - + the inverse of what `UA1000` requires. The two then take turns: the style stage reorders, the + analyzers stage puts it back, the file on disk never changes, and `dotnet format` reports nothing + to do while `dotnet format --verify-no-changes` exits 2 forever. It is not a diagnostic and no + severity setting reaches it. Rider is unaffected, so this only bites the command line and CI. ## Turning things off