Skip to content

todo.md

Latest commit

 

History

History
177 lines (140 loc) · 16.6 KB

todo.md

File metadata and controls

177 lines (140 loc) · 16.6 KB

TODO: Review whether the C# 13 test verifier should use real reference assemblies (e.g., a future ReferenceAssemblies.Net.NetX_Y that includes System.Threading.Lock) instead of the current minimal stub in CSharp13CodeFixVerifier<TAnalyzer, TCodeFix>.

C# 13 implementation follow-ups (shortcuts / known limitations)

  • Partial properties/indexers refactoring: indexers excluded

  • C# 13 test infrastructure: CSharp13CodeFixVerifier pins ReferenceAssemblies.Net.Net90

    • Limitation: Tests using CSharp13CodeFixVerifier<TAnalyzer, TCodeFix> compile against .NET 9 reference assemblies regardless of the repo’s baseline.
    • Why: Needed a “modern target framework” so preview C# 13 features that depend on runtime support can compile in fixed-state (see comment in verifier).
    • Long-term fix: Revisit whether .NET 9 is the right baseline for tests (or whether it should be conditional / centralized), and switch to the smallest reference set that still supports the required C# 13 features.
    • Pointers: CSharp13CodeFixVerifier.CreateTest().

  • System.Threading.Lock is injected as a test stub

    • Limitation: The verifier injects a minimal System.Threading.Lock implementation into both test and fixed states, which can mask real API shape differences.
    • Why: Current reference assemblies used by tests don’t provide System.Threading.Lock, but safety checkers/code fixes need the symbol to resolve.
    • Long-term fix: Remove the stub and rely on real reference assemblies once they include System.Threading.Lock (or use a dedicated reference assembly package that contains it).
    • Pointers: CSharp13CodeFixVerifier.LockStubSource, UseSystemThreadingLockSafetyChecker.

Inventory: original-sharpen vs sharpen.analyzer

Scope

You asked for an inventory of all analyzers and fix providers in the old project ./original-sharpen and a comparison with what is implemented in the new project ./sharpen.analyzer.

Important terminology mismatch:

  • In original-sharpen, “analyzers” are engine analyzers implementing ISingleSyntaxTreeAnalyzer and returning AnalysisResult objects. “Fixes” are suggestions (ISharpenSuggestion) surfaced by the VS extension UI.
  • In sharpen.analyzer, analyzers are Roslyn DiagnosticAnalyzer implementations and fixes are Roslyn CodeFixProvider implementations.

This report compares capabilities by:

  1. enumerating the old engine analyzers from SharpenAnalyzersHolder.Analyzers
  2. verifying the corresponding new analyzer/fix exists, and
  3. spot-checking implementation content (not only file names) for a few representative rules.

Findings

1) Old project (original-sharpen): where analysis/fixes live

The old implementation is split into:

2) Old project inventory: Engine analyzers (and their suggestions)

From SharpenAnalyzersHolder.Analyzers, the old project contains the following capabilities:

C# 3.0

  • UseVarKeywordInVariableDeclarationWithObjectCreation

C# 5.0 (Async/Await)

  • ConsiderAwaitingEquivalentAsynchronousMethodAndMakingTheCallerAsynchronous
  • AwaitEquivalentAsynchronousMethod
  • AwaitTaskDelayInsteadOfCallingThreadSleep
  • AwaitTaskInsteadOfCallingTaskWait
  • AwaitTaskInsteadOfCallingTaskResult
  • AwaitTaskWhenAllInsteadOfCallingTaskWaitAll
  • AwaitTaskWhenAnyInsteadOfCallingTaskWaitAny

C# 6.0 (Expression-bodied members, nameof)

  • UseExpressionBodyForGetOnlyProperties
  • UseExpressionBodyForGetOnlyIndexers
  • UseNameofExpressionForThrowingArgumentExceptions
  • UseNameofExpressionInDependencyPropertyDeclarations

