diff --git a/src/FSharpLint.Core/Rules/Conventions/NoPartialFunctions.fs b/src/FSharpLint.Core/Rules/Conventions/NoPartialFunctions.fs index bdc1789b9..bce1f3715 100644 --- a/src/FSharpLint.Core/Rules/Conventions/NoPartialFunctions.fs +++ b/src/FSharpLint.Core/Rules/Conventions/NoPartialFunctions.fs @@ -9,6 +9,7 @@ open FSharpLint.Framework.Ast open FSharpLint.Framework.Rules open FSharp.Compiler.CodeAnalysis open FSharp.Compiler.Symbols +open FSharp.Compiler.Syntax [] type Config = { @@ -110,7 +111,124 @@ let private checkIfPartialIdentifier (config:Config) (identifier:string) (range: TypeChecks = [] }) -let private isNonStaticInstanceMemberCall (checkFile:FSharpCheckFileResults) names range:(Option) = +let rec private tryFindTypedExpression (range: Range) (expression: FSharpExpr) = + let tryFindFirst exprs = + exprs |> Seq.choose (tryFindTypedExpression range) |> Seq.tryHead + if expression.Range = range then + Some expression + else + match expression with + | FSharpExprPatterns.AddressOf(lvalueExpr) -> + tryFindTypedExpression range lvalueExpr + | FSharpExprPatterns.AddressSet(lvalueExpr, rvalueExpr) -> + tryFindTypedExpression range lvalueExpr |> Option.orElse (tryFindTypedExpression range rvalueExpr) + | FSharpExprPatterns.Application(funcExpr, _typeArgs, argExprs) -> + (funcExpr :: argExprs) |> tryFindFirst + | FSharpExprPatterns.Call(objExprOpt, _memberOrFunc, _typeArgs1, _typeArgs2, argExprs) -> + (List.append (Option.toList objExprOpt) argExprs) |> tryFindFirst + | FSharpExprPatterns.Coerce(_targetType, inpExpr) -> + tryFindTypedExpression range inpExpr + | FSharpExprPatterns.FastIntegerForLoop(startExpr, limitExpr, consumeExpr, _isUp) -> + [ startExpr; limitExpr; consumeExpr ] |> tryFindFirst + | FSharpExprPatterns.ILAsm(_asmCode, _typeArgs, argExprs) -> + tryFindFirst argExprs + | FSharpExprPatterns.ILFieldGet (objExprOpt, _fieldType, _fieldName) -> + objExprOpt |> Option.bind (tryFindTypedExpression range) + | FSharpExprPatterns.ILFieldSet (objExprOpt, _fieldType, _fieldName, valueExpr) -> + objExprOpt |> Option.bind (tryFindTypedExpression range) |> Option.orElse (tryFindTypedExpression range valueExpr) + | FSharpExprPatterns.IfThenElse (guardExpr, thenExpr, elseExpr) -> + [ guardExpr; thenExpr; elseExpr ] |> tryFindFirst + | FSharpExprPatterns.Lambda(_lambdaVar, bodyExpr) -> + tryFindTypedExpression range bodyExpr + | FSharpExprPatterns.Let((_bindingVar, bindingExpr), bodyExpr) -> + tryFindTypedExpression range bindingExpr |> Option.orElse (tryFindTypedExpression range bodyExpr) + | FSharpExprPatterns.LetRec(recursiveBindings, bodyExpr) -> + recursiveBindings + |> Seq.choose (fun (_, expr) -> tryFindTypedExpression range expr) + |> Seq.tryHead + |> Option.orElse (tryFindTypedExpression range bodyExpr) + | FSharpExprPatterns.NewArray(_arrayType, argExprs) -> + tryFindFirst argExprs + | FSharpExprPatterns.NewDelegate(_delegateType, delegateBodyExpr) -> + tryFindTypedExpression range delegateBodyExpr + | FSharpExprPatterns.NewObject(_objType, _typeArgs, argExprs) -> + tryFindFirst argExprs + | FSharpExprPatterns.NewRecord(_recordType, argExprs) -> + tryFindFirst argExprs + | FSharpExprPatterns.NewAnonRecord(_recordType, argExprs) -> + tryFindFirst argExprs + | FSharpExprPatterns.NewTuple(_tupleType, argExprs) -> + tryFindFirst argExprs + | FSharpExprPatterns.NewUnionCase(_unionType, _unionCase, argExprs) -> + tryFindFirst argExprs + | FSharpExprPatterns.Quote(quotedExpr) -> + tryFindTypedExpression range quotedExpr + | FSharpExprPatterns.FSharpFieldGet(objExprOpt, _recordOrClassType, _fieldInfo) -> + objExprOpt |> Option.bind (tryFindTypedExpression range) + | FSharpExprPatterns.AnonRecordGet(objExpr, _recordOrClassType, _fieldInfo) -> + tryFindTypedExpression range objExpr + | FSharpExprPatterns.FSharpFieldSet(objExprOpt, _recordOrClassType, _fieldInfo, argExpr) -> + objExprOpt |> Option.bind (tryFindTypedExpression range) |> Option.orElse (tryFindTypedExpression range argExpr) + | FSharpExprPatterns.Sequential(firstExpr, secondExpr) -> + tryFindTypedExpression range firstExpr |> Option.orElse (tryFindTypedExpression range secondExpr) + | FSharpExprPatterns.TryFinally(bodyExpr, finalizeExpr) -> + tryFindTypedExpression range bodyExpr |> Option.orElse (tryFindTypedExpression range finalizeExpr) + | FSharpExprPatterns.TryWith(bodyExpr, _, _, _catchVar, catchExpr) -> + tryFindTypedExpression range bodyExpr |> Option.orElse (tryFindTypedExpression range catchExpr) + | FSharpExprPatterns.TupleGet(_tupleType, _tupleElemIndex, tupleExpr) -> + tryFindTypedExpression range tupleExpr + | FSharpExprPatterns.DecisionTree(decisionExpr, decisionTargets) -> + tryFindTypedExpression range decisionExpr + |> Option.orElse (decisionTargets |> Seq.choose (fun (_, expr) -> tryFindTypedExpression range expr) |> Seq.tryHead) + | FSharpExprPatterns.DecisionTreeSuccess (_decisionTargetIdx, decisionTargetExprs) -> + tryFindFirst decisionTargetExprs + | FSharpExprPatterns.TypeLambda(_genericParam, bodyExpr) -> + tryFindTypedExpression range bodyExpr + | FSharpExprPatterns.TypeTest(_ty, inpExpr) -> + tryFindTypedExpression range inpExpr + | FSharpExprPatterns.UnionCaseSet(unionExpr, unionType, unionCase, unionCaseField, valueExpr) -> + tryFindTypedExpression range unionExpr |> Option.orElse (tryFindTypedExpression range valueExpr) + | FSharpExprPatterns.UnionCaseGet(unionExpr, _unionType, _unionCase, _unionCaseField) -> + tryFindTypedExpression range unionExpr + | FSharpExprPatterns.UnionCaseTest(unionExpr, _unionType, _unionCase) -> + tryFindTypedExpression range unionExpr + | FSharpExprPatterns.UnionCaseTag(unionExpr, _unionType) -> + tryFindTypedExpression range unionExpr + | FSharpExprPatterns.ObjectExpr(_objType, baseCallExpr, overrides, interfaceImplementations) -> + let interfaceImlps = interfaceImplementations |> List.collect snd + baseCallExpr :: (List.append overrides interfaceImlps |> Seq.cast |> Seq.toList) + |> tryFindFirst + | FSharpExprPatterns.TraitCall(_sourceTypes, _traitName, _typeArgs, _typeInstantiation, _argTypes, argExprs) -> + tryFindFirst argExprs + | FSharpExprPatterns.ValueSet(_valToSet, valueExpr) -> + tryFindTypedExpression range valueExpr + | FSharpExprPatterns.WhileLoop(guardExpr, bodyExpr) -> + tryFindTypedExpression range guardExpr |> Option.orElse (tryFindTypedExpression range bodyExpr) + | _ -> None + +let private getTypedExpressionForRange (checkFile:FSharpCheckFileResults) (range: Range) = + let expressions = + match checkFile.ImplementationFile with + | Some implementationFile -> + let rec getExpressions declarations = + seq { + for declaration in declarations do + match declaration with + | FSharpImplementationFileDeclaration.Entity(entity, subDecls) -> + yield! getExpressions subDecls + | FSharpImplementationFileDeclaration.MemberOrFunctionOrValue(_,_,body) -> + yield body + | _ -> () + } + + getExpressions implementationFile.Declarations + | None -> Seq.empty + + expressions + |> Seq.choose (tryFindTypedExpression range) + |> Seq.tryHead + +let private isNonStaticInstanceMemberCall (checkFile:FSharpCheckFileResults) names (range: Range) :(Option) = let typeChecks = (partialInstanceMemberIdentifiers @@ -125,7 +243,7 @@ let private isNonStaticInstanceMemberCall (checkFile:FSharpCheckFileResults) nam let isSourcePropSameAsReplacementProp = List.tryFind (fun sourceInstanceMemberName -> sourceInstanceMemberName = instanceMemberNameOnly) names match isSourcePropSameAsReplacementProp with | Some _ -> - let typeName = fullyQualifiedInstanceMember.Substring(0, fullyQualifiedInstanceMember.Length - instanceMemberNameOnly.Length - 1) + let typeName = fullyQualifiedInstanceMember.Substring(0, fullyQualifiedInstanceMember.Length - instanceMemberNameOnly.Length - 1) let partialAssemblySignature = checkFile.PartialAssemblySignature let isEntityOfType (entity:FSharpEntity) = @@ -180,6 +298,55 @@ let private isNonStaticInstanceMemberCall (checkFile:FSharpCheckFileResults) nam | None -> None | Some instanceMember -> instanceMember +let private checkMemberCallOnExpression + (checkFile: FSharpCheckFileResults) + (flieContent: string) + (range: Range) + (originalRange: Range): array = + match getTypedExpressionForRange checkFile range with + | Some expression -> + partialInstanceMemberIdentifiers + |> Map.toList + |> List.choose (fun (fullyQualifiedInstanceMember, replacementStrategy) -> + let typeName = fullyQualifiedInstanceMember.Split(".").[0] + let fsharpType = expression.Type + + let matchesType = + match typeName with + | "Option" -> + // see https://stackoverflow.com/a/70282499/544947 + fsharpType.HasTypeDefinition + && fsharpType.TypeDefinition.Namespace = Some "Microsoft.FSharp.Core" + && fsharpType.TypeDefinition.CompiledName = "option`1" + | "Map" -> + fsharpType.HasTypeDefinition + && fsharpType.TypeDefinition.Namespace = Some "Microsoft.FSharp.Collections" + && fsharpType.TypeDefinition.CompiledName = "FSharpMap`2" + | "List" -> + fsharpType.HasTypeDefinition + && fsharpType.TypeDefinition.Namespace = Some "Microsoft.FSharp.Collections" + && fsharpType.TypeDefinition.CompiledName = "list`1" + | _ -> + fsharpType.HasTypeDefinition + && fsharpType.TypeDefinition.FullName = typeName + + if matchesType then + match replacementStrategy with + | PatternMatch -> + Some { Range = originalRange + Message = String.Format(Resources.GetString "RulesConventionsNoPartialFunctionsPatternMatchError", fullyQualifiedInstanceMember) + SuggestedFix = None + TypeChecks = (fun () -> true) |> List.singleton } + | Function replacementFunctionName -> + Some { Range = originalRange + Message = String.Format(Resources.GetString "RulesConventionsNoPartialFunctionsReplacementError", replacementFunctionName, fullyQualifiedInstanceMember) + SuggestedFix = Some (lazy ( Some { FromText = (ExpressionUtilities.tryFindTextOfRange originalRange flieContent).Value ; FromRange = originalRange; ToText = replacementFunctionName })) + TypeChecks = (fun () -> true) |> List.singleton } + else + None) + |> List.toArray + | None -> Array.empty + let private runner (config:Config) (args:AstNodeRuleParams) = match (args.AstNode, args.CheckInfo) with | (AstNode.Identifier (identifier, range), Some checkInfo) -> @@ -195,6 +362,11 @@ let private runner (config:Config) (args:AstNodeRuleParams) = | Some warningDetails -> warningDetails |> Array.singleton | _ -> Array.Empty() + | (Ast.Expression(SynExpr.DotGet(expr, _, LongIdentWithDots(_identifiers, _), _range)), Some checkInfo) -> + let originalRange = expr.Range + let expr = ExpressionUtilities.removeParens expr + + checkMemberCallOnExpression checkInfo args.FileContent expr.Range originalRange | _ -> Array.empty let rule config =