Skip to content

Commit

Permalink
Use correct visibility of get/set property and member. (#2903)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
nojaf authored Jun 19, 2023
1 parent 4f2f9c1 commit d18cc65
Show file tree
Hide file tree
Showing 3 changed files with 144 additions and 7 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
112 changes: 112 additions & 0 deletions src/Fantomas.Core.Tests/StructTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -189,3 +189,115 @@ struct // 1
// 5
|} // 6
"""

[<Test>]
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
"""

[<Test>]
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
"""

[<Test>]
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) = ()
"""

[<Test>]
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) = ()
"""
34 changes: 27 additions & 7 deletions src/Fantomas.Core/ASTTransformer.fs
Original file line number Diff line number Diff line change
Expand Up @@ -2566,14 +2566,24 @@ 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
InlineKeyword = inlineKw }) ->
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 ], _) ] ->
Expand Down Expand Up @@ -2601,7 +2611,7 @@ let mkPropertyGetSetBinding

PropertyGetSetBindingNode(
Option.map (stn "inline") inlineKw,
mkSynAccess ao,
mkSynAccess accessibility,
leadingKeyword,
pats,
returnTypeNode,
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit d18cc65

Please sign in to comment.