C# 7.0 (Expression-bodied members, out vars)

  • UseExpressionBodyForConstructors
  • UseExpressionBodyForDestructors
  • UseExpressionBodyForGetAccessorsInProperties
  • UseExpressionBodyForGetAccessorsInIndexers
  • UseExpressionBodyForSetAccessorsInProperties
  • UseExpressionBodyForSetAccessorsInIndexers
  • UseExpressionBodyForLocalFunctions
  • UseOutVariablesInMethodInvocations
  • UseOutVariablesInObjectCreations
  • DiscardOutVariablesInMethodInvocations
  • DiscardOutVariablesInObjectCreations

C# 7.1 (default literal)

  • UseDefaultExpressionInReturnStatements
  • UseDefaultExpressionInOptionalMethodParameters
  • UseDefaultExpressionInOptionalConstructorParameters

C# 8.0

  • EnableNullableContextAndDeclareIdentifierAsNullableAnalyzer
  • ConsiderAwaitingEquivalentAsynchronousMethodAndYieldingIAsyncEnumerableAnalyzer
  • ReplaceUsingStatementWithUsingDeclarationAnalyzer
  • ReplaceSwitchStatementWithSwitchExpressionAnalyzer
  • UseNullCoalescingAssignmentOperatorInsteadOfAssigningResultOfTheNullCoalescingOperatorAnalyzer

3) New project (sharpen.analyzer): analyzers / code fix providers

The new project contains Roslyn analyzers and code fix providers under:

Additionally, the new project includes analyzers beyond the old project (C# 9-12), e.g. UseTopLevelStatementsAnalyzer, UsePrimaryConstructorAnalyzer.

4) Content-based comparison (spot checks)

4.1 Out variables: object creations

Old engine:

New Roslyn analyzer:

Conclusion: capability exists in new project and the core detection logic is present.

4.2 Out variables: discard in method invocations

Old engine:

New Roslyn analyzer:

  • DiscardOutVariablesInMethodInvocationsAnalyzer registers on SyntaxKind.InvocationExpression and:
    • iterates arguments
    • filters out keyword + identifier expression
    • calls OutVariableCandidateHelper.IsCandidate(..., outArgumentCanBeDiscarded: true)
    • reports diagnostic on the out keyword location

Conclusion: capability exists in new project and the “discard vs out var” distinction is preserved.

4.3 Expression-bodied set accessor in indexers

New Roslyn code fix:

  • UseExpressionBodyForSetAccessorsInIndexersCodeFixProvider performs a concrete syntax transformation:
    • finds the AccessorDeclarationSyntax at diagnostic span
    • validates it is a set accessor inside an IndexerDeclarationSyntax
    • requires a block body with exactly one ExpressionStatementSyntax
    • replaces { expr; } with => expr; while attempting to preserve trivia

Conclusion: new project not only detects but also provides an automated fix for this rule.

4.4 Expression-bodied set accessor in properties (deeper check)

Old engine:

New project:

Conclusion: UseExpressionBodyForSetAccessorsInProperties appears to be a real missing capability in the new project (not just a naming mismatch).

5) Delta: “in original but not in new”

After re-checking the new project’s analyzer/fix inventory (and not relying only on file names), the earlier “missing” list was incorrect.

The following capabilities from the old engine are present in the new project (examples):

Confirmed missing capability:

  • UseExpressionBodyForSetAccessorsInProperties exists in the old engine list (see UseExpressionBodyForSetAccessorsInProperties) but does not appear to exist in the new project (no analyzer, no rule descriptor, no code fix provider).

Notes / limitations

  • The old project’s “fixes” are not Roslyn CodeFixProviders; they are suggestions surfaced by the VS extension UI. So this report compares capabilities, not 1:1 type equivalence.
  • I only did content spot-checks for a subset of rules (out vars + expression-bodied rule(s)). A full content-level diff for every rule would require reading each old suggestion implementation and its new analyzer + code fix pair.