Skip to content

Commit

Permalink
NoPartialFunctions: fix case for expressions
Browse files Browse the repository at this point in the history 
Fixed NoPartialFunctions rule for case when instance member is
called on arbitrary expression, not just dotted identifier.
  • Loading branch information
@webwarrior-ws
webwarrior-ws committed Dec 27, 2023
1 parent 4b6e192 commit 791df60
Showing 1 changed file with 174 additions and 2 deletions.
176 changes: 174 additions & 2 deletions src/FSharpLint.Core/Rules/Conventions/NoPartialFunctions.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ open FSharpLint.Framework.Ast
open FSharpLint.Framework.Rules
open FSharp.Compiler.CodeAnalysis
open FSharp.Compiler.Symbols
open FSharp.Compiler.Syntax

[<RequireQualifiedAccess>]
type Config = {
Expand Down Expand Up @@ -110,7 +111,124 @@ let private checkIfPartialIdentifier (config:Config) (identifier:string) (range:
TypeChecks = []
})

let private isNonStaticInstanceMemberCall (checkFile:FSharpCheckFileResults) names range:(Option<WarningDetails>) =
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<FSharpExpr> |> 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<WarningDetails>) =

let typeChecks =
(partialInstanceMemberIdentifiers
Expand All @@ -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) =
Expand Down Expand Up @@ -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<WarningDetails> =
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) ->
Expand All @@ -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 =
Expand Down

0 comments on commit 791df60

Please sign in to comment.