From 0a3521f00710b1a916c013b9c33bf032f8c11350 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20O=2E=20S=C3=B8rensen?= Date: Wed, 7 Oct 2026 19:23:00 +0200 Subject: [PATCH] feat!: Report operators that lead a wrapped line (NOTA0003). A new analyser, a code fix that moves only the operator, and their unit tests. Both assemblies ship inside the package under analyzers/dotnet/cs, built against Roslyn 4.8 so SDK 8 consumers do not skip them with CS9057. The convention flips to match: dotnet_style_operator_placement_when_wrapping is end_of_line, and Rider's three csharp_wrap_before_* keys are set the same way. IDE0024 is also raised to a warning, so operator overloads get block bodies. Breaking: existing leading operators and expression-bodied operators become warnings. dotnet format converts both; the README's upgrade section has the commands. verify.sh and verify-package.sh now assert NOTA0003 reports, and that nothing reports the trailing form the fix produces. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/build-and-publish.yml | 6 + ...ta.CodeAnalysis.Analysers.CodeFixes.csproj | 28 ++ .../OperatorPlacementCodeFix.cs | 67 +++++ .../Nota.CodeAnalysis.Analysers.Test.csproj | 30 +++ .../OperatorPlacementAnalyserTests.cs | 128 +++++++++ .../OperatorPlacementTests.cs | 250 ++++++++++++++++++ .../AnalyzerReleases.Shipped.md | 3 + .../AnalyzerReleases.Unshipped.md | 8 + .../LeadingOperator.cs | 95 +++++++ .../Nota.CodeAnalysis.Analysers.csproj | 34 +++ .../OperatorPlacement.cs | 116 ++++++++ .../OperatorPlacementAnalyser.cs | 55 ++++ .../Nota.CodeAnalysis.Verification.csproj | 6 + Nota.CodeAnalysis.Verification/README.md | 33 ++- .../Samples/LeadingOperator.cs | 31 +++ .../verify-package.sh | 10 +- Nota.CodeAnalysis.Verification/verify.sh | 15 +- Nota.CodeAnalysis.sln | 42 +++ Nota.CodeAnalysis/Nota.CodeAnalysis.csproj | 26 ++ .../content/Nota.CodeAnalysis.globalconfig | 18 +- README.md | 44 ++- 21 files changed, 1029 insertions(+), 16 deletions(-) create mode 100644 Nota.CodeAnalysis.Analysers.CodeFixes/Nota.CodeAnalysis.Analysers.CodeFixes.csproj create mode 100644 Nota.CodeAnalysis.Analysers.CodeFixes/OperatorPlacementCodeFix.cs create mode 100644 Nota.CodeAnalysis.Analysers.Test/Nota.CodeAnalysis.Analysers.Test.csproj create mode 100644 Nota.CodeAnalysis.Analysers.Test/OperatorPlacementAnalyserTests.cs create mode 100644 Nota.CodeAnalysis.Analysers.Test/OperatorPlacementTests.cs create mode 100644 Nota.CodeAnalysis.Analysers/AnalyzerReleases.Shipped.md create mode 100644 Nota.CodeAnalysis.Analysers/AnalyzerReleases.Unshipped.md create mode 100644 Nota.CodeAnalysis.Analysers/LeadingOperator.cs create mode 100644 Nota.CodeAnalysis.Analysers/Nota.CodeAnalysis.Analysers.csproj create mode 100644 Nota.CodeAnalysis.Analysers/OperatorPlacement.cs create mode 100644 Nota.CodeAnalysis.Analysers/OperatorPlacementAnalyser.cs create mode 100644 Nota.CodeAnalysis.Verification/Samples/LeadingOperator.cs diff --git a/.github/workflows/build-and-publish.yml b/.github/workflows/build-and-publish.yml index 8a8c687..21922e7 100644 --- a/.github/workflows/build-and-publish.yml +++ b/.github/workflows/build-and-publish.yml @@ -54,6 +54,12 @@ jobs: - name: Build run: dotnet build Nota.CodeAnalysis.sln --configuration Release + # Unit tests for this repository's own analysers (NOTA0003): what they report, and that the + # code fix moves operators without changing anything but whitespace. The scripts below prove + # the rule reaches a consumer; these prove it is right when it gets there. + - name: Test the analysers + run: dotnet test Nota.CodeAnalysis.sln --configuration Release --no-build + - name: Verify the rules report run: ./Nota.CodeAnalysis.Verification/verify.sh diff --git a/Nota.CodeAnalysis.Analysers.CodeFixes/Nota.CodeAnalysis.Analysers.CodeFixes.csproj b/Nota.CodeAnalysis.Analysers.CodeFixes/Nota.CodeAnalysis.Analysers.CodeFixes.csproj new file mode 100644 index 0000000..1956d89 --- /dev/null +++ b/Nota.CodeAnalysis.Analysers.CodeFixes/Nota.CodeAnalysis.Analysers.CodeFixes.csproj @@ -0,0 +1,28 @@ + + + + + netstandard2.0 + latest + enable + false + true + true + + + + + + + + + + + + diff --git a/Nota.CodeAnalysis.Analysers.CodeFixes/OperatorPlacementCodeFix.cs b/Nota.CodeAnalysis.Analysers.CodeFixes/OperatorPlacementCodeFix.cs new file mode 100644 index 0000000..6f22e7f --- /dev/null +++ b/Nota.CodeAnalysis.Analysers.CodeFixes/OperatorPlacementCodeFix.cs @@ -0,0 +1,67 @@ +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Composition; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.Text; + +namespace Nota.CodeAnalysis.Analysers.CodeFixes; + +/// +/// Fixes NOTA0003 by moving the operator to the end of the line before it. +/// +[ExportCodeFixProvider(Microsoft.CodeAnalysis.LanguageNames.CSharp, Name = nameof(OperatorPlacementCodeFix))] +[Shared] +public sealed class OperatorPlacementCodeFix : CodeFixProvider +{ + private const string Title = "Move the operator to the end of the line before"; + + /// + public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(OperatorPlacementAnalyser.DiagnosticId); + + /// + public override FixAllProvider GetFixAllProvider() + { + return FixAllProvider.Create(FixAllAsync); + } + + /// + public override Task RegisterCodeFixesAsync(CodeFixContext context) + { + foreach (Diagnostic diagnostic in context.Diagnostics) + { + context.RegisterCodeFix( + CodeAction.Create(Title, + cancellationToken => FixAsync(context.Document, [diagnostic], cancellationToken), + nameof(OperatorPlacementCodeFix)), + diagnostic); + } + + return Task.CompletedTask; + } + + private static async Task FixAllAsync(FixAllContext context, Document document, ImmutableArray diagnostics) + { + return await FixAsync(document, diagnostics, context.CancellationToken).ConfigureAwait(false); + } + + private static async Task FixAsync(Document document, IEnumerable diagnostics, CancellationToken cancellationToken) + { + SyntaxNode? root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + SourceText text = await document.GetTextAsync(cancellationToken).ConfigureAwait(false); + + if (root is null) + { + return document; + } + + HashSet positions = [.. diagnostics.Select(d => d.Location.SourceSpan.Start)]; + + return document.WithText(OperatorPlacement.Apply(root, text, positions)); + } +} diff --git a/Nota.CodeAnalysis.Analysers.Test/Nota.CodeAnalysis.Analysers.Test.csproj b/Nota.CodeAnalysis.Analysers.Test/Nota.CodeAnalysis.Analysers.Test.csproj new file mode 100644 index 0000000..d36f018 --- /dev/null +++ b/Nota.CodeAnalysis.Analysers.Test/Nota.CodeAnalysis.Analysers.Test.csproj @@ -0,0 +1,30 @@ + + + + net10.0 + enable + false + Exe + + + + + + + + + all + runtime; build; native; contentfiles; analyzers; buildtransitive + + + + + + + + + + + + + diff --git a/Nota.CodeAnalysis.Analysers.Test/OperatorPlacementAnalyserTests.cs b/Nota.CodeAnalysis.Analysers.Test/OperatorPlacementAnalyserTests.cs new file mode 100644 index 0000000..7534174 --- /dev/null +++ b/Nota.CodeAnalysis.Analysers.Test/OperatorPlacementAnalyserTests.cs @@ -0,0 +1,128 @@ +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; +using System.Threading.Tasks; + +using AwesomeAssertions; + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Nota.CodeAnalysis.Analysers.Test; + +public class OperatorPlacementAnalyserTests +{ + /// + /// Runs NOTA0003 over , placed inside a method, and returns the operators it + /// reported. + /// + internal static async Task> ReportedOperatorsAsync(string body) + { + IReadOnlyList diagnostics = await DiagnosticsAsync(Wrap(body)); + + return diagnostics.Select(d => d.Location.SourceTree!.GetText().ToString(d.Location.SourceSpan)).ToList(); + } + + internal static async Task> DiagnosticsAsync(string source) + { + SyntaxTree tree = CSharpSyntaxTree.ParseText(source, path: "Test.cs"); + CSharpCompilation compilation = CSharpCompilation.Create("Test", [tree]); + ImmutableArray diagnostics = await compilation.WithAnalyzers([new OperatorPlacementAnalyser()]) + .GetAnalyzerDiagnosticsAsync(TestContext.Current.CancellationToken); + + return diagnostics.Where(d => d.Id == OperatorPlacementAnalyser.DiagnosticId).ToList(); + } + + internal static string Wrap(string body) + { + return "class C\n{\n object M(bool a, bool b, int x, int y, string? s, object o)\n {\n" + body + "\n }\n}\n"; + } + + [Theory] + [InlineData("return a\n && b;", "&&")] + [InlineData("return a\n || b;", "||")] + [InlineData("return x\n + y;", "+")] + [InlineData("return x\n - y;", "-")] + [InlineData("return x\n * y;", "*")] + [InlineData("return x\n | y;", "|")] + [InlineData("return x\n != y;", "!=")] + [InlineData("return x\n < y;", "<")] + [InlineData("return s\n ?? \"\";", "??")] + [InlineData("return o\n as string;", "as")] + [InlineData("return o\n is int;", "is")] + [InlineData("return o\n is int i;", "is")] + [InlineData("return o is 1\n or 2;", "or")] + [InlineData("return o is not null\n and not 2;", "and")] + public async Task ReportsAnOperatorThatLeadsItsLine(string body, string op) + { + IReadOnlyList reported = await ReportedOperatorsAsync(body); + + reported.Should().Equal(op); + } + + [Fact] + public async Task ReportsBothHalvesOfAConditional() + { + IReadOnlyList reported = await ReportedOperatorsAsync("return a\n ? x\n : y;"); + + reported.Should().Equal("?", ":"); + } + + [Fact] + public async Task ReportsEachOperatorOfAChain() + { + IReadOnlyList reported = await ReportedOperatorsAsync("return a\n && b\n && a\n || b;"); + + reported.Should().HaveCount(3); + } + + [Theory] + [InlineData("return a &&\n b;")] + [InlineData("return a ?\n x :\n y;")] + [InlineData("return o is 1 or\n 2;")] + [InlineData("return a && b;")] + [InlineData("return a ? x : y;")] + [InlineData("return a\n .ToString();")] + [InlineData("System.Func f = ()\n => 1;\nreturn f;")] + [InlineData("return @\"x\n && y\";")] + [InlineData("// a\n// && b\nreturn a;")] + public async Task LeavesEverythingElse(string body) + { + IReadOnlyList reported = await ReportedOperatorsAsync(body); + + reported.Should().BeEmpty(); + } + + /// SA1128 wants a constructor initializer on its own line, and this must not argue. + [Fact] + public async Task LeavesAConstructorInitializerAndABaseList() + { + const string source = "class B\n{\n public B(int x)\n {\n }\n}\n\nclass D\n : B\n{\n public D()\n : base(1)\n {\n }\n}\n"; + + IReadOnlyList diagnostics = await DiagnosticsAsync(source); + + diagnostics.Should().BeEmpty(); + } + + [Fact] + public async Task LeavesGeneratedCode() + { + const string source = "// \nclass C\n{\n bool M(bool a, bool b)\n {\n return a\n && b;\n }\n}\n"; + + IReadOnlyList diagnostics = await DiagnosticsAsync(source); + + diagnostics.Should().BeEmpty(); + } + + [Fact] + public async Task ReportsAtTheOperator() + { + IReadOnlyList diagnostics = await DiagnosticsAsync(Wrap("return a\n && b;")); + + Diagnostic diagnostic = diagnostics.Should().ContainSingle().Subject; + diagnostic.Location.GetLineSpan().StartLinePosition.Should().Be(new Microsoft.CodeAnalysis.Text.LinePosition(5, 4)); + diagnostic.GetMessage(System.Globalization.CultureInfo.InvariantCulture).Should().StartWith("'&&' leads its line"); + diagnostic.Severity.Should().Be(DiagnosticSeverity.Warning); + } +} diff --git a/Nota.CodeAnalysis.Analysers.Test/OperatorPlacementTests.cs b/Nota.CodeAnalysis.Analysers.Test/OperatorPlacementTests.cs new file mode 100644 index 0000000..784b0c4 --- /dev/null +++ b/Nota.CodeAnalysis.Analysers.Test/OperatorPlacementTests.cs @@ -0,0 +1,250 @@ +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; +using System.Threading.Tasks; + +using AwesomeAssertions; + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Text; + +using Nota.CodeAnalysis.Analysers.CodeFixes; + +namespace Nota.CodeAnalysis.Analysers.Test; + +/// +/// Moving an operator: where it goes, where the operand it leaves behind ends up, and that nothing but +/// whitespace changes. +/// +public class OperatorPlacementTests +{ + private static string Rewrite(string source) + { + SyntaxTree tree = CSharpSyntaxTree.ParseText(source); + SourceText rewritten = OperatorPlacement.Apply(tree.GetRoot(), tree.GetText(), positions: null); + SyntaxTree after = CSharpSyntaxTree.ParseText(rewritten); + + SyntaxFactory.AreEquivalent(tree.GetRoot(), after.GetRoot(), topLevel: false) + .Should().BeTrue("moving an operator changes whitespace and nothing else"); + + return rewritten.ToString(); + } + + [Fact] + public void AnOperatorAlignedUnderTheFirstOperandLeavesTheOperandThere() + { + const string before = """ + if (a + && b + && c) + """; + const string after = """ + if (a && + b && + c) + """; + + Rewrite(before).Should().Be(after); + } + + [Fact] + public void AnOperatorHungInTheMarginLeavesTheOperandWhereItIs() + { + const string before = """ + x = "one" + + "two" + + "three"; + """; + const string after = """ + x = "one" + + "two" + + "three"; + """; + + Rewrite(before).Should().Be(after); + } + + [Fact] + public void AConditionalIndentedUnderItsStatementKeepsTheIndent() + { + const string before = """ + var x = a + ? b + : c; + """; + const string after = """ + var x = a ? + b : + c; + """; + + Rewrite(before).Should().Be(after); + } + + [Fact] + public void APatternCombinatorMoves() + { + const string before = """ + bool r = s is A + or B + or C; + """; + const string after = """ + bool r = s is A or + B or + C; + """; + + Rewrite(before).Should().Be(after); + } + + [Fact] + public void TheOperatorGoesBeforeACommentEndingTheLine() + { + const string before = """ + bool r = a // why a + && b; + """; + const string after = """ + bool r = a && // why a + b; + """; + + Rewrite(before).Should().Be(after); + } + + [Fact] + public void ACommentLineBetweenStaysWhereItIs() + { + const string before = """ + bool r = a + // why b + && b; + """; + const string after = """ + bool r = a && + // why b + b; + """; + + Rewrite(before).Should().Be(after); + } + + [Fact] + public void AnOperatorAloneOnItsLineTakesTheLineWithIt() + { + const string before = """ + bool r = a + && + b; + """; + const string after = """ + bool r = a && + b; + """; + + Rewrite(before).Should().Be(after); + } + + [Fact] + public void NestedOperatorsAllMove() + { + const string before = """ + var x = (a + || b) + ? p + + q + : r; + """; + const string after = """ + var x = (a || + b) ? + p + + q : + r; + """; + + Rewrite(before).Should().Be(after); + } + + [Fact] + public void AnOperatorBehindADirectiveIsLeftForAPerson() + { + const string before = """ + bool r = a + #if true + && b + #endif + ; + """; + + Rewrite(before).Should().Be(before); + } + + [Fact] + public void AnOperatorFollowedByACommentIsLeftForAPerson() + { + const string before = """ + bool r = a + && /* why */ b; + """; + + Rewrite(before).Should().Be(before); + } + + [Fact] + public void OnlyTheRequestedOperatorsMove() + { + const string before = """ + bool r = a + && b; + bool s = c + && d; + """; + SyntaxTree tree = CSharpSyntaxTree.ParseText(before, cancellationToken: TestContext.Current.CancellationToken); + int second = before.LastIndexOf("&&", System.StringComparison.Ordinal); + + string rewritten = OperatorPlacement.Apply(tree.GetRoot(TestContext.Current.CancellationToken), + tree.GetText(TestContext.Current.CancellationToken), + new HashSet { second }).ToString(); + + rewritten.Should().Be(""" + bool r = a + && b; + bool s = c && + d; + """); + } + + /// + /// The fix end to end, through the provider Rider calls: the diagnostic the analyser raises is one + /// the fix accepts, and once applied the analyser has nothing left to say. + /// + [Fact] + public async Task TheCodeFixClearsTheDiagnostic() + { + string source = OperatorPlacementAnalyserTests.Wrap("return a\n && b\n && a;"); + using AdhocWorkspace workspace = new(); + Document document = workspace.AddProject("Test", LanguageNames.CSharp).AddDocument("Test.cs", source); + IReadOnlyList diagnostics = await OperatorPlacementAnalyserTests.DiagnosticsAsync(source); + OperatorPlacementCodeFix fix = new(); + List actions = []; + + Diagnostic first = diagnostics.OrderBy(d => d.Location.SourceSpan.Start).First(); + + CodeFixContext context = new(document, first, (action, _) => actions.Add(action), TestContext.Current.CancellationToken); + await fix.RegisterCodeFixesAsync(context); + ImmutableArray operations = await actions.Should().ContainSingle().Subject + .GetOperationsAsync(TestContext.Current.CancellationToken); + Solution fixedSolution = operations.OfType().Single().ChangedSolution; + SourceText fixedText = await fixedSolution.GetDocument(document.Id)!.GetTextAsync(TestContext.Current.CancellationToken); + + fixedText.ToString().Should().Contain("return a &&\n b\n && a;", + "only the operator the diagnostic names moves"); + fix.FixableDiagnosticIds.Should().Equal(OperatorPlacementAnalyser.DiagnosticId); + fix.GetFixAllProvider().Should().NotBeNull(); + } +} diff --git a/Nota.CodeAnalysis.Analysers/AnalyzerReleases.Shipped.md b/Nota.CodeAnalysis.Analysers/AnalyzerReleases.Shipped.md new file mode 100644 index 0000000..60c1edf --- /dev/null +++ b/Nota.CodeAnalysis.Analysers/AnalyzerReleases.Shipped.md @@ -0,0 +1,3 @@ +; Shipped analyzer releases +; https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md + diff --git a/Nota.CodeAnalysis.Analysers/AnalyzerReleases.Unshipped.md b/Nota.CodeAnalysis.Analysers/AnalyzerReleases.Unshipped.md new file mode 100644 index 0000000..3c686bb --- /dev/null +++ b/Nota.CodeAnalysis.Analysers/AnalyzerReleases.Unshipped.md @@ -0,0 +1,8 @@ +; Unshipped analyzer release +; https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md + +### New Rules + +Rule ID | Category | Severity | Notes +--------|----------|----------|------- +NOTA0003 | Layout | Warning | OperatorPlacementAnalyser diff --git a/Nota.CodeAnalysis.Analysers/LeadingOperator.cs b/Nota.CodeAnalysis.Analysers/LeadingOperator.cs new file mode 100644 index 0000000..259c1d6 --- /dev/null +++ b/Nota.CodeAnalysis.Analysers/LeadingOperator.cs @@ -0,0 +1,95 @@ +using System.Collections.Generic; + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Text; + +namespace Nota.CodeAnalysis.Analysers; + +/// +/// An operator that begins a wrapped line instead of ending the line before it. +/// +public readonly struct LeadingOperator +{ + /// + /// Initializes a new instance of the struct. + /// + /// The operator token. + /// The first token of the leftmost operand the operator joins. + public LeadingOperator(SyntaxToken op, SyntaxToken firstOperand) + { + Operator = op; + FirstOperand = firstOperand; + } + + /// Gets the operator token. + public SyntaxToken Operator { get; } + + /// + /// Gets the first token of the leftmost operand the operator joins - where a hand-aligned + /// expression lines its operands up. + /// + public SyntaxToken FirstOperand { get; } + + /// + /// Every operator in that is the first token on its line. + /// + /// + /// Binary operators, the pattern combinators and and or, is, and both halves + /// of a conditional. Not the : of a constructor initializer or a base list, which SA1128 + /// puts on its own line, nor the . of a call chain or =>, which are not + /// operators joining two operands. + /// + /// The syntax tree's root. + /// The text was parsed from. + /// The leading operators, in document order within each expression. + public static IEnumerable FindAll(SyntaxNode root, SourceText text) + { + foreach (SyntaxNode node in root.DescendantNodes()) + { + switch (node) + { + case BinaryExpressionSyntax binary when LeadsItsLine(binary.OperatorToken, text): + yield return new LeadingOperator(binary.OperatorToken, binary.Left.GetFirstToken()); + break; + + case BinaryPatternSyntax pattern when LeadsItsLine(pattern.OperatorToken, text): + yield return new LeadingOperator(pattern.OperatorToken, pattern.Left.GetFirstToken()); + break; + + case IsPatternExpressionSyntax isPattern when LeadsItsLine(isPattern.IsKeyword, text): + yield return new LeadingOperator(isPattern.IsKeyword, isPattern.Expression.GetFirstToken()); + break; + + case ConditionalExpressionSyntax conditional: + SyntaxToken condition = conditional.Condition.GetFirstToken(); + + if (LeadsItsLine(conditional.QuestionToken, text)) + { + yield return new LeadingOperator(conditional.QuestionToken, condition); + } + + if (LeadsItsLine(conditional.ColonToken, text)) + { + yield return new LeadingOperator(conditional.ColonToken, condition); + } + + break; + } + } + } + + private static bool LeadsItsLine(SyntaxToken token, SourceText text) + { + SyntaxToken previous = token.GetPreviousToken(); + + if (previous.RawKind == 0 || token.IsMissing) + { + return false; + } + + int previousLine = text.Lines.GetLineFromPosition(previous.Span.End).LineNumber; + + return previousLine < text.Lines.GetLineFromPosition(token.SpanStart).LineNumber; + } +} diff --git a/Nota.CodeAnalysis.Analysers/Nota.CodeAnalysis.Analysers.csproj b/Nota.CodeAnalysis.Analysers/Nota.CodeAnalysis.Analysers.csproj new file mode 100644 index 0000000..3a477ec --- /dev/null +++ b/Nota.CodeAnalysis.Analysers/Nota.CodeAnalysis.Analysers.csproj @@ -0,0 +1,34 @@ + + + + + netstandard2.0 + latest + enable + false + true + true + + + + + + + + + + + + + diff --git a/Nota.CodeAnalysis.Analysers/OperatorPlacement.cs b/Nota.CodeAnalysis.Analysers/OperatorPlacement.cs new file mode 100644 index 0000000..4b5406e --- /dev/null +++ b/Nota.CodeAnalysis.Analysers/OperatorPlacement.cs @@ -0,0 +1,116 @@ +using System.Collections.Generic; +using System.Linq; + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Text; + +namespace Nota.CodeAnalysis.Analysers; + +/// +/// Moves a leading operator to the end of the line before it. +/// +/// +/// +/// Here rather than in the code fix so that it needs nothing beyond the compiler: the fix and a +/// whole-tree rewrite call the same code. +/// +/// +/// The operand left behind takes the operator's column, which keeps an operator that was aligned +/// under the first operand aligned. The exception is an operator hung in the margin so that the +/// operands line up - return a over + b - where the operand is already where it +/// belongs and stays there. +/// +/// +public static class OperatorPlacement +{ + /// + /// Moves every leading operator in whose position is in + /// , or every one when is null. + /// + /// + /// An operator with a comment or directive where the edits would go is left for a person, and the + /// analyser goes on reporting it. + /// + /// The syntax tree's root. + /// The text was parsed from. + /// Operator start positions to move, or null for all of them. + /// The rewritten text; itself when nothing could be moved. + public static SourceText Apply(SyntaxNode root, SourceText text, ISet? positions) + { + List changes = []; + + // How far each rewritten line's operand moved left. An operand lined up under one that has + // itself moved has to move with it, and the first operand is always on an earlier line. + Dictionary shifts = []; + + foreach (LeadingOperator leading in LeadingOperator.FindAll(root, text) + .Where(l => positions is null || positions.Contains(l.Operator.SpanStart)) + .OrderBy(l => l.Operator.SpanStart)) + { + SyntaxToken op = leading.Operator; + SyntaxToken previous = op.GetPreviousToken(); + SyntaxToken next = op.GetNextToken(); + TextLine line = text.Lines.GetLineFromPosition(op.SpanStart); + + if (op.LeadingTrivia.Any(IsDirective) || + op.TrailingTrivia.Any(t => !IsSpace(t)) || + previous.TrailingTrivia.Any(IsDirective) || + !string.IsNullOrWhiteSpace(text.ToString(TextSpan.FromBounds(line.Start, op.SpanStart)))) + { + continue; + } + + changes.Add(new TextChange(new TextSpan(previous.Span.End, 0), " " + op.Text)); + + if (text.Lines.GetLineFromPosition(next.SpanStart).LineNumber != line.LineNumber) + { + changes.Add(new TextChange(line.SpanIncludingLineBreak, string.Empty)); + continue; + } + + int opColumn = op.SpanStart - line.Start; + int nextColumn = next.SpanStart - line.Start; + int firstColumn = Column(text, leading.FirstOperand.SpanStart); + int firstLine = text.Lines.GetLineFromPosition(leading.FirstOperand.SpanStart).LineNumber; + int inherited = 0; + + if (shifts.TryGetValue(firstLine, out (int operandStart, int shift) moved) && + leading.FirstOperand.SpanStart >= moved.operandStart) + { + inherited = moved.shift; + } + + int column = opColumn; + + if (nextColumn == firstColumn) + { + column = nextColumn - inherited; + } + else if (opColumn == firstColumn) + { + column = opColumn - inherited; + } + + changes.Add(new TextChange(TextSpan.FromBounds(line.Start, next.SpanStart), new string(' ', column))); + shifts[line.LineNumber] = (next.SpanStart, nextColumn - column); + } + + return changes.Count == 0 ? text : text.WithChanges(changes.OrderBy(c => c.Span.Start)); + } + + private static int Column(SourceText text, int position) + { + return position - text.Lines.GetLineFromPosition(position).Start; + } + + private static bool IsDirective(SyntaxTrivia trivia) + { + return trivia.IsDirective || trivia.IsKind(SyntaxKind.DisabledTextTrivia); + } + + private static bool IsSpace(SyntaxTrivia trivia) + { + return trivia.IsKind(SyntaxKind.WhitespaceTrivia) || trivia.IsKind(SyntaxKind.EndOfLineTrivia); + } +} diff --git a/Nota.CodeAnalysis.Analysers/OperatorPlacementAnalyser.cs b/Nota.CodeAnalysis.Analysers/OperatorPlacementAnalyser.cs new file mode 100644 index 0000000..72be775 --- /dev/null +++ b/Nota.CodeAnalysis.Analysers/OperatorPlacementAnalyser.cs @@ -0,0 +1,55 @@ +using System.Collections.Immutable; + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Text; + +namespace Nota.CodeAnalysis.Analysers; + +/// +/// NOTA0003: an operator that wraps a line ends the line it belongs to rather than leading the next. +/// +/// +/// dotnet_style_operator_placement_when_wrapping states the rule for Roslyn's formatter, and +/// Rider's csharp_wrap_before_* settings state it for Rider, but neither reports code that +/// already breaks it - IDE0055 steers a rewrap and does not audit what is there. This does. +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class OperatorPlacementAnalyser : DiagnosticAnalyzer +{ + /// The diagnostic id. + public const string DiagnosticId = "NOTA0003"; + + private static readonly DiagnosticDescriptor Rule = new( + DiagnosticId, + "Operator leads a wrapped line", + "'{0}' leads its line; move it to the end of the line before and leave the rest of the expression as it is", + "Layout", + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: "When an expression wraps, the operator ends the line it follows and the next line starts with the operand. " + + "Moving the operator is the whole fix: a wrapped condition or ternary is fine as it is and does not need " + + "rewriting as an if/else."); + + /// + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); + + /// + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + context.RegisterSyntaxTreeAction(AnalyseTree); + } + + private static void AnalyseTree(SyntaxTreeAnalysisContext context) + { + SyntaxNode root = context.Tree.GetRoot(context.CancellationToken); + SourceText text = context.Tree.GetText(context.CancellationToken); + + foreach (LeadingOperator leading in LeadingOperator.FindAll(root, text)) + { + context.ReportDiagnostic(Diagnostic.Create(Rule, leading.Operator.GetLocation(), leading.Operator.Text)); + } + } +} diff --git a/Nota.CodeAnalysis.Verification/Nota.CodeAnalysis.Verification.csproj b/Nota.CodeAnalysis.Verification/Nota.CodeAnalysis.Verification.csproj index d77e0c2..61c1ca4 100644 --- a/Nota.CodeAnalysis.Verification/Nota.CodeAnalysis.Verification.csproj +++ b/Nota.CodeAnalysis.Verification/Nota.CodeAnalysis.Verification.csproj @@ -54,4 +54,10 @@ + + + + + diff --git a/Nota.CodeAnalysis.Verification/README.md b/Nota.CodeAnalysis.Verification/README.md index e82f821..2bbbdd9 100644 --- a/Nota.CodeAnalysis.Verification/README.md +++ b/Nota.CodeAnalysis.Verification/README.md @@ -48,16 +48,26 @@ looks redundant and is not: without it `IDE0005` silently stops reporting. ## What `verify.sh` asserts -`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` | +The files in `Samples/` 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` | +| `NOTA0003` | an operator leading a wrapped line | `LeadingOperator.cs` | + +`LeadingOperator.cs` also wraps the same expression the way `NOTA0003` wants, and `verify.sh` fails if +anything at all reports on it. That is what its code fix produces, and a fix that some other rule - +StyleCop's `SA1003`, say, or `IDE0055` - wants undone is a loop rather than a convention. Nothing does +today; this is what would say so if a StyleCop upgrade changed that. + +`NOTA0003` comes from `Nota.CodeAnalysis.Analysers` in this repository, referenced as an analyser +rather than through the package. Its own behaviour is covered by the unit tests in +`Nota.CodeAnalysis.Analysers.Test`; this only proves it loads and reports. Two more come from `build/Nota.CodeAnalysis.targets` rather than an analyser, and so are the only rules here that cannot be confirmed by reading a severity out of the globalconfig - they have to @@ -136,6 +146,9 @@ rule per analyser - plus a deliberately mis-encoded file for `NOTA0001`, which p the only rule with an opt-out that defaults to enforcing and so the only one where a wrong default would ship as silence. It fails on `CS9057` too, since that is a warning nothing else would notice. +`NOTA0003` proves the package's own analyser assembly was packed under `analyzers/dotnet/cs`, which +nothing inside the solution can see. + It also asserts the opposite of everything above: that `NOTA0001` and `NOTA0002` report the consumer's own files and **nothing else**. The throwaway project references `Microsoft.NET.Test.Sdk` purely for what that drags in - a source file of its own, from the read-only NuGet cache, carrying a diff --git a/Nota.CodeAnalysis.Verification/Samples/LeadingOperator.cs b/Nota.CodeAnalysis.Verification/Samples/LeadingOperator.cs new file mode 100644 index 0000000..e8bd825 --- /dev/null +++ b/Nota.CodeAnalysis.Verification/Samples/LeadingOperator.cs @@ -0,0 +1,31 @@ +namespace Nota.Verification; + +/// +/// An operator leading a wrapped line, which NOTA0003 reports, asserted by verify.sh. +/// +/// The second method wraps the same expression the way the rule wants, and must stay quiet - from +/// NOTA0003 and from StyleCop alike, since a rule whose fix another rule reports is a loop rather +/// than a convention. +/// +public static class LeadingOperator +{ + /// The operator leads the second line. + /// The first operand. + /// The second operand. + /// Both. + public static bool Leading(bool a, bool b) + { + return a + && b; + } + + /// The operator ends the first line. + /// The first operand. + /// The second operand. + /// Both. + public static bool Trailing(bool a, bool b) + { + return a && + b; + } +} diff --git a/Nota.CodeAnalysis.Verification/verify-package.sh b/Nota.CodeAnalysis.Verification/verify-package.sh index f7642b2..33ccd0b 100755 --- a/Nota.CodeAnalysis.Verification/verify-package.sh +++ b/Nota.CodeAnalysis.Verification/verify-package.sh @@ -118,6 +118,13 @@ public static class Broken /// An unbound property is Serilog003, from SerilogAnalyzer. public static void LogIt() => Log.Information("Hello {Name}"); + + /// A leading operator is NOTA0003, from the package's own analyser. + public static bool Both(bool a, bool b) + { + return a + && b; + } } EOF @@ -151,7 +158,8 @@ output="$(cd "$app" && dotnet build --no-incremental -v:m 2>&1 || true)" # 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 NOTA0002 SA1208 VSTHRD100 UA1000 Serilog003" +# NOTA0003 the package's own analyser was packed under analyzers/dotnet/cs, and loaded on this Roslyn +expected="IDE0008 NOTA0001 NOTA0002 NOTA0003 SA1208 VSTHRD100 UA1000 Serilog003" missing="" for rule in $expected; do diff --git a/Nota.CodeAnalysis.Verification/verify.sh b/Nota.CodeAnalysis.Verification/verify.sh index c65e0a5..876e007 100755 --- a/Nota.CodeAnalysis.Verification/verify.sh +++ b/Nota.CodeAnalysis.Verification/verify.sh @@ -43,7 +43,9 @@ done # NOTA0002 a source file carrying a UTF-8 byte order mark - Samples/BomMarked.cs. Also from the # targets, and the reason the samples are exempt from verify-encoding.sh. A warning # where NOTA0001 is an error, which is why the grep below accepts either -expected=(IDE0005 IDE0008 UA1000 UA1001 SA1208 SA1516 NOTA0001 NOTA0002) +# NOTA0003 an operator leading a wrapped line - see Samples/LeadingOperator.cs. From this +# repository's own analyser, so it also proves the analyser project loads at all +expected=(IDE0005 IDE0008 UA1000 UA1001 SA1208 SA1516 NOTA0001 NOTA0002 NOTA0003) # VerifyRules is what pulls Samples/ into the compilation. Without it the project builds empty, which # is what every other build of this solution wants. @@ -56,6 +58,17 @@ for rule in "${expected[@]}"; do fi done +# 3. Samples/LeadingOperator.cs wraps the same expression both ways. The leading operator is the one +# NOTA0003 reports; the trailing one is what its fix produces, and nothing may report that - not +# NOTA0003, and not a StyleCop or formatting rule wanting the operator back where it was. A rule +# whose fix another rule reports is a loop, not a convention. +leading="$(grep 'LeadingOperator\.cs(' <<<"$output" | sed 's/ \[.*//' | sort -u || true)" +unexpected="$(grep -v 'LeadingOperator\.cs(19,13): warning NOTA0003:' <<<"$leading" || true)" + +if [ -n "$unexpected" ]; then + failures+=("Samples/LeadingOperator.cs reported more than the one leading operator:"$'\n'"$unexpected") +fi + if [ ${#failures[@]} -ne 0 ]; then printf 'Verification failed:\n' >&2 printf ' - %s\n' "${failures[@]}" >&2 diff --git a/Nota.CodeAnalysis.sln b/Nota.CodeAnalysis.sln index 10f67ea..51e3909 100644 --- a/Nota.CodeAnalysis.sln +++ b/Nota.CodeAnalysis.sln @@ -7,6 +7,12 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Nota.CodeAnalysis", "Nota.C EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Nota.CodeAnalysis.Verification", "Nota.CodeAnalysis.Verification\Nota.CodeAnalysis.Verification.csproj", "{1053892B-03AC-49E2-8E7F-4E83D05A835D}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Nota.CodeAnalysis.Analysers", "Nota.CodeAnalysis.Analysers\Nota.CodeAnalysis.Analysers.csproj", "{C8110B1B-6C3D-4536-8DFA-C32A0DC1CBD5}" +EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Nota.CodeAnalysis.Analysers.CodeFixes", "Nota.CodeAnalysis.Analysers.CodeFixes\Nota.CodeAnalysis.Analysers.CodeFixes.csproj", "{E18676A5-1BBE-46B6-89C7-9D998958BADC}" +EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Nota.CodeAnalysis.Analysers.Test", "Nota.CodeAnalysis.Analysers.Test\Nota.CodeAnalysis.Analysers.Test.csproj", "{60A323E5-8CBC-4896-993B-2DE191673226}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -41,6 +47,42 @@ Global {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 + {C8110B1B-6C3D-4536-8DFA-C32A0DC1CBD5}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {C8110B1B-6C3D-4536-8DFA-C32A0DC1CBD5}.Debug|Any CPU.Build.0 = Debug|Any CPU + {C8110B1B-6C3D-4536-8DFA-C32A0DC1CBD5}.Debug|x64.ActiveCfg = Debug|Any CPU + {C8110B1B-6C3D-4536-8DFA-C32A0DC1CBD5}.Debug|x64.Build.0 = Debug|Any CPU + {C8110B1B-6C3D-4536-8DFA-C32A0DC1CBD5}.Debug|x86.ActiveCfg = Debug|Any CPU + {C8110B1B-6C3D-4536-8DFA-C32A0DC1CBD5}.Debug|x86.Build.0 = Debug|Any CPU + {C8110B1B-6C3D-4536-8DFA-C32A0DC1CBD5}.Release|Any CPU.ActiveCfg = Release|Any CPU + {C8110B1B-6C3D-4536-8DFA-C32A0DC1CBD5}.Release|Any CPU.Build.0 = Release|Any CPU + {C8110B1B-6C3D-4536-8DFA-C32A0DC1CBD5}.Release|x64.ActiveCfg = Release|Any CPU + {C8110B1B-6C3D-4536-8DFA-C32A0DC1CBD5}.Release|x64.Build.0 = Release|Any CPU + {C8110B1B-6C3D-4536-8DFA-C32A0DC1CBD5}.Release|x86.ActiveCfg = Release|Any CPU + {C8110B1B-6C3D-4536-8DFA-C32A0DC1CBD5}.Release|x86.Build.0 = Release|Any CPU + {E18676A5-1BBE-46B6-89C7-9D998958BADC}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {E18676A5-1BBE-46B6-89C7-9D998958BADC}.Debug|Any CPU.Build.0 = Debug|Any CPU + {E18676A5-1BBE-46B6-89C7-9D998958BADC}.Debug|x64.ActiveCfg = Debug|Any CPU + {E18676A5-1BBE-46B6-89C7-9D998958BADC}.Debug|x64.Build.0 = Debug|Any CPU + {E18676A5-1BBE-46B6-89C7-9D998958BADC}.Debug|x86.ActiveCfg = Debug|Any CPU + {E18676A5-1BBE-46B6-89C7-9D998958BADC}.Debug|x86.Build.0 = Debug|Any CPU + {E18676A5-1BBE-46B6-89C7-9D998958BADC}.Release|Any CPU.ActiveCfg = Release|Any CPU + {E18676A5-1BBE-46B6-89C7-9D998958BADC}.Release|Any CPU.Build.0 = Release|Any CPU + {E18676A5-1BBE-46B6-89C7-9D998958BADC}.Release|x64.ActiveCfg = Release|Any CPU + {E18676A5-1BBE-46B6-89C7-9D998958BADC}.Release|x64.Build.0 = Release|Any CPU + {E18676A5-1BBE-46B6-89C7-9D998958BADC}.Release|x86.ActiveCfg = Release|Any CPU + {E18676A5-1BBE-46B6-89C7-9D998958BADC}.Release|x86.Build.0 = Release|Any CPU + {60A323E5-8CBC-4896-993B-2DE191673226}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {60A323E5-8CBC-4896-993B-2DE191673226}.Debug|Any CPU.Build.0 = Debug|Any CPU + {60A323E5-8CBC-4896-993B-2DE191673226}.Debug|x64.ActiveCfg = Debug|Any CPU + {60A323E5-8CBC-4896-993B-2DE191673226}.Debug|x64.Build.0 = Debug|Any CPU + {60A323E5-8CBC-4896-993B-2DE191673226}.Debug|x86.ActiveCfg = Debug|Any CPU + {60A323E5-8CBC-4896-993B-2DE191673226}.Debug|x86.Build.0 = Debug|Any CPU + {60A323E5-8CBC-4896-993B-2DE191673226}.Release|Any CPU.ActiveCfg = Release|Any CPU + {60A323E5-8CBC-4896-993B-2DE191673226}.Release|Any CPU.Build.0 = Release|Any CPU + {60A323E5-8CBC-4896-993B-2DE191673226}.Release|x64.ActiveCfg = Release|Any CPU + {60A323E5-8CBC-4896-993B-2DE191673226}.Release|x64.Build.0 = Release|Any CPU + {60A323E5-8CBC-4896-993B-2DE191673226}.Release|x86.ActiveCfg = Release|Any CPU + {60A323E5-8CBC-4896-993B-2DE191673226}.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 80c7361..8e53cd9 100644 --- a/Nota.CodeAnalysis/Nota.CodeAnalysis.csproj +++ b/Nota.CodeAnalysis/Nota.CodeAnalysis.csproj @@ -29,6 +29,32 @@ + + + + + + + + $(TargetsForTfmSpecificContentInPackage);NotaPackAnalysers + + + + + + + + + + + + diff --git a/Nota.CodeAnalysis/content/Nota.CodeAnalysis.globalconfig b/Nota.CodeAnalysis/content/Nota.CodeAnalysis.globalconfig index 8ddd9a8..25f92a2 100644 --- a/Nota.CodeAnalysis/content/Nota.CodeAnalysis.globalconfig +++ b/Nota.CodeAnalysis/content/Nota.CodeAnalysis.globalconfig @@ -107,7 +107,18 @@ dotnet_naming_style.pascal_case.required_prefix = dotnet_naming_style.pascal_case.required_suffix = dotnet_naming_style.pascal_case.word_separator = dotnet_naming_style.pascal_case.capitalization = pascal_case -dotnet_style_operator_placement_when_wrapping = beginning_of_line +# A wrapped expression ends its line with the operator, and the next line starts with the operand. +# dotnet_style_operator_placement_when_wrapping steers Roslyn's formatter when it rewraps, and the +# csharp_wrap_before_* keys say the same to Rider's. Neither reports code that is already wrapped +# the other way; NOTA0003, below, is what reports, and carries a code fix. +# +# The Rider keys are here in case Rider reads them from a global analyzer config, which is +# unverified - Rider certainly reads them from .editorconfig, and a repository whose Rider still +# wraps before the operator should copy these three lines there. Roslyn ignores them either way. +dotnet_style_operator_placement_when_wrapping = end_of_line +csharp_wrap_before_binary_opsign = false +csharp_wrap_before_binary_pattern_op = false +csharp_wrap_before_ternary_opsigns = false dotnet_style_coalesce_expression = true:suggestion dotnet_style_null_propagation = true:suggestion dotnet_style_prefer_is_null_check_over_reference_equality_method = true:suggestion @@ -150,7 +161,7 @@ csharp_style_prefer_top_level_statements = true:silent csharp_style_prefer_primary_constructors = true:suggestion csharp_style_expression_bodied_methods = false:silent csharp_style_expression_bodied_constructors = false:silent -csharp_style_expression_bodied_operators = false:silent +csharp_style_expression_bodied_operators = false:warning csharp_style_expression_bodied_properties = true:silent csharp_style_expression_bodied_indexers = true:silent csharp_style_expression_bodied_accessors = true:silent @@ -557,6 +568,9 @@ dotnet_diagnostic.IDE0063WithoutSuggestion.severity = none dotnet_diagnostic.IDE0066.severity = none dotnet_diagnostic.IDE0066WithoutSuggestion.severity = none dotnet_diagnostic.IDE0290.severity = none +# An operator leading a wrapped line, from this package's own analyser. Stated although warning is +# already its default, so that what it is set to can be read here like every other rule. +dotnet_diagnostic.NOTA0003.severity = warning dotnet_diagnostic.SA0000.severity = warning dotnet_diagnostic.SA1101.severity = none dotnet_diagnostic.SA1116.severity = none diff --git a/README.md b/README.md index 346541b..bc5da07 100644 --- a/README.md +++ b/README.md @@ -35,8 +35,9 @@ Four properties are switched on, in `build/Nota.CodeAnalysis.props`: | `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`. +SerilogAnalyzer, and UsingLayoutAnalyser. One more, Nota's own (`NOTA0003`, with its code fix), is +carried inside the package itself, built from `Nota.CodeAnalysis.Analysers`. Around 400 rule +severities are set in `content/Nota.CodeAnalysis.globalconfig`. ## Configuring it @@ -81,6 +82,12 @@ Most of this is unsurprising. These are the ones that catch people out: Reporting that left `NotaValidateSourceEncoding=false` as the only route to a clean build, which throws away `NOTA0001` too - a rule against corruption talking people into switching off the only guard against it. +- **`NOTA0003`**: when an expression wraps, the operator ends the line and the next line starts + with the operand - `a &&` then `b`, not `a` then `&& b`. Binary operators, `is`/`as`, the pattern + combinators `and`/`or`, and both halves of `?:`; not `.`, `=>`, or the `:` of a base list or + constructor initializer. The code fix moves only the operator and leaves the expression alone - a + wrapped condition or ternary is fine as it is. An existing repository is converted in one pass + with `dotnet format analyzers --diagnostics NOTA0003 --severity warn`. - **`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`. @@ -186,6 +193,32 @@ measured, it transcodes such a file to UTF-8, which preserves the text and still arrives as a whole-file diff, and a regenerated service reference or migration will be UTF-16 again next time the generator runs. +## Upgrading from 2.3 + +The operator placement convention has flipped. `dotnet_style_operator_placement_when_wrapping` was +`beginning_of_line` and is now `end_of_line`, and `NOTA0003` reports every operator that still leads +a wrapped line - as a warning, so a repository with `TreatWarningsAsErrors` will fail to build until +they are moved. One command moves them all, and changes nothing but whitespace: + +```sh +dotnet format analyzers --diagnostics NOTA0003 --severity warn +``` + +`IDE0024` is now a warning too: an operator overload has a block body, not `=>`. It was always the +preference, just never reported. `dotnet format style --diagnostics IDE0024 --severity warn` converts +them. + +The package also sets Rider's wrap keys, but whether Rider reads them from a global analyzer config +rather than `.editorconfig` is unverified. If Rider still puts the operator first when it wraps - +producing fresh warnings - add them to your `.editorconfig`: + +```ini +[*.cs] +csharp_wrap_before_binary_opsign = false +csharp_wrap_before_binary_pattern_op = false +csharp_wrap_before_ternary_opsigns = false +``` + ## Upgrading The two encoding changes came one version apart and land in the same place, so they are described @@ -259,6 +292,13 @@ before changing rules. Both run on pull requests as well as on `main`. +Nota's own analysers live in `Nota.CodeAnalysis.Analysers`, their code fixes in +`Nota.CodeAnalysis.Analysers.CodeFixes` - a separate assembly, because a fix needs Workspaces and the +command-line compiler does not carry it. Both reference Roslyn 4.8 and must stay there or below: an +analyser built against a newer compiler than the consumer's SDK is skipped with `CS9057`, a warning, +and its rules silently vanish. `dotnet test` runs their unit tests in +`Nota.CodeAnalysis.Analysers.Test`; a new rule goes in `AnalyzerReleases.Unshipped.md` as well. + Releases are cut by tagging: ```sh