Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add quickfix suggestions for several rules #649

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,39 @@ open FSharp.Compiler.Syntax
open FSharpLint.Framework.Ast
open FSharpLint.Framework.Rules

let private checkForBindingToAWildcard pattern range =
let private checkForBindingToAWildcard pattern range fileContent (expr: SynExpr) letBindingRange =
let rec findWildAndIgnoreParens = function
| SynPat.Paren(pattern, _) -> findWildAndIgnoreParens pattern
| SynPat.Wild(_) -> true
| _ -> false

if findWildAndIgnoreParens pattern then
{ Range = range
Message = Resources.GetString("RulesFavourIgnoreOverLetWildError")
SuggestedFix = None
TypeChecks = [] } |> Array.singleton
else
Array.empty
match ExpressionUtilities.tryFindTextOfRange expr.Range fileContent with
| Some exprText ->
if findWildAndIgnoreParens pattern then
{ Range = range
Message = Resources.GetString("RulesFavourIgnoreOverLetWildError")
SuggestedFix = Some (lazy (Some({ FromRange = letBindingRange
FromText = fileContent
ToText = sprintf "(%s) |> ignore" exprText })))
TypeChecks = [] } |> Array.singleton
else
Array.empty
| None -> Array.empty

let private runner (args:AstNodeRuleParams) =
match args.AstNode with
| AstNode.Binding(SynBinding(_, _, _, _, _, _, _, pattern, _, _, range, _))
when Helper.Binding.isLetBinding args.NodeIndex args.SyntaxArray ->
checkForBindingToAWildcard pattern range
| 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)) :: _ ->
Some(range)
| _ -> None

match bindingRange with
| Some letBindingRange ->
checkForBindingToAWildcard pattern range args.FileContent expr letBindingRange
| None -> Array.empty
| _ -> Array.empty

/// Checks if any code uses 'let _ = ...' and suggests to use the ignore function.
Expand Down
16 changes: 11 additions & 5 deletions src/FSharpLint.Core/Rules/Conventions/Binding/UselessBinding.fs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ open FSharp.Compiler.CodeAnalysis
open FSharpLint.Framework.Ast
open FSharpLint.Framework.Rules

let private checkForUselessBinding (checkInfo:FSharpCheckFileResults option) pattern expr range =
let private checkForUselessBinding (checkInfo:FSharpCheckFileResults option) pattern expr range maybeSuggestedFix =
match checkInfo with
| Some checkInfo ->
let rec findBindingIdentifier = function
Expand Down Expand Up @@ -42,17 +42,23 @@ let private checkForUselessBinding (checkInfo:FSharpCheckFileResults option) pat
|> Option.map (fun ident ->
{ Range = range
Message = Resources.GetString("RulesUselessBindingError")
SuggestedFix = None
SuggestedFix = Some (lazy(maybeSuggestedFix))
TypeChecks = [ checkNotMutable ident ] })
|> Option.toArray
| _ -> Array.empty

let private runner (args:AstNodeRuleParams) =
let maybeSuggestedFix =
match args.GetParents(args.NodeIndex) with
| AstNode.ModuleDeclaration(SynModuleDecl.Let(_, _, range)) :: _ ->
Some({ FromRange = range; FromText = "let"; ToText = "" })
| AstNode.Expression(SynExpr.LetOrUse(_, false, _, _, range)) :: _ ->
Some({ FromRange = range; FromText = "use"; ToText = "" })
| _ -> None
match args.AstNode with
| AstNode.Binding(SynBinding(_, _, _, isMutable, _, _, _, pattern, _, expr, range, _))
when Helper.Binding.isLetBinding args.NodeIndex args.SyntaxArray
&& not isMutable ->
checkForUselessBinding args.CheckInfo pattern expr range
when maybeSuggestedFix.IsSome && not isMutable ->
checkForUselessBinding args.CheckInfo pattern expr range maybeSuggestedFix
| _ ->
Array.empty

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,11 @@ let runner (config: Config) args =
if identifiers.Length = 2 then
match identifiers with
| head::_ when isNotConsistent head.idText symbol ->
let suggestedFix = lazy(Some({ FromRange = head.idRange; FromText = head.idText; ToText = symbol }))
let error =
{ Range = range
Message = String.Format(Resources.GetString "RulesFavourConsistentThis", config.Symbol)
SuggestedFix = None
SuggestedFix = Some suggestedFix
TypeChecks = List.Empty }
|> Array.singleton
error
Expand Down
9 changes: 5 additions & 4 deletions src/FSharpLint.Core/Rules/Conventions/FavourReRaise.fs
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,24 @@ open FSharpLint.Framework.Ast
open FSharpLint.Framework.Rules

