Skip to content

Commit

Permalink
Always consider the range of the accessibility of properties. (#2907)
Browse files Browse the repository at this point in the history
  • Loading branch information
@nojaf
nojaf authored Jun 20, 2023
1 parent 702694e commit fc65e0c
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 35 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# Changelog

## [6.0.7] - 2023-06-20

### Fixed
* Private keyword is lost from getter with unit. [#2906](https://github.com/fsprojects/fantomas/issues/2906)

## [6.0.6] - 2023-06-19

### Fixed
Expand Down
37 changes: 37 additions & 0 deletions src/Fantomas.Core.Tests/ClassTests.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1293,3 +1293,40 @@ type MyClass(a: int, b: int) =
[<Obsolete("Do not use")>]
new(x: int) = MyClass(x, x)
"""

[<Test>]
let ``access modifier on get unit property gets simplified, 2906`` () =
formatSourceString
false
"""
type X() =
member private this.Y with get() = "meh"
member this.Z with private get() = "foo"
"""
config
|> prepend newline
|> should
equal
"""
type X() =
member private this.Y = "meh"
member private this.Z = "foo"
"""

[<Test>]
let ``access modifier on set property, 2906`` () =
formatSourceString
false
"""
type X() =
member private this.Y with set _ = ()
"""
config
|> prepend newline
|> should
equal
"""
type X() =
member private this.Y
with set _ = ()
"""
82 changes: 47 additions & 35 deletions src/Fantomas.Core/ASTTransformer.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2561,29 +2561,20 @@ let mkWithGetSet (withKeyword: range option) (getSet: GetSetKeywords option) =

let mkPropertyGetSetBinding
(creationAide: CreationAide)
(accessibility: SynAccess option)
(leadingKeyword: SingleTextNode)
(binding: SynBinding)
: PropertyGetSetBindingNode =
match binding with
| SynBinding(
headPat = SynPat.LongIdent(
longDotId = lid; extraId = Some extraIdent; accessibility = ao; argPats = SynArgPats.Pats ps)
headPat = SynPat.LongIdent(extraId = Some extraIdent; 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 @@ -2628,7 +2619,9 @@ let mkMemberDefn (creationAide: CreationAide) (md: SynMemberDefn) =
| SynMemberDefn.ImplicitInherit(t, e, _, StartRange 7 (mInherit, _)) ->
mkInheritConstructor creationAide t e mInherit memberDefinitionRange
|> MemberDefn.ImplicitInherit
| SynMemberDefn.GetSetMember(Some(SynBinding(ao,

// Transforms: `member this.Y with get() = "meh"` into `member this.Y = "meh"`
| SynMemberDefn.GetSetMember(Some(SynBinding(_,
kind,
isInline,
isMutable,
Expand All @@ -2638,10 +2631,10 @@ let mkMemberDefn (creationAide: CreationAide) (md: SynMemberDefn) =
SynPat.LongIdent(lid,
extraId,
typarDecls,
SynArgPats.Pats [ SynPat.Paren(SynPat.Const(SynConst.Unit,
_),
_) ],
None,
SynArgPats.Pats [ SynPat.Paren(
pat = SynPat.Const(
constant = SynConst.Unit)) ],
ao,
mPat),
ri,
e,
Expand All @@ -2652,12 +2645,11 @@ let mkMemberDefn (creationAide: CreationAide) (md: SynMemberDefn) =
_,
{ GetKeyword = Some _ }) ->

let pat =
SynPat.LongIdent(lid, extraId, typarDecls, SynArgPats.Pats([]), None, mPat)
let pat = SynPat.LongIdent(lid, extraId, typarDecls, SynArgPats.Pats([]), ao, mPat)

mkBinding
creationAide
(SynBinding(ao, kind, isInline, isMutable, ats, px, valData, pat, ri, e, bindingRange, dp, trivia))
(SynBinding(None, kind, isInline, isMutable, ats, px, valData, pat, ri, e, bindingRange, dp, trivia))
|> MemberDefn.Member
| SynMemberDefn.Member(
memberDefn = SynBinding(
Expand Down Expand Up @@ -2784,15 +2776,12 @@ let mkMemberDefn (creationAide: CreationAide) (md: SynMemberDefn) =
SetKeyword = Some setKeyword
WithKeyword = withKeyword
AndKeyword = andKeyword }) ->
let firstAccessibility, firstBinding, lastBinding =

let firstAccessibility, firstBinding, firstKeyword, lastBinding, lastKeyword =
if Position.posLt getKeyword.Start setKeyword.Start then
visGet,
mkPropertyGetSetBinding creationAide (stn "get" getKeyword) getBinding,
Some(mkPropertyGetSetBinding creationAide (stn "set" setKeyword) setBinding)
visGet, getBinding, (stn "get" getKeyword), setBinding, (stn "set" setKeyword)
else
visSet,
mkPropertyGetSetBinding creationAide (stn "set" setKeyword) setBinding,
Some(mkPropertyGetSetBinding creationAide (stn "get" getKeyword) getBinding)
visSet, setBinding, (stn "set" setKeyword), getBinding, (stn "get" getKeyword)

// Only use the accessibility of the first binding if the keyword came before the member identifier.
let accessibility =
Expand All @@ -2803,6 +2792,22 @@ let mkMemberDefn (creationAide: CreationAide) (md: SynMemberDefn) =
else
None)

let firstBinding =
match firstBinding with
| SynBinding(headPat = SynPat.LongIdent(accessibility = Some vis)) when
rangeBeforePos memberName.Range vis.Range.Start
->
mkPropertyGetSetBinding creationAide (Some vis) firstKeyword firstBinding
| _ -> mkPropertyGetSetBinding creationAide None firstKeyword firstBinding

let lastBinding =
match lastBinding with
| SynBinding(headPat = SynPat.LongIdent(accessibility = Some vis)) when
rangeBeforePos memberName.Range vis.Range.Start
->
mkPropertyGetSetBinding creationAide (Some vis) lastKeyword lastBinding
| _ -> mkPropertyGetSetBinding creationAide None lastKeyword lastBinding

MemberDefnPropertyGetSetNode(
mkXmlDoc px,
mkAttributes creationAide ats,
Expand All @@ -2813,27 +2818,25 @@ let mkMemberDefn (creationAide: CreationAide) (md: SynMemberDefn) =
stn "with" withKeyword,
firstBinding,
Option.map (stn "and") andKeyword,
lastBinding,
Some lastBinding,
memberDefinitionRange
)
|> MemberDefn.PropertyGetSet
| SynMemberDefn.GetSetMember(None,
Some(SynBinding(
accessibility = ao
attributes = ats
xmlDoc = px
headPat = SynPat.LongIdent(longDotId = memberName)
headPat = SynPat.LongIdent(longDotId = memberName; accessibility = ao)
trivia = { LeadingKeyword = lk
InlineKeyword = inlineKw }) as binding),
_,
{ WithKeyword = withKeyword
GetKeyword = getKeyword
SetKeyword = setKeyword })
| SynMemberDefn.GetSetMember(Some(SynBinding(
accessibility = ao
attributes = ats
xmlDoc = px
headPat = SynPat.LongIdent(longDotId = memberName)
headPat = SynPat.LongIdent(longDotId = memberName; accessibility = ao)
trivia = { LeadingKeyword = lk
InlineKeyword = inlineKw }) as binding),
None,
Expand All @@ -2842,17 +2845,26 @@ let mkMemberDefn (creationAide: CreationAide) (md: SynMemberDefn) =
GetKeyword = getKeyword
SetKeyword = setKeyword }) ->

let visMember, visProperty =
match ao with
| None -> None, None
| Some ao ->
if rangeBeforePos ao.Range memberName.Range.Start then
Some ao, None
else
None, Some ao

match getKeyword, setKeyword with
| Some getKeyword, None ->
let bindingNode =
mkPropertyGetSetBinding creationAide (stn "get" getKeyword) binding
mkPropertyGetSetBinding creationAide visProperty (stn "get" getKeyword) binding

MemberDefnPropertyGetSetNode(
mkXmlDoc px,
mkAttributes creationAide ats,
mkSynLeadingKeyword lk,
Option.map (stn "inline") inlineKw,
mkSynAccess ao,
mkSynAccess visMember,
mkSynLongIdent memberName,
stn "with" withKeyword,
bindingNode,
Expand All @@ -2863,14 +2875,14 @@ let mkMemberDefn (creationAide: CreationAide) (md: SynMemberDefn) =
|> MemberDefn.PropertyGetSet
| None, Some setKeyword ->
let bindingNode =
mkPropertyGetSetBinding creationAide (stn "set" setKeyword) binding
mkPropertyGetSetBinding creationAide visProperty (stn "set" setKeyword) binding

MemberDefnPropertyGetSetNode(
mkXmlDoc px,
mkAttributes creationAide ats,
mkSynLeadingKeyword lk,
Option.map (stn "inline") inlineKw,
mkSynAccess ao,
mkSynAccess visMember,
mkSynLongIdent memberName,
stn "with" withKeyword,
bindingNode,
Expand Down

0 comments on commit fc65e0c

Please sign in to comment.