Skip to content

Commit 10662ae

Browse files
jgardellajgardella
authored
Add new rule NoPartialFunctions (#453)
* Start implementing rule for no partial functions * Add collection functions * Add quickfix for partial function replacement * Move error strings to resources file * Add config for NoPartialFunctions rule * Add doc page for rule * Update changelog * Add Map.find and Map.tryFind * Bump version * Update list of rules in docs Co-authored-by: Jason Gardella <[email protected]>
1 parent 0cc5dd5 commit 10662ae

File tree

15 files changed

+231
-12
lines changed

15 files changed

+231
-12
lines changed

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file.
44
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
55
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
66

7+
## [0.18.0]
8+
### Added
9+
- New rule `NoPartialFunctions (FL0066)`.
10+
711
## [0.17.1] - 2020-11-25
812
- Fix for records being counted as a nested statement #464 [@davidtgillard]
913

‎docs/content/how-tos/rule-configuration.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,3 +98,4 @@ The following rules can be specified for linting.
9898
- [TrailingNewLineInFile (FL0063)](rules/FL0063.html)
9999
- [NoTabCharacters (FL0064)](rules/FL0064.html)
100100
- [Hints (FL0065)](rules/FL0065.html)
101+
- [NoPartialFunctions (FL0066)](rules/FL0066.html)

‎docs/content/how-tos/rules/FL0066.md‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
---
2+
title: FL0066
3+
category: how-to
4+
hide_menu: true
5+
---
6+
7+
# NoPartialFunctions (FL0066)
8+
9+
## Cause
10+
11+
A partial function was used.
12+
13+
## Rationale
14+
15+
Partial functions are only valid for certain inputs; for invalid inputs they will throw an exception. In most cases,
16+
it is better to use a non-partial version of the function (e.g. `List.tryFind` instead of `List.find`) or use pattern matching,
17+
so that you need to explicitly handle those cases which would cause an exception in the partial version.
18+
19+
## How To Fix
20+
21+
Use non-partial function or pattern matching.
22+
23+
## Rule Settings
24+
25+
{
26+
"noPartialFunctions": {
27+
"enabled": false,
28+
"config": {
29+
"allowedPartials": [],
30+
"additionalPartials": []
31+
}
32+
}
33+
}
34+
35+
* *allowedPartials* - list of strings representing partial functions to allow (e.g. `"List.tryFind"`)
36+
* *additionalPartials* - list of strings representing additional partial functions to disallow

‎src/FSharpLint.Core/Application/Configuration.fs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,8 @@ type Configuration =
422422
TrailingWhitespaceOnLine:RuleConfig<TrailingWhitespaceOnLine.Config> option
423423
MaxLinesInFile:RuleConfig<MaxLinesInFile.Config> option
424424
TrailingNewLineInFile:EnabledConfig option
425-
NoTabCharacters:EnabledConfig option }
425+
NoTabCharacters:EnabledConfig option
426+
NoPartialFunctions:RuleConfig<NoPartialFunctions.Config> option }
426427
with
427428
static member Zero = {
428429
Global = None
@@ -496,6 +497,7 @@ with
496497
MaxLinesInFile = None
497498
TrailingNewLineInFile = None
498499
NoTabCharacters = None
500+
NoPartialFunctions = None
499501
}
500502

501503
// fsharplint:enable RecordFieldNames
@@ -632,6 +634,7 @@ let flattenConfig (config:Configuration) =
632634
config.MaxLinesInFile |> Option.bind (constructRuleWithConfig MaxLinesInFile.rule)
633635
config.TrailingNewLineInFile |> Option.bind (constructRuleIfEnabled TrailingNewLineInFile.rule)
634636
config.NoTabCharacters |> Option.bind (constructRuleIfEnabled NoTabCharacters.rule)
637+
config.NoPartialFunctions |> Option.bind (constructRuleWithConfig NoPartialFunctions.rule)
635638
|] |> Array.choose id
636639

637640
let astNodeRules = ResizeArray()

