Skip to content

Commit fb6c03d

Browse files
xperiandriclaude
xperiandri
and
claude
committed
Add a refactoring between reference and struct tuples
Ctrl+. on a tuple expression, pattern or annotated tuple type converts it to the other kind and follows the value through the solution: annotations of values, parameters, record fields and function results it flows through, tuple patterns taking it apart, and the arguments and values flowing into it. Uses that cannot be followed (fst, snd, generic collections) are left for the compiler to report. CreateWithCodeAndDependency now tells FCS about both files, so the second file can be type-checked. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 17f8eb7 commit fb6c03d

22 files changed

Lines changed: 1363 additions & 1 deletion

‎docs/release-notes/.VisualStudio/18.vNext.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
* Code-fixes for FS3888 (compiler-semantic attribute on the `.fs` but not the `.fsi`): copy the attribute into the `.fsi`, or remove it from the `.fs`. ([Issue #19560](https://github.com/dotnet/fsharp/issues/19560), [PR #19880](https://github.com/dotnet/fsharp/pull/19880))
44
* Expand `<inheritdoc/>` in IDE tooltips, completion, and signature help, inheriting XML documentation from base classes, interfaces, overridden members, and constructors. ([Issue #19175](https://github.com/dotnet/fsharp/issues/19175), [PR #19188](https://github.com/dotnet/fsharp/pull/19188))
5+
* Refactoring to convert a tuple between a reference tuple and a struct tuple, following its value through the solution: annotations of values, parameters, record fields and results it flows through, tuple patterns that take it apart, and the arguments and values that flow into it. What cannot be followed (`fst`, `snd`, generic collections) is left for the compiler to report.
56

67
### Fixed
78

‎vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,9 @@
105105
<Compile Include="Refactor\ChangeTypeofWithNameToNameofExpression.fs" />
106106
<Compile Include="Refactor\AddExplicitTypeToParameter.fs" />
107107
<Compile Include="Refactor\ChangeDerefToValueRefactoring.fs" />
108+
<Compile Include="Refactor\TupleConversion.fs" />
109+
<Compile Include="Refactor\TuplePropagation.fs" />
110+
<Compile Include="Refactor\ConvertTuple.fs" />
108111
<Compile Include="CodeFixes\IFSharpCodeFix.fs" />
109112
<Compile Include="CodeFixes\CodeFixHelpers.fs" />
110113
<Compile Include="CodeFixes\ChangeEqualsInFieldTypeToColon.fs" />

‎vsintegration/src/FSharp.Editor/FSharp.Editor.resx‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,4 +368,10 @@ Use live (unsaved) buffers for analysis</value>
368368
<data name="ReturnsHeader" xml:space="preserve">
369369
<value>Returns:</value>
370370
</data>
371+
<data name="ConvertToStructTuple" xml:space="preserve">
372+
<value>Convert to struct tuple</value>
373+
</data>
374+
<data name="ConvertToReferenceTuple" xml:space="preserve">
375+
<value>Convert to reference tuple</value>
376+
</data>
371377
</root>

‎vsintegration/src/FSharp.Editor/Refactor/ConvertTuple.fs‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information.
2+
3+
namespace Microsoft.VisualStudio.FSharp.Editor
4+
5+
open System
6+
open System.Composition
7+
open System.Threading
8+
open System.Threading.Tasks
9+
10+
open Microsoft.CodeAnalysis
11+
open Microsoft.CodeAnalysis.CodeActions
12+
open Microsoft.CodeAnalysis.CodeRefactorings
13+
14+
open FSharp.Compiler.Syntax
15+
open FSharp.Compiler.Text
16+
17+
open CancellableTasks
18+
open TupleConversion
19+
20+
[<ExportCodeRefactoringProvider(FSharpConstants.FSharpLanguageName, Name = "ConvertTuple"); Shared>]
21+
type internal FSharpConvertTupleRefactoring [<ImportingConstructor>] () =
22+
inherit CodeRefactoringProvider()
23+
24+
static let hasSignatureFile (document: Document) =
25+
let signaturePath = document.FilePath + "i"
26+
27+
document.Project.Documents
28+
|> Seq.exists (fun d -> String.Equals(d.FilePath, signaturePath, StringComparison.OrdinalIgnoreCase))
29+
30+
static let isInQuotation (caretNode: CaretNode) =
31+
let path =
32+
match caretNode with
33+
| CaretNode.Expr(path = path)
34+
| CaretNode.Pat(path = path)
35+
| CaretNode.Type(annotated = Annotated.Pattern(path = path))
36+
| CaretNode.Type(annotated = Annotated.Expression(path = path)) -> path
37+
| CaretNode.Type _ -> []
38+
39+
path
40+
|> List.exists (function
41+
| SyntaxNode.SynExpr(SynExpr.Quote _) -> true
42+
| _ -> false)
43+
44+
override _.ComputeRefactoringsAsync context =
45+
cancellableTask {
46+
let document = context.Document
47+
48+
if not (document.IsFSharpSignatureFile || hasSignatureFile document) then
49+
let! cancellationToken = CancellableTask.getCancellationToken ()
50+
let! sourceText = document.GetTextAsync cancellationToken
51+
let! parseResults = document.GetFSharpParseResultsAsync(nameof FSharpConvertTupleRefactoring)
52+
53+
let caret =
54+
let linePosition = sourceText.Lines.GetLinePosition context.Span.Start
55+
Position.mkPos (Line.fromZ linePosition.Line) linePosition.Character
56+
57+
match tryCaretNode caret parseResults.ParseTree with
58+
| ValueSome caretNode when not (isInQuotation caretNode) ->
59+
let title =
60+
if isStructNode caretNode then
61+
SR.ConvertToReferenceTuple()
62+
else
63+
SR.ConvertToStructTuple()
64+
65+
let changedSolution =
66+
cancellableTask {
67+
let! converted = TuplePropagation.tryConvert document caretNode (nameof FSharpConvertTupleRefactoring)
68+
69+
return
70+
match converted with
71+
| ValueSome solution -> solution
72+
| ValueNone -> document.Project.Solution
73+
}
74+
75+
let action =
76+
CodeAction.Create(
77+
title,
78+
Func<CancellationToken, Task<Solution>>(fun cancellationToken ->
79+
CancellableTask.start cancellationToken changedSolution),
80+
title
81+
)
82+
83+
context.RegisterRefactoring action
84+
| _ -> ()
85+
}
86+
|> CancellableTask.startAsTask context.CancellationToken

‎vsintegration/src/FSharp.Editor/Refactor/TupleConversion.fs‎

Lines changed: 224 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,224 @@
1+
// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information.
2+
3+
module internal Microsoft.VisualStudio.FSharp.Editor.TupleConversion
4+
5+
open System
6+
7+
open Microsoft.CodeAnalysis.Text
8+
9+
open FSharp.Compiler.Syntax
10+
open FSharp.Compiler.Text
11+
12+
let spanOf (sourceText: SourceText) (m: range) =
13+
RoslynHelpers.FSharpRangeToTextSpan(sourceText, m)
14+
15+
let isSame (node: 'T) (other: 'T) = obj.ReferenceEquals(node, other)
16+
17+
let containsPos (m: range) (position: pos) =
18+
Position.posGeq position m.Start && Position.posGeq m.End position
19+
20+
let rec stripParenTypes (ty: SynType) =
21+
match ty with
22+
| SynType.Paren(innerType = inner) -> stripParenTypes inner
23+
| _ -> ty
24+
25+
/// `struct` and the blanks after it, at the start of a struct tuple's range.
26+
let private structKeyword (sourceText: SourceText) (m: range) =
27+
let start = (spanOf sourceText m).Start
28+
let mutable finish = start + "struct".Length
29+
30+
while finish < sourceText.Length && Char.IsWhiteSpace sourceText[finish] do
31+
finish <- finish + 1
32+
33+
TextSpan.FromBounds(start, finish)
34+
35+
/// The position of the `(` that, with its `)`, encloses only the span and blanks.
36+
let private tryEnclosingParen (sourceText: SourceText) (span: TextSpan) =
37+
let mutable before = span.Start - 1
38+
39+
while before >= 0 && Char.IsWhiteSpace sourceText[before] do
40+
before <- before - 1
41+
42+
let mutable after = span.End
43+
44+
while after < sourceText.Length && Char.IsWhiteSpace sourceText[after] do
45+
after <- after + 1
46+
47+
if
48+
before >= 0
49+
&& after < sourceText.Length
50+
&& sourceText[before] = '('
51+
&& sourceText[after] = ')'
52+
then
53+
ValueSome before
54+
else
55+
ValueNone
56+
57+
/// Whether the text between start and finish is a whole generic argument: `<` or `,` before it, `>` or `,` after.
58+
let private isGenericArgument (sourceText: SourceText) (start: int) (finish: int) =
59+
let mutable before = start - 1
60+
61+
while before >= 0 && Char.IsWhiteSpace sourceText[before] do
62+
before <- before - 1
63+
64+
let mutable after = finish
65+
66+
while after < sourceText.Length && Char.IsWhiteSpace sourceText[after] do
67+
after <- after + 1
68+
69+
before >= 0
70+
&& after < sourceText.Length
71+
&& (sourceText[before] = '<' || sourceText[before] = ',')
72+
&& (sourceText[after] = '>' || sourceText[after] = ',')
73+
74+
/// Changes giving a tuple type the target kind; a whole annotation also loses the parentheses `struct` needed.
75+
let typeChanges (sourceText: SourceText) (toStruct: bool) (isWholeAnnotation: bool) (tupleType: SynType) =
76+
match tupleType with
77+
| SynType.Tuple(isStruct = isStruct; range = m) when isStruct <> toStruct ->
78+
let span = spanOf sourceText m
79+
80+
if toStruct then
81+
match tryEnclosingParen sourceText span with
82+
| ValueSome openParen -> [ TextChange(TextSpan(openParen, 0), "struct ") ]
83+
| ValueNone ->
84+
[
85+
TextChange(TextSpan(span.Start, 0), "struct (")
86+
TextChange(TextSpan(span.End, 0), ")")
87+
]
88+
else
89+
let keyword = structKeyword sourceText m
90+
91+
if isWholeAnnotation || isGenericArgument sourceText keyword.Start span.End then
92+
[
93+
TextChange(TextSpan(keyword.Start, keyword.Length + 1), "")
94+
TextChange(TextSpan(span.End - 1, 1), "")
95+
]
96+
else
97+
[ TextChange(keyword, "") ]
98+
| _ -> []
99+
100+
/// Whether the tuple is the argument list of a method, constructor or union case call rather than a tuple value.
101+
let isArgumentList (tuple: SynExpr) (path: SyntaxVisitorPath) =
102+
match path with
103+
| SyntaxNode.SynExpr(SynExpr.Paren(expr = inner) as paren) :: SyntaxNode.SynExpr(SynExpr.App(flag = ExprAtomicFlag.Atomic; argExpr = arg) | SynExpr.New(
104+
expr = arg)) :: _ -> isSame inner tuple && isSame arg paren
105+
| _ -> false
106+
107+
/// Changes giving a tuple expression the target kind; ValueNone when it is not a tuple or cannot change in place.
108+
let tryExprChanges (sourceText: SourceText) (toStruct: bool) (tuple: SynExpr) (path: SyntaxVisitorPath) =
109+
match tuple with
110+
| SynExpr.Tuple(isStruct = isStruct) when isStruct = toStruct -> ValueSome []
111+
| SynExpr.Tuple(range = m) when not toStruct -> ValueSome [ TextChange(structKeyword sourceText m, "") ]
112+
| SynExpr.Tuple(range = m) ->
113+
match path with
114+
| SyntaxNode.SynExpr(SynExpr.Paren(expr = inner; range = parenRange)) :: _ when isSame inner tuple ->
115+
ValueSome [ TextChange(TextSpan((spanOf sourceText parenRange).Start, 0), "struct ") ]
116+
| _ when m.StartLine = m.EndLine ->
117+
let span = spanOf sourceText m
118+
119+
ValueSome
120+
[
121+
TextChange(TextSpan(span.Start, 0), "struct (")
122+
TextChange(TextSpan(span.End, 0), ")")
123+
]
124+
| _ -> ValueNone
125+
| _ -> ValueNone
126+
127+
/// Changes giving a tuple pattern the target kind; ValueNone when it is not a tuple or cannot change in place.
128+
let tryPatChanges (sourceText: SourceText) (toStruct: bool) (tuple: SynPat) (path: SyntaxVisitorPath) =
129+
match tuple with
130+
| SynPat.Tuple(isStruct = isStruct) when isStruct = toStruct -> ValueSome []
131+
| SynPat.Tuple(range = m) when not toStruct -> ValueSome [ TextChange(structKeyword sourceText m, "") ]
132+
| SynPat.Tuple(range = m) ->
133+
match path with
134+
| SyntaxNode.SynPat(SynPat.Paren(pat = inner; range = parenRange)) :: _ when isSame inner tuple ->
135+
ValueSome [ TextChange(TextSpan((spanOf sourceText parenRange).Start, 0), "struct ") ]
136+
| _ when m.StartLine = m.EndLine ->
137+
let span = spanOf sourceText m
138+
139+
ValueSome
140+
[
141+
TextChange(TextSpan(span.Start, 0), "struct (")
142+
TextChange(TextSpan(span.End, 0), ")")
143+
]
144+
| _ -> ValueNone
145+
| _ -> ValueNone
146+
147+
/// The innermost tuple type within the type that contains the position.
148+
let rec tryTupleTypeAt (position: pos) (ty: SynType) =
149+
if not (containsPos ty.Range position) then
150+
ValueNone
151+
else
152+
let inner =
153+
match ty with
154+
| SynType.Paren(innerType = inner)
155+
| SynType.Array(elementType = inner)
156+
| SynType.WithGlobalConstraints(typeName = inner) -> tryTupleTypeAt position inner
157+
| SynType.App(typeName = typeName; typeArgs = typeArgs)
158+
| SynType.LongIdentApp(typeName = typeName; typeArgs = typeArgs) ->
159+
typeName :: typeArgs |> Seq.tryPickV (tryTupleTypeAt position)
160+
| SynType.Fun(argType = argType; returnType = returnType) -> [ argType; returnType ] |> Seq.tryPickV (tryTupleTypeAt position)
161+
| SynType.Tuple(path = segments) ->
162+
segments
163+
|> Seq.tryPickV (function
164+
| SynTupleTypeSegment.Type element -> tryTupleTypeAt position element
165+
| _ -> ValueNone)
166+
| _ -> ValueNone
167+
168+
match inner, ty with
169+
| ValueSome _, _ -> inner
170+
| ValueNone, SynType.Tuple _ -> ValueSome ty
171+
| ValueNone, _ -> ValueNone
172+
173+
/// What a tuple type under the caret annotates.
174+
[<RequireQualifiedAccess; NoComparison; NoEquality>]
175+
type Annotated =
176+
| Pattern of pat: SynPat * path: SyntaxVisitorPath
177+
| Expression of expr: SynExpr * path: SyntaxVisitorPath
178+
| Return of binding: SynBinding
179+
| Field of field: SynField
180+
181+
[<RequireQualifiedAccess; NoComparison; NoEquality>]
182+
type CaretNode =
183+
| Expr of tuple: SynExpr * path: SyntaxVisitorPath
184+
| Pat of tuple: SynPat * path: SyntaxVisitorPath
185+
| Type of tuple: SynType * annotated: Annotated * isWholeAnnotation: bool
186+
187+
let isStructNode (node: CaretNode) =
188+
match node with
189+
| CaretNode.Expr(tuple = SynExpr.Tuple(isStruct = isStruct))
190+
| CaretNode.Pat(tuple = SynPat.Tuple(isStruct = isStruct))
191+
| CaretNode.Type(tuple = SynType.Tuple(isStruct = isStruct)) -> isStruct
192+
| _ -> false
193+
194+
/// The innermost tuple expression, pattern or annotated tuple type under the caret.
195+
let tryCaretNode (caret: pos) (parseTree: ParsedInput) =
196+
let annotationAt (annotation: SynType) (annotated: Annotated) =
197+
tryTupleTypeAt caret annotation
198+
|> ValueOption.map (fun tuple -> CaretNode.Type(tuple, annotated, isSame (stripParenTypes annotation) tuple))
199+
200+
(ValueNone, parseTree)
201+
||> ParsedInput.fold (fun found path node ->
202+
match node with
203+
| SyntaxNode.SynExpr(SynExpr.Tuple(range = m) as tuple) when containsPos m caret && not (isArgumentList tuple path) ->
204+
ValueSome(CaretNode.Expr(tuple, path))
205+
| SyntaxNode.SynPat(SynPat.Tuple(range = m) as tuple) when containsPos m caret -> ValueSome(CaretNode.Pat(tuple, path))
206+
| SyntaxNode.SynPat(SynPat.Typed(targetType = annotation) as pat) when containsPos annotation.Range caret ->
207+
annotationAt annotation (Annotated.Pattern(pat, path))
208+
|> ValueOption.orElse found
209+
| SyntaxNode.SynExpr(SynExpr.Typed(targetType = annotation) as expr) when containsPos annotation.Range caret ->
210+
annotationAt annotation (Annotated.Expression(expr, path))
211+
|> ValueOption.orElse found
212+
| SyntaxNode.SynBinding(SynBinding(returnInfo = Some(SynBindingReturnInfo(typeName = annotation))) as binding) when
213+
containsPos annotation.Range caret
214+
->
215+
annotationAt annotation (Annotated.Return binding) |> ValueOption.orElse found
216+
| SyntaxNode.SynTypeDefn(SynTypeDefn(
217+
typeRepr = SynTypeDefnRepr.Simple(SynTypeDefnSimpleRepr.Record(recordFieldsAndSpreads = fields), _))) ->
218+
fields
219+
|> Seq.tryPickV (function
220+
| SynFieldOrSpread.Field(SynField(fieldType = annotation) as field) when containsPos annotation.Range caret ->
221+
annotationAt annotation (Annotated.Field field)
222+
| _ -> ValueNone)
223+
|> ValueOption.orElse found
224+
| _ -> found)

0 commit comments

Comments
 (0)