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 an attribute enforcing the use of named parameters at callsite #414

Open
baronfel opened this issue Oct 20, 2016 · 16 comments
Open

Add an attribute enforcing the use of named parameters at callsite #414

baronfel opened this issue Oct 20, 2016 · 16 comments

Comments

@baronfel
Copy link
Contributor

baronfel commented Oct 20, 2016

Submitted by Gauthier Segay on 2/19/2015 12:00:00 AM
4 votes on UserVoice prior to migration

In some code, it's critical to have function/methods called with parameter names at call site, for readability reasons, but also for correctness. I would like F# to enforce this using an attribute for example: [<EnforceNamedParametersAtCallSite>]. Suppose we have this code (for sake of showing the idea):

type Foo() =
    member x.Bar(b, a) = (a, b)

let foo = new Foo()
foo.Bar(1.00, 0.00)
foo.Bar(a = 1.00, b = 0.00)

now suppose we have this slight change in the code (just swapped the parameter names)

type Foo() =
    member x.Bar(b, a) = (a, b)

this would not produce the expected result foo.Bar(1.00, 0.00). With the proposed feature:

type Foo() =
    [<EnforceNamedParametersAtCallSite>]
    member x.Bar(b, a) = (a, b)

foo.Bar(1.00, 0.00) // doesn't compile, expects usage of parameter names

Another example is with FSharp.Data.SqlClient's SqlCommandProvider, which defines methods taking parameters based on their order of appearance in the sql code.

samplesql1.sql
------------------------------------
declare @numerator float
declare @denominator float
set @numerator = @a
set @denominator = @b
select @numerator / @denominator
samplesql2.sql
------------------------------------
declare @numerator float
declare @denominator float
set @denominator = @b // swapped this line, didn't change anything else
set @numerator = @a
select @numerator / @denominator
sample1.fsx
#r @"..\tools\nuget\packages\FSharp.Data.SqlClient\lib\net40\FSharp.Data.SqlClient.dll"
open FSharp.Data
type divideInSql = SqlCommandProvider< "sample1.sql", YourConnectionString>
let cmd = new divideInSql()
cmd.Execute(0.0, 1.0) // work but wait bellow (don't want that to compile)
cmd.Execute(a = 0.0, b = 1.0) // works
cmd.Execute(b = 1.0, a = 0.0) // works
sample2.fsx
#r @"..\tools\nuget\packages\FSharp.Data.SqlClient\lib\net40\FSharp.Data.SqlClient.dll"
open FSharp.Data
type divideInSql = SqlCommandProvider< "sample2.sql", YourConnectionString>
let cmd = new divideInSql()
cmd.Execute(0.0, 1.0) // fails divide by 0 (don't want that to compile)
cmd.Execute(a = 0.0, b = 1.0) // works
cmd.Execute(b = 1.0, a = 0.0) // works

We have seen by now that change of order in a sql file provokes havoc. API design wise, some people might want to enforce that callsite will use named parameters to avoid those kind of misshaps.
I hope the description is clear enough.

Original UserVoice Submission
Archived Uservoice Comments

@dsyme dsyme removed the open label Oct 29, 2016
@dsyme dsyme changed the title An attribute enforcing usage (in F# code) of named parameters at callsite An attribute enforcing the use of named parameters at callsite Oct 29, 2016
@dsyme dsyme changed the title An attribute enforcing the use of named parameters at callsite Add an attribute enforcing the use of named parameters at callsite Oct 29, 2016
@smoothdeveloper
Copy link
Contributor

smoothdeveloper commented Feb 1, 2020

@dsyme I have a bit of prototype code making this check work at callsite.

Can this suggestion be (edit) considered for marked "approved on principle", in which case I'll work on RFC adressing the behaviour in greater details.

The compiler should also have checks on the method definitions, and probably fair amount of edge cases I'm not considering.

image

@smoothdeveloper
Copy link
Contributor

smoothdeveloper commented Feb 1, 2020

There was one comment from @dsyme on UserVoice:

I can see the rationale. It feels uncomfortable to add a bespoke attribute for this though I don't have specific better suggestions as yet. There's nothing quite like this in the F# language design, apart from the requirement to name record fields in the { a=1; b = 2} construction syntax, which is of course a related problem.

edit:

I feel there is something remotely related in F# language: RequireQualifiedAccess where you need to name the module or the discriminated union type.

@charlesroddie
Copy link

Interesting. A little unclear how F#-specific this is. There might be similar syntax in C#: there instead of parameters they have fields that you initialize using FieldName = ...., and if this initialization is compulsary ( dotnet/csharplang#2328 ) the syntax could be similar to this proposal.

@gusty
Copy link

gusty commented Feb 2, 2020

I'm generally in favor of this suggestion, but I think this should be implemented together with another suggestion (still not submitted) which would allow to curry the parameters by indicating its order in a succinct way.

Something like myMethod{name,address,phone} which would de-sugar to fun (a, b, c) -> myMethod(name=a, address=b, phone=c).

Note that here's only 3 parameters, in real code this is normally much more and when you have 8 or more parameters writing that "fun" is not that fun.

@smoothdeveloper
Copy link
Contributor

There is a roslyn analyzer doing the same:
https://github.com/mykolav/require-named-args-fs

Also an implementation in Dart:
https://stackoverflow.com/questions/49768439/i-want-to-use-named-parameters-in-dart-for-clarity-how-should-i-handle-them

@dsyme, @cartermp, any feedback wether the feature is a good fit for F#?

The minimum changes don't seem to amount to a lot of code:

dotnet/fsharp@main...smoothdeveloper:argument-names-enforced

@cartermp
Copy link
Member

cartermp commented Nov 8, 2020

I'd personally be in favor but this is a @dsyme call

@smoothdeveloper
Copy link
Contributor

There is another existing feature which is alike in the compiler: [<RequiresExplicitTypeArguments>]

type [<Measure>] cm
type [<Measure>] km
let [<RequiresExplicitTypeArguments>] removeUoM<[<Measure>] 'a> (v: float<'a>) = float v
let k = removeUoM<cm> 1.<cm>
//let bad = removeUoM<cm> 1.<km>
let bad2 = removeUoM 1.<cm> // error FS0685: The generic function 'removeUoM' must be given explicit type argument(s)