let private runner (args: AstNodeRuleParams) =
let generateError range =
let generateError suggestedFix range =
{ Range = range
Message = Resources.GetString "RulesFavourReRaise"
SuggestedFix = None
SuggestedFix = Some suggestedFix
TypeChecks = List.empty }
|> Array.singleton

let rec checkExpr (expr) maybeIdent =
match expr with
| SynExpr.App (_, _, SynExpr.Ident raiseId, expression, range) when raiseId.idText = "raise" ->
let suggestedFix = lazy(Some({ FromRange = range; FromText = raiseId.idText; ToText = "reraise()" }))
match expression with
| SynExpr.Ident ident ->
match maybeIdent with
| Some id when id = ident.idText ->
generateError range
generateError suggestedFix range
| _ -> Array.empty
| SynExpr.LongIdent (_, LongIdentWithDots (id, _), _, range) -> generateError range
| SynExpr.LongIdent (_, LongIdentWithDots (id, _), _, range) -> generateError suggestedFix range
| _ -> Array.empty
| SynExpr.TryWith (expressions, _, clauseList, _expression, _range, _, _) as expr ->
clauseList
Expand Down
10 changes: 9 additions & 1 deletion src/FSharpLint.Core/Rules/Typography/NoTabCharacters.fs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,15 @@ let checkNoTabCharacters literalStrings (args:LineRuleParams) =
if isInLiteralString literalStrings range |> not then
{ Range = range
Message = Resources.GetString("RulesTypographyTabCharacterError")
SuggestedFix = None
SuggestedFix =
Some(
lazy
(Some(
{ FromRange = range
FromText = "\t"
ToText = String.replicate args.GlobalConfig.numIndentationSpaces " " }
))
)
TypeChecks = [] } |> Array.singleton
else
Array.empty
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,40 @@ let a = List.iter (fun x -> ()) []"""

Assert.IsFalse(this.ErrorsExist)

[<Test>]
member this.LetWildcardUnitValueSuggestedFix() =
let source = """
module Program

let _ = ()"""
let expected = """
module Program

(()) |> ignore"""
this.Parse source

Assert.IsTrue(this.ErrorExistsAt(4, 4))

let result = this.ApplyQuickFix source

Assert.AreEqual(expected, result)

[<Test>]
member this.LetWildCardInParanUnitValueSuggestedFix() =
let source = """
module Program

let ((((_)))) = List.iter (fun x -> ()) []"""

let expected = """
module Program

(List.iter (fun x -> ()) []) |> ignore"""

this.Parse source

Assert.IsTrue(this.ErrorExistsAt(4, 4))

let result = this.ApplyQuickFix source

Assert.AreEqual(expected, result)
43 changes: 43 additions & 0 deletions tests/FSharpLint.Core.Tests/Rules/Binding/UselessBinding.fs
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,46 @@ type Cat() =
reader.ReadToEnd()""")

Assert.IsFalse(this.ErrorsExist)

[<Test>]
member this.UselessBindingSuggestedFix() =
let source = """
module Program

let a = 10
let a = a"""
let expected = """
module Program

let a = 10
"""

