F# 9 changes the validation of attribute usage targets for union cases.

[eiriktsarpalis]

I noticed the following regression when trying to build an old F# project using the .NET 9 sdk. The following code

open System.Runtime.Serialization

type CartEvent =
    | [<DataMember(Name = "cartCreated")>] CartCreated

now fails to compile with the error

error FS0842: This attribute is not valid for use on this language element

Technically, this error is correct because DataMemberAttribute specifies AttributeTarget.Field ||| AttributeTarget.Property however this is breaking a common pattern used when serializing events modelled as discriminated unions.

So I have the following questions:

1. Was this change made intentionally rectifying lack of attribute target validation by the compiler?
2. What attribute AttributeTarget should library authors be specifying for attributes intended for application to union cases?

cc @bartelink

[T-Gro]

This was done as part of fsharp/fslang-suggestions#1375 , via a series of changes implemented by @edgarfgp . It has been indeed intentional to rectify a prior lack of enforcement.

Unrestricted attribute would fit here best, but I assume that is not an option?
Due to the different lowering techniques for union types that are to be supported, I cannot think of a fixed one that would work well without edge cases.

[T-Gro]

Due to the lowering differences, the attribute targets are branched based on the numbers of fields:

if rfields.IsEmpty then AttributeTargets.Property else AttributeTargets.Method

[edgarfgp]

@eiriktsarpalis are you using 9.0.102 ? I do not seem to reproduce this for

open System.Runtime.Serialization

type CartEvent =
    | [<DataMember(Name = "cartCreated")>] CartCreated

1. Was this change made intentionally rectifying lack of attribute target validation by the compiler?

Yes

2. What attribute AttributeTarget should library authors be specifying for attributes intended for application to union cases?

open System

[<AttributeUsage(AttributeTargets.Property, AllowMultiple = false)>]
type PropertyOnlyAttribute() = 
  inherit Attribute()
  
[<AttributeUsage(AttributeTargets.Method, AllowMultiple = false)>]
type MethodOnlyAttribute() = 
  inherit Attribute()
  
[<AttributeUsage(AttributeTargets.Property ||| AttributeTargets.Method, AllowMultiple = false)>]
type PropertyAndMethodAttribute()
    = inherit Attribute()

type CartEvent =
    | [<PropertyOnly>] CartCreated
    | [<MethodOnly>] CartUpdated of int
    | [<PropertyAndMethod>] CartDeleted of int

[eiriktsarpalis]

Does the above workaround still result in the custom attribute being reported when I call UnionCaseInfo.GetCustomAttributes() for the relevant union case?

are you using 9.0.102 ?

Yes

[edgarfgp]

Does the above workaround still result in the custom attribute being reported when I call UnionCaseInfo.GetCustomAttributes() for the relevant union case?

I believe so. If you run

let a =
    typeof<CartEvent>
    |> FSharpType.GetUnionCases
    |> Array.map (fun x -> x.GetCustomAttributes())

You will get.

val a: obj array array =
  [|[|FSI_0007+ProperlyOnlyAttribute;
      Microsoft.FSharp.Core.CompilationMappingAttribute|];
    [|FSI_0007+MethodOnlyAttribute;
      Microsoft.FSharp.Core.CompilationMappingAttribute|];
    [|FSI_0007+PropertyAndMethodAttribute;
      Microsoft.FSharp.Core.CompilationMappingAttribute|]|]

[T-Gro]

Do we need a dedicated docs section about attributes on unions and their cases perharps?

[eiriktsarpalis]

While I get why each of the union case arities are mapped to Property and Method respectively, I don't necessarily think this would be intuitive to a library author or somebody consuming the attributes. I don't think there's a perfect solution here, but I can propose a couple alternatives:

1. Allow AttributeTarget.Type: while technically not all union cases a represented as types in IL, logically they are the closest concept corresponding to a union case.
2. Revert validation to pre F# 9 behavior. This should avoid breaking existing source code that applies [<DataMember>] attributes to union cases.

[T-Gro]

If DataMember is capable on support symbols beyond its attribute targets, wouldn't it then be possible to change it's declaration?

If there are libraries that do support invalid combinations of symbols and attribute targets, I think we could go with an opt-in way to trigger warnings instead of errors.

[eiriktsarpalis]

If DataMember is capable on support symbols beyond its attribute targets, wouldn't it then be possible to change it's declaration?

Types under System.Runtime.Serialization are considered legacy nowadays and we try not to make any changes to them. Apart from that though DataMemberAttribute is a type intended for use on properties and fields, so I doubt F# union cases would meet the bar for inclusion. The correct annotation should have been DataContractAttribute, although this is not allowed on union cases even with older versions of F#. It's been too long to remember exactly, but I suspect that might have been the reason that forced me to switch to DataMemberAttribute back in the day.

[T-Gro]

I don't want us to mix up two things when coming up with a solution:

• Staying as correct as possible for reflection-based-libraries which might not have any explicit F# support. But still, we can try to keep a correct mapping between AttributeTargets and emitted IL, at least where possible.

• Providing an annotation solution for libraries which are aware of the F# type system and are using e.g. Fsharp.Core reflection to work with it.

Allowing Target.Type could be helpful for the latter. But it also wouldn't solve the situation with DataMemberAttribute , would it?

[eiriktsarpalis]

But it also wouldn't solve the situation with DataMemberAttribute , would it?

