Skip to content
4 changes: 2 additions & 2 deletions Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
<PackageVersion Include="BenchmarkDotNet" Version="0.15.2" />
<PackageVersion Include="BenchmarkDotNet.Diagnostics.Windows" Version="0.15.2" />
<PackageVersion Include="FParsec" Version="1.1.1" />
<PackageVersion Include="FSharp.Compiler.Service" Version="43.9.201" />
<PackageVersion Include="FSharp.Compiler.Service" Version="43.12.201" />
<PackageVersion Include="FSharp.Control.Reactive" Version="6.1.2" />
<PackageVersion Include="FSharp.Core" Version="9.0.201" />
<PackageVersion Include="FSharp.Core" Version="10.1.201" />
<PackageVersion Include="FSharpx.Collections" Version="3.1.0" />
<PackageVersion Include="Ionide.ProjInfo.ProjectSystem" Version="0.70.2" />
<PackageVersion Include="Ionide.ProjInfo.FCS" Version="0.70.2" />
Expand Down
11 changes: 1 addition & 10 deletions src/FSharpLint.Core/Application/Configuration.fs
Original file line number Diff line number Diff line change
Expand Up @@ -133,16 +133,7 @@ module IgnoreFiles =

// Non-standard record field naming for config serialization.
// fsharplint:disable RecordFieldNames
type RuleConfig<'Config

// Workaround to FSharp Compiler Service bug (see https://github.com/dotnet/fsharp/issues/19118).
// The bug is fixed already (see PR https://github.com/dotnet/fsharp/pull/19123 and commit
// https://github.com/dotnet/fsharp/commit/89d788641914c5d0b87fddfa11f4df0b5cfaa73d) and might be available in
// the next version of FCS.
// The current latest version of FCS (43.10.101) as of writing this comment does not contain that fix.
when 'Config: comparison

