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

Better ranges for CE let! and use! error reporting. #17712

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

edgarfgp
Copy link
Contributor

@edgarfgp edgarfgp commented Sep 12, 2024

Description

Better ranges for CE let! and use! error reporting. Continuation of #17671

Before

  • let! in an CE builder
Screenshot 2024-09-20 at 11 20 12
  • let! in a query builder
Screenshot 2024-09-20 at 11 20 49
  • use! in an CE builder
Screenshot 2024-09-20 at 11 26 03
  • use! in a query builder
Screenshot 2024-09-20 at 11 28 00

After

  • let! in an CE builder
let run r2 r3 =
    result {
        let! a = r2
        ^^^^
        return! a
    }
  • let! in a query builder
query {
    let! x = failwith ""
    ^^^^
    yield 1
}
  • use! in an CE builder
let run r2 r3 =
    result {
        use! a = r2
        ^^^^
        return! a
    }
  • use! in a query builder
query {
    use! x = failwith ""
    ^^^^
    yield 1
}

Checklist

  • Test cases added
  • Release notes entry updated

Copy link
Contributor

github-actions bot commented Sep 12, 2024

❗ Release notes required


✅ Found changes and release notes in following paths:

Change path Release notes path Description
src/Compiler docs/release-notes/.FSharp.Compiler.Service/9.0.200.md

@edgarfgp edgarfgp force-pushed the better-ranges-for-ce-error-reporting branch from 1f0ec69 to d618bc8 Compare September 16, 2024 18:17
@edgarfgp edgarfgp force-pushed the better-ranges-for-ce-error-reporting branch from 61f0d94 to e94d0b4 Compare September 18, 2024 20:01
@edgarfgp edgarfgp closed this Sep 19, 2024
@edgarfgp edgarfgp reopened this Sep 19, 2024
@edgarfgp edgarfgp changed the title Better ranges for CE error reporting Better ranges for CE let! and use! error reporting. Sep 19, 2024
@edgarfgp edgarfgp marked this pull request as ready for review September 20, 2024 10:39
@edgarfgp edgarfgp requested a review from a team as a code owner September 20, 2024 10:39
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) }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is confusing that we are using SynExpr.LetOrUseBang to represent DO_BANG

Comment on lines -62 to -64
neg61.fs(97,13,97,33): typecheck error FS3143: 'let!', 'use!' and 'do!' expressions may not be used in queries

neg61.fs(102,13,102,28): typecheck error FS3145: This is not a known query operator. Query operators are identifiers such as 'select', 'where', 'sortBy', 'thenBy', 'groupBy', 'groupValBy', 'join', 'groupJoin', 'sumBy' and 'averageBy', defined using corresponding methods on the 'QueryBuilder' type.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We now will show

  • 1 error when let!, use! and do! are involved in queries.
  • 1 error for for anything else that is not allowed.

Note: we not longer show 2 errors at the same time which seems unnecessary specially when using let!, 'use! and 'do!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants