feat(FC0007): Statement blocks should be separated by a blank line (opt-in) - #424
Open
MODUSCarstenScholling wants to merge 4 commits into
Open
Conversation
- Replace WellKnownFixAllProviders.BatchFixer with a custom FixAllProvider.Create(FixAllAsync) that removes all matching ParameterSyntax nodes in one root.RemoveNodes pass. BatchFixer's ReplaceNode(parameterList, …) merge dropped all but one edit when multiple parameters in the same signature were flagged. - Split registration into three CodeActions with distinct EquivalenceKeys (.All, .RegularProcedure, .EventSubscriber). FixAllAsync reads CodeActionEquivalenceKey to scope the batch; single-fix path uses RemoveNode(KeepNoTrivia) so trivia handling matches FixAll. - Guard fixAllSpans with !IsDefaultOrEmpty and fall back to GetDocumentDiagnosticsAsync — the AL SDK reports HasValue=true with an empty array under document-scope FixAll. - Add multiline HasFix fixture and three HasFixAll fixtures (single-method dual removal, multi-method, event subscriber). All 25 ParameterNotReferenced tests pass. - Update codefix-development, testing, and LC0095 instruction files: document the custom FixAllProvider exception, the HasFixAll test pattern, new design decisions, and known-issues entry.
…ne" (opt-in) Introduce FC0007, a disabled-by-default formatting rule that enforces blank-line separation around multi-line control-flow blocks (if, case, repeat, while, for, foreach), before scope-leaving statements (exit, built-in Error), and optionally before the else keyword of if/end-else constructs. Configurable via alcops.json under a new StatementBlockSpacing object with five properties: ControlFlowBefore, ControlFlowAfter, ScopeLeavingMode, ElseChainBeforeMode, and OneLinerMode. Three are real C# enums serialized as case-insensitive strings via JsonStringEnumConverter (net8+) and StringEnumConverter (netstandard2.1). Also: - Colocate the new settings type in Settings/StatementBlockSpacingSettings.cs - Harden ALCopsSettingsProvider: malformed alcops.json now silently falls back to defaults (JsonException swallowed in DeserializeSettings) - Add reusable schema-parity guard test template (StatementBlockSpacingSchema) against alcops.schema.json - 21 tests (18 rule + 3 schema-parity), all green No CodeFix. Additional spacing axes are captured on the FC0007 Roadmap.
- Detach StatementBlocksSeparatedByBlankLine from the exception-harness bridge back to plain DiagnosticAnalyzer. Matches the current policy in analyzer-exception-harness.instructions.md — no production analyzer adopts the harness until issue ALCops#389 (AL1003 loader bug) is resolved. - Unify test class on a single [Test]+[TestCase] pattern: two parametrized methods (HasDiagnostic / NoDiagnostic) driven by a named settings dictionary. Drop the fixtureKind parameter — LoadFixtureAsync resolves the folder from the (globally unique) fixture name, removing the "HasDiagnostic" string leaking into NoDiagnostic test cases. - Make StatementBlockSpacingSchema inherit NavCodeAnalysisBase per testing convention (mirrors NamingPatternSettings). - Sync common-library.instructions.md with reality: remove documentation for the removed GetSettings(string?) legacy overload and non-existent ClearCache() method. - Drop a stray blank line in FormattingCop DiagnosticDescriptors.
Collection expressions on ImmutableArray<T> require the CollectionBuilder attribute, which is only present in System.Collections.Immutable v6+ (net8+). The netstandard2.1 target pulls v5.0.0 and therefore fails with CS9210. Replace `[]` and `[.. src]` in GetSiblingStatements with ImmutableArray<T>.Empty and .ToImmutableArray() respectively. Local build (net8 only) missed this; CI covers all three TFMs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
feat(FC0007): Statement blocks should be separated by a blank line (opt-in)
Adds FC0007 — Statement blocks should be separated by a blank line to
ALCops.FormattingCop. Opt-in (disabled by default) and fully configurable viaalcops.json. Closes #.What it flags
if,case,repeat,while,for,foreach) without a blank line before them and/or after their terminatingend;.exit,exit(<expr>), and built-inError(...)calls that are not preceded by a blank line.
end elsetransition inif … end else … endchains.Single-line (one-liner) control-flow statements are ignored unless
OneLinerModeis set toAll.Configuration surface
New object in
alcops.json:ControlFlowBeforebooltrueControlFlowAfterbooltrueScopeLeavingModeOff/ExitOnly/ErrorOnly/ExitAndErrorExitAndErrorElseChainBeforeModeOff/RequireBlankOffOneLinerModeOff/AllOffEnums are real C# types (
ALCops.Common.Settings.{Scope,ElseChainBefore,OneLiner}Mode),serialized case-insensitively via
JsonStringEnumConverteronnet8+andStringEnumConverteronnetstandard2.1. Both converter registrations wereadded to
ALCopsSettingsProvider.Provider hardening (small, but notable)
ALCopsSettingsProvider.DeserializeSettingsnow swallowsJsonExceptionandreturns defaults. Users with a broken
alcops.json(e.g. an unknown enumvalue) previously would have crashed the analyzer callback and produced an
AD0001; now the rule simply behaves as if no config file existed. A dedicatedtest guards this behavior with a fixture containing
"ScopeLeavingMode":"NotAnEnumValue".Schema parity guard
Settings/alcops.schema.jsonmirrors the three FC0007 enums. Any drift iscaught by
StatementBlockSpacingSchema.cs, a NUnit test template that reflectsover the C# enum values and compares them against the
enumarrays in theschema JSON. Reusable pattern for future enum-typed settings.
Test coverage
one-liner / disabled-by-default / malformed-config regression).
Tests follow a single
[Test] + [TestCase]pattern across two parametrizedmethods (
HasDiagnostic/NoDiagnostic) driven by a named settingsdictionary. The fixture folder (
HasDiagnostic/vsNoDiagnostic/) isresolved implicitly by
LoadFixtureAsync— test signatures stay free ofstorage-layout details.
Docs
.github/instructions/fc0007-statement-blocks-separated-by-blank-line.instructions.mdwith design decisions, test summary counts, known issues, and a Phase-2
roadmap (
CaseBranchMode,GuardClauseMode,LoopControlBeforeMode,TreatCommentOnlyLinesAsSeparator,SkipDirectiveBoundaries).common-library.instructions.mdupdated for the newStatementBlockSpacingsetting and the "add a new setting" enum-converter walkthrough.
alcops.dev
at
content/docs/analyzers/formattingcop/fc0007.md.Not in this PR (roadmap)
Implements #423