Adaptation to breaking FCS changes in sdk 9.0 and 10.0 #3167
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| { | ||
| "sdk": { | ||
| "version": "9.0.300", | ||
| "rollForward": "latestPatch" | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -248,6 +248,7 @@ let ``#help without string`` () = | |
| #help List.map | ||
| """ | ||
|
|
||
| // As of F# 10.0, warn directives are treated as trivia like #if, so argruments are not formatted | ||
| [<Test>] | ||
| let ``#nowarn with integer`` () = | ||
| formatSourceString | ||
|
|
@@ -259,5 +260,5 @@ let ``#nowarn with integer`` () = | |
| |> should | ||
| equal | ||
| """ | ||
| #nowarn 1182 | ||
|
Contributor
There was a problem hiding this comment. Hmm, seems like a step back, though.
Author
There was a problem hiding this comment. Warn directives are now handled exactly like conditional directives as whole-line trivia.
Contributor
There was a problem hiding this comment. Well, yes, but you don't have the
Author
There was a problem hiding this comment. I had the numbers actually at some point (as int only, as they are no longer parsed by the parser). But when I saw how the conditional directives are treated, I went for the same simple whole-line-trivia approach. After all, both conditional and warn directives are preprocessor directives that are not really part of the proper syntax tree.
Contributor
There was a problem hiding this comment. I can't say I really follow here, looking at this example, the AST does have proper nodes for the expression.
Author
There was a problem hiding this comment. Yes, but I believe the expressions are only used to direct the computation of the different trees.
Contributor
There was a problem hiding this comment. Interesting, I assumed wrongly that we were using the AST node to actually print the hash directive expressions, looks like we indeed took a shortcut and took the original source. We would look at the nodes honestly. Anyway, circling back to the new thing, how are you getting the values of the
Author
There was a problem hiding this comment. Long story :-).
Author
There was a problem hiding this comment. I added the WarnDirectives entry in the AST only for Fantomas. (Again, like the ConditionalDirectives entry.) |
||
| """ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -966,22 +966,22 @@ let mkExpr (creationAide: CreationAide) (e: SynExpr) : Expr = | |
| | SynExpr.AddressOf(false, e, _, StartRange 2 (ampersandToken, _range)) -> | ||
| ExprSingleNode(stn "&&" ampersandToken, false, false, mkExpr creationAide e, exprRange) | ||
| |> Expr.Single | ||
| | SynExpr.YieldOrReturn((true, _), e, StartRange 5 (yieldKeyword, _range), _) -> | ||
Martin521 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| ExprSingleNode(stn "yield" yieldKeyword, true, true, mkExpr creationAide e, exprRange) | ||
| |> Expr.Single | ||
| | SynExpr.YieldOrReturn((false, _), e, StartRange 6 (returnKeyword, _range), _) -> | ||
| ExprSingleNode(stn "return" returnKeyword, true, true, mkExpr creationAide e, exprRange) | ||
| |> Expr.Single | ||
| | SynExpr.YieldOrReturnFrom((true, _), e, StartRange 6 (yieldBangKeyword, _range), _) -> | ||
| ExprSingleNode(stn "yield!" yieldBangKeyword, true, true, mkExpr creationAide e, exprRange) | ||
| |> Expr.Single | ||
| | SynExpr.YieldOrReturnFrom((false, _), e, StartRange 7 (returnBangKeyword, _range), _) -> | ||
| ExprSingleNode(stn "return!" returnBangKeyword, true, true, mkExpr creationAide e, exprRange) | ||
| |> Expr.Single | ||
| | SynExpr.Do(e, StartRange 2 (doKeyword, _range)) -> | ||
| ExprSingleNode(stn "do" doKeyword, true, true, mkExpr creationAide e, exprRange) | ||
| |> Expr.Single | ||
| | SynExpr.DoBang(e, StartRange 3 (doBangKeyword, _range), _) -> | ||
| ExprSingleNode(stn "do!" doBangKeyword, true, true, mkExpr creationAide e, exprRange) | ||
| |> Expr.Single | ||
| | SynExpr.Fixed(e, StartRange 5 (fixedKeyword, _range)) -> | ||
|
|
@@ -1029,9 +1029,7 @@ let mkExpr (creationAide: CreationAide) (e: SynExpr) : Expr = | |
| let fieldNodes = | ||
| recordFields | ||
| |> List.choose (function | ||
| | SynExprRecordField((fieldName, _), Some mEq, Some expr, m, _) -> | ||
| Some( | ||
| RecordFieldNode(mkSynLongIdent creationAide fieldName, stn "=" mEq, mkExpr creationAide expr, m) | ||
| ) | ||
|
|
@@ -1144,7 +1142,7 @@ let mkExpr (creationAide: CreationAide) (e: SynExpr) : Expr = | |
| _, | ||
| pat, | ||
| e1, | ||
| SynExpr.YieldOrReturn((true, _), e2, _, _), | ||
| StartRange 3 (mFor, _)) -> | ||
| ExprForEachNode( | ||
| stn "for" mFor, | ||
|
|
@@ -2809,7 +2807,7 @@ let mkMemberDefn (creationAide: CreationAide) (md: SynMemberDefn) = | |
| let memberDefinitionRange = md.Range | ||
|
|
||
| match md with | ||
| | SynMemberDefn.ImplicitInherit(t, e, _, StartRange 7 (mInherit, _), _) -> | ||
|
||
| mkInheritConstructor creationAide t e mInherit memberDefinitionRange | ||
| |> MemberDefn.ImplicitInherit | ||
|
|
||
|
|
@@ -2870,9 +2868,12 @@ let mkMemberDefn (creationAide: CreationAide) (md: SynMemberDefn) = | |
| ) | ||
| |> MemberDefn.ExplicitCtor | ||
| | SynMemberDefn.Member(memberDefn, _) -> mkBinding creationAide memberDefn |> MemberDefn.Member | ||
| | SynMemberDefn.Inherit(baseTypeOpt, _, StartRange 7 (mInherit, _), _) -> | ||
| match baseTypeOpt with | ||
| | Some baseType -> | ||
| MemberDefnInheritNode(stn "inherit" mInherit, mkType creationAide baseType, memberDefinitionRange) | ||
| |> MemberDefn.Inherit | ||
| | None -> failwith "successful parse shouldn't have any unfinished inherit" | ||
| | SynMemberDefn.ValField(f, _) -> mkSynField creationAide f |> MemberDefn.ValField | ||
| | SynMemberDefn.LetBindings( | ||
| bindings = [ SynBinding(trivia = { LeadingKeyword = SynLeadingKeyword.Extern _ }) as binding ]) -> | ||
|
|
@@ -3598,25 +3599,27 @@ let mkSigFile | |
| let mds = List.map (mkModuleOrNamespaceSig creationAide) contents | ||
| Oak(phds, mds, m) | ||
|
|
||
| let includeTrivia (baseRange: range) (trivia: ParsedInputTrivia) : range = | ||
| let ranges = | ||
| [ yield! | ||
| List.map | ||
| (function | ||
| | CommentTrivia.LineComment m | ||
| | CommentTrivia.BlockComment m -> m) | ||
| trivia.CodeComments | ||
| yield! | ||
| List.map | ||
| (function | ||
| | ConditionalDirectiveTrivia.If(range = range) | ||
| | ConditionalDirectiveTrivia.Else(range = range) | ||
| | ConditionalDirectiveTrivia.EndIf(range = range) -> range) | ||
| trivia.ConditionalDirectives | ||
| yield! | ||
| List.map | ||
| (function | ||
| | WarnDirectiveTrivia.Nowarn range | ||
| | WarnDirectiveTrivia.Warnon range -> range) | ||
| trivia.WarnDirectives ] | ||
|
|
||
| (baseRange, ranges) | ||
| ||> List.fold (fun acc triviaRange -> | ||
|
|
@@ -3668,7 +3671,7 @@ let mkFullTreeRange ast = | |
| | Some lastModule -> mkSynModuleOrNamespaceFullRange lastModule | ||
|
|
||
| let astRange = unionRanges startPos endPos | ||
| includeTrivia astRange trivia | ||
|
|
||
| | ParsedInput.SigFile(ParsedSigFileInput(hashDirectives = directives; contents = modules; trivia = trivia)) -> | ||
| let startPos = | ||
|
|
@@ -3688,7 +3691,7 @@ let mkFullTreeRange ast = | |
| | Some lastModule -> mkSynModuleOrNamespaceSigFullRange lastModule | ||
|
|
||
| let astRange = unionRanges startPos endPos | ||
| includeTrivia astRange trivia | ||
|
|
||
| let mkOak (sourceText: ISourceText option) (ast: ParsedInput) = | ||
| let creationAide = { SourceText = sourceText } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I no longer remember how this stuff works, but can we stick to dotnet 8 SDK and versions? Or do you really need 9?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Due to the new nullness-related constructs, compiling the current compiler service requires 9.0 or higher for both fsc and FSharp.Core.
That's why I wrote above that this PR might be a draft one for quite some time.
I am assuming you might want to wait for the LTS SDK 10.
Regarding features I don't think there is a big pressure to move. The current Fantomas can deal with sources that contain nullness constructs or #warnon.
It is just that the changes accumulate. I naively just wanted to make the changes for "scoped nowarn" but found I had to deal with three other breaking changes also.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have any more details on that? What exactly in FSharp.Core 9 does any of this depend on?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is all about nullness.
If I compile with SDK 9.0.300 and FSharp.Core 8.0.100, I get 85 errors in the Fantomas.FCS project, mostly about unknown
objnullandNonNull.