From d18cc65a46e8ad9d29a7f1c6c21702551aefc049 Mon Sep 17 00:00:00 2001 From: Florian Verdonck Date: Mon, 19 Jun 2023 16:54:04 +0200 Subject: [PATCH] Use correct visibility of get/set property and member. (#2903) * Move the visibility of a get/set property to the member. * Add issue number * Revert changes. * Verify if the accessibility is part of the PropertyGetSetBindingNode or the parent MemberDefnPropertyGetSetNode. * Add changelog entry. * Use rangeBeforePos from FCS to determine what came before what. --- CHANGELOG.md | 5 ++ src/Fantomas.Core.Tests/StructTests.fs | 112 +++++++++++++++++++++++++ src/Fantomas.Core/ASTTransformer.fs | 34 ++++++-- 3 files changed, 144 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 56c61eea7a..60c0598737 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## [6.0.6] - 2023-06-19 + +### Fixed +* Setter is also private. [#2902](https://github.com/fsprojects/fantomas/issues/2902) + ## [6.0.5] - 2023-06-06 ### Fixed diff --git a/src/Fantomas.Core.Tests/StructTests.fs b/src/Fantomas.Core.Tests/StructTests.fs index fc6152e0d0..1e15689463 100644 --- a/src/Fantomas.Core.Tests/StructTests.fs +++ b/src/Fantomas.Core.Tests/StructTests.fs @@ -189,3 +189,115 @@ struct // 1 // 5 |} // 6 """ + +[] +let ``second binding does not contain accessibility, 2902`` () = + formatSourceString + false + """ +module Telplin + +type T = + struct + member private this.X with get () : int = 1 and set (_:int) = () + end +""" + config + |> prepend newline + |> should + equal + """ +module Telplin + +type T = + struct + member private this.X + with get (): int = 1 + and set (_: int) = () + end +""" + +[] +let ``different accessibility on setter`` () = + formatSourceString + false + """ +module Telplin + +type T = + struct + member this.X with get () : int = 1 and private set (_:int) = () + member this.Y with internal get () : int = 1 and private set (_:int) = () + member private this.Z with get () : int = 1 and set (_:int) = () + member this.S with internal set (_:int) = () and private get () : int = 1 + end +""" + config + |> prepend newline + |> should + equal + """ +module Telplin + +type T = + struct + member this.X + with get (): int = 1 + and private set (_: int) = () + + member this.Y + with internal get (): int = 1 + and private set (_: int) = () + + member private this.Z + with get (): int = 1 + and set (_: int) = () + + member this.S + with internal set (_: int) = () + and private get (): int = 1 + end +""" + +[] +let ``private setter on next line`` () = + formatSourceString + false + """ +type Y = + member this.X + with get (): int = 1 + and private set (_: int) = () +""" + config + |> prepend newline + |> should + equal + """ +type Y = + member this.X + with get (): int = 1 + and private set (_: int) = () +""" + +[] +let ``private property with identifier on next line`` () = + formatSourceString + false + """ +type Y = + member private + this.X + with get (): int = 1 + and set (_: int) = () +""" + config + |> prepend newline + |> should + equal + """ +type Y = + member private this.X + with get (): int = 1 + and set (_: int) = () +""" diff --git a/src/Fantomas.Core/ASTTransformer.fs b/src/Fantomas.Core/ASTTransformer.fs index 1c02a27f01..3d1d316db6 100644 --- a/src/Fantomas.Core/ASTTransformer.fs +++ b/src/Fantomas.Core/ASTTransformer.fs @@ -2566,7 +2566,8 @@ let mkPropertyGetSetBinding : PropertyGetSetBindingNode = match binding with | SynBinding( - headPat = SynPat.LongIdent(extraId = Some extraIdent; accessibility = ao; argPats = SynArgPats.Pats ps) + headPat = SynPat.LongIdent( + longDotId = lid; extraId = Some extraIdent; accessibility = ao; argPats = SynArgPats.Pats ps) returnInfo = returnInfo expr = expr trivia = { EqualsRange = Some mEq @@ -2574,6 +2575,15 @@ let mkPropertyGetSetBinding let e = parseExpressionInSynBinding returnInfo expr let returnTypeNode = mkBindingReturnInfo creationAide returnInfo + // Only use the accessibility of the property binding if the keyword came after the member identifier. + let accessibility = + ao + |> Option.bind (fun vis -> + if rangeBeforePos lid.Range vis.Range.Start then + Some vis + else + None) + let pats = match ps with | [ SynPat.Tuple(false, [ p1; p2; p3 ], [ comma ], _) ] -> @@ -2601,7 +2611,7 @@ let mkPropertyGetSetBinding PropertyGetSetBindingNode( Option.map (stn "inline") inlineKw, - mkSynAccess ao, + mkSynAccess accessibility, leadingKeyword, pats, returnTypeNode, @@ -2763,32 +2773,42 @@ let mkMemberDefn (creationAide: CreationAide) (md: SynMemberDefn) = ) |> MemberDefn.AbstractSlot | SynMemberDefn.GetSetMember(Some(SynBinding( - accessibility = ao attributes = ats xmlDoc = px - headPat = SynPat.LongIdent(longDotId = memberName) + headPat = SynPat.LongIdent(longDotId = memberName; accessibility = visGet) trivia = { LeadingKeyword = lk InlineKeyword = inlineKw }) as getBinding), - Some setBinding, + Some(SynBinding(headPat = SynPat.LongIdent(accessibility = visSet)) as setBinding), _, { GetKeyword = Some getKeyword SetKeyword = Some setKeyword WithKeyword = withKeyword AndKeyword = andKeyword }) -> - let firstBinding, lastBinding = + let firstAccessibility, firstBinding, lastBinding = if Position.posLt getKeyword.Start setKeyword.Start then + visGet, mkPropertyGetSetBinding creationAide (stn "get" getKeyword) getBinding, Some(mkPropertyGetSetBinding creationAide (stn "set" setKeyword) setBinding) else + visSet, mkPropertyGetSetBinding creationAide (stn "set" setKeyword) setBinding, Some(mkPropertyGetSetBinding creationAide (stn "get" getKeyword) getBinding) + // Only use the accessibility of the first binding if the keyword came before the member identifier. + let accessibility = + firstAccessibility + |> Option.bind (fun vis -> + if rangeBeforePos vis.Range memberName.Range.Start then + Some vis + else + None) + MemberDefnPropertyGetSetNode( mkXmlDoc px, mkAttributes creationAide ats, mkSynLeadingKeyword lk, Option.map (stn "inline") inlineKw, - mkSynAccess ao, + mkSynAccess accessibility, mkSynLongIdent memberName, stn "with" withKeyword, firstBinding,