Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always consider the range of the accessibility of properties. #2907

Merged
merged 1 commit into from
Jun 20, 2023
Merged

Always consider the range of the accessibility of properties. #2907

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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