Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 18 additions & 9 deletions Nota.CodeAnalysis.Verification/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
26 changes: 26 additions & 0 deletions Nota.CodeAnalysis.Verification/Samples/Unseparated.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
using System;
using System.Xml;
using Nota.Vendor;

namespace Nota.Verification;

/// <summary>
/// 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.
/// </summary>
public static class Unseparated
{
public static string First => typeof(Thing).Name;
public static string Second => typeof(XmlDocument).Name + Environment.NewLine;
}
8 changes: 6 additions & 2 deletions Nota.CodeAnalysis.Verification/verify.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
29 changes: 21 additions & 8 deletions Nota.CodeAnalysis/content/Nota.CodeAnalysis.globalconfig
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down