Skip to content

Commit dd1b30e

Browse files
auduchinokauduchinok
committed
RIDER-124491 Completion/import/extensions: fix applicable type, accessbility, and imported scope checks
Also add more interrupt checks (cherry picked from commit 47f3383e0ac26fe9c0d352ad57aca608a501e166) GitOrigin-RevId: 5a46255c99d3b5c57929e23990de6d9cc030937c
1 parent 567fbc0 commit dd1b30e

20 files changed

+151
-56
lines changed

‎ReSharper.FSharp/src/FSharp/FSharp.Psi.Features/src/CodeCompletion/Rules/ImportExtensionMemberRule.fs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ open JetBrains.ReSharper.Plugins.FSharp.Psi.Services.Util.FSharpCompletionUtil
1717
open JetBrains.ReSharper.Plugins.FSharp.Psi.Tree
1818
open JetBrains.ReSharper.Psi
1919
open JetBrains.UI.RichText
20-
open JetBrains.Util.Extension
2120

2221
[<Language(typeof<FSharpLanguage>)>]
2322
type ImportExtensionMemberRule() =
@@ -43,7 +42,7 @@ type ImportExtensionMemberRule() =
4342
override this.AddLookupItems(context, collector) =
4443
let refExpr = getRefExpr context
4544
let members =
46-
FSharpExtensionMemberUtil.getNonImportedExtensionMembers None refExpr
45+
FSharpExtensionMemberUtil.getNonImportedExtensionMembers context.NodeInFile None refExpr
4746
|> FSharpExtensionMemberUtil.groupByNameAndNs
4847

4948
let iconManager = context.BasicContext.Solution.GetComponent<PsiIconManager>()

‎ReSharper.FSharp/src/FSharp/FSharp.Psi.Intentions/src/QuickFixes/ImportMemberFix.fs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ type FSharpImportExtensionMemberFix(reference: IReference) =
121121
if isNull refExpr then [] else
122122

123123
let name = reference.GetName()
124-
FSharpExtensionMemberUtil.getNonImportedExtensionMembers (Some name) refExpr
124+
FSharpExtensionMemberUtil.getNonImportedExtensionMembers refExpr (Some name) refExpr
125125
|> FSharpExtensionMemberUtil.groupByNameAndNs
126126
|> Seq.map (snd >> Seq.tryHead)
127127
|> Seq.choose id

‎ReSharper.FSharp/src/FSharp/FSharp.Psi.Services/src/Util/FSharpExtensionMemberUtil.fs‎

Lines changed: 32 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,13 @@ open JetBrains.ReSharper.Plugins.FSharp.Psi
99
open JetBrains.ReSharper.Plugins.FSharp.Psi.Features.Util
1010
open JetBrains.ReSharper.Plugins.FSharp.Psi.Impl
1111
open JetBrains.ReSharper.Plugins.FSharp.Psi.Impl.Cache2.Compiled
12-
open JetBrains.ReSharper.Plugins.FSharp.Psi.Resolve
1312
open JetBrains.ReSharper.Plugins.FSharp.Psi.Tree
1413
open JetBrains.ReSharper.Plugins.FSharp.Psi.Util
1514
open JetBrains.ReSharper.Psi
1615
open JetBrains.ReSharper.Psi.ExtensionsAPI.Caches2.ExtensionMethods.Queries
1716
open JetBrains.ReSharper.Psi.Modules
1817
open JetBrains.ReSharper.Psi.Resolve.TypeInference
18+
open JetBrains.ReSharper.Psi.Tree
1919
open JetBrains.ReSharper.Psi.Util
2020
open JetBrains.Util
2121
open JetBrains.Util.Extension
@@ -109,7 +109,7 @@ let (|FSharpExtensionMember|_|) (typeMember: ITypeMember) =
109109
| FSharpCompiledExtensionMember _ -> ValueSome()
110110
| _ -> ValueNone
111111

112-
let getExtensionMembersForType (context: IFSharpTreeNode) (fcsType: FSharpType) isStaticContext (nameOpt: string option) =
112+
let getExtensionMembersForType (context: ITreeNode) (fcsType: FSharpType) isStaticContext (nameOpt: string option) =
113113
if isNull fcsType then EmptyList.InstanceList else
114114

115115
let psiModule = context.GetPsiModule()
@@ -134,7 +134,6 @@ let getExtensionMembersForType (context: IFSharpTreeNode) (fcsType: FSharpType)
134134
typeElements.AsReadOnly()
135135

136136
let openedModulesProvider = OpenedModulesProvider(context)
137-
let accessContext = FSharpAccessContext(context)
138137

