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

Reset Langver for OverloadsForCustomOperations #10164

Merged
merged 1 commit into from
Sep 21, 2020

Conversation

KevinRansom
Copy link
Member

This change produces unhelpful error messages. We should improve the messages before promoting this to FSharp5.0
This code produces:

let q1 = // no errors
    query {
        for d in [1..10] do
            if d > 3 then
                select d
    }

let q2 = 
    query {
        for d in [1..10] do
            where (d > 3)
                select d
    }

Produces:

error FS3095: 'select' is not used correctly. This is a custom operation in this query or computation expression.
error FS0039: The value or constructor 'd' is not defined.
error FS0501: The member or object constructor 'Where' takes 2 argument(s) but is here given 4. The required signature is 'member Linq.QueryBuilder.Where : source:Linq.QuerySource<'T,'Q> * predicate:('T -> bool) -> Linq.QuerySource<'T,'Q>'.

Instead of:

error FS3099: 'where' is used with an incorrect number of arguments. This is a custom operation in this query or computation expression. Expected 1 argument(s), but given 3.

I have to say both are pretty rubbish ...

I would be open to the argument that we will allow the update to happen, and someone will commit to fix the error messages.

@KevinRansom KevinRansom reopened this Sep 21, 2020
@KevinRansom
Copy link
Member Author

Nope, I can't do it:
This code:

// #Conformance #DataExpressions #Query
// DevDiv:210830, groupValBy with poor diagnostics
//<Expects status="error" span="(9,6-9,32)" id="FS0001">This expression was expected to have type.+'System\.Linq\.IGrouping<'a,'b>'.+but here has type.+'unit'</Expects>

let words = ["blueberry"; "chimpanzee"; ]
let wordGroups =
query {
   for w in words do
     groupValBy w w.[0]. into g
     select (g.Key, g.ToArray())
}

produces these errors:

c:\kevinransom\fsharp>artifacts\bin\fsc\Debug\net472\fsc C:\kevinransom\fsharp\tests\fsharpqa\Source\Conformance\Expressions\DataExpressions\QueryExpressions\E_BadGroupValBy02.fs
Microsoft (R) F# Compiler version 11.0.0.0 for F# 5.0
Copyright (c) Microsoft Corporation. All Rights Reserved.

C:\kevinransom\fsharp\tests\fsharpqa\Source\Conformance\Expressions\DataExpressions\QueryExpressions\E_BadGroupValBy02.fs(7,1): 
warning FS0058: Possible incorrect indentation: this token is offside of context started at position (6:1). Try indenting this token further or using standard formatting conventions.

C:\kevinransom\fsharp\tests\fsharpqa\Source\Conformance\Expressions\DataExpressions\QueryExpressions\E_BadGroupValBy02.fs(7,1): 
warning FS0058: Possible incorrect indentation: this token is offside of context started at position (6:1). Try indenting this token further or using standard formatting conventions.

C:\kevinransom\fsharp\tests\fsharpqa\Source\Conformance\Expressions\DataExpressions\QueryExpressions\E_BadGroupValBy02.fs(9,6): 
error FS3099: 'groupValBy' is used with an incorrect number of arguments. This is a custom operation in this query or computation expression. Expected 2 argument(s), but given 3.

C:\kevinransom\fsharp\tests\fsharpqa\Source\Conformance\Expressions\DataExpressions\QueryExpressions\E_BadGroupValBy02.fs(9,6): 
error FS0001: This expression was expected to have type
    'System.Linq.IGrouping<'a,'b>'
but here has type
    'unit'

@KevinRansom KevinRansom merged commit 53e58f4 into dotnet:release/dev16.8 Sep 21, 2020
@KevinRansom KevinRansom deleted the langver branch October 29, 2020 18:57
nosami pushed a commit to xamarin/visualfsharp that referenced this pull request Feb 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants