Skip to content

Commit 493711a

Browse files
committed
Generate/overrides: fix isOverriden check for generic members
1 parent 376bf38 commit 493711a

File tree

11 files changed

+143
-9
lines changed

11 files changed

+143
-9
lines changed

ReSharper.FSharp/src/FSharp.Psi.Services/src/Generate/GenerateProvider.fs

Lines changed: 51 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
namespace JetBrains.ReSharper.Plugins.FSharp.Psi.Features
22

3+
open System
34
open System.Collections.Generic
45
open FSharp.Compiler.Symbols
56
open JetBrains.Application.Progress
@@ -20,7 +21,6 @@ open JetBrains.ReSharper.Psi.Impl
2021
open JetBrains.ReSharper.Psi.Tree
2122
open JetBrains.ReSharper.Psi.Util
2223
open JetBrains.ReSharper.Resources.Shell
23-
open JetBrains.Util
2424

2525
[<Language(typeof<FSharpLanguage>)>]
2626
type FSharpGeneratorContextFactory() =
@@ -70,6 +70,13 @@ type FSharpGeneratorElement(element, mfvInstance: FcsMfvInstance, addTypes) =
7070
override x.ToString() = element.ToString()
7171

7272

73+
[<Flags>]
74+
type PropertyOverrideState =
75+
| None = 0
76+
| Getter = 1
77+
| Setter = 2
78+
79+
7380
[<GeneratorElementProvider(GeneratorStandardKinds.Overrides, typeof<FSharpLanguage>)>]
7481
[<GeneratorElementProvider(GeneratorStandardKinds.MissingMembers, typeof<FSharpLanguage>)>]
7582
type FSharpOverridableMembersProvider() =
@@ -113,7 +120,7 @@ type FSharpOverridableMembersProvider() =
113120
| Some baseType when baseType.HasTypeDefinition -> loop [] baseType
114121
| _ -> []
115122

116-
let ownMembersIds =
123+
let ownMembersDescriptors =
117124
typeElement.GetMembers()
118125
|> Seq.collect (fun typeMember ->
119126
if typeMember :? IFSharpGeneratedElement then Seq.empty else
@@ -140,7 +147,39 @@ type FSharpOverridableMembersProvider() =
140147
|> Seq.toList
141148
fcsEntityInstance, mfvInstances)
142149

143-
let alreadyOverriden = HashSet()
150+
let alreadyOverriden = Dictionary<OverridableMemberInstance, PropertyOverrideState>()
151+
152+
let addOverrides (memberInstance: OverridableMemberInstance) =
153+
for overridableMemberInstance in OverridableMemberImpl.GetImmediateOverride(memberInstance) do
154+
let state =
155+
match memberInstance.Member with
156+
| :? IProperty as prop ->
157+
(if prop.IsReadable then PropertyOverrideState.Getter else PropertyOverrideState.None) |||
158+
(if prop.IsReadable then PropertyOverrideState.Getter else PropertyOverrideState.None)
159+
| _ -> PropertyOverrideState.None
160+
161+
let state =
162+
match alreadyOverriden.TryGetValue(overridableMemberInstance) with
163+
| true, existingState -> existingState ||| state
164+
| _ -> state
165+
166+
alreadyOverriden[overridableMemberInstance] <- state
167+
168+
let isOverridden (memberInstance: OverridableMemberInstance) =
169+
match alreadyOverriden.TryGetValue(memberInstance) with
170+
| false, _ -> false
171+
| true, state ->
172+
173+
let prop = memberInstance.Member.As<IProperty>()
174+
isNull prop ||
175+
176+
(not prop.IsReadable || (state &&& PropertyOverrideState.Getter <> enum 0)) &&
177+
(not prop.IsWritable || (state &&& PropertyOverrideState.Setter <> enum 0))
178+
179+
for KeyValue(_, memberInstance) in memberInstances do
180+
let fsTypeMember = memberInstance.Member.As<IFSharpTypeMember>()
181+
if isNull fsTypeMember || fsTypeMember.IsVisibleFromFSharp then
182+
addOverrides memberInstance
144183

145184
let overridableMemberInstances =
146185
baseFcsMembers |> List.collect (fun (_, mfvInstances) ->
@@ -153,15 +192,18 @@ type FSharpOverridableMembersProvider() =
153192
| null -> mfv.GetXmlDocId()
154193
| typeMember -> XMLDocUtil.GetTypeMemberXmlDocId(typeMember, typeMember.ShortName)
155194

156-
if ownMembersIds.Contains(xmlDocId) then None else
195+
if ownMembersDescriptors.Contains(xmlDocId) then None else
157196

158197
let mutable memberInstance = Unchecked.defaultof<_>
159198
if not (memberInstances.TryGetValue(xmlDocId, &memberInstance)) then None else
160-
if alreadyOverriden.Contains(memberInstance) then None else
161199

162-
OverridableMemberImpl.GetImmediateOverride(memberInstance) |> alreadyOverriden.AddRange
163-
Some (memberInstance.Member, mfvInstance))
164-
|> Seq.toList)
200+
if isOverridden memberInstance then None else
201+
202+
addOverrides memberInstance
203+
Some (memberInstance.Member, mfvInstance)
204+
)
205+
|> Seq.toList
206+
)
165207

166208
let needsTypesAnnotations =
167209
overridableMemberInstances
@@ -182,7 +224,7 @@ type FSharpOverridableMembersProvider() =
182224
prop.Setter :> IOverridableMember, { mfvInstance with Mfv = mfv.SetterMethod } ])
183225
|> Seq.map (fun (m, mfvInstance) ->
184226
FSharpGeneratorElement(m, mfvInstance, needsTypesAnnotations.Contains(mfvInstance.Mfv)))
185-
|> Seq.filter (fun i -> not (ownMembersIds.Contains(i.TestDescriptor)))
227+
|> Seq.filter (fun i -> not (ownMembersDescriptors.Contains(i.TestDescriptor)))
186228
|> Seq.distinctBy (fun i -> i.TestDescriptor) // todo: better way to check shadowing/overriding members
187229
|> Seq.filter (fun i -> not missingMembersOnly || i.Member.IsAbstract)
188230
|> Seq.iter context.ProvidedElements.Add
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// ${KIND:Overrides}
2+
// ${SELECT0:ToString():System.String}
3+
4+
[<AbstractClass>]
5+
type A<'T1>() =
6+
abstract P: 'T1
7+
8+
[<AbstractClass>]
9+
type T() ={caret}
10+
inherit A<int>()
11+
12+
override this.P = 1
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
Provided elements:
2+
0: ToString():System.String
3+
1: Equals(System.Object):System.Boolean
4+
2: GetHashCode():System.Int32
5+
3: Finalize():System.Void
6+
7+
// ${KIND:Overrides}
8+
// ${SELECT0:ToString():System.String}
9+
10+
[<AbstractClass>]
11+
type A<'T1>() =
12+
abstract P: 'T1
13+
14+
[<AbstractClass>]
15+
type T() =
16+
inherit A<int>()
17+
18+
override this.P = 1
19+
override this.ToString() = {selstart}failwith "todo"{selend}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
[<AbstractClass>]
2+
type A<'T>() =
3+
abstract P1: 'T list
4+
abstract P2: int
5+
6+
type {caret}B() =
7+
inherit A<int>()
8+
9+
override this.P1 = []
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
[<AbstractClass>]
2+
type A<'T>() =
3+
abstract P1: 'T list
4+
abstract P2: int
5+
6+
type B() =
7+
inherit A<int>()
8+
9+
override this.P1 = []
10+
override this.P2 = {selstart}failwith "todo"{selend}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
[<AbstractClass>]
2+
type A<'T>() =
3+
abstract M1: unit -> 'T list
4+
abstract M2: unit -> int
5+
6+
type {caret}B() =
7+
inherit A<int>()
8+
9+
override this.M1() = []
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
[<AbstractClass>]
2+
type A<'T>() =
3+
abstract M1: unit -> 'T list
4+
abstract M2: unit -> int
5+
6+
type B() =
7+
inherit A<int>()
8+
9+
override this.M1() = []
10+
override this.M2() = {selstart}failwith "todo"{selend}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
[<AbstractClass>]
2+
type A<'T>() =
3+
abstract M1: 'T list -> unit
4+
abstract M2: int -> unit
5+
6+
type {caret}B() =
7+
inherit A<int>()
8+
9+
override this.M1 _ = ()
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
[<AbstractClass>]
2+
type A<'T>() =
3+
abstract M1: 'T list -> unit
4+
abstract M2: int -> unit
5+
6+
type B() =
7+
inherit A<int>()
8+
9+
override this.M1 _ = ()
10+
override this.M2(var0) = {selstart}failwith "todo"{selend}

ReSharper.FSharp/test/src/FSharp.Intentions.Tests/src/QuickFixes/GenerateInterfaceMembersFixTest.fs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,4 +82,7 @@ type GenerateMissingMembersFixTest() =
8282
[<Test>] member x.``Property 01``() = x.DoNamedTest()
8383
[<Test>] member x.``Same name 01``() = x.DoNamedTest()
8484
[<Test>] member x.``Same name 02``() = x.DoNamedTest()
85+
[<Test>] member x.``Substitution 01``() = x.DoNamedTest()
86+
[<Test>] member x.``Substitution 02``() = x.DoNamedTest()
87+
[<Test>] member x.``Substitution 03``() = x.DoNamedTest()
8588
[<Test>] member x.``Super 01 - Different type parameter name``() = x.DoNamedTest()

0 commit comments

Comments
 (0)