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