diff --git a/tests/BeforeAfter.Tests/ExtractMethod/Fixtures/Demo.cs b/tests/BeforeAfter.Tests/ExtractMethod/Fixtures/Demo.cs index df5fa1a..b79215f 100644 --- a/tests/BeforeAfter.Tests/ExtractMethod/Fixtures/Demo.cs +++ b/tests/BeforeAfter.Tests/ExtractMethod/Fixtures/Demo.cs @@ -115,4 +115,37 @@ public class Demo { return (double)total / _seed; } + + /// Bucket (em 04 codegen): a local DECLARED BEFORE the selection + /// that the selection writes and the tail reads — the promoted signature + /// keeps it as a ref parameter (the write-back already flows the + /// value out, so the result is not re-returned; void) and the call site + /// is a plain call, no assignment: Extract(ref acc, n);. + public int Accumulate(int n) + { + int acc = 0; + for (int i = 0; i < n; i++) + { + acc += i; + } + + return acc * 2; + } + + /// Bucket (em 04 codegen): TWO locals declared inside, both + /// written inside and read after — the promoted signature returns a + /// tuple (int, int) and the call site deconstructs it: + /// var (x, y) = Extract(n);. + public (int, int) Pairwise(int n) + { + int x = 0; + int y = 1; + for (int i = 0; i < n; i++) + { + x += i; + y += i * 2; + } + + return (x + y, x - y); + } } \ No newline at end of file diff --git a/tests/BeforeAfter.Tests/ExtractMethod/RefactoringTests.cs b/tests/BeforeAfter.Tests/ExtractMethod/RefactoringTests.cs new file mode 100644 index 0000000..46b09bd --- /dev/null +++ b/tests/BeforeAfter.Tests/ExtractMethod/RefactoringTests.cs @@ -0,0 +1,227 @@ +using ExtractMethod.Tooling; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace BeforeAfter.Tests.ExtractMethod; + +/// +/// Tests for yak em 04 (codegen): generate the refactoring from the em 03 +/// suggestion — new method (promoted signature, parameter names from the +/// source symbols) + rewritten call site + transformed tree — and the +/// round-trip check (the transformed tree compiles with zero diagnostics). +/// +/// Fixture line anchors: the em 01/02/03 anchors (ScoreReads 56..58, +/// Summarize 68..76 / partial 68..72, Heaviest 87..93) plus the em 04 +/// additions Accumulate for-loop 127..130 and Pairwise 141..147. +/// +public class RefactoringTests +{ + // --------------------------------------------------------------------- + // Summarize partial (68..72): the declared-inside `total` promotes to the + // plain return of the new method; at the call site it is re-declared from + // the result. The new body gains the appended `return total;`. + // --------------------------------------------------------------------- + [Fact] + public void Summarize_partial_selection_generates_returning_method_and_declaration_call() + { + var (method, calls, tree) = Generate(68, 72); + + Assert.Equal("Extract", method.Identifier.ValueText); + Assert.Equal("int", method.ReturnType.ToString()); + Assert.Equal(new[] { "int limit" }, method.ParameterList.Parameters.Select(p => p.ToString())); + + var body = (BlockSyntax)method.Body!; + Assert.Equal(3, body.Statements.Count); // total declaration, for-loop, appended return + Assert.Equal("return total;", body.Statements[2].ToString()); + + // `total` was declared inside the selection: re-declared at the call + // site from the result. + Assert.Single(calls); + Assert.Equal("int total = Extract(limit);", calls[0].ToString()); + + VerifyZeroDiagnostics(tree); + } + + // --------------------------------------------------------------------- + // Summarize full body (68..76): the selection ends the (non-void) + // enclosing method and the trailing `return message;` is nameable — the + // call site becomes `return Extract(limit);` and NO extra return is + // appended to the new body (the existing one is kept). + // --------------------------------------------------------------------- + [Fact] + public void Summarize_full_body_selection_becomes_return_call() + { + var (method, calls, tree) = Generate(68, 76); + + Assert.Equal("Extract", method.Identifier.ValueText); + Assert.Equal("string", method.ReturnType.ToString()); + Assert.Equal(new[] { "int limit" }, method.ParameterList.Parameters.Select(p => p.ToString())); + + var body = (BlockSyntax)method.Body!; + Assert.Equal(5, body.Statements.Count); // total, for, scaled, message, return — unchanged + Assert.Equal("return message;", body.Statements[^1].ToString()); + + Assert.Single(calls); + Assert.Equal("return Extract(limit);", calls[0].ToString()); + + VerifyZeroDiagnostics(tree); + } + + // --------------------------------------------------------------------- + // Heaviest full body (87..93): the trailing `return best;` names the + // single return slot, which matches the enclosing method's return type — + // again a `return Extract(...)` call site, with the widgets/count + // parameters as plain in-params. + // --------------------------------------------------------------------- + [Fact] + public void Heaviest_full_body_selection_becomes_return_call() + { + var (method, calls, tree) = Generate(87, 93); + + Assert.Equal("Widget", method.ReturnType.ToString()); + Assert.Equal(new[] { "int count", "List widgets" }, + method.ParameterList.Parameters.Select(p => p.ToString())); + + var body = (BlockSyntax)method.Body!; + Assert.Equal(3, body.Statements.Count); + Assert.Equal("return best;", body.Statements[^1].ToString()); + + Assert.Single(calls); + Assert.Equal("return Extract(count, widgets);", calls[0].ToString()); + + VerifyZeroDiagnostics(tree); + } + + // --------------------------------------------------------------------- + // ScoreReads (56..58): the enclosing method returns int but the selection + // ends with the COMPOSITE `return score + bonus;` — not nameable in v1, + // so the promoted signature is void and codegen must REFUSE (the report + // note already says: extract a local first, then re-run). + // --------------------------------------------------------------------- + [Fact] + public void ScoreReads_refuses_when_the_composite_trailing_return_is_not_nameable() + { + var (tree, compilation) = DemoFixture.Load(); + var model = compilation.GetSemanticModel(tree); + + var resolved = SelectionResolver.Resolve(tree, 56, 58); + Assert.True(resolved.Succeeded, resolved.Error); + + var suggestion = DataFlowClassifier.Classify(model, resolved); + var signature = SignatureBuilder.Build(model, resolved, suggestion); + Assert.Equal("void", signature.ReturnType); + + var ex = Assert.Throws( + () => RefactoringGenerator.Generate(model, resolved, signature)); + Assert.Contains("extract it into a local first", ex.Message, StringComparison.Ordinal); + } + + // --------------------------------------------------------------------- + // Accumulate for-loop (127..130): `acc` is declared BEFORE the selection, + // written inside and read after — the promoted signature keeps it as a + // ref parameter (the write-back flows the value out; void return, no + // return slot) and the call site is a plain call with `ref acc`. + // --------------------------------------------------------------------- + [Fact] + public void Accumulate_forloop_selection_calls_with_ref_write_back() + { + var (method, calls, tree) = Generate(127, 130); + + Assert.Equal("void", method.ReturnType.ToString()); + Assert.Equal(new[] { "ref int acc", "int n" }, + method.ParameterList.Parameters.Select(p => p.ToString())); + + var body = (BlockSyntax)method.Body!; + Assert.Single(body.Statements); // the for-loop; no return appended (void) + + Assert.Single(calls); + Assert.Equal("Extract(ref acc, n);", calls[0].ToString()); + + VerifyZeroDiagnostics(tree); + } + + // --------------------------------------------------------------------- + // Pairwise (141..147): two declared-inside locals both written inside and + // read after — the promoted signature returns the tuple `(int, int)` and + // the call site deconstructs it; the new body gains `return (x, y);`. + // --------------------------------------------------------------------- + [Fact] + public void Pairwise_selection_generates_tuple_return_and_deconstruction_call() + { + var (method, calls, tree) = Generate(141, 147); + + Assert.Equal("(int, int)", method.ReturnType.ToString()); + Assert.Equal(new[] { "int n" }, method.ParameterList.Parameters.Select(p => p.ToString())); + + var body = (BlockSyntax)method.Body!; + Assert.Equal(4, body.Statements.Count); // x, y, for-loop, appended tuple return + Assert.Equal("return (x, y);", body.Statements[^1].ToString()); + + Assert.Single(calls); + Assert.Equal("var (x, y) = Extract(n);", calls[0].ToString()); + + VerifyZeroDiagnostics(tree); + } + + // --------------------------------------------------------------------- + // The unified diff, pinned exactly for a minimal edit (hunk header, + // context, deletion before insertion). + // --------------------------------------------------------------------- + [Fact] + public void UnifiedDiff_pins_the_hunk_format() + { + var diff = UnifiedDiff.Diff( + "a\nb\nc\n", "a\nX\nc\nd\n", "Demo.cs", "Demo.cs (generated)"); + + Assert.Equal( + "--- Demo.cs\n" + + "+++ Demo.cs (generated)\n" + + "@@ -1,3 +1,4 @@\n" + + " a\n" + + "-b\n" + + "+X\n" + + " c\n" + + "+d\n", + diff); + } + + // --------------------------------------------------------------------- + // Identical texts produce a header with no hunks. + // --------------------------------------------------------------------- + [Fact] + public void UnifiedDiff_of_identical_texts_is_header_only() + { + Assert.Equal("--- a\n+++ b\n", UnifiedDiff.Diff("same\n", "same\n", "a", "b")); + } + + // --------------------------------------------------------------------- + + private static (MethodDeclarationSyntax Method, List Calls, SyntaxTree Tree) + Generate(int startLine, int endLine) + { + var (tree, compilation) = DemoFixture.Load(); + var model = compilation.GetSemanticModel(tree); + + var resolved = SelectionResolver.Resolve(tree, startLine, endLine); + Assert.True(resolved.Succeeded, resolved.Error); + + var suggestion = DataFlowClassifier.Classify(model, resolved); + var signature = SignatureBuilder.Build(model, resolved, suggestion); + + var refactoring = RefactoringGenerator.Generate(model, resolved, signature); + return (refactoring.NewMethod, refactoring.CallStatements.ToList(), refactoring.TransformedTree); + } + + /// + /// The yak's round-trip check: the transformed tree must compile with + /// ZERO diagnostics under the same scratch compilation the input file + /// compiles under. + /// + private static void VerifyZeroDiagnostics(SyntaxTree tree) + { + var compilation = CompilationLoader.CreateCompilation(tree, "DemoFixture"); + var diagnostics = compilation.GetDiagnostics().ToList(); + Assert.True(diagnostics.Count == 0, string.Join("\n", diagnostics)); + } +} \ No newline at end of file diff --git a/tools/ExtractMethod/Program.cs b/tools/ExtractMethod/Program.cs index 4a87268..afe0317 100644 --- a/tools/ExtractMethod/Program.cs +++ b/tools/ExtractMethod/Program.cs @@ -46,18 +46,52 @@ if (!resolved.Succeeded) } // 4. compose the full suggestion (em 02 buckets + em 03 extract-first and -// signature) and print the report. A composition failure is a semantic -// resolution error: same exit code as the resolver, message on stderr, -// but never a stack trace. +// signature), generate the refactoring (em 04), verify it and print the +// diff. Any failure is a clean error: message on stderr, same exit code +// as the resolver, never a stack trace. var model = compilation.GetSemanticModel(tree); try { var extraction = ExtractionReporter.Compose(model, resolved); Console.Write(ReportFormatter.Format(extraction)); + + // 5. generate the refactoring from the suggestion (em 04 codegen): new + // method + rewritten call site + transformed tree. + var refactoring = RefactoringGenerator.Generate(model, resolved, extraction.Signature); + + // 6. round-trip check: the generated code must compile with zero + // diagnostics (same scratch-compilation path as the input file). + var transformedCompilation = CompilationLoader.CreateCompilation( + refactoring.TransformedTree, Path.GetFileNameWithoutExtension(file)); + var diagnostics = transformedCompilation.GetDiagnostics().ToList(); + if (diagnostics.Count > 0) + { + Console.Error.WriteLine( + $"error: round-trip check failed — the generated code has {diagnostics.Count} diagnostic(s):"); + foreach (var diagnostic in diagnostics) + { + Console.Error.WriteLine($" {diagnostic}"); + } + + Console.Error.WriteLine("---- generated code ----"); + Console.Error.Write(refactoring.TransformedText); + return SelectionResolver.ExitError; + } + + // 7. v1 decision (em 04): print a unified diff to stdout — PREVIEW, never + // an in-place file rewrite. Applying the diff (or a future --write + // flag with backup semantics) is a deliberate next step, because + // mutating the caller's file is a different safety policy than + // generating code. + Console.Write(UnifiedDiff.Diff( + tree.ToString(), + refactoring.TransformedText, + Path.GetFileName(file), + Path.GetFileName(file) + " (generated)")); } catch (Exception e) when (e is InvalidOperationException or ArgumentException) { - Console.Error.WriteLine($"error: data-flow analysis failed: {e.Message}"); + Console.Error.WriteLine($"error: {e.Message}"); return SelectionResolver.ExitError; } diff --git a/tools/ExtractMethod/Tooling/RefactoringGenerator.cs b/tools/ExtractMethod/Tooling/RefactoringGenerator.cs new file mode 100644 index 0000000..7a50324 --- /dev/null +++ b/tools/ExtractMethod/Tooling/RefactoringGenerator.cs @@ -0,0 +1,399 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace ExtractMethod.Tooling; + +/// +/// The generated extract-method refactoring (yak em 04 codegen): the NEW +/// method (the em 03 promoted signature; parameter names come straight from +/// the source symbols), the CALL STATEMENTS that replace the selection in the +/// enclosing method, and the fully transformed syntax tree. +/// +/// The extracted method, ready to be read. Built +/// with SyntaxFactory and NORMALIZED (whitespace) before insertion, so its +/// text is the readable shape that lands in the file — not the trivia-less +/// builder output. +/// The statement(s) that replace the selected +/// run at the call site (one statement, or a capture + destructure pair for +/// a mixed tuple return), normalized like . +/// The whole input tree with the selection +/// moved out and the new method added next to the enclosing one — the +/// round-trip check compiles exactly this tree. +public sealed record Refactoring( + MethodDeclarationSyntax NewMethod, + IReadOnlyList CallStatements, + SyntaxTree TransformedTree) +{ + public string TransformedText => TransformedTree.ToString(); +} + +/// +/// Emits the refactoring behind the em 03 suggestion. +/// +/// WHY no identifier rewriting at all: the suggested parameter names ARE the +/// source variable names (em 02/03 promotion), so the moved statements bind +/// unchanged — every local/parameter read inside the selection is a parameter +/// of the new method, every variable declared inside moves with the body, and +/// the remaining reads are type members visible from the same class +/// (the new method is inserted into the SAME containing type). This is the +/// property the resolver (one method body, whole statements) and the +/// classifier (params bucket = exactly the local/parameter reads) jointly +/// guarantee; a generic SyntaxRewriter renaming pass would be dead weight in +/// v1. What IS generated structurally: +/// - the new method declaration (signature line, static-ness mirrored from +/// the enclosing method, body = selected statements + appended +/// return ... for return slots the selection did not already end +/// with a return of), +/// - the call statement(s) replacing the selection, +/// - the insertion of the new method right after the enclosing one. +/// +/// CALL-SITE RULES (per promoted return slots, v1): +/// - void → a plain call expression statement. A ref parameter already flows +/// the value back (e.g. Accumulate: Extract(ref acc, n);). +/// - one slot DECLARED inside the selection → redeclared at the call site +/// from the result: int total = Extract(limit);. +/// - one slot declared OUTSIDE → plain assignment: acc = Extract(...);. +/// - several slots ALL declared inside → tuple deconstruction: +/// var (x, y) = Extract(n);. +/// - several slots, any declared outside → capture + ItemN assignments +/// (the result is evaluated ONCE, then assigned member-wise). +/// - the enclosing method's return type is non-void and the selection ends +/// the method → the call site must be return Extract(...); and the +/// suggested return type must equal the enclosing one (otherwise: refuse). +/// +/// REFUSALS (loud, never a silent wrong refactoring): +/// - the selection contains a return that does NOT end the enclosing method +/// (v1: select to the end of the method); +/// - the suggested return type does not match the enclosing method's return +/// type — including the em 03 "composite trailing return" case, where the +/// signature is void although the enclosing method returns a value (the +/// report's note already says: extract a local first, then re-run). +/// +/// BUILDER STYLE: single-slot declarations and assignments are hand-built +/// with SyntaxFactory; the deconstruction call var (x, y) = ...; and +/// the tuple return return (x, y); are PARSED from a text skeleton and +/// the call slot swapped in via ReplaceNode — the parser knows the +/// designation/tuple-element shapes a v1 builder would get wrong. (Parse +/// diagnostics would surface as red nodes and fail the round-trip check +/// loudly, not silently.) +/// +/// KNOWN v1 APPROXIMATIONS: type display strings are re-parsed for the +/// generated declarations (a tuple return with a comma-bearing generic type +/// element is not supported); the capture name result could in theory +/// collide with an enclosing local; the name check "suggested return type == +/// enclosing return type" is a string comparison, not symbol equality. +/// +public static class RefactoringGenerator +{ + public static Refactoring Generate(SemanticModel model, SelectionReport selection, SignatureSuggestion signature) + { + var method = selection.Method!; + var body = method.Body!; + + var declaredInside = SignatureBuilder.DeclaredInsideNames(model, selection); + var slots = signature.ReturnSlots.ToList(); + var slotNames = slots.Select(s => s.Name).ToList(); + + var containsReturn = selection.Statements.OfType().Any(); + var selectionEndsBody = ReferenceEquals(selection.Statements[^1], body.Statements[^1]); + var enclosingNonVoid = method.ReturnType is not PredefinedTypeSyntax voidType + || !voidType.Keyword.IsKind(SyntaxKind.VoidKeyword); + var enclosingReturnType = method.ReturnType.ToString(); + + // ---- soundness refusals (see class comment) ---- + if (containsReturn && enclosingNonVoid) + { + if (!selectionEndsBody) + { + throw new InvalidOperationException( + "the selection contains a return that does not end the enclosing method — select to the end of the method (v1 codegen)"); + } + + if (signature.ReturnType != enclosingReturnType) + { + throw new InvalidOperationException( + signature.ReturnType == "void" + ? "the selection ends with the method's return, but its value is not nameable in v1 — " + + "extract it into a local first (see the report note), then re-run" + : $"the suggested return type {signature.ReturnType} does not match the enclosing method's " + + $"return type {enclosingReturnType} — refusing to generate"); + } + } + + // ---- the call expression (parameter names = source names, ref-ness kept) ---- + var arguments = signature.Params + .Select(p => + { + var argument = SyntaxFactory.Argument(SyntaxFactory.IdentifierName(p.Name)); + return p.ByRef + ? argument.WithRefOrOutKeyword(SyntaxFactory.Token(SyntaxKind.RefKeyword)) + : argument; + }) + .ToList(); + var invocation = SyntaxFactory.InvocationExpression( + SyntaxFactory.IdentifierName(signature.MethodName)) + .WithArgumentList(SyntaxFactory.ArgumentList(SyntaxFactory.SeparatedList(arguments))) + // Normalized once, here: the PARSED deconstruction skeleton (see + // BuildCallStatements) must not be re-normalized (the normalizer + // drops the space after the contextual keyword `var` before `(`), + // so it receives the invocation already properly spaced. + .NormalizeWhitespace(); + + // Normalizing only the GENERATED nodes keeps the original trivia — + // and therefore the doc comments — untouched (a whole-root + // NormalizeWhitespace corrupts doc-comment trivia in a way Roslyn's + // XML-doc writer rejects with CS1569, and it would also reformat the + // entire file, making the printed diff useless). The call statements + // and the new method get their indentation/end-of-line grafted from + // the selection they replace, so the edit slots into the original + // formatting. + var callStatements = BuildCallStatements(slots, slotNames, invocation, declaredInside, + callMustReturn: containsReturn && enclosingNonVoid).ToList(); + var indent = SyntaxFactory.TriviaList( + selection.Statements[0].GetLeadingTrivia().Where(t => t.IsKind(SyntaxKind.WhitespaceTrivia))); + for (var index = 0; index < callStatements.Count; index++) + { + var call = callStatements[index].WithLeadingTrivia(indent); + if (index == callStatements.Count - 1) + { + // The replaced statement's trailing end-of-line kept the + // following token (the next statement or the closing brace) + // on its own line — carry it over. + call = call.WithTrailingTrivia(selection.Statements[^1].GetTrailingTrivia()); + } + + callStatements[index] = call; + } + + // Normalize the new method INSIDE a throwaway class wrapper: the + // normalizer computes indentation from nesting depth, and a bare + // method node has none — its body would come out flush-left. At class + // depth the signature lands one level in and the body two. The grafted + // leading/trailing trivia below replaces whatever the wrapper put + // around the member. + var newMethod = (MethodDeclarationSyntax)SyntaxFactory.ClassDeclaration("__normalizer__") + .WithMembers(SyntaxFactory.SingletonList( + BuildNewMethod(method, selection, signature, slots, slotNames))) + .NormalizeWhitespace() + .Members[0]; + var methodIndent = method.GetLeadingTrivia() + .LastOrDefault(t => t.IsKind(SyntaxKind.WhitespaceTrivia)); + newMethod = newMethod + .WithLeadingTrivia(SyntaxFactory.TriviaList(SyntaxFactory.EndOfLine("\n"), methodIndent)) + .WithTrailingTrivia(SyntaxFactory.EndOfLine("\n")); + + // ---- apply to the tree: swap the selection for the call, add the method ---- + var allStatements = body.Statements; + var firstIndex = allStatements.IndexOf(selection.Statements[0]); // reference identity: same tree + var newStatements = new List(); + newStatements.AddRange(allStatements.Take(firstIndex)); + newStatements.AddRange(callStatements); + newStatements.AddRange(allStatements.Skip(firstIndex + selection.Statements.Count)); + var rewrittenMethod = method.WithBody(body.WithStatements(SyntaxFactory.List(newStatements))); + + // The new method lands right after the enclosing one, in the SAME type + // (that is what keeps the extract-first own-class members in scope). + var containingType = method.AncestorsAndSelf().OfType().First(); + var memberIndex = containingType.Members.IndexOf(method); + var newType = containingType.WithMembers( + containingType.Members.Replace(method, rewrittenMethod).Insert(memberIndex + 1, newMethod)); + + var tree = model.SyntaxTree; + var visited = new SingleNodeReplacer(containingType, newType).Visit(tree.GetRoot()); + if (visited is null) + { + throw new InvalidOperationException("the type replacement rewrote the root away (internal error)"); + } + + return new Refactoring(newMethod, callStatements, tree.WithRootAndOptions(visited, tree.Options)); + } + + /// + /// The one place a SyntaxRewriter enters this tool (yak em 04's stated + /// curriculum): swapping a single node by REFERENCE identity. The typed + /// With*-slices (WithBody, WithMembers) cover the local changes; only the + /// "put the rewritten type back into the root" step is a whole-tree walk. + /// + private sealed class SingleNodeReplacer : CSharpSyntaxRewriter + { + private readonly SyntaxNode _oldNode; + private readonly SyntaxNode _newNode; + + public SingleNodeReplacer(SyntaxNode oldNode, SyntaxNode newNode) + { + _oldNode = oldNode; + _newNode = newNode; + } + + public override SyntaxNode? Visit(SyntaxNode? node) + => ReferenceEquals(node, _oldNode) ? _newNode : base.Visit(node); + } + + /// The call statement(s) replacing the selection (see class comment). + private static IReadOnlyList BuildCallStatements( + IReadOnlyList slots, + IReadOnlyList slotNames, + InvocationExpressionSyntax invocation, + ISet declaredInside, + bool callMustReturn) + { + if (callMustReturn) + { + // The selection ends the enclosing (non-void) method: the call is + // the return value, the slots live in the new method. + return [Norm(SyntaxFactory.ReturnStatement(invocation))]; + } + + if (slots.Count == 0) + { + return [Norm(SyntaxFactory.ExpressionStatement(invocation))]; + } + + if (slots.Count == 1) + { + var slot = slots[0]; + if (declaredInside.Contains(slot.Name)) + { + // Declared inside the selection: redeclare it at the call site + // from the result (the declaration statement moved with the body). + var declarator = SyntaxFactory.VariableDeclarator(slot.Name) + .WithInitializer(SyntaxFactory.EqualsValueClause(invocation)); + var declaration = SyntaxFactory.VariableDeclaration( + ParseType(slot.Type), SyntaxFactory.SingletonSeparatedList(declarator)); + return [Norm(SyntaxFactory.LocalDeclarationStatement(declaration))]; + } + + // Declared outside: the caller already owns it (a parameter, or a + // local declared before the selection) — plain assignment. + return [Norm(SyntaxFactory.ExpressionStatement(SyntaxFactory.AssignmentExpression( + SyntaxKind.SimpleAssignmentExpression, + SyntaxFactory.IdentifierName(slot.Name), + invocation)))]; + } + + // Several return slots. + if (slots.All(s => declaredInside.Contains(s.Name))) + { + // All declared inside: tuple deconstruction at the call site. + // Parsed from a skeleton (see class comment — builder style), the + // call swapped in at the placeholder identifier. NOT normalized: + // the skeleton text is already well-spaced, and NormalizeWhitespace + // would drop the space after the contextual keyword `var` (it is + // an identifier token to the normalizer, so `var (` becomes + // `var(`). + return [ParseStatementWithCall($"var ({string.Join(", ", slotNames)}) = __call__;", invocation)]; + } + + // Mixed: evaluate the call ONCE into a capture, then assign each slot + // from the matching ItemN. (Capture name collision with an enclosing + // local is a documented v1 limitation.) + var statements = new List + { + Norm(SyntaxFactory.LocalDeclarationStatement(SyntaxFactory.VariableDeclaration( + SyntaxFactory.PredefinedType(SyntaxFactory.Token(SyntaxKind.VarKeyword)), + SyntaxFactory.SingletonSeparatedList(SyntaxFactory.VariableDeclarator("result") + .WithInitializer(SyntaxFactory.EqualsValueClause(invocation)))))), + }; + for (var index = 0; index < slots.Count; index++) + { + statements.Add(Norm(SyntaxFactory.ExpressionStatement(SyntaxFactory.AssignmentExpression( + SyntaxKind.SimpleAssignmentExpression, + SyntaxFactory.IdentifierName(slots[index].Name), + SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, + SyntaxFactory.IdentifierName("result"), + SyntaxFactory.IdentifierName($"Item{index + 1}")))))); + } + + return statements; + } + + /// + /// Normalizes a builder-made statement: SyntaxFactory tokens carry no + /// trivia, so without this return and Extract would render + /// glued together. Only used for BUILDER statements — the parsed skeleton + /// statements are already well-spaced (see the deconstruction comment). + /// + private static StatementSyntax Norm(StatementSyntax statement) => statement.NormalizeWhitespace(); + + /// + /// Parses (containing the placeholder + /// identifier __call__) and swaps the placeholder for the built + /// — the parser supplies the syntax shapes + /// (tuple designations, tuple elements) the v1 builder avoids. + /// + private static StatementSyntax ParseStatementWithCall(string statementText, InvocationExpressionSyntax invocation) + { + var parsed = SyntaxFactory.ParseStatement(statementText); + var placeholder = parsed.DescendantNodes() + .OfType() + .First(n => n.Identifier.ValueText == "__call__"); + return parsed.ReplaceNode(placeholder, invocation); + } + + /// + /// The extracted method: the promoted signature, static-ness mirrored from + /// the enclosing method, body = the selected statements plus an appended + /// return for the return slots (skipped when the selection already + /// ends in a return — a second one would be unreachable). The appended + /// tuple return is parsed from its text skeleton (see class comment). + /// + private static MethodDeclarationSyntax BuildNewMethod( + MethodDeclarationSyntax enclosingMethod, + SelectionReport selection, + SignatureSuggestion signature, + IReadOnlyList slots, + IReadOnlyList slotNames) + { + // A static enclosing method can only read statics and parameters, so + // the extracted body needs no instance — mirror the static modifier. + var modifiers = enclosingMethod.Modifiers.Any(m => m.IsKind(SyntaxKind.StaticKeyword)) + ? new[] { SyntaxFactory.Token(SyntaxKind.StaticKeyword) } + : Array.Empty(); + + var parameters = signature.Params + .Select(p => + { + var parameter = SyntaxFactory.Parameter(SyntaxFactory.Identifier(p.Name)) + .WithType(ParseType(p.Type)); + return p.ByRef + ? parameter.WithModifiers(SyntaxFactory.TokenList( + SyntaxFactory.Token(SyntaxKind.RefKeyword))) + : parameter; + }) + .ToList(); + + var bodyStatements = selection.Statements.ToList(); + if (slots.Count > 0 && selection.Statements[^1] is not ReturnStatementSyntax) + { + // The slot order here is the same name-ordered sequence the tuple + // ReturnType was built from, so the emitted value matches the + // declared element types. + bodyStatements.Add(slots.Count == 1 + ? SyntaxFactory.ReturnStatement(SyntaxFactory.IdentifierName(slotNames[0])) + : SyntaxFactory.ParseStatement($"return ({string.Join(", ", slotNames)});")); + } + + return SyntaxFactory.MethodDeclaration(ParseType(signature.ReturnType), SyntaxFactory.Identifier(signature.MethodName)) + .WithModifiers(SyntaxFactory.TokenList(modifiers)) + .WithParameterList(SyntaxFactory.ParameterList(SyntaxFactory.SeparatedList(parameters))) + .WithBody(SyntaxFactory.Block(SyntaxFactory.List(bodyStatements))); + } + + /// + /// Re-parses a type display string (em 02/03 are plain strings) in type + /// context — ParseTypeName handles plain names, generics and tuple + /// display strings (T1, T2) alike. + /// + /// EXCEPT void: parsing "void" in TYPE context yields a node the + /// compiler rejects as a method return type with CS1547 (keyword 'void' + /// cannot be used in this context) — the parser that reads real method + /// declarations produces PredefinedType(Token(VoidKeyword)), so + /// that is what a void signature must be built from. + /// Single choke point for the string-to-syntax hop; its correctness is + /// enforced by the round-trip check and the tests, not by construction. + /// + private static TypeSyntax ParseType(string text) => text == "void" + ? SyntaxFactory.PredefinedType(SyntaxFactory.Token(SyntaxKind.VoidKeyword)) + : SyntaxFactory.ParseTypeName(text); +} \ No newline at end of file diff --git a/tools/ExtractMethod/Tooling/SignatureBuilder.cs b/tools/ExtractMethod/Tooling/SignatureBuilder.cs index 0c4cdc1..dcfb7ea 100644 --- a/tools/ExtractMethod/Tooling/SignatureBuilder.cs +++ b/tools/ExtractMethod/Tooling/SignatureBuilder.cs @@ -10,10 +10,16 @@ namespace ExtractMethod.Tooling; /// v1 always suggests Extract. /// The coherent parameter list (ref-ness preserved from /// the classification). +/// The individual return slot candidates (name + +/// type), name-ordered — the same order the tuple +/// was built from, and the order em 04's codegen emits them in the appended +/// return (...) statement and the call-side declaration. Empty for a +/// void extraction. public sealed record SignatureSuggestion( string ReturnType, string MethodName, - IReadOnlyList Params); + IReadOnlyList Params, + IReadOnlyList ReturnSlots); /// /// Turns the raw em 02 buckets into ONE coherent signature line. The buckets @@ -66,7 +72,7 @@ public static class SignatureBuilder _ => $"({string.Join(", ", returnSlots.Select(r => r.Type))})", }; - return new SignatureSuggestion(returnType, "Extract", parameters); + return new SignatureSuggestion(returnType, "Extract", parameters, returnSlots); } /// diff --git a/tools/ExtractMethod/Tooling/UnifiedDiff.cs b/tools/ExtractMethod/Tooling/UnifiedDiff.cs new file mode 100644 index 0000000..bc28d55 --- /dev/null +++ b/tools/ExtractMethod/Tooling/UnifiedDiff.cs @@ -0,0 +1,187 @@ +using System.Text; + +namespace ExtractMethod.Tooling; + +/// +/// Line-based unified diff (v1, hand-rolled, no dependencies): an LCS over +/// lines is backtracked into equal/delete/insert runs, and the runs are +/// printed as unified hunks with 3 lines of context (hunks closer than +/// 2×context lines apart merge). The shape is the familiar +/// ---/+++/@@ one — enough for a human to review the generated +/// refactoring; no patch-file semantics are promised. +/// +public static class UnifiedDiff +{ + private const int Context = 3; + + public static string Diff(string original, string modified, string originalLabel, string modifiedLabel) + { + var a = SplitLines(original); + var b = SplitLines(modified); + + var sb = new StringBuilder($"--- {originalLabel}\n+++ {modifiedLabel}\n"); + foreach (var hunk in BuildHunks(a, b)) + { + sb.Append(hunk); + } + + return sb.ToString(); + } + + /// + /// Split on newlines, dropping a trailing carriage return (CRLF tolerance). + /// A trailing newline does NOT start a new line: "a\n" is ONE line, so the + /// phantom empty element a naive Split('\n') leaves at the end is dropped + /// (it would inflate every hunk header by one). The empty string is zero + /// lines. + /// + private static string[] SplitLines(string text) + { + if (text.Length == 0) + { + return []; + } + + var lines = text.Split('\n').Select(line => line.TrimEnd('\r')).ToArray(); + return lines[^1].Length == 0 ? lines[..^1] : lines; + } + + private enum Op { Equal, Del, Ins } + + /// + /// LCS + backtracking + hunk assembly. lcs[i, j] is the length of + /// the longest common suffix of a[i..] and b[j..]. + /// + private static IReadOnlyList BuildHunks(string[] a, string[] b) + { + var n = a.Length; + var m = b.Length; + + var lcs = new int[n + 1, m + 1]; + for (var row = n - 1; row >= 0; row--) + { + for (var col = m - 1; col >= 0; col--) + { + lcs[row, col] = a[row] == b[col] + ? lcs[row + 1, col + 1] + 1 + : Math.Max(lcs[row + 1, col], lcs[row, col + 1]); + } + } + + // Backtrack: equal lines advance both; on a mismatch prefer a deletion + // (ties go to delete, which keeps the printed block old-then-new). + var ops = new List<(Op Tag, int A, int B)>(); + var i = 0; + var j = 0; + while (i < n && j < m) + { + if (a[i] == b[j]) + { + ops.Add((Op.Equal, i, j)); + i++; + j++; + } + else if (lcs[i + 1, j] >= lcs[i, j + 1]) + { + ops.Add((Op.Del, i, j)); + i++; + } + else + { + ops.Add((Op.Ins, i, j)); + j++; + } + } + + while (i < n) + { + ops.Add((Op.Del, i, j)); + i++; + } + + while (j < m) + { + ops.Add((Op.Ins, i, j)); + j++; + } + + // A "region" is a maximal run of non-equal ops: [AStart, AEnd) × + // [BStart, BEnd) in the two line arrays. + var regions = new List<(int AStart, int AEnd, int BStart, int BEnd)>(); + var index = 0; + while (index < ops.Count) + { + if (ops[index].Tag == Op.Equal) + { + index++; + continue; + } + + var first = ops[index]; + var regionAStart = first.Tag == Op.Del ? first.A : (index > 0 ? ops[index - 1].A + 1 : 0); + var regionBStart = first.B; + while (index < ops.Count && ops[index].Tag != Op.Equal) + { + index++; + } + + var last = ops[index - 1]; + regions.Add((regionAStart, + last.Tag == Op.Del ? last.A + 1 : last.A, + regionBStart, + last.Tag == Op.Ins ? last.B + 1 : last.B)); + } + + // Extend every region by Context lines (clamped), then merge regions + // whose extended ranges overlap. + var hunks = new List<(int AStart, int AEnd, int BStart, int BEnd)>(); + foreach (var (aStart, aEnd, bStart, bEnd) in regions) + { + var (haStart, haEnd) = (Math.Max(0, aStart - Context), Math.Min(n, aEnd + Context)); + var (hbStart, hbEnd) = (Math.Max(0, bStart - Context), Math.Min(m, bEnd + Context)); + if (hunks.Count > 0) + { + var prev = hunks[^1]; + if (haStart <= prev.AEnd && hbStart <= prev.BEnd) + { + hunks[^1] = (prev.AStart, Math.Max(prev.AEnd, haEnd), prev.BStart, Math.Max(prev.BEnd, hbEnd)); + continue; + } + } + + hunks.Add((haStart, haEnd, hbStart, hbEnd)); + } + + return hunks.Select(hunk => RenderHunk(ops, a, b, hunk.AStart, hunk.AEnd, hunk.BStart, hunk.BEnd)).ToList(); + } + + private static string RenderHunk( + IReadOnlyList<(Op Tag, int A, int B)> ops, + string[] a, + string[] b, + int aStart, + int aEnd, + int bStart, + int bEnd) + { + var sb = new StringBuilder(); + sb.AppendLine($"@@ -{aStart + 1},{aEnd - aStart} +{bStart + 1},{bEnd - bStart} @@"); + foreach (var (tag, aIndex, bIndex) in ops) + { + switch (tag) + { + case Op.Equal when aStart <= aIndex && aIndex < aEnd: + sb.Append(' ').AppendLine(a[aIndex]); + break; + case Op.Del when aStart <= aIndex && aIndex < aEnd: + sb.Append('-').AppendLine(a[aIndex]); + break; + case Op.Ins when bStart <= bIndex && bIndex < bEnd: + sb.Append('+').AppendLine(b[bIndex]); + break; + } + } + + return sb.ToString(); + } +}