this.Parse source

Assert.IsTrue(this.ErrorExistsAt(5, 4))

let result = this.ApplyQuickFix source

Assert.AreEqual(expected, result)

[<Test>]
member this.UselessBindingWithParensSuggestedFix() =
let source = """
module Program

let a = 10
let ((a)) = ((a))"""

let expected = """
module Program

let a = 10
"""

this.Parse source

Assert.IsTrue(this.ErrorExistsAt(5, 4))

let result = this.ApplyQuickFix source

Assert.AreEqual(expected, result)
Original file line number Diff line number Diff line change
Expand Up @@ -101,3 +101,77 @@ type TorMessageDigest(isSha256: bool) =

Assert.IsTrue this.NoErrorsExist

[<Test>]
member this.InConsistentSelfShouldProduceErrorSuggestedFix() =
let source = """
type Foo =
{ Bar : Baz }
member self.FooBar =
()
member this.FooBarBaz x =
failwith "foobarbaz" """

let expected = """
type Foo =
{ Bar : Baz }
member this.FooBar =
()
member this.FooBarBaz x =
failwith "foobarbaz" """

this.Parse source

Assert.IsTrue this.ErrorsExist

let result = this.ApplyQuickFix source

Assert.AreEqual(expected, result)

[<Test>]
member this.InConsistentSelfShouldProduceErrorSuggestedFix_2() =
let source = """
type Foo =
{ Bar : Baz }
member this.FooBar =
()
member self.FooBarBaz x =
failwith "foobarbaz" """

let expected = """
type Foo =
{ Bar : Baz }
member this.FooBar =
()
member this.FooBarBaz x =
failwith "foobarbaz" """

this.Parse source

Assert.IsTrue this.ErrorsExist
Assert.IsTrue(this.ErrorExistsAt(6, 11))

let result = this.ApplyQuickFix source

Assert.AreEqual(expected, result)

[<Test>]
member this.InConsistentSelfShouldProduceErrorSuggestedFix_3() =
let source = """
type CustomerName(firstName, middleInitial, lastName) =
member this.FirstName = firstName
member self.MiddleInitial = middleInitial
member this.LastName = lastName """
let expected = """
type CustomerName(firstName, middleInitial, lastName) =
member this.FirstName = firstName
member this.MiddleInitial = middleInitial
member this.LastName = lastName """

this.Parse source

Assert.IsTrue this.ErrorsExist
Assert.IsTrue(this.ErrorExistsAt(4, 11))

let result = this.ApplyQuickFix source

Assert.AreEqual(expected, result)
27 changes: 27 additions & 0 deletions tests/FSharpLint.Core.Tests/Rules/Conventions/FavourReRaise.fs
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,30 @@ with
raise ex """

this.AssertNoWarnings()

[<Test>]
member this.``using raise ex must generate error suggested fix``() =
let source = """
try
foo ()
with
| ex ->
if someCondition then
raise ex """

let expected = """
try
foo ()
with
| ex ->
if someCondition then
reraise() """

this.Parse source

Assert.IsTrue(this.ErrorsExist)
Assert.IsTrue(this.ErrorExistsAt(7, 8))

let result = this.ApplyQuickFix source

Assert.AreEqual(expected, result)
13 changes: 13 additions & 0 deletions tests/FSharpLint.Core.Tests/Rules/Typography/NoTabCharacters.fs
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,16 @@ type TestTypographyTabCharacterInFile() =

Assert.IsFalse(this.ErrorExistsAt(2, 23))
Assert.IsFalse(this.ErrorExistsAt(4, 13))

[<Test>]
member this.TabCharacterInFileSuggestedFix() =
let source = "\t"
let expected = String.replicate 4 " "
this.Parse source

Assert.IsTrue(this.ErrorExistsAt(1, 0))

let result = this.ApplyQuickFix source

Assert.AreEqual(expected, result)

Loading