Skip to content

Commit

Permalink
Better ranges for CE let! and use! error reporting. (dotnet#17712)
Browse files Browse the repository at this point in the history
  • Loading branch information
@edgarfgp @esbenbjerre
edgarfgp authored and esbenbjerre committed Sep 30, 2024
1 parent 2fb0d04 commit d380caa
Show file tree
Hide file tree
Showing 13 changed files with 250 additions and 42 deletions.
1 change: 1 addition & 0 deletions docs/release-notes/.FSharp.Compiler.Service/9.0.200.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,6 @@
### Changed

* Make ILTypeDef interface impls calculation lazy. ([PR #17392](https://github.com/dotnet/fsharp/pull/17392))
* Better ranges for CE `let!` and `use!` error reporting. ([PR #17712](https://github.com/dotnet/fsharp/pull/17712))

### Breaking Changes
49 changes: 24 additions & 25 deletions src/Compiler/Checking/Expressions/CheckComputationExpressions.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1589,10 +1589,10 @@ let rec TryTranslateComputationExpression
Some(TranslateComputationExpression ceenv CompExprTranslationPass.Initial q varSpace innerComp2 translatedCtxt)

else

if ceenv.isQuery && not (innerComp1.IsArbExprAndThusAlreadyReportedError) then
match innerComp1 with
| SynExpr.JoinIn _ -> () // an error will be reported later when we process innerComp1 as a sequential
| SynExpr.JoinIn _ -> ()
| SynExpr.DoBang(range = m) -> errorR (Error(FSComp.SR.tcBindMayNotBeUsedInQueries (), m))
| _ -> errorR (Error(FSComp.SR.tcUnrecognizedQueryOperator (), innerComp1.RangeOfFirstPortion))

match
Expand Down Expand Up @@ -1854,12 +1854,14 @@ let rec TryTranslateComputationExpression
// or
// --> build.BindReturn(e1, (fun _argN -> match _argN with pat -> expr-without-return))
| SynExpr.LetOrUseBang(
bindDebugPoint = spBind; isUse = false; isFromSource = isFromSource; pat = pat; rhs = rhsExpr; andBangs = []; body = innerComp) ->

let mBind =
match spBind with
| DebugPointAtBinding.Yes m -> m
| _ -> rhsExpr.Range
bindDebugPoint = spBind
isUse = false
isFromSource = isFromSource
pat = pat
rhs = rhsExpr
andBangs = []
body = innerComp
trivia = { LetOrUseBangKeyword = mBind }) ->

if ceenv.isQuery then
error (Error(FSComp.SR.tcBindMayNotBeUsedInQueries (), mBind))
Expand Down Expand Up @@ -1900,20 +1902,17 @@ let rec TryTranslateComputationExpression
pat = SynPat.Named(ident = SynIdent(id, _); isThisVal = false) as pat
rhs = rhsExpr
andBangs = []
body = innerComp)
body = innerComp
trivia = { LetOrUseBangKeyword = mBind })
| SynExpr.LetOrUseBang(
bindDebugPoint = spBind
isUse = true
isFromSource = isFromSource
pat = SynPat.LongIdent(longDotId = SynLongIdent(id = [ id ])) as pat
rhs = rhsExpr
andBangs = []
body = innerComp) ->

let mBind =
match spBind with
| DebugPointAtBinding.Yes m -> m
| _ -> rhsExpr.Range
body = innerComp
trivia = { LetOrUseBangKeyword = mBind }) ->

if ceenv.isQuery then
error (Error(FSComp.SR.tcBindMayNotBeUsedInQueries (), mBind))
Expand Down Expand Up @@ -1988,9 +1987,9 @@ let rec TryTranslateComputationExpression
Some(translatedCtxt bindExpr)

// 'use! pat = e1 ... in e2' where 'pat' is not a simple name -> error
| SynExpr.LetOrUseBang(isUse = true; pat = pat; andBangs = andBangs) ->
| SynExpr.LetOrUseBang(isUse = true; andBangs = andBangs; trivia = { LetOrUseBangKeyword = mBind }) ->
if isNil andBangs then
error (Error(FSComp.SR.tcInvalidUseBangBinding (), pat.Range))
error (Error(FSComp.SR.tcInvalidUseBangBinding (), mBind))
else
let m =
match andBangs with
Expand All @@ -2013,17 +2012,17 @@ let rec TryTranslateComputationExpression
rhs = letRhsExpr
andBangs = andBangBindings
body = innerComp
range = letBindRange) ->
trivia = { LetOrUseBangKeyword = mBind }) ->
if not (cenv.g.langVersion.SupportsFeature LanguageFeature.AndBang) then
error (Error(FSComp.SR.tcAndBangNotSupported (), comp.Range))
let andBangRange =
match andBangBindings with
| [] -> comp.Range
| h :: _ -> h.Trivia.AndBangKeyword

if ceenv.isQuery then
error (Error(FSComp.SR.tcBindMayNotBeUsedInQueries (), letBindRange))
error (Error(FSComp.SR.tcAndBangNotSupported (), andBangRange))

let mBind =
match spBind with
| DebugPointAtBinding.Yes m -> m
| _ -> letRhsExpr.Range
if ceenv.isQuery then
error (Error(FSComp.SR.tcBindMayNotBeUsedInQueries (), mBind))

let sources =
(letRhsExpr
Expand Down
7 changes: 6 additions & 1 deletion src/Compiler/SyntaxTree/SyntaxTrivia.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,15 @@ type SynExprLetOrUseTrivia =
[<NoEquality; NoComparison>]
type SynExprLetOrUseBangTrivia =
{
LetOrUseBangKeyword: range
EqualsRange: range option
}

static member Zero: SynExprLetOrUseBangTrivia = { EqualsRange = None }
static member Zero: SynExprLetOrUseBangTrivia =
{
LetOrUseBangKeyword = Range.Zero
EqualsRange = None
}

[<NoEquality; NoComparison>]
type SynExprMatchTrivia =
Expand Down
2 changes: 2 additions & 0 deletions src/Compiler/SyntaxTree/SyntaxTrivia.fsi
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,8 @@ type SynExprLetOrUseTrivia =
[<NoEquality; NoComparison>]
type SynExprLetOrUseBangTrivia =
{
/// The syntax range of the `let!` or `use!` keyword.
LetOrUseBangKeyword: range
/// The syntax range of the `=` token.
EqualsRange: range option
}
Expand Down
8 changes: 4 additions & 4 deletions src/Compiler/pars.fsy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4419,7 +4419,7 @@ declExpr:
{ let spBind = DebugPointAtBinding.Yes(rhs2 parseState 1 5)
let mEquals = rhs parseState 3
let m = unionRanges (rhs parseState 1) $8.Range
let trivia: SynExprLetOrUseBangTrivia = { EqualsRange = Some mEquals }
let trivia: SynExprLetOrUseBangTrivia = { LetOrUseBangKeyword = rhs parseState 1 ; EqualsRange = Some mEquals }
SynExpr.LetOrUseBang(spBind, ($1 = "use"), true, $2, $4, $7, $8, m, trivia) }

| OBINDER headBindingPattern EQUALS typedSequentialExprBlock hardwhiteDefnBindingsTerminator opt_OBLOCKSEP moreBinders typedSequentialExprBlock %prec expr_let
Expand All @@ -4428,7 +4428,7 @@ declExpr:
let spBind = DebugPointAtBinding.Yes(unionRanges (rhs parseState 1) $4.Range)
let mEquals = rhs parseState 3
let m = unionRanges (rhs parseState 1) $8.Range
let trivia: SynExprLetOrUseBangTrivia = { EqualsRange = Some mEquals }
let trivia: SynExprLetOrUseBangTrivia = { LetOrUseBangKeyword = rhs parseState 1 ; EqualsRange = Some mEquals }
SynExpr.LetOrUseBang(spBind, ($1 = "use"), true, $2, $4, $7, $8, m, trivia) }

| OBINDER headBindingPattern EQUALS typedSequentialExprBlock hardwhiteDefnBindingsTerminator opt_OBLOCKSEP error %prec expr_let
Expand All @@ -4437,12 +4437,12 @@ declExpr:
let mEquals = rhs parseState 3
let mAll = unionRanges (rhs parseState 1) (rhs parseState 7)
let m = $4.Range.EndRange // zero-width range
let trivia: SynExprLetOrUseBangTrivia = { EqualsRange = Some mEquals }
let trivia: SynExprLetOrUseBangTrivia = { LetOrUseBangKeyword = rhs parseState 1 ; EqualsRange = Some mEquals }
SynExpr.LetOrUseBang(spBind, ($1 = "use"), true, $2, $4, [], SynExpr.ImplicitZero m, mAll, trivia) }

| DO_BANG typedSequentialExpr IN opt_OBLOCKSEP typedSequentialExprBlock %prec expr_let
{ let spBind = DebugPointAtBinding.NoneAtDo
let trivia: SynExprLetOrUseBangTrivia = { EqualsRange = None }
let trivia: SynExprLetOrUseBangTrivia = { LetOrUseBangKeyword = Range.Zero; EqualsRange = None }
SynExpr.LetOrUseBang(spBind, false, true, SynPat.Const(SynConst.Unit, $2.Range), $2, [], $5, unionRanges (rhs parseState 1) $5.Range, trivia) }