> = {
type RuleConfig<'Config> = {
Enabled:bool
Config:'Config option
}
Expand Down
50 changes: 27 additions & 23 deletions src/FSharpLint.Core/Framework/Ast.fs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ module Ast =
| SynModuleDecl.NestedModule(componentInfo, _, moduleDeclarations, _, _, _) ->
List.revIter (ModuleDeclaration >> add) moduleDeclarations
add <| ComponentInfo componentInfo
| SynModuleDecl.Let(_, bindings, _) -> List.revIter (Binding >> add) bindings
| SynModuleDecl.Let(_, bindings, _, _) -> List.revIter (Binding >> add) bindings
| SynModuleDecl.Expr(expression, _) -> add <| Expression expression
| SynModuleDecl.Types(typeDefinitions, _) -> List.revIter (TypeDefinition >> add) typeDefinitions
| SynModuleDecl.Exception(SynExceptionDefn.SynExceptionDefn(repr, _, members, _), _) ->
Expand Down Expand Up @@ -204,7 +204,7 @@ module Ast =
| SynMemberDefn.ImplicitInherit(synType, expression, _, _, _) ->
add <| Expression expression
add <| Type synType
| SynMemberDefn.LetBindings(bindings, _, _, _) -> List.revIter (Binding >> add) bindings
| SynMemberDefn.LetBindings(bindings, _, _, _, _) -> List.revIter (Binding >> add) bindings
| SynMemberDefn.Interface(synType, _, Some(members), _) ->
List.revIter (MemberDefinition >> add) members
add <| Type synType
Expand Down Expand Up @@ -240,7 +240,7 @@ module Ast =
| SynPat.Attrib(pattern, _, _)
| SynPat.Paren(pattern, _) -> add <| Pattern pattern
| SynPat.Named(_) -> ()
| SynPat.Record(patternsAndIdentifier, _) -> List.revIter (fun (_, _, pattern) -> pattern |> Pattern |> add) patternsAndIdentifier
| SynPat.Record(patPairFieldList, _) -> patPairFieldList |> List.revIter(_.Pattern >> Pattern >> add)
| SynPat.Const(_)
| SynPat.Wild(_)
| SynPat.FromParseError(_)
Expand Down Expand Up @@ -278,6 +278,8 @@ module Ast =
| SynExpr.Lazy(expression, _)
| SynExpr.TraitCall(_, _, expression, _)
| SynExpr.YieldOrReturn(_, expression, _, _)
| SynExpr.AnonRecd(_, Some (expression, _), _, _, _)
| SynExpr.IndexFromEnd(expression, _)
| SynExpr.YieldOrReturnFrom(_, expression, _, _) -> add <| Expression expression
| SynExpr.SequentialOrImplicitYield(_, expression1, expression2, ifNotExpression, _) ->
addMany [Expression expression1; Expression expression2; Expression ifNotExpression]
Expand All @@ -296,23 +298,12 @@ module Ast =
| SynExpr.Tuple(_, expressions, _, _)
| SynExpr.ArrayOrList(_, expressions, _) -> List.revIter (Expression >> add) expressions
| SynExpr.Record(_, Some(expr, _), _, _) -> add <| Expression expr
| SynExpr.Record(_, None, _, _) -> ()
| SynExpr.AnonRecd(_, Some (expr,_), _, _, _) ->
add <| Expression expr
| SynExpr.AnonRecd(_, None, _, _, _) -> ()
| SynExpr.ObjExpr(synType, _, _, bindings, _, _, _, _) ->
List.revIter (Binding >> add) bindings
add <| Type synType
| SynExpr.DotNamedIndexedPropertySet(expression, _, expression1, expression2, _)
| SynExpr.For(_, _, _, _, expression, _, expression1, expression2, _) ->
addMany [Expression expression2; Expression expression1; Expression expression]
| SynExpr.LetOrUseBang(_, _, _, pattern, rightHandSide, andBangs, leftHandSide, _, _) ->
addMany [Expression rightHandSide; Expression leftHandSide]
// TODO: is the the correct way to handle the new `and!` syntax?
List.iter (fun (SynExprAndBang(_, _, _, pattern, body, _, _)) ->
addMany [Expression body; Pattern pattern]
) andBangs
add <| Pattern pattern
| SynExpr.ForEach(_, _, _, _, pattern, expression, expression1, _) ->
addMany [Expression expression1; Expression expression; Pattern pattern]
| SynExpr.MatchLambda(_, _, matchClauses, _, _) ->
Expand All @@ -330,9 +321,22 @@ module Ast =
| SynExpr.Upcast(expression, synType, _)
| SynExpr.Downcast(expression, synType, _) ->
addMany [Type synType; Expression expression]
| SynExpr.LetOrUse(_, _, bindings, expression, _, _) ->
add <| Expression expression
// regular let or use
| ExpressionUtilities.LetOrUse({Bindings = bindings; Body = body}, false, _) ->
add <| Expression body
List.revIter (Binding >> add) bindings
// let! or use!
| ExpressionUtilities.LetOrUse({Bindings = bindings; Body = body}, true, _) ->
match bindings with
| firstBinding :: andBangs ->
match firstBinding with
| SynBinding(headPat = pattern; expr = rightHandSide) ->
addMany [Expression rightHandSide; Expression body]
List.iter (fun (SynBinding(headPat = pattern; expr = body)) ->
addMany [Expression body; Pattern pattern]
) andBangs
add <| Pattern pattern
| [] -> () // error case. @@TODO@@ any other handling needed here?
| SynExpr.Ident(ident) -> add <| Identifier([ident.idText], ident.idRange)
| SynExpr.LongIdent(_, SynLongIdent(ident, _, _), _, range) ->
add <| Identifier(List.map (fun (identifier: Ident) -> identifier.idText) ident, range)
Expand Down Expand Up @@ -362,17 +366,17 @@ module Ast =
| SynExpr.DotLambda(_)
| SynExpr.App(_)
| SynExpr.Fixed(_) -> ()
*)
| SynExpr.Record(_, None, _, _) -> ()
| SynExpr.AnonRecd(_, None, _, _, _) -> ()
| SynExpr.Typar(_) -> ()
| SynExpr.WhileBang(_, expression, expression1, _) ->
*)
| SynExpr.WhileBang(_, expression1, expression2, _) ->
add <| Expression expression1
add <| Expression expression
add <| Expression expression2
| SynExpr.DebugPoint(_debugPoint, _, innerExpr) ->
add <| Expression innerExpr
| SynExpr.Dynamic(funcExpr, _, argExpr, _) ->
addMany [Expression funcExpr; Expression argExpr]
| SynExpr.IndexFromEnd(expr, _) ->
add <| Expression expr
| SynExpr.IndexRange(expr1, _, expr2, _, _, _) ->
expr1 |> Option.iter (Expression >> add)
expr2 |> Option.iter (Expression >> add)
Expand Down Expand Up @@ -417,7 +421,7 @@ module Ast =
| SynArgPats.Pats(patterns) ->
patterns |> List.revIter (Pattern >> add)
| SynArgPats.NamePatPairs(namePatterns, _, _) ->
namePatterns |> List.revIter (fun (_, _, pattern) -> pattern |> Pattern |> add)
namePatterns |> List.revIter (_.Pattern >> Pattern >> add)

