Files
db-subclass-to-dto/tools/ExtractMethod/Tooling/DataFlowClassifier.cs
T
mostalive 49a5633a7d em 03: extract-first scan + full suggestion report
- ExtractFirstScanner: reads-only scan of non-local/param read expressions
  (member/field/property access, element access, invocation, cast), deduped
  by symbol+text with occurrence counts; invocations flagged (hoisting
  changes eval count); own-class field/property reads marked optional.
  Assignment LHS skipped whole (decision 5) — surfaced as a report note.
- SignatureBuilder: promotes bucket overlaps into one coherent signature
  (declared-inside vars drop from params; declared-inside return candidates
  become the return; outside candidates already covered by their ref
  write-back stay ref params). Trailing composite returns stay void with a
  note suggesting an extract-variable first.
- ExtractionReporter + ReportFormatter: single composition shared by CLI and
  tests; report = header, params (ref), returns, locals, extract-first
  (count + flags), notes, suggested-signature line last.
- DataFlowClassifier: Returns now carry types (ReturnSuggestion) — the
  signature line needs them.
- Displays: compact symbol/type formatting for the report.
- Fixture: RepeatReads (dedupe ×2 + skipped field write), Casts (cast
  candidate) appended at the end so all pinned line numbers stay put.
2026-09-12 18:26:20 +01:00

316 lines
16 KiB
C#