139138
let matchesType (typeMember: ITypeMember) : bool =
140139
match typeMember with
@@ -158,26 +157,22 @@ let getExtensionMembersForType (context: IFSharpTreeNode) (fcsType: FSharpType)
158157
let parameters = method.Parameters
159158
if parameters.Count = 0 then false else
160159

161-
let consumer = RecursiveConsumer(method.TypeParameters.ToIReadOnlyList())
162-
let typeInferenceMatcher = CLRTypeInferenceMatcher.Instance
163-
typeInferenceMatcher.Match(TypeInferenceKind.LowerBound, exprType, parameters[0].Type, consumer)
160+
let thisParameter = parameters[0]
161+
let parameterType = thisParameter.Type
162+
let parameterKind = thisParameter.Kind
163+
let typeParameters = method.TypeParameters.ToIReadOnlyList()
164+
let conversionRule = ClrPredefinedTypeConversionRule.INSTANCE
165+
if typeParameters.Count > 0 then
166+
let substitution = TypeInferenceUtil.InferTypes(FSharpLanguage.Instance, psiModule, exprType, parameterType, typeParameters, conversionRule)
167+
isNotNull substitution &&
168+
169+
let substitutedType = substitution.Apply(parameterType)
170+
conversionRule.HasExtensionMethodThisArgumentConversion(exprType, substitutedType, parameterKind, false)
171+
else
172+
conversionRule.HasExtensionMethodThisArgumentConversion(exprType, parameterType, parameterKind, false)
164173

165174
| _ -> false
166175

167-
let isAccessible (typeMember: ITypeMember) =
168-
let isTypeAccessible =
169-
let containingType = typeMember.ContainingType
170-
let accessRightsOwner = containingType :?> IAccessRightsOwner
171-
match accessRightsOwner.GetAccessRights() with
172-
| AccessRights.PUBLIC -> true
173-
| _ -> containingType.Module.AreInternalsVisibleTo(psiModule)
174-
175-
isTypeAccessible &&
176-
177-
match typeMember with
178-
| FSharpExtensionMember _ -> true
179-
| _ -> AccessUtil.IsSymbolAccessible(typeMember, accessContext)
180-
181176
let matchesName (typeMember: ITypeMember) =
182177
match nameOpt with
183178
| None -> true
@@ -211,27 +206,35 @@ let getExtensionMembersForType (context: IFSharpTreeNode) (fcsType: FSharpType)
211206

212207
| _ -> not isStaticContext
213208

209+
let isInScope (typeMember: ITypeMember) : bool =
210+
match typeMember with
211+
| FSharpExtensionMember -> FSharpImportUtil.areMembersInScope openedModulesProvider typeMember.ContainingType
212+
| _ -> FSharpImportUtil.isExtensionMemberInScope openedModulesProvider typeMember
213+
214214
let isApplicable (typeMember: ITypeMember) =
215215
resolvesAsExtensionMember typeMember &&
216216
matchesName typeMember &&
217-
not (FSharpImportUtil.isTypeMemberInScope openedModulesProvider typeMember) &&
218-
isAccessible typeMember &&
217+
not (isInScope typeMember) &&
218+
FSharpAccessRightUtil.IsAccessible(typeMember.ContainingType, context) &&
219219
matchesCallingConvention typeMember &&
220220
matchesType typeMember
221221

222-
let query = ExtensionMembersQuery(solution.GetPsiServices(), FSharpRequest(psiModule, exprType, nameOpt))
223-
let methods = query.EnumerateMembers() |> List.ofSeq
222+
let query = ExtensionMembersQuery(solution.GetPsiServices(), FSharpRequest(psiModule, exprType, nameOpt)) //.MaybeForReceiverType(exprType)
223+
224+
let result = List()
225+
for typeMember in query.EnumerateMembers() do
226+
Interruption.Current.CheckAndThrow()
227+
if isApplicable typeMember then
228+
result.Add(typeMember)
224229

225-
methods
226-
|> Seq.filter isApplicable
227-
|> List :> _
230+
result
228231

229-
let getNonImportedExtensionMembers (nameOpt: string option) (refExpr: IReferenceExpr) : IList<ITypeMember> =
232+
let getNonImportedExtensionMembers (context: ITreeNode) (nameOpt: string option) (refExpr: IReferenceExpr) : IList<ITypeMember> =
230233
if isNull refExpr then EmptyList.InstanceList else
231234

232235
let isStaticContext = FSharpExpressionUtil.isStaticContext refExpr.Qualifier
233236
let fcsType = getQualifierFcsType refExpr
234-
getExtensionMembersForType refExpr fcsType isStaticContext nameOpt
237+
getExtensionMembersForType context fcsType isStaticContext nameOpt
235238