let inline private typeRepresentationChildren node add =
match node with
Expand Down Expand Up @@ -474,7 +478,7 @@ module Ast =
| Else(expression)
| Expression(expression) -> expressionChildren expression add

| File(ParsedInput.ImplFile(ParsedImplFileInput(_, _, _, _, _, moduleOrNamespaces, _, _, _))) ->
| File(ParsedInput.ImplFile(ParsedImplFileInput(contents = moduleOrNamespaces))) ->
moduleOrNamespaces |> List.revIter (ModuleOrNamespace >> add)

| UnionCase(unionCase) -> unionCaseChildren unionCase add
Expand Down
9 changes: 9 additions & 0 deletions src/FSharpLint.Core/Framework/Utilities.fs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,15 @@ module ExpressionUtilities =
(range.StartLine, range.StartColumn) >= (containingRange.StartLine, containingRange.StartColumn)
&& (range.EndLine, range.EndColumn) <= (containingRange.EndLine, containingRange.EndColumn)

/// Active pattern to match any SynExpr.LetOrUse
/// Returns a tuple of (isBang, isUse, record) allowing matching on both booleans and accessing the full record
/// Borrowed from https://github.com/nojaf/fsharp/blob/b3d90dc6ab9a9cedad4b8702fd8625f8f8175ae1/src/Compiler/SyntaxTree/SyntaxTreeOps.fs#L138
[<return: Struct>]
let (|LetOrUse|_|) (expr: SynExpr) =
match expr with
| SynExpr.LetOrUse(letOrUse) -> ValueSome(letOrUse, letOrUse.IsBang, letOrUse.IsUse)
| _ -> ValueNone

module String =

open System.IO
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@ let isLetBinding index (syntaxArray:AbstractSyntaxArray.Node []) =
if index > 0 then
match syntaxArray.[syntaxArray.[index].ParentIndex].Actual with
| AstNode.ModuleDeclaration(SynModuleDecl.Let(_))
| AstNode.Expression(SynExpr.LetOrUse(_, false, _, _, _, _)) -> true
| AstNode.Expression(ExpressionUtilities.LetOrUse(_, false, false)) -> true
| _ -> false
else false
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ let private runner (args:AstNodeRuleParams) =
| AstNode.Binding(SynBinding(_, _, _, _, _, _, _, pattern, _, expr, range, _, _)) ->
let bindingRange =
match args.GetParents(args.NodeIndex) with
| AstNode.ModuleDeclaration(SynModuleDecl.Let(_, _, range)) :: _
| AstNode.Expression(SynExpr.LetOrUse(_, false, _, _, range, _)) :: _ ->
| AstNode.ModuleDeclaration(SynModuleDecl.Let(_, _, range, _)) :: _
| AstNode.Expression(ExpressionUtilities.LetOrUse({Range = range}, false, false)) :: _ ->
Some(range)
| _ -> None

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ let private runner (args:AstNodeRuleParams) =