‎src/FSharpLint.Core/DefaultConfiguration.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,13 @@
268268
},
269269
"trailingNewLineInFile": { "enabled": false },
270270
"noTabCharacters": { "enabled": true },
271+
"noPartialFunctions": {
272+
"enabled": false,
273+
"config": {
274+
"allowedPartials": [],
275+
"additionalPartials": []
276+
}
277+
},
271278
"hints": {
272279
"add": [
273280
"not (a = b) ===> a <> b",

‎src/FSharpLint.Core/FSharpLint.Core.fsproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
<Compile Include="Rules\Conventions\RecursiveAsyncFunction.fs" />
4646
<Compile Include="Rules\Conventions\RedundantNewKeyword.fs" />
4747
<Compile Include="Rules\Conventions\NestedStatements.fs" />
48+
<Compile Include="Rules\Conventions\NoPartialFunctions.fs" />
4849
<Compile Include="Rules\Conventions\RaiseWithTooManyArguments\RaiseWithTooManyArgumentsHelper.fs" />
4950
<Compile Include="Rules\Conventions\RaiseWithTooManyArguments\FailwithWithSingleArgument.fs" />
5051
<Compile Include="Rules\Conventions\RaiseWithTooManyArguments\RaiseWithSingleArgument.fs" />

‎src/FSharpLint.Core/Framework/AbstractSyntaxArray.fs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ module AbstractSyntaxArray =
164164
/// Get hash code of an ast node to be used for the fuzzy match of hints against the ast.
165165
let private getHashCode node =
166166
match node with
167-
| Identifier(idents) -> getIdentHash idents
167+
| Identifier(idents, _) -> getIdentHash idents
168168
| Pattern(SynPat.Const(SynConst.Bool(x), _))
169169
| Expression(SynExpr.Const(SynConst.Bool(x), _)) -> hash x
170170
| Pattern(SynPat.Const(SynConst.Byte(x), _))

‎src/FSharpLint.Core/Framework/Ast.fs‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
namespace FSharpLint.Framework
22

3+
open FSharp.Compiler.Range
4+
35
/// Used to walk the FSharp Compiler's abstract syntax tree,
46
/// so that each node can be visited by a list of visitors.
57
module Ast =
@@ -30,7 +32,7 @@ module Ast =
3032
| ConstructorArguments of SynArgPats
3133
| TypeParameter of SynTypar
3234
| InterfaceImplementation of SynInterfaceImpl
33-
| Identifier of string list
35+
| Identifier of string list * range : range
3436
| File of ParsedInput
3537
| LambdaBody of SynExpr
3638
| LambdaArg of SynSimplePats
@@ -326,9 +328,9 @@ module Ast =
326328
| SynExpr.LetOrUse(_, _, bindings, expression, _) ->
327329
add <| Expression expression
328330
bindings |> List.revIter (Binding >> add)
329-
| SynExpr.Ident(ident) -> add <| Identifier([ident.idText])
330-
| SynExpr.LongIdent(_, LongIdentWithDots(ident, _), _, _) ->
331-
add <| Identifier(ident |> List.map (fun x -> x.idText))
331+
| SynExpr.Ident(ident) -> add <| Identifier([ident.idText], ident.idRange)
332+
| SynExpr.LongIdent(_, LongIdentWithDots(ident, _), _, range) ->
333+
add <| Identifier(ident |> List.map (fun x -> x.idText), range)
332334
| SynExpr.IfThenElse(cond, body, Some(elseExpr), _, _, _, _) ->
333335
add <| Else elseExpr
334336
add <| Expression body
@@ -374,7 +376,7 @@ module Ast =
374376
add <| Type synType
375377
add <| SimplePattern simplePattern
376378
| SynSimplePat.Attrib(simplePattern, _, _) -> add <| SimplePattern simplePattern
377-
| SynSimplePat.Id(identifier, _, _, _, _, _) -> add <| Identifier([identifier.idText])
379+
| SynSimplePat.Id(identifier, _, _, _, _, _) -> add <| Identifier([identifier.idText], identifier.idRange)
378380

379381
let inline private matchChildren node add =
380382
match node with

‎src/FSharpLint.Core/Rules/Conventions/NoPartialFunctions.fs‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
module FSharpLint.Rules.NoPartialFunctions
2+
3+
open System
4+
open FSharp.Compiler.Range
5+
open FSharpLint.Framework
6+
open FSharpLint.Framework.Suggestion
7+
open FSharpLint.Framework.Ast
8+
open FSharpLint.Framework.Rules
9+
10+
[<RequireQualifiedAccess>]
11+
type Config = {
12+
AllowedPartials:string list
13+
AdditionalPartials:string list
14+
}
15+
16+
type private Replacement =
17+
| PatternMatch
18+
| Function of functionName:string
19+
20+
let private partialFunctionIdentifiers =
21+
[
22+
// Option
23+
("Option.get", PatternMatch)
24+
25+
// Map
26+
("Map.find", Function "Map.tryFind")
27+
("Map.findKey", Function "Map.tryFindKey")
28+
29+
// Array
30+
("Array.exactlyOne", Function "Array.tryExactlyOne")
31+
("Array.get", Function "Array.tryItem")
32+
("Array.item", Function "Array.tryItem")
33+
("Array.find", Function "Array.tryFind")
34+
("Array.findIndex", Function "Array.tryFindIndex")
35+
("Array.findBack", Function "Array.tryFindBack")
36+
("Array.head", Function "Array.tryHead")
37+
("Array.last", Function "Array.tryLast")
38+
("Array.tail", Function "Array.tryLast")
39+
("Array.reduce", Function "Array.fold")
40+
("Array.reduceBack", Function "Array.foldBack")
41+
("Array.pick", Function "Array.tryPick")
42+
43+
// Seq
44+
("Seq.exactlyOne", Function "Seq.tryExactlyOne")
45+
("Seq.item", Function "Seq.tryItem")
46+
("Seq.find", Function "Seq.tryFind")
47+
("Seq.findIndex", Function "Seq.tryFindIndex")
48+
("Seq.findBack", Function "Seq.tryFindBack")
49+
("Seq.head", Function "Seq.tryHead")
50+
("Seq.last", Function "Seq.tryLast")
51+
("Seq.tail", Function "Seq.tryLast")
52+
("Seq.reduce", Function "Seq.fold")
53+
("Seq.reduceBack", Function "Seq.foldBack")
54+
("Seq.pick", Function "Seq.tryPick")
55+
56+
// List
57+
("List.exactlyOne", Function "List.tryExactlyOne")
58+
("List.item", Function "List.tryItem")
59+
("List.find", Function "List.tryFind")
60+
("List.findIndex", Function "List.tryFindIndex")
61+
("List.findBack", Function "List.tryFindBack")
62+
("List.head", Function "List.tryHead")
63+
("List.last", Function "List.tryLast")
64+
("List.tail", Function "List.tryLast")
65+
("List.reduce", Function "List.fold")
66+
("List.reduceBack", Function "List.foldBack")
67+
("List.pick", Function "List.tryPick")
68+
] |> Map.ofList
69+
70+
let private checkIfPartialIdentifier (config:Config) (identifier:string) (range:range) =
71+
if List.contains identifier config.AllowedPartials then
72+
None
73+
elif List.contains identifier config.AdditionalPartials then
74+
Some {
75+
Range = range
76+
Message = String.Format(Resources.GetString ("RulesConventionsNoPartialFunctionsAdditionalError"), identifier)
77+
SuggestedFix = None
78+
TypeChecks = []
79+
}
80+
else
81+
Map.tryFind identifier partialFunctionIdentifiers
82+
|> Option.filter (fun _ -> not (List.contains identifier config.AllowedPartials))
83+
|> Option.map (function
84+
| PatternMatch ->
85+
{
86+
Range = range
87+
Message = String.Format(Resources.GetString ("RulesConventionsNoPartialFunctionsPatternMatchError"), identifier)
88+
SuggestedFix = None
89+
TypeChecks = []
90+
}
91+
| Function replacementFunction ->
92+
{
93+
Range = range
94+
Message = String.Format(Resources.GetString "RulesConventionsNoPartialFunctionsReplacementError", replacementFunction, identifier)
95+
SuggestedFix = Some (lazy ( Some { FromText = identifier; FromRange = range; ToText = replacementFunction }))
96+
TypeChecks = []
97+
})
98+
99+
let private runner (config:Config) (args:AstNodeRuleParams) =
100+
match args.AstNode with
101+
| AstNode.Identifier (identifier, range) ->
102+
checkIfPartialIdentifier config (String.concat "." identifier) range
103+
|> Option.toArray
104+
| _ ->
105+
Array.empty
106+
107+
let rule config =
108+
{ Name = "NoPartialFunctions"
109+
Identifier = Identifiers.NoPartialFunctions
110+
RuleConfig = { AstNodeRuleConfig.Runner = runner config
111+
Cleanup = ignore } }
112+
|> AstNodeRule

‎src/FSharpLint.Core/Rules/Identifiers.fs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,3 +70,4 @@ let MaxLinesInFile = identifier 62
7070
let TrailingNewLineInFile = identifier 63
7171
let NoTabCharacters = identifier 64
7272
let Hints = identifier 65
73+
let NoPartialFunctions = identifier 66

0 commit comments

Comments
 (0)