236239
let groupByNameAndNs members =
237240
members |> Seq.groupBy (fun (typeMember: ITypeMember) ->

‎ReSharper.FSharp/src/FSharp/FSharp.Psi.Services/src/Util/FSharpImportUtil.fs‎

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,12 @@
22
module JetBrains.ReSharper.Plugins.FSharp.Psi.Services.Util.FSharpImportUtil
33

44
open JetBrains.ReSharper.Plugins.FSharp.Psi
5+
open JetBrains.ReSharper.Plugins.FSharp.Psi.Impl
56
open JetBrains.ReSharper.Plugins.FSharp.Psi.Util
67
open JetBrains.ReSharper.Psi
78

89
let private isTypeOrNsInScope (openedModulesProvider: OpenedModulesProvider) (declaredElement: IClrDeclaredElement) =
9-
let name = getQualifiedName declaredElement
10+
let name = getClrName declaredElement
1011
let scopes = openedModulesProvider.OpenedModuleScopes.GetValuesSafe(name)
1112
OpenScope.inAnyScope openedModulesProvider.Context scopes
1213

@@ -15,22 +16,27 @@ let areMembersInScope (openedModulesProvider: OpenedModulesProvider) (typeElemen
1516
| :? IFSharpModule as fsModule ->
1617
isTypeOrNsInScope openedModulesProvider fsModule
1718

18-
| containingType ->
19-
let ns = containingType.GetContainingNamespace()
19+
| typeElement ->
20+
let ns = typeElement.GetContainingNamespace()
2021
isTypeOrNsInScope openedModulesProvider ns
2122

22-
let isTypeMemberInScope (openedModulesProvider: OpenedModulesProvider) (typeMember: ITypeMember) =
23-
areMembersInScope openedModulesProvider typeMember.ContainingType
23+
let isExtensionMemberInScope (openedModulesProvider: OpenedModulesProvider) (typeMember: ITypeMember) =
24+
let containingTypeOrNs: IClrDeclaredElement =
25+
let typeElement = typeMember.ContainingType
26+
match typeElement.GetContainingType() with
27+
| null -> typeElement.GetNamespace()
28+
| containingType -> containingType
2429

25-
let isTypeElementInScope (openedModulesProvider: OpenedModulesProvider) (typeElement: ITypeElement) =
26-
isTypeOrNsInScope openedModulesProvider typeElement ||
30+
isTypeOrNsInScope openedModulesProvider containingTypeOrNs
2731

32+
let isTypeElementInScope (openedModulesProvider: OpenedModulesProvider) (typeElement: ITypeElement) =
2833
match typeElement.GetContainingType() with
29-
| :? IFSharpModule as fsModule ->
30-
isTypeOrNsInScope openedModulesProvider fsModule
31-
3234
| null ->
3335
let ns = typeElement.GetContainingNamespace()
3436
isTypeOrNsInScope openedModulesProvider ns
3537

36-
| _ -> false
38+
| :? IFSharpModule as fsModule ->
39+
isTypeOrNsInScope openedModulesProvider fsModule
40+
41+
| typeElement ->
42+
areMembersInScope openedModulesProvider typeElement

‎ReSharper.FSharp/src/FSharp/FSharp.Psi.Services/src/Util/OpensUtil.fs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,12 @@ type OpenedModulesProvider(context: ITreeNode) =
400400
let scope = OpenScope.Range(moduleDecl.GetTreeTextRange())
401401
importQualifiedName scope moduleDecl.ClrName
402402

403+
let fsModule = moduleDecl.DeclaredElement.As<IFSharpModule>()
404+
if isNotNull fsModule && fsModule.IsAutoOpen then
405+
let parent = moduleDecl.Parent
406+
let scope = OpenScope.Range(TreeTextRange(moduleDecl.GetTreeEndOffset(), parent.GetTreeEndOffset()))
407+
importQualifiedName scope moduleDecl.ClrName
408+
403409
let namedModuleDecl = moduleDecl.As<INamedModuleDeclaration>()
404410
if isNotNull namedModuleDecl then
405411
importQualifiedName scope namedModuleDecl.NamespaceName

‎ReSharper.FSharp/test/data/features/completion/list/Local val - Short lambda 01.fs.gold‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
Completion: Basic
2-
Count: 128
2+
Count: 123
33
Focus: Soft
44
Range: "|> List.map _.♦"
55

@@ -53,15 +53,13 @@ GetReverseIndex int
5353
Aggregate |(in System.Linq)
5454
All |(in System.Linq)
5555
Ancestors |(in System.Xml.Linq)
56-
AncestorsAndSelf |(in System.Xml.Linq)
5756
Any |(in System.Linq)
5857
Append |(in System.Linq)
5958
AsEnumerable |(in System.Linq)
6059
AsMemory |(in System)
6160
AsParallel |(in System.Linq)
6261
AsQueryable |(in System.Linq)
6362
AsSpan |(in System)
64-
Attributes |(in System.Xml.Linq)
6563
Average |(in System.Linq)
6664
Cast |(in System.Linq)
6765
Chunk |(in System.Linq)
@@ -71,9 +69,7 @@ CopyToDataTable |(in System.Data)
7169
Count |(in System.Linq)
7270
DefaultIfEmpty |(in System.Linq)
7371
DescendantNodes |(in System.Xml.Linq)
74-
DescendantNodesAndSelf |(in System.Xml.Linq)
7572
Descendants |(in System.Xml.Linq)
76-
DescendantsAndSelf |(in System.Xml.Linq)
7773
Distinct |(in System.Linq)
7874
DistinctBy |(in System.Linq)
7975
ElementAt |(in System.Linq)
@@ -83,7 +79,6 @@ Except |(in System.Linq)
8379
ExceptBy |(in System.Linq)
8480
First |(in System.Linq)
8581
FirstOrDefault |(in System.Linq)
86-
GetReverseIndex |(in Microsoft.FSharp.Core.Operators.ArrayExtensions)
8782
GroupBy |(in System.Linq)
8883
GroupJoin |(in System.Linq)
8984
InDocumentOrder |(in System.Xml.Linq)
@@ -186,15 +181,13 @@ TryCopyTo bool
186181
Aggregate |(in System.Linq)
187182
All |(in System.Linq)
188183
Ancestors |(in System.Xml.Linq)
189-
AncestorsAndSelf |(in System.Xml.Linq)
190184
Any |(in System.Linq)
191185
Append |(in System.Linq)
192186
AsEnumerable |(in System.Linq)
193187
AsMemory |(in System)
194188
AsParallel |(in System.Linq)
195189
AsQueryable |(in System.Linq)
196190
AsSpan |(in System)
197-
Attributes |(in System.Xml.Linq)
198191
Average |(in System.Linq)
199192
Cast |(in System.Linq)
200193
Chunk |(in System.Linq)
@@ -204,9 +197,7 @@ CopyToDataTable |(in System.Data)
204197
Count |(in System.Linq)
205198
DefaultIfEmpty |(in System.Linq)
206199
DescendantNodes |(in System.Xml.Linq)
207-
DescendantNodesAndSelf |(in System.Xml.Linq)
208200
Descendants |(in System.Xml.Linq)
209-
DescendantsAndSelf |(in System.Xml.Linq)
210201
Distinct |(in System.Linq)
211202
DistinctBy |(in System.Linq)
212203
ElementAt |(in System.Linq)
@@ -216,7 +207,6 @@ Except |(in System.Linq)
216207
ExceptBy |(in System.Linq)
217208
First |(in System.Linq)
218209
FirstOrDefault |(in System.Linq)
219-
GetReverseIndex |(in Microsoft.FSharp.Core.Operators.ArrayExtensions)
220210
GroupBy |(in System.Linq)
221211
GroupJoin |(in System.Linq)
222212
InDocumentOrder |(in System.Xml.Linq)

‎ReSharper.FSharp/test/data/features/quickFixes/import/extension/Attribute - Source 01.fs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
module Top
2+
3+
open System.Runtime.CompilerServices
4+
5+
module Nested =
6+
type Extensions =
7+
[<Extension>]
8+
static member Method(s: string) = ()
9+
10+
"".Method{caret}

‎ReSharper.FSharp/test/data/features/quickFixes/import/extension/Attribute - Source 01.fs.gold‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
module Top
2+
3+
open System.Runtime.CompilerServices
4+
5+
module Nested =
6+
type Extensions =
7+
[<Extension>]
8+
static member Method(s: string) = ()
9+
10+
open Nested
11+
12+
"".Method{caret}

‎ReSharper.FSharp/test/data/features/quickFixes/import/extension/Attribute - Source 02.fs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
module Top
2+
3+
open System.Runtime.CompilerServices
4+
5+
module Nested =
6+
type Extensions =
7+
[<Extension>]
8+
static member Method(s: string) = ()
9+
10+
"".Method{caret}()

‎ReSharper.FSharp/test/data/features/quickFixes/import/extension/Attribute - Source 02.fs.gold‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
module Top
2+
3+
open System.Runtime.CompilerServices
4+
5+
module Nested =
6+
type Extensions =
7+
[<Extension>]
8+
static member Method(s: string) = ()
9+
10+
open Nested
11+
12+
"".Method{caret}()

0 commit comments

Comments
 (0)