| ODO_BANG typedSequentialExprBlock hardwhiteDefnBindingsTerminator %prec expr_let
Expand Down
196 changes: 196 additions & 0 deletions tests/FSharp.Compiler.ComponentTests/Language/ComputationExpressionTests.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -246,4 +246,200 @@ let run r2 r3 =
|> shouldFail
|> withDiagnostics [
(Error 3345, Line 22, Col 9, Line 22, Col 13, "use! may not be combined with and!")
]

[<Fact>]
let ``This control construct may only be used if the computation expression builder defines a 'Bind' method`` () =
Fsx """
module Result =
let zip x1 x2 =
match x1,x2 with
| Ok x1res, Ok x2res -> Ok (x1res, x2res)
| Error e, _ -> Error e
| _, Error e -> Error e
type ResultBuilder() =
member _.MergeSources(t1: Result<'T,'U>, t2: Result<'T1,'U>) = Result.zip t1 t2
member _.BindReturn(x: Result<'T,'U>, f) = Result.map f x
member _.Delay(f) = f()
member _.TryWith(r: Result<'T,'U>, f) =
match r with
| Ok x -> Ok x
| Error e -> f e
let result = ResultBuilder()
let run r2 r3 =
result {
let! a = r2
return! a
}
"""
|> ignoreWarnings
|> typecheck
|> shouldFail
|> withDiagnostics [
(Error 708, Line 23, Col 9, Line 23, Col 13, "This control construct may only be used if the computation expression builder defines a 'Bind' method")
]

[<Fact>]
let ``This control construct may only be used if the computation expression builder defines a 'Using' method`` () =
Fsx """
module Result =
let zip x1 x2 =
match x1,x2 with
| Ok x1res, Ok x2res -> Ok (x1res, x2res)
| Error e, _ -> Error e
| _, Error e -> Error e
type ResultBuilder() =
member _.MergeSources(t1: Result<'T,'U>, t2: Result<'T1,'U>) = Result.zip t1 t2
member _.BindReturn(x: Result<'T,'U>, f) = Result.map f x
member _.Delay(f) = f()
member _.TryWith(r: Result<'T,'U>, f) =
match r with
| Ok x -> Ok x
| Error e -> f e
let result = ResultBuilder()
let run r2 r3 =
result {
use! a = r2
return! a
}
"""
|> ignoreWarnings
|> typecheck
|> shouldFail
|> withDiagnostics [
(Error 708, Line 23, Col 9, Line 23, Col 13, "This control construct may only be used if the computation expression builder defines a 'Using' method")
]

[<Fact>]
let ``do! expressions may not be used in queries`` () =
Fsx """
query {
do! failwith ""
yield 1
}
"""
|> ignoreWarnings
|> typecheck
|> shouldFail
|> withDiagnostics [
(Error 3143, Line 3, Col 5, Line 3, Col 20, "'let!', 'use!' and 'do!' expressions may not be used in queries")
]

[<Fact>]
let ``let! expressions may not be used in queries`` () =
Fsx """
query {
let! x = failwith ""
yield 1
}
"""
|> ignoreWarnings
|> typecheck
|> shouldFail
|> withDiagnostics [
(Error 3143, Line 3, Col 5, Line 3, Col 9, "'let!', 'use!' and 'do!' expressions may not be used in queries")
]

[<Fact>]
let ``let!, and! expressions may not be used in queries`` () =
Fsx """
query {
let! x = failwith ""
and! y = failwith ""
yield 1
}
"""
|> ignoreWarnings
|> typecheck
|> shouldFail
|> withDiagnostics [
(Error 3143, Line 3, Col 5, Line 3, Col 9, "'let!', 'use!' and 'do!' expressions may not be used in queries")
]

[<Fact>]
let ``use! expressions may not be used in queries`` () =
Fsx """
query {
use! x = failwith ""
yield 1
}
"""
|> ignoreWarnings
|> typecheck
|> shouldFail
|> withDiagnostics [
(Error 3143, Line 3, Col 5, Line 3, Col 9, "'let!', 'use!' and 'do!' expressions may not be used in queries")
]

[<Fact>]
let ``do! expressions may not be used in queries(SynExpr.Sequential)`` () =
Fsx """
query {
for c in [1..10] do
do! failwith ""
yield 1
}
"""
|> ignoreWarnings
|> typecheck
|> shouldFail
|> withDiagnostics [
(Error 3143, Line 4, Col 5, Line 4, Col 20, "'let!', 'use!' and 'do!' expressions may not be used in queries")
]