let maybeSuggestedFix =
match args.GetParents(args.NodeIndex) with
| AstNode.ModuleDeclaration(SynModuleDecl.Let(_, _, range)) :: _ ->
| AstNode.ModuleDeclaration(SynModuleDecl.Let(_, _, range, _)) :: _ ->
Some({ FromRange = range; FromText = "let"; ToText = String.Empty })
| AstNode.Expression(SynExpr.LetOrUse(_, false, _, _, range, _)) :: _ ->
| AstNode.Expression(ExpressionUtilities.LetOrUse({Range = range}, false, false)) :: _ ->
Some({ FromRange = range; FromText = "use"; ToText = String.Empty })
| _ -> None
match args.AstNode with
Expand Down
6 changes: 3 additions & 3 deletions src/FSharpLint.Core/Rules/Conventions/DisallowShadowing.fs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ let rec private processExpressions
(processArgs: SynSimplePats -> bool)
(expressions: list<SynExpr>) =
match expressions with
| SynExpr.LetOrUse(_, _, bindings, _, _, _) :: rest ->
| ExpressionUtilities.LetOrUse({Bindings = bindings}, false, _) :: rest ->
bindings |> List.exists processBinding
|| processExpressions processBinding processArgs rest
| SynExpr.Sequential(_, _, expr1, expr2, _, _) :: rest ->
Expand Down Expand Up @@ -60,7 +60,7 @@ let rec private processPatterns (definitionsAndPatterns: list<array<FSharpSymbol
| (definitions, SynPat.Paren(pat, _)) :: rest ->
processPatterns ((definitions, pat) :: rest)
| (definitions, SynPat.Record(fieldPats, _)) :: rest ->
processPatterns ((fieldPats |> List.map (fun (_, _, pat) -> (definitions, pat))) @ rest)
processPatterns ((fieldPats |> List.map (fun patPairField -> (definitions, patPairField.Pattern))) @ rest)
| (definitions, SynPat.Tuple(_, pats, _, _)) :: rest ->
processPatterns ((pats |> List.map (fun pat -> (definitions, pat))) @ rest)
| (definitions, SynPat.Typed(pat, _, _)) :: rest ->
Expand Down Expand Up @@ -114,7 +114,7 @@ let runner (args: AstNodeRuleParams) =

let processModuleDeclaration (moduleDecl: SynModuleDecl) =
match moduleDecl with
| SynModuleDecl.Let(_, bindings, _) -> bindings |> List.exists processBinding
| SynModuleDecl.Let(_, bindings, _, _) -> bindings |> List.exists processBinding
| _ -> false

let processExpression (expr: SynExpr) = processExpressions processBinding processArgs (List.singleton expr)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ let runner args =
|> List.exists
(fun decl ->
match decl with
| SynModuleDecl.Let(_, bindings, _) ->
| SynModuleDecl.Let(_, bindings, _, _) ->
bindings |> List.exists isBindingOfIdentifierToTemplate
| _ -> false)
| _ -> false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ let runner (args: AstNodeRuleParams) =
funcs
|> List.choose (processFunction false checkInfo funcs)
|> List.toArray
| AstNode.Expression(SynExpr.LetOrUse(true, _, bindings, _, _, _)), Some checkInfo ->
| AstNode.Expression(ExpressionUtilities.LetOrUse({IsRecursive = true; Bindings = bindings}, false, _)), Some checkInfo ->
match UnneededRecKeyword.getRecursiveFunctionsFromBindings bindings with
| [] -> Array.empty
| funcs ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ let rec private collectMemberBindings (acc: list<FunctionBinding>) (memberDefns:
collectMemberBindings (collectBindings (List.singleton binding) @ acc) rest
| SynMemberDefn.GetSetMember(getMember, setMember, _, _) :: rest ->
collectMemberBindings (collectBindings ((Option.toList getMember) @ (Option.toList setMember)) @ acc) rest
| SynMemberDefn.LetBindings(bindings, _, _, _) :: rest ->
| SynMemberDefn.LetBindings(bindings, _, _, _, _) :: rest ->
collectMemberBindings (collectBindings bindings @ acc) rest
| SynMemberDefn.Interface(_, _, Some(members), _) :: rest ->
collectMemberBindings acc (members @ rest)
Expand All @@ -45,7 +45,7 @@ let rec private collectMemberBindings (acc: list<FunctionBinding>) (memberDefns:
let runner (args: AstNodeRuleParams) =
let collectTopLevelFunctionBindings (declaration: SynModuleDecl): list<FunctionBinding> =
match declaration with
| SynModuleDecl.Let(_, bindings, _) -> collectBindings bindings
| SynModuleDecl.Let(_, bindings, _, _) -> collectBindings bindings
| _ -> List.empty

match args.AstNode with
Expand Down Expand Up @@ -75,7 +75,7 @@ let runner (args: AstNodeRuleParams) =
collectBindings (List.singleton binding)
| SynMemberDefn.GetSetMember(getMember, setMember, _, _) ->
collectBindings ((Option.toList getMember) @ (Option.toList setMember))
| SynMemberDefn.LetBindings(bindings, _, _, _) ->
| SynMemberDefn.LetBindings(bindings, _, _, _, _) ->
collectBindings bindings
| SynMemberDefn.Interface(_, _, Some(members), _) ->
collectMemberBindings List.empty members
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,17 +64,17 @@ let rec private processLetBinding (instanceNames: Set<string>) (body: SynExpr) (
and [<TailCall>] processExpression (expression: SynExpr) (continuation: unit -> array<WarningDetails>) : array<WarningDetails> =
Array.append
(match expression with
| SynExpr.LetOrUse(_, _, bindings, body, _, _) ->
let instanceNames = extraFromBindings bindings List.Empty |> Set.ofList
processLetBinding instanceNames body returnEmptyArray
| SynExpr.LetOrUse letOrUse when not letOrUse.IsBang ->

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is using when because when I tried the active pattern I got error FS3569: The member or function 'processLetBinding' has the 'TailCallAttribute' attribute, but is not being used in a tail recursive way.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what the TailCall attrib is for: so that the compiler makes sure you're using a tail call recursive algorithm. If you mark a func as TailCall without changing the algorithm to be a tail-call-recursive, compilation will fail.

let instanceNames = extraFromBindings letOrUse.Bindings List.Empty |> Set.ofList
processLetBinding instanceNames letOrUse.Body returnEmptyArray
| SynExpr.Sequential(_, _, expr1, expr2, _, _) ->
processExpression expr1 (fun () -> processExpression expr2 returnEmptyArray)
| _ -> Array.empty)
(continuation())

let runner args =
match args.AstNode with
| Binding(SynBinding(_, _, _, _, _, _, _, _, _, SynExpr.LetOrUse(_, _, bindings, body, _, _), _, _, _)) ->
| Binding(SynBinding(_, _, _, _, _, _, _, _, _, ExpressionUtilities.LetOrUse({Bindings = bindings; Body = body}, false, _), _, _, _)) ->
let instanceNames = extraFromBindings bindings List.Empty |> Set.ofList
processLetBinding instanceNames body returnEmptyArray
| Match(SynMatchClause(_, _, expr, _, _, _)) ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ let rec private processExpressions (errorsSoFar: array<WarningDetails>) (args: A
| SynExpr.Record(_, _, synExprRecordFields, _) :: tail ->
let mapping =
function
| SynExprRecordField(_, _, expr, _) -> expr
| SynExprRecordField(_, _, expr, _, _) -> expr
let fieldExpressions =
synExprRecordFields
|> List.choose mapping
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,10 @@ let runner (args:AstNodeRuleParams) =

let identifiers =
match args.AstNode with
| AstNode.Expression(SynExpr.LetOrUseBang(_, _, _, pat, _, _, _, _, _)) ->
getParameterWithBelowMinimumLength [pat]
| AstNode.Expression(ExpressionUtilities.LetOrUse({Bindings = binding :: _}, true, _)) ->
match binding with
| SynBinding(headPat = pat) ->
getParameterWithBelowMinimumLength [pat]
| AstNode.Expression(SynExpr.Lambda(_, _, lambdaArgs, _, _, _, _)) ->
let lambdaIdent = FunctionReimplementation.getLambdaParamIdent lambdaArgs
match lambdaIdent with
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ type private Func =
[<TailCall>]
let rec private getBindings (acc: list<SynBinding>) (declarations: list<SynModuleDecl>) =
match declarations with
| SynModuleDecl.Let(_, bindings, _) :: rest -> getBindings (bindings @ acc) rest
| SynModuleDecl.Let(_, bindings, _, _) :: rest -> getBindings (bindings @ acc) rest
| SynModuleDecl.NestedModule(_, _, innerDecls, _, _, _) :: rest -> getBindings acc (innerDecls @ rest)
| [] -> acc
| _ :: rest -> getBindings acc rest
Expand Down Expand Up @@ -145,7 +145,7 @@ let runner (config: Config) (args: AstNodeRuleParams) =
| SynArgPats.NamePatPairs(pairs, _, _) ->
let argsList =
pairs
|> List.map (fun (ident, _, _) -> ident.idRange)
|> List.map _.Range
|> List.choose (fun range -> ExpressionUtilities.tryFindTextOfRange range args.FileContent)
" " + String.Join(" ", argsList)
| SynArgPats.Pats([ SynPat.Paren(SynPat.Const(SynConst.Unit, _), _) ]) ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ let checkRecursiveAsyncFunction (args:AstNodeRuleParams) (range:Range) (doBangEx
breadcrumbs
|> List.collect (fun crumb ->
match crumb with
| AstNode.ModuleDeclaration (SynModuleDecl.Let (true, bindings, _)) ->
| AstNode.ModuleDeclaration (SynModuleDecl.Let (true, bindings, _, _)) ->
bindings
| AstNode.Expression (SynExpr.LetOrUse (true, false, bindings, _, _, _)) ->
| AstNode.Expression (ExpressionUtilities.LetOrUse ({IsRecursive = true; Bindings = bindings}, false, false)) ->
bindings
| _ -> List.Empty)
|> List.choose getFunctionNameFromAsyncCompExprBinding
Expand Down
Loading
Loading