@dsyme is that helping making a good case for this feature?

@charlesroddie
Copy link

charlesroddie commented Nov 19, 2020

The C# suggestion is here: dotnet/csharplang#1005

If there is to be some attribute, it would be good to discuss with C#/.Net. It wouldn't be good if there is an F# attribute that C#/.Net consumers can't understand, and then a C#/.Net attribute down the line that F# has to implement separately.

@Evangelink
Copy link

There is a roslyn analyzer doing the same:
mykolav/require-named-args-fs

I think it's a little different to have an analyzer (which I would +1) rather than having an attribute to enforce the use. Let's say you create a library and use enforce (with an analyzer) this practice on your codebase then I am still able to make a choice on my side while with the attribute I would be also forced to follow this pattern.

For me this is more on the coding style side and so it should not be forced to users.

Besides, you could also create some record type and take this as parameter instead of multiple arguments, this would force having "named" arguments.

Just my 2 cents and based on what I see from this suggestion.

@smoothdeveloper
Copy link
Contributor

Besides, you could also create some record type and take this as parameter instead of multiple arguments, this would force having "named" arguments.

This becomes problematic for type providers, there is no provision in the SDK to be able to emit provided record types.

@dsyme
Copy link
Collaborator

dsyme commented Jan 12, 2021

I'd personally be in favor but this is a @dsyme call

Yes I think this is a good suggestion.

Perhaps call the attribute RequireNamedArgument

@smoothdeveloper
Copy link
Contributor

Thanks @dsyme, adjusting the name (BTW we sadly have some inconsistency between Require and Requires).

I'm resuming the investigation, I wonder if it makes sense if @cartermp, @davidfowl or others would like to loop back with C#/BCL people, the attribute may find its way in the BCL, it may be good to get their opinion on the name too.

Will fill in a RFC as I progress, I appreciate if a placeholder for it could be added so it is in the right spot / has the right name.

@davidfowl
Copy link

I feel like python has this feature? Doesn't it?

@smoothdeveloper
Copy link
Contributor

smoothdeveloper commented Mar 29, 2021

@davidfowl not too familiar, but section "Requiring your arguments be named" indicates it does, seemingly on argument by argument basis.

edit: seems to allow for mix of it, and capturing positional arguments separately, akin to C# and F# allowing freely to use named arguments only after positional, but embedding that choice in the signature of the member.

@LiteracyFanatic
Copy link

I would love to have this feature. My use case is working with EF Core. Ideally one could just use records for everything, but navigation properties don't play well their inability to be declared as null. Using classes instead resolves that issue and gives access to optional arguments constructor arguments which is actually an improvement to the ergonomics for keys and default values which currently need to be explicitly set to 0 or Unchecked.defaultof. However, classes offer much weaker guarantees of correct initialization as they are changed throughout the lifetime of a project. It's far too easy to pass arguments of the same type in the wrong order by accident causing hard to track down bugs, so the ability to enforce the use of named parameters would be excellent.

@smoothdeveloper
Copy link
Contributor

@LiteracyFanatic, thanks for your feedback, please check fsharp/fslang-design#538 and https://github.com/fsharp/fslang-design/blob/main/drafts/FS-1095-requirenamedargumentattribute.md for further details.

You may surface your eagerness about this feature at dotnet/runtime#51451 so it would also be picked up by roslyn analyzers and having the attribute defined in the BCL.

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

No branches or pull requests

9 participants