[<Fact>]
let ``let! expressions may not be used in queries(SynExpr.Sequential)`` () =
Fsx """
query {
for c in [1..10] do
let! x = failwith ""
yield 1
}
"""
|> ignoreWarnings
|> typecheck
|> shouldFail
|> withDiagnostics [
(Error 3143, Line 4, Col 5, Line 4, Col 9, "'let!', 'use!' and 'do!' expressions may not be used in queries")
]

[<Fact>]
let ``let!, and! expressions may not be used in queries(SynExpr.Sequential)`` () =
Fsx """
query {
for c in [1..10] do
let! x = failwith ""
and! y = failwith ""
yield 1
}
"""
|> ignoreWarnings
|> typecheck
|> shouldFail
|> withDiagnostics [
(Error 3143, Line 4, Col 5, Line 4, Col 9, "'let!', 'use!' and 'do!' expressions may not be used in queries")
]

[<Fact>]
let ``use! expressions may not be used in queries(SynExpr.Sequential)`` () =
Fsx """
query {
for c in [1..10] do
use! x = failwith ""
yield 1
}
"""
|> ignoreWarnings
|> typecheck
|> shouldFail
|> withDiagnostics [
(Error 3143, Line 4, Col 5, Line 4, Col 9, "'let!', 'use!' and 'do!' expressions may not be used in queries")
]
4 changes: 3 additions & 1 deletion ...FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.debug.bsl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10203,7 +10203,9 @@ FSharp.Compiler.SyntaxTrivia.SynExprLetOrUseBangTrivia: FSharp.Compiler.SyntaxTr
FSharp.Compiler.SyntaxTrivia.SynExprLetOrUseBangTrivia: Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Text.Range] EqualsRange
FSharp.Compiler.SyntaxTrivia.SynExprLetOrUseBangTrivia: Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Text.Range] get_EqualsRange()
FSharp.Compiler.SyntaxTrivia.SynExprLetOrUseBangTrivia: System.String ToString()
FSharp.Compiler.SyntaxTrivia.SynExprLetOrUseBangTrivia: Void .ctor(Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Text.Range])
FSharp.Compiler.SyntaxTrivia.SynExprLetOrUseBangTrivia: FSharp.Compiler.Text.Range LetOrUseBangKeyword
FSharp.Compiler.SyntaxTrivia.SynExprLetOrUseBangTrivia: FSharp.Compiler.Text.Range get_LetOrUseBangKeyword()
FSharp.Compiler.SyntaxTrivia.SynExprLetOrUseBangTrivia: Void .ctor(FSharp.Compiler.Text.Range, Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Text.Range])
FSharp.Compiler.SyntaxTrivia.SynExprLetOrUseTrivia: FSharp.Compiler.SyntaxTrivia.SynExprLetOrUseTrivia Zero
FSharp.Compiler.SyntaxTrivia.SynExprLetOrUseTrivia: FSharp.Compiler.SyntaxTrivia.SynExprLetOrUseTrivia get_Zero()
FSharp.Compiler.SyntaxTrivia.SynExprLetOrUseTrivia: Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Text.Range] InKeyword
Expand Down
4 changes: 3 additions & 1 deletion ...harp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.release.bsl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10203,7 +10203,9 @@ FSharp.Compiler.SyntaxTrivia.SynExprLetOrUseBangTrivia: FSharp.Compiler.SyntaxTr
FSharp.Compiler.SyntaxTrivia.SynExprLetOrUseBangTrivia: Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Text.Range] EqualsRange
FSharp.Compiler.SyntaxTrivia.SynExprLetOrUseBangTrivia: Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Text.Range] get_EqualsRange()
FSharp.Compiler.SyntaxTrivia.SynExprLetOrUseBangTrivia: System.String ToString()
FSharp.Compiler.SyntaxTrivia.SynExprLetOrUseBangTrivia: Void .ctor(Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Text.Range])
FSharp.Compiler.SyntaxTrivia.SynExprLetOrUseBangTrivia: FSharp.Compiler.Text.Range LetOrUseBangKeyword
FSharp.Compiler.SyntaxTrivia.SynExprLetOrUseBangTrivia: FSharp.Compiler.Text.Range get_LetOrUseBangKeyword()
FSharp.Compiler.SyntaxTrivia.SynExprLetOrUseBangTrivia: Void .ctor(FSharp.Compiler.Text.Range, Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Text.Range])
FSharp.Compiler.SyntaxTrivia.SynExprLetOrUseTrivia: FSharp.Compiler.SyntaxTrivia.SynExprLetOrUseTrivia Zero
FSharp.Compiler.SyntaxTrivia.SynExprLetOrUseTrivia: FSharp.Compiler.SyntaxTrivia.SynExprLetOrUseTrivia get_Zero()
FSharp.Compiler.SyntaxTrivia.SynExprLetOrUseTrivia: Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Text.Range] InKeyword
Expand Down
Loading

0 comments on commit d380caa

Please sign in to comment.