using System.Collections.Immutable;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
namespace ExtractMethod.Tooling;
// ---------------------------------------------------------------------------
// Yak em 02: data-flow classification. Turns a resolved selection into the
// three buckets the extract-method report is built from (params / returns /
// locals), using Roslyn's DataFlowAnalysis. Names and types are plain strings
// for v1; the records are the stable contract em 03 (extract-first + report)
// and em 04 (codegen) build on.
// ---------------------------------------------------------------------------
/// <summary>One suggested parameter of the extracted method (v1: plain strings).</summary>
/// <param name="Name">Variable name as it appears in the source.</param>
/// <param name="Type">Type as a display string (e.g. <c>int</c>, <c>string</c>).</param>
/// <param name="ByRef">True when the extraction must take the variable by <c>ref</c>
/// (write-back required); see <see cref="DataFlowClassifier.IsReassignedInside"/> for
/// the precise v1 rule.</param>
public sealed record ParamSuggestion(string Name, string Type, bool ByRef);
/// <summary>One return candidate of the extracted method (v1: plain strings).</summary>
/// <param name="Name">Variable name as it appears in the source.</param>
/// <param name="Type">Type as a display string — the signature line of em 03
/// prints it as the extracted method's return type.</param>
public sealed record ReturnSuggestion(string Name, string Type);
/// <summary>The classified buckets of one selection (v1: symbol names as plain strings).</summary>
/// <param name="Params">Variables the selection READS and the new method therefore
/// receives: read locals + the enclosing method's parameters (parent decision #6).
/// A variable may also appear in <see cref="Locals"/> and/or <see cref="Returns"/> —
/// v1 reports per-bucket; em 03's signature builder dedupes into a coherent
/// signature (a variable DECLARED inside the selection cannot be a parameter).</param>
/// <param name="Returns">Variables written inside AND read after the selection
/// (tail data flow), plus the value of a trailing <c>return X;</c> when X is a
/// simple name — they must flow out of the extraction. Typed since em 03: the
/// signature line needs the return type, not just the name.</param>
/// <param name="Locals">Variables written inside and never read after: scratch
/// locals of the new method.</param>
public sealed record ExtractionSuggestion(
IReadOnlyList<ParamSuggestion> Params,
IReadOnlyList<ReturnSuggestion> Returns,
IReadOnlyList<string> Locals);
/// <summary>
/// Buckets a <see cref="SelectionReport"/> via Roslyn data-flow analysis.
///
/// Two regions are analyzed, in the SAME tree and compilation (so symbols keep
/// their identity and <see cref="SymbolEqualityComparer"/> matches them across
/// regions):
/// 1. the selection itself (parent spec: ReadInside / WrittenInside /
/// WrittenOutside),
/// 2. the TAIL: the statements AFTER the selection in the same enclosing
/// block — its ReadInside is the "read after" set (parent decision #4;
/// branch-insensitive over-approximation, see <see cref="Classify"/>).
///
/// WHY the two-argument overload and not a synthetic BlockSyntax (parent
/// decision #3 was "spike it"): the em 01 spike proved a synthetic block is
/// NOT part of the tree, so <c>AnalyzeDataFlow(block)</c> throws
/// <c>ArgumentException: statements not within tree</c>. Making it work would
/// mean re-parsing the method body, which RE-BINDS every symbol — the tail's
/// symbols would no longer equal the selection's, and the
/// written-inside ∩ read-after intersection (the return bucket) would break.
/// The two-argument overload analyzes a contiguous run in one statement list,
/// and both endpoints ARE tree nodes — no re-parse, identity preserved. It is
/// the LIVE path; the synthetic block is the DEAD path (pinned in tests).
/// </summary>
public static class DataFlowClassifier
{
/// <summary>
/// Data flow of the selected statements themselves.
/// Single statement → the one-argument overload; many contiguous
/// statements → <see cref="SemanticModel.AnalyzeDataFlow(SyntaxNode, SyntaxNode)"/>.
/// The resolver guarantees the selection is a contiguous run of whole
/// statements in one statement list, which is exactly that overload's
/// contract (both endpoints in the tree, same parent list).
/// </summary>
public static DataFlowAnalysis AnalyzeSelectionFlow(SemanticModel model, SelectionReport selection)
=> AnalyzeRegion(model, selection.Statements);
/// <summary>
/// Data flow of the statements AFTER the selection in the same enclosing
/// block (parent decision #4's "tail flow"). Returns null when the
/// selection already reaches the end of the block (nothing to read after).
/// The tail is again a contiguous run in one statement list, so the same
/// two-argument overload applies — no synthetic block needed, and symbol
/// identity with the selection analysis is preserved (see class comment).
/// </summary>
public static DataFlowAnalysis? AnalyzeTailFlow(SemanticModel model, SelectionReport selection)
{
var body = selection.Method?.Body;
if (body is null)
{
return null; // resolution succeeded but had no body: nothing to do (defensive)
}
var all = body.Statements;
int lastIndex = all.IndexOf(selection.Statements[^1]); // reference identity: same tree
int tailCount = all.Count - lastIndex - 1;
if (tailCount == 0)
{
return null;
}
return AnalyzeRegion(model, all.Skip(lastIndex + 1).Take(tailCount).ToList());
}
/// <summary>
/// The three report buckets for a resolved selection.
///
/// RULES (parent spec):
/// - params: every variable READ inside that is a local or the enclosing
/// method's parameter (decision #6 keeps fields/properties/statics out —
/// they are the extract-first bucket of em 03). The implicit `this`
/// parameter is filtered out: it is the instance, not a passable value.
/// - returns: written inside AND read after the selection (tail flow),
/// plus the value of a trailing `return X;` when X is a simple name.
/// - locals: written inside and never read after → scratch locals.
///
/// OVER-APPROXIMATION (decision #4, kept for v1, comment is the contract):
/// "read after" is ReadInside of the whole tail, branch-insensitively. We
/// do NOT track whether the write from inside the selection actually
/// reaches each tail read (e.g. the variable could be overwritten in the
/// tail before its next read). Consequences: some variables are reported
/// as returns that a precise analysis would classify as locals. Accepted.
///
/// Overlap between buckets is a v1 artifact of the parent spec, not a bug:
/// a variable that is read, declared and reassigned inside (e.g. `total`)
/// legitimately lands in params (by-ref: a write-back is owed) AND in
/// locals/returns. em 03 promotes such variables into the signature.
/// </summary>
public static ExtractionSuggestion Classify(SemanticModel model, SelectionReport selection)
{
var selectionFlow = AnalyzeSelectionFlow(model, selection);
if (!selectionFlow.Succeeded)
{
// Bindable code that Roslyn cannot analyze means a tool bug or an
// unhandled file shape; be loud instead of silently producing an
// empty classification.
throw new InvalidOperationException(
"data-flow analysis of the selection failed to bind (Succeeded == false)");
}
var readInside = selectionFlow.ReadInside;
var writtenInside = selectionFlow.WrittenInside;
// WrittenOutside is not needed for the v1 buckets (a variable written
// outside the selection and read inside is already a by-value in-param;
// one written inside and outside is caught by IsReassignedInside).
// It is still computed/exposed via AnalyzeSelectionFlow — the sets are
// the curriculum — but unused here by design.
// ReadInside is ImmutableArray (not a set): fine for Contains lookups.
var readAfter = AnalyzeTailFlow(model, selection)?.ReadInside ?? ImmutableArray<ISymbol>.Empty;
// ---- returns: written inside ∧ read after, plus trailing return X ----
// Typed since em 03 (the signature line needs the return type). The
// symbols come straight from the flow sets, so types are exact; the
// trailing-return candidate resolves its own identifier below.
var returnSymbols = writtenInside
.Where(v => readAfter.Contains(v))
.ToDictionary(v => v.Name, VariableTypeString, StringComparer.Ordinal);
var trailingReturn = TrailingReturnSuggestion(model, selection);
if (trailingReturn is { } candidate)
{
returnSymbols.TryAdd(candidate.Name, candidate.Type);
}
var returnSuggestions = returnSymbols
.Select(kv => new ReturnSuggestion(kv.Key, kv.Value))
.OrderBy(r => r.Name, StringComparer.Ordinal)
.ToList();
// ---- locals: written inside, never read after -> scratch locals ----
var localNames = writtenInside
.Where(v => !returnSymbols.ContainsKey(v.Name))
.Select(v => v.Name)
.OrderBy(n => n, StringComparer.Ordinal)
.ToList();
// ---- params: reads that are locals / enclosing-method parameters ----
var paramSuggestions = readInside
.Where(v => v is ILocalSymbol or IParameterSymbol)
.Where(v => v is not IParameterSymbol { IsThis: true }) // `this` is the instance, not a value
.OrderBy(v => v.Name, StringComparer.Ordinal)
.Select(v => new ParamSuggestion(
v.Name,
VariableTypeString(v),
IsReassignedInside(model, v, selection.Statements)))
.ToList();
return new ExtractionSuggestion(paramSuggestions, returnSuggestions, localNames);
}
/// <summary>
/// Display string of a variable's type. ISymbol has no Type member; only
/// locals and parameters carry one. Used for the params bucket AND (since
/// em 03) the returns bucket — the signature line prints it.
/// </summary>
private static string VariableTypeString(ISymbol variable) => variable switch
{
ILocalSymbol local => local.Type?.ToDisplayString(Displays.TypeFormat) ?? "unknown",
IParameterSymbol parameter => parameter.Type?.ToDisplayString(Displays.TypeFormat) ?? "unknown",
_ => "unknown",
};
/// <summary>
/// Analyzes a contiguous run of statements (one or many) in one statement
/// list. One statement → the one-argument overload; several → the two-
/// argument overload. Both endpoints are real tree nodes; a synthetic
/// block would throw "statements not within tree" (see class comment).
/// </summary>
private static DataFlowAnalysis AnalyzeRegion(SemanticModel model, IReadOnlyList<StatementSyntax> statements)
{
// Both overloads are nullable-annotated in Roslyn 5.x; null here means
// the region could not be bound, which the caller treats like
// Succeeded == false (loud failure, never a silent empty result).
DataFlowAnalysis? flow = statements.Count == 1
? model.AnalyzeDataFlow(statements[0])
: model.AnalyzeDataFlow(statements[0], statements[^1]);
return flow ?? throw new InvalidOperationException(
$"data-flow analysis returned null for a {statements.Count}-statement region (unable to bind)");
}
/// <summary>
/// The ByRef flag (parent spec: "mark ref if also written inside").
///
/// REFINEMENT, with the WHY: a variable's own declaration write (its
/// initializer) must NOT force a ref — the extraction declares the
/// variable itself and the caller never owed a write-back. Only a
/// REASSIGNMENT inside the selection (assignment, compound assignment,
/// ++/--, ref/out argument) represents a value written back to a variable
/// the caller already owns, which is what demands by-ref. Without this
/// refinement, every local declared-and-read inside (ScoreReads' seed and
/// score, per the note-6 fixture map) would be wrongly flagged ref.
/// </summary>
private static bool IsReassignedInside(SemanticModel model, ISymbol variable, IReadOnlyList<StatementSyntax> statements)
{
foreach (var statement in statements)
{
foreach (var node in statement.DescendantNodesAndSelf())
{
switch (node)
{
// `x = ...`, `x += ...`, `x ??= ...`, `x ?? y` is read, etc.:
// the LHS of any AssignmentExpression is a write position.
case AssignmentExpressionSyntax a when RefersTo(a.Left, model, variable):
return true;
// `x++`, `++x`, `x--`, `--x` (read-modify-write). The
// unary-expression Kind distinguishes inc/dec from other
// unary operators (e.g. `-x`, `!x` are reads only).
case PostfixUnaryExpressionSyntax p
when p.Kind() is SyntaxKind.PostIncrementExpression or SyntaxKind.PostDecrementExpression
&& RefersTo(p.Operand, model, variable):
case PrefixUnaryExpressionSyntax q
when q.Kind() is SyntaxKind.PreIncrementExpression or SyntaxKind.PreDecrementExpression
&& RefersTo(q.Operand, model, variable):
return true;
// `Foo(ref x)`, `Foo(out x)`: ref/out arguments write the
// variable (out even more so).
case ArgumentSyntax arg
when !arg.RefOrOutKeyword.IsKind(SyntaxKind.None)
&& RefersTo(arg.Expression, model, variable):
return true;
}
}
}
return false;
}
/// <summary>True when <paramref name="expr"/> is a simple reference to <paramref name="variable"/>.</summary>
private static bool RefersTo(ExpressionSyntax expr, SemanticModel model, ISymbol variable)
=> expr is IdentifierNameSyntax
&& SymbolEqualityComparer.Default.Equals(model.GetSymbolInfo(expr).Symbol, variable);
/// <summary>
/// Parent spec: "if the selection ends with `return X;`, X is the candidate."
/// Returns the simple-name candidate with its type, or null when the
/// returned expression is not a simple name — an expression like
/// `score + bonus` is not nameable as a v1 string (composite return
/// expressions are extract-first shape for em 03: the report suggests
/// extracting it into a local first). Empty return ('return;') has no
/// candidate.
/// </summary>
private static ReturnSuggestion? TrailingReturnSuggestion(SemanticModel model, SelectionReport selection)
{
var last = selection.Statements[^1];
if (last is not ReturnStatementSyntax { Expression: IdentifierNameSyntax name })
{
return null;
}
var type = model.GetSymbolInfo(name).Symbol switch
{
ILocalSymbol local => local.Type?.ToDisplayString(Displays.TypeFormat) ?? "unknown",
IParameterSymbol parameter => parameter.Type?.ToDisplayString(Displays.TypeFormat) ?? "unknown",
_ => "unknown",
};
return new ReturnSuggestion(name.Identifier.ValueText, type);
}
/// <summary>
/// The trailing return statement when its expression is NOT a simple
/// name (and not absent) — the composite-return shape the em 03 report
/// flags as "extract a local first" (see <see cref="TrailingReturnSuggestion"/>).
/// </summary>
public static ReturnStatementSyntax? CompositeTrailingReturn(SelectionReport selection)
=> selection.Statements[^1] is ReturnStatementSyntax { Expression: not null and not IdentifierNameSyntax } ret
? ret
: null;
}