Skip to content

Add analyzer λ1012: prefer the …OrNone method over …OrDefault with a null check - #973

Merged
FreeApophis merged 6 commits into
mainfrom
analyzer-prefer-or-none
Oct 20, 2026
Merged

FreeApophis merged 6 commits into
mainfrom
analyzer-prefer-or-none

Conversation

@FreeApophis

Copy link
Copy Markdown
Member

Summary

  • New opt-in rule λ1012 (Warning): FirstOrDefault, LastOrDefault, SingleOrDefault and ElementAtOrDefault whose result is immediately tested for null (?? fallback, ?., is null/is not null, == null/!= null, is { } x, is not { } x, is T t). The …OrNone variants make the missing element a None instead of a null every caller has to check by hand.
  • Only sequences of a reference type that is not nullable-annotated are reported: for IEnumerable<string?> a null result is ambiguous between "no element" and "the element is null". The overloads with an explicit default value, the static call form and IQueryable (where FirstOrDefault translates to the query) are left alone. The rule is silent when the …OrNone method is not available.
  • Code fix: xs.FirstOrDefault() ?? fallback becomes xs.FirstOrNone().GetOrElse(fallback); a throw fallback becomes the lazy GetOrElse(() => throw …). It adds using Funcky.Extensions; next to the existing usings (file level or inside the namespace) when the method does not bind yet. The other null-check shapes are reported without a fix, the rule doc lists the idiomatic replacement for each.
  • New helper UsingDirectives.AddUsing in the code fix project (parsed directive, trivia copied from the last existing using, so no CRLF/LF normalization).
  • Fills the free ID λ1012 (Add a code fix for CS8714 on Option.Select rewriting to SelectMany with Option.FromNullable #965 became a code fix without a rule).
  • Docs: λ1012.md, rule table, SUMMARY, changelog, AnalyzerReleases.Unshipped.

Note for reviewers: the operation tree presents an extension call unreduced (TargetMethod is the static method with the receiver as first argument), so the matcher checks IsExtensionMethod and derives the extension-call form from the syntax.

Test plan

  • 6 new tests in PreferOrNoneTest: fix with using added at file level and inside a namespace, every method/overload incl. predicate, index, parentheses and throw fallback, the no-fix shapes, no null check, value types / nullable elements / IQueryable / static form not reported
  • 82 analyzer tests pass, Roslyn 4.0 variant builds
  • dotnet build Funcky.slnx -warnaserror --no-incremental green (no hits in Funcky itself)

🤖 Generated with Claude Code

…null check

FirstOrDefault, LastOrDefault, SingleOrDefault and ElementAtOrDefault whose
result is immediately tested for null (??, ?., is null, is { } x, == null,
is T t, …) are reported; the …OrNone variants return an Option instead.
Only sequences of a non-nullable reference type are considered, the
overloads with an explicit default value and IQueryable are left alone.

The code fix rewrites `xs.FirstOrDefault() ?? fallback` to
`xs.FirstOrNone().GetOrElse(fallback)` (a throw fallback becomes lazy) and
adds `using Funcky.Extensions;` next to the existing usings when the name
does not bind yet. The other shapes have no single idiomatic replacement
and are reported without a fix.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… usings

Keeps the helper identical to the one in the λ1018 branch.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The analyzer misses intended nullable-aware cases, and code fixes can change behavior or produce compilation errors.

5 open issues
What changed in this PR

Adds λ1012 to encourage using Funcky’s option-returning LINQ methods instead of checking …OrDefault results for null.

Changes:

  • Adds null-check detection and a code fix for ?? expressions.
  • Adds import insertion support and analyzer/code-fix tests.
  • Documents the rule and registers it in release notes.
File Description
Funcky.Analyzers/​Funcky.Analyzers/​OrNone/​PreferOrNoneAnalyzer.cs Implements λ1012 detection.
Funcky.Analyzers/​Funcky.Analyzers/​AnalyzerReleases.Unshipped.md Registers the warning.
Funcky.Analyzers/​Funcky.Analyzers.Test/​OrNone/​PreferOrNoneTest.cs Tests diagnostics and rewrites.
Funcky.Analyzers/​Funcky.Analyzers.CodeFixes/​OrNone/​PreferOrNoneCodeFix.cs Rewrites coalescing expressions.
Funcky.Analyzers/​Funcky.Analyzers.CodeFixes/​CodeAnalysisExtensions/​UsingDirectives.cs Adds extension namespace imports.
Documentation/​src/​SUMMARY.md Links the rule documentation.
Documentation/​src/​analyzer-rules/​λ1012.md Explains usage and replacements.
Documentation/​src/​analyzer-rules/​analyzer-rules.md Adds the rule to the index.
changelog.md Announces the analyzer and fix.

Review effort: Balanced

Comment thread Funcky.Analyzers/Funcky.Analyzers.CodeFixes/OrNone/PreferOrNoneCodeFix.cs Outdated
Comment thread Funcky.Analyzers/Funcky.Analyzers.CodeFixes/OrNone/PreferOrNoneCodeFix.cs Outdated
Comment thread Funcky.Analyzers/Funcky.Analyzers.CodeFixes/OrNone/PreferOrNoneCodeFix.cs Outdated
Comment thread Funcky.Analyzers/Funcky.Analyzers/OrNone/PreferOrNoneAnalyzer.cs Outdated
FreeApophis and others added 4 commits October 20, 2026 15:15
… and decline widened types

- The analyzer checks the element type argument instead of the return type, which the nullable-annotated LINQ always
  annotates, and uses the Funcky.Style category.
- The fix keeps explicit type arguments when renaming, offers nothing when the `??` has a wider type than the element,
  passes a fallback that does work as a lambda (declining when it cannot move into one) and inserts the using outside a
  conditional region with formatting trivia only. The directive-safe insertion is shared with the λ1017 fix.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…y, and behind leading #define directives

The directive-safe insertion of the λ1017 fix moves into the shared UsingDirectives helper, which copies only whitespace
and line endings from its neighbour. In a file without usings a leading `#define` or `#undef` moves in front of the new
using, since those directives must precede every token.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@FreeApophis
FreeApophis merged commit 8fa49a7 into main Oct 20, 2026
10 checks passed
@FreeApophis
FreeApophis deleted the analyzer-prefer-or-none branch October 20, 2026 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants