diff --git a/tests/BeforeAfter.Tests/ExtractMethod/DataFlowClassificationTests.cs b/tests/BeforeAfter.Tests/ExtractMethod/DataFlowClassificationTests.cs new file mode 100644 index 0000000..c3f14d4 --- /dev/null +++ b/tests/BeforeAfter.Tests/ExtractMethod/DataFlowClassificationTests.cs @@ -0,0 +1,185 @@ +using ExtractMethod.Tooling; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace BeforeAfter.Tests.ExtractMethod; + +/// +/// Tests for yak em 02: the data-flow classifier (buckets: params / returns / +/// locals) over the checked-in Demo.cs fixture. Line anchors are the exact +/// fixture lines pinned by em 01 (see Demo.cs header comment): +/// ScoreReads body 56..58, Summarize body 68..76 (for-loop 69..72), +/// Heaviest body 87..93. +/// +public class DataFlowClassificationTests +{ + // --------------------------------------------------------------------- + // in-param bucket (ScoreReads, full body 56..58). + // ReadInside = {_seed→field, seed, score, bonus}; the classifier must + // report the reads that are locals/params — seed/score/bonus — as in- + // params, and must NOT leak `this` (the implicit this-read behind the + // _seed field access; filtered via IParameterSymbol.IsThis) into params, + // nor the _seed field itself (decision #6 keeps fields for em 03). + // WrittenInside = {seed, score}; no tail, trailing return is an expression + // (score + bonus) which v1 cannot name -> no returns, both written vars + // are scratch locals. + // --------------------------------------------------------------------- + [Fact] + public void ScoreReads_full_body_reads_report_three_in_params_and_no_returns() + { + var suggestion = ResolveAndClassify(56, 58); + + Assert.Equal(new[] { "bonus", "score", "seed" }, suggestion.Params.Select(p => p.Name)); + // None is reassigned inside (declaration initializers are not write-backs) -> no ref. + Assert.All(suggestion.Params, p => Assert.False(p.ByRef)); + Assert.Empty(suggestion.Returns); + // v1 reports the read locals ALSO as scratch locals (decision #6 over- + // reports local reads); em 03 dedupes them into the signature. + Assert.Equal(new[] { "score", "seed" }, suggestion.Locals); + } + + // --------------------------------------------------------------------- + // multi-statement range case (Summarize 68..72: LocalDeclarationStatement + // + ForStatement): the resolution, the selection data flow AND the tail + // data flow all go through the two-argument overload with Succeeded=true + // (the synth-block trick from parent decision #3 is the DEAD path — see + // Test for it below). Buckets: total written inside + read at line 74 in + // the tail -> return; for-var i never read after -> local. + // --------------------------------------------------------------------- + [Fact] + public void Summarize_multi_statement_range_flows_via_two_argument_overload_and_buckets_total_return_i_local() + { + var (tree, compilation) = DemoFixture.Load(); + var model = compilation.GetSemanticModel(tree); + var report = SelectionResolver.Resolve(tree, 68, 72); + Assert.True(report.Succeeded, report.Error); + + var selectionFlow = DataFlowClassifier.AnalyzeSelectionFlow(model, report); + Assert.True(selectionFlow.Succeeded, "two-argument AnalyzeDataFlow must succeed on a multi-statement selection"); + + var tailFlow = DataFlowClassifier.AnalyzeTailFlow(model, report); + Assert.NotNull(tailFlow); + Assert.True(tailFlow.Succeeded, "two-argument AnalyzeDataFlow must succeed on the (multi-statement) tail region"); + + var suggestion = DataFlowClassifier.Classify(model, report); + Assert.Equal(new[] { "total" }, suggestion.Returns); + Assert.Equal(new[] { "i" }, suggestion.Locals); + + // limit is read in the for-header (68..72) — a plain by-value in-param. + var limit = Assert.Single(suggestion.Params.Where(p => p.Name == "limit")); + Assert.False(limit.ByRef); + } + + // --------------------------------------------------------------------- + // return + ref-param buckets (Summarize, full body 68..76). + // Returns: the tail is empty, so the tail-flow rule yields nothing; the + // selection ends with `return message;` -> message is the trailing-return + // candidate (parent spec). Locals: everything else written inside: + // {i, scaled, total}. + // Params (all ReadInside locals/params): the differential ref check — + // total is REASSIGNED inside (total += i at line 71) -> ByRef; scaled and + // limit are never reassigned (declaration write only) -> plain in-params. + // --------------------------------------------------------------------- + [Fact] + public void Summarize_full_body_returns_trailing_message_and_flags_reassigned_total_as_ref() + { + var suggestion = ResolveAndClassify(68, 76); + + Assert.Equal(new[] { "message" }, suggestion.Returns); + Assert.Equal(new[] { "i", "scaled", "total" }, suggestion.Locals); + + var total = Assert.Single(suggestion.Params.Where(p => p.Name == "total")); + Assert.True(total.ByRef, "total is reassigned inside (total += i) -> the extraction owes a write-back"); + + var scaled = Assert.Single(suggestion.Params.Where(p => p.Name == "scaled")); + Assert.False(scaled.ByRef, "scaled is only declared inside, never reassigned"); + + var limit = Assert.Single(suggestion.Params.Where(p => p.Name == "limit")); + Assert.False(limit.ByRef); + } + + // --------------------------------------------------------------------- + // return + extract-first isolation (Heaviest, full body 87..93). + // Returns: `return best;` is the last statement -> best (trailing-return + // rule again). Written inside but never read after: i -> local. + // Params: reads are best/count/i/widgets; best and i are reassigned + // inside -> ref. Nothing extract-first (widgets[i] indexer, Bigger + // invocation — em 03 territory) may leak into any bucket: the params list + // is exactly the four variable reads. + // --------------------------------------------------------------------- + [Fact] + public void Heaviest_full_body_returns_best_locals_i_and_leaks_no_extract_first_names() + { + var suggestion = ResolveAndClassify(87, 93); + + Assert.Equal(new[] { "best" }, suggestion.Returns); + Assert.Equal(new[] { "i" }, suggestion.Locals); + Assert.Equal(new[] { "best", "count", "i", "widgets" }, suggestion.Params.Select(p => p.Name)); + Assert.Equal(new[] { true, false, true, false }, suggestion.Params.Select(p => p.ByRef)); + } + + // --------------------------------------------------------------------- + // single-statement fallback (ScoreReads, line 58 alone: `return ...;`). + // The one-argument AnalyzeDataFlow overload is the live path for a single + // statement. The trailing return is `score + bonus` — an expression, not + // a simple name — so v1 reports no return candidate (composite return + // expressions are extract-first shape for em 03) and no locals. + // --------------------------------------------------------------------- + [Fact] + public void Single_statement_selection_uses_one_argument_overload_and_composite_trailing_return_is_not_nameable() + { + var (tree, compilation) = DemoFixture.Load(); + var model = compilation.GetSemanticModel(tree); + var report = SelectionResolver.Resolve(tree, 58, 58); + Assert.True(report.Succeeded, report.Error); + + Assert.True(DataFlowClassifier.AnalyzeSelectionFlow(model, report).Succeeded); + + var suggestion = DataFlowClassifier.Classify(model, report); + Assert.Equal(new[] { "bonus", "score" }, suggestion.Params.Select(p => p.Name)); + Assert.All(suggestion.Params, p => Assert.False(p.ByRef)); + Assert.Empty(suggestion.Returns); + Assert.Empty(suggestion.Locals); + } + + // --------------------------------------------------------------------- + // DEAD PATH (parent decision #3, pinned so the choice is self-explanatory + // to future readers): a synthetic BlockSyntax re-parents tree nodes that + // are not in the tree, and AnalyzeDataFlow refuses with + // ArgumentException("statements not within tree"). That is why the live + // path uses the two-argument overload (both endpoints are real tree + // nodes) — which also keeps symbol identity across selection and tail + // analyses, the prerequisite for the written-inside ∩ read-after + // intersection of the return bucket. + // --------------------------------------------------------------------- + [Fact] + public void Synthetic_block_data_flow_throws_not_within_tree_so_the_two_argument_overload_stays_live() + { + var (tree, compilation) = DemoFixture.Load(); + var model = compilation.GetSemanticModel(tree); + + var summarize = tree.GetRoot().DescendantNodes() + .OfType() + .Single(m => m.Identifier.ValueText == "Summarize"); + var statements = summarize.Body!.Statements; + + // Same node instances, new (synthetic) parent — this is the shape + // parent decision #3 proposed for multi-statement ranges and tails. + var syntheticBlock = SyntaxFactory.Block(statements[2], statements[3], statements[4]); + + var ex = Assert.Throws(() => model.AnalyzeDataFlow(syntheticBlock)); + Assert.Contains("not within tree", ex.Message, StringComparison.Ordinal); + } + + private static ExtractionSuggestion ResolveAndClassify(int startLine, int endLine) + { + var (tree, compilation) = DemoFixture.Load(); + var model = compilation.GetSemanticModel(tree); + + var report = SelectionResolver.Resolve(tree, startLine, endLine); + Assert.True(report.Succeeded, report.Error); + + return DataFlowClassifier.Classify(model, report); + } +} \ No newline at end of file diff --git a/tests/BeforeAfter.Tests/ExtractMethod/DemoFixtureLoader.cs b/tests/BeforeAfter.Tests/ExtractMethod/DemoFixtureLoader.cs new file mode 100644 index 0000000..9e5a082 --- /dev/null +++ b/tests/BeforeAfter.Tests/ExtractMethod/DemoFixtureLoader.cs @@ -0,0 +1,32 @@ +using ExtractMethod.Tooling; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; + +namespace BeforeAfter.Tests.ExtractMethod; + +/// +/// Shared entry point for the ExtractMethod tests: the checked-in Demo.cs +/// fixture is DATA (excluded from this project's compilation, copied to the +/// output dir) and must ALWAYS be loaded through the tool's own +/// so tests exercise the exact parse + +/// scratch-compilation path the CLI uses. +/// +internal static class DemoFixture +{ + /// Where the fixture lands after the csproj copies it (link Fixtures/). + public static string Path => + System.IO.Path.Combine(AppContext.BaseDirectory, "Fixtures", "Demo.cs"); + + /// Parse + scratch-compile the fixture once per caller. + public static (SyntaxTree Tree, CSharpCompilation Compilation) Load() + { + if (!File.Exists(Path)) + { + throw new FileNotFoundException($"fixture missing at {Path}"); + } + + var tree = CompilationLoader.ParseFile(Path); + var compilation = CompilationLoader.CreateCompilation(tree, "DemoFixture"); + return (tree, compilation); + } +} \ No newline at end of file diff --git a/tests/BeforeAfter.Tests/ExtractMethod/DemoFixtureTests.cs b/tests/BeforeAfter.Tests/ExtractMethod/DemoFixtureTests.cs index d92fbe5..fafe6f4 100644 --- a/tests/BeforeAfter.Tests/ExtractMethod/DemoFixtureTests.cs +++ b/tests/BeforeAfter.Tests/ExtractMethod/DemoFixtureTests.cs @@ -13,16 +13,7 @@ namespace BeforeAfter.Tests.ExtractMethod; /// public class DemoFixtureTests { - private static readonly string FixturePath = - Path.Combine(AppContext.BaseDirectory, "Fixtures", "Demo.cs"); - - private static (SyntaxTree Tree, CSharpCompilation Compilation) LoadFixture() - { - Assert.True(File.Exists(FixturePath), $"fixture missing at {FixturePath}"); - var tree = CompilationLoader.ParseFile(FixturePath); - var compilation = CompilationLoader.CreateCompilation(tree, "DemoFixture"); - return (tree, compilation); - } + private static (SyntaxTree Tree, CSharpCompilation Compilation) LoadFixture() => DemoFixture.Load(); // --------------------------------------------------------------------- // (a) The fixture is pristine under the scratch compilation: if this @@ -77,6 +68,9 @@ public class DemoFixtureTests var dataFlow = compilation.GetSemanticModel(tree).AnalyzeDataFlow(forStatement); + // AnalyzeDataFlow is nullable-annotated in Roslyn 5.x: assert non-null + // so the dereference below is provably safe (and test intent explicit). + Assert.NotNull(dataFlow); Assert.True(dataFlow.Succeeded); } diff --git a/tools/ExtractMethod/Program.cs b/tools/ExtractMethod/Program.cs index 3a8d571..76f0b09 100644 --- a/tools/ExtractMethod/Program.cs +++ b/tools/ExtractMethod/Program.cs @@ -1,4 +1,5 @@ using ExtractMethod.Tooling; +using Microsoft.CodeAnalysis; // CLIs are boring on purpose: argument parsing and printing live here, all // Roslyn logic lives in Tooling/ so the tests can drive it directly. @@ -31,7 +32,7 @@ var tree = CompilationLoader.ParseFile(file); // 2. build the scratch compilation (refs from TRUSTED_PLATFORM_ASSEMBLIES) var compilation = CompilationLoader.CreateCompilation(tree, Path.GetFileNameWithoutExtension(file)); -if (compilation.GetDiagnostics().Any(d => d.Severity == Microsoft.CodeAnalysis.DiagnosticSeverity.Error)) +if (compilation.GetDiagnostics().Any(d => d.Severity == DiagnosticSeverity.Error)) { Console.Error.WriteLine("warning: the file does not compile cleanly under a plain Roslyn compilation; reporting syntax-level resolution only"); } @@ -48,4 +49,46 @@ Console.WriteLine( $"{report.Count} statement(s) selected, lines {report.StartLine}..{report.EndLine} " + $"in {report.Method?.Identifier.ValueText}(): " + string.Join(", ", report.Kinds)); + +// 4. classify the data flow into the raw buckets (em 02: no pretty report yet) +// A classification failure is a semantic resolution error: same exit code as +// the resolver, message on stderr, but never a stack trace. +ExtractionSuggestion suggestion; +try +{ + suggestion = DataFlowClassifier.Classify(compilation.GetSemanticModel(tree), report); +} +catch (Exception e) when (e is InvalidOperationException or ArgumentException) +{ + Console.Error.WriteLine($"error: data-flow analysis failed: {e.Message}"); + return SelectionResolver.ExitError; +} + +foreach (var param in suggestion.Params) +{ + Console.WriteLine($"params: {param.Name} ({param.Type}){(param.ByRef ? " [ref]" : " [in]")}"); +} +if (suggestion.Params.Count == 0) +{ + Console.WriteLine("params: (none)"); +} + +foreach (var name in suggestion.Returns) +{ + Console.WriteLine($"returns: {name}"); +} +if (suggestion.Returns.Count == 0) +{ + Console.WriteLine("returns: (none)"); +} + +foreach (var name in suggestion.Locals) +{ + Console.WriteLine($"locals: {name}"); +} +if (suggestion.Locals.Count == 0) +{ + Console.WriteLine("locals: (none)"); +} + return 0; \ No newline at end of file diff --git a/tools/ExtractMethod/Tooling/DataFlowClassifier.cs b/tools/ExtractMethod/Tooling/DataFlowClassifier.cs new file mode 100644 index 0000000..c9a891c --- /dev/null +++ b/tools/ExtractMethod/Tooling/DataFlowClassifier.cs @@ -0,0 +1,277 @@ +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. +// --------------------------------------------------------------------------- + +/// One suggested parameter of the extracted method (v1: plain strings). +/// Variable name as it appears in the source. +/// Type as a display string (e.g. int, string). +/// True when the extraction must take the variable by ref +/// (write-back required); see for +/// the precise v1 rule. +public sealed record ParamSuggestion(string Name, string Type, bool ByRef); + +/// The classified buckets of one selection (v1: symbol names as plain strings). +/// 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 and/or — +/// v1 reports per-bucket and em 03 dedupes into a coherent signature. +/// Variables written inside AND read after the selection +/// (tail data flow), plus the value of a trailing return X; when X is a +/// simple name — they must flow out of the extraction. +/// Variables written inside and never read after: scratch +/// locals of the new method. +public sealed record ExtractionSuggestion( + IReadOnlyList Params, + IReadOnlyList Returns, + IReadOnlyList Locals); + +/// +/// Buckets a via Roslyn data-flow analysis. +/// +/// Two regions are analyzed, in the SAME tree and compilation (so symbols keep +/// their identity and 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 ). +/// +/// 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 AnalyzeDataFlow(block) throws +/// ArgumentException: statements not within tree. 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). +/// +public static class DataFlowClassifier +{ + /// + /// Data flow of the selected statements themselves. + /// Single statement → the one-argument overload; many contiguous + /// statements → . + /// 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). + /// + public static DataFlowAnalysis AnalyzeSelectionFlow(SemanticModel model, SelectionReport selection) + => AnalyzeRegion(model, selection.Statements); + + /// + /// 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). + /// + 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()); + } + + /// + /// 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. + /// + 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.Empty; + + var trailingReturn = TrailingReturnName(selection); + + // ---- returns: written inside ∧ read after, plus trailing return X ---- + var returnNames = writtenInside + .Where(v => readAfter.Contains(v)) + .Select(v => v.Name) + .Concat(trailingReturn is { } name ? new[] { name } : Array.Empty()) + .Distinct(StringComparer.Ordinal) + .OrderBy(n => n, StringComparer.Ordinal) + .ToList(); + + // ---- locals: written inside, never read after -> scratch locals ---- + var localNames = writtenInside + .Where(v => !returnNames.Contains(v.Name, StringComparer.Ordinal)) + .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, + ParamTypeString(v), + IsReassignedInside(model, v, selection.Statements))) + .ToList(); + + return new ExtractionSuggestion(paramSuggestions, returnNames, localNames); + } + + /// + /// Display string of the variable's type. ISymbol has no Type member; + /// only locals and parameters carry one (the params bucket is restricted + /// to exactly those kinds, hence the match). + /// + private static string ParamTypeString(ISymbol variable) => variable switch + { + ILocalSymbol local => local.Type?.ToDisplayString() ?? "unknown", + IParameterSymbol parameter => parameter.Type?.ToDisplayString() ?? "unknown", + _ => "unknown", + }; + + /// + /// 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). + /// + private static DataFlowAnalysis AnalyzeRegion(SemanticModel model, IReadOnlyList 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)"); + } + + /// + /// 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. + /// + private static bool IsReassignedInside(SemanticModel model, ISymbol variable, IReadOnlyList 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; + } + + /// True when is a simple reference to . + private static bool RefersTo(ExpressionSyntax expr, SemanticModel model, ISymbol variable) + => expr is IdentifierNameSyntax + && SymbolEqualityComparer.Default.Equals(model.GetSymbolInfo(expr).Symbol, variable); + + /// + /// Parent spec: "if the selection ends with `return X;`, X is the candidate." + /// Returns the simple-name candidate, 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). Empty return ('return;') has no candidate. + /// + private static string? TrailingReturnName(SelectionReport selection) + { + var last = selection.Statements[^1]; + return last is ReturnStatementSyntax { Expression: IdentifierNameSyntax name } ? name.Identifier.ValueText : null; + } +} \ No newline at end of file