It would not, but it's probably not the end of the world.

[T-Gro]

We might re-think attributes on DUs (and how they are lowered) in the context of C# unions as well, assuming they will also have a similar situation of having >1 lowering schemes for 1 syntactical construct.

@edgarfgp : Do you have any idea on how to make DataMemberAttribute work on union cases?
What do you think about optional flag to do "errors as warnings" for attribute enforcement?

[edgarfgp]

@T-Gro

Like other OSS Libraries have done so far:

1. Update their attributes e.g DataMemberAttribute to use the a more permissive targets AttributeTargets [<AttributeUsage(AttributeTargets.Property ||| AttributeTargets.Method, AllowMultiple = false)>]

2. Errors as warnings might be an option. But if people have TreatWarningAsErrors will still result in an error.

Before F#9 the AttributeTarget analysis was non existent. So now we are doing the "right thing" but I understand that not all people would be able/desire to update there libraries.

[eiriktsarpalis]

1. Update their attributes e.g DataMemberAttribute to use the a more permissive targets AttributeTargets [<AttributeUsage(AttributeTargets.Property ||| AttributeTargets.Method, AllowMultiple = false)>]

Wouldn't a side-effect of that be that the attributes can now be applied to properties and methods outside of unions?

[eiriktsarpalis]

Before F#9 the AttributeTarget analysis was non existent.

Pretty sure that older versions were rejecting AttributeTarget.Type though, right?

[edgarfgp]

Before F#9 the AttributeTarget analysis was non existent.

Pretty sure that older versions were rejecting AttributeTarget.Type though, right?

I think so. I guess it was almost non existent then :)

[T-Gro]

Orthogonal idea:
Would FsharpAttributeTargets.UnionCase make any sense?
That would require a language suggestion about a new attribute in FSharp.Core and F# typechecking rule to check it, but it would be a clean way to annotate from an F# perspective ;; as opposed to the existing IL-only view.

[TheAngryByrd]

We're seeing this on FSharp.Analyzers.SDK when using Argu

Error: /home/runner/work/FSharp.Analyzers.SDK/FSharp.Analyzers.SDK/src/FSharp.Analyzers.Cli/Program.fs(21,9): error FS0842: This attribute is not valid for use on this language element [/home/runner/work/FSharp.Analyzers.SDK/FSharp.Analyzers.SDK/src/FSharp.Analyzers.Cli/FSharp.Analyzers.Cli.fsproj]

https://github.com/ionide/FSharp.Analyzers.SDK/actions/runs/13298889551/job/37136571174#step:4:51

type Arguments =
    | Project of string list
    | Analyzers_Path of string list
    | [<EqualsAssignment; AltCommandLine("-p:"); AltCommandLine("-p")>] Property of string * string

https://github.com/ionide/FSharp.Analyzers.SDK/blob/9a294ebd8be287eaff43c4035412136eb2c948e0/src/FSharp.Analyzers.Cli/Program.fs#L18-L21

[edgarfgp]

@TheAngryByrd i believe Argu updated the attribute targets in 6.2.4 https://github.com/fsprojects/Argu/releases/tag/6.2.4

[hyazinthh]

I have issues with struct unions and [<RequireQualifiedAccess>]:

[<Struct>]
[<RequireQualifiedAccess>]
type GlobalTime =
    | Timestamp of MicroTime
    | Infinity

This should be valid or am I missing something? Wasn't sure if I should open another issue for this.

[edgarfgp]

@hyazinthh What is the error reported in your case.?. [<Struct>] and [<RequireQualifiedAccess>] works as before. See compiler tests. Please make sure that you are using an updated FSharp.Core version.

[hyazinthh]

@edgarfgp it's the same FS0842 error message.

Thanks for the FSharp.Core hint, seems like this only happens when you build against a FSharp.Core version before 8.0.300.

We usually target the lowest possible version of FSharp.Core in our libraries to avoid conflicts with other third-party libraries that may force an older specific version. Currently, we target 8.0.100 so updating should be fine.

Yet, this is pretty unexpected behavior.

[T-Gro]

The attributes and their targets have a special position - even though they ship as part of the runtime library (FSharp.Core), the targets of the build-in attributes are needed at compile-time.

In this specific scenario, it should be possible for you to built against a higher FSharp.Core, and let apps pin down a lower one.

(but it is of course dangerous for using other possibly new/updated APIs in FSharp.Core)

[pleaseletmesearch]

Just another record of someone encountering the breaking change above. I can work around all of it except the [<Struct; RequireQualifiedAccess>] type MyDu = | Foo, for which the only workaround is to build my library against a higher FSharp.Core and then have a consumer use a lower FSharp.Core, which as you say is dangerous: it now essentially requires integration tests to do safely, where we test that a consuming application with a lower FSharp.Core can operate using my library which expects the higher FSharp.Core. Doing this "properly" and not unsafely, the .NET SDK upgrade requires me to upgrade packages in my library and propagate them to its consumers, which is deeply unexpected and not very pleasant.

[edgarfgp]

Orthogonal idea: Would FsharpAttributeTargets.UnionCase make any sense? That would require a language suggestion about a new attribute in FSharp.Core and F# typechecking rule to check it, but it would be a clean way to annotate from an F# perspective ;; as opposed to the existing IL-only view.

Yeah this definitely would be a better way of handling this. I would be ok to tackle this at some point before version 10