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

Missing attributes information on setter of a property (in FSharpMemberOrFunctionOrValue type) #18157

Open
MangelMaxime opened this issue Dec 17, 2024 · 7 comments

Comments

@MangelMaxime
Copy link

Hello,

I am not sure how to report this bug because this is related to the internals of F# Compiler itself, and happening inside of Fable transformation.

In Fable, we want to erase types and properties decorated with [<Erase>].

For example, this code should emit nothing.

[<Erase>]
type internal LanguageInjectionAttribute() =
    inherit Attribute()
    
    [<Erase>]
    member val Prefix = "" with get, set

However, currently it does not seems possible to do it for the setter because it is missing the [<Erase>] attribute information:

Image

Please tell me, if I can do anything to reword my issue or help you identify the cause of this problem.

@vzarytovskii
Copy link
Member

Yeah, I don't think attributes are universally propagated to property methods. One of the reasons is that targets on attributes can be incorrect for methods. Which is fine from the runtime perspective, but semantically incorrect.

@T-Gro
Copy link
Member

T-Gro commented Jan 2, 2025

You should be able to find the attribute on the symbol for the whole Property (and not just getter/setter).
Is it there?

@MangelMaxime
Copy link
Author

@T-Gro What do you mean by Symbol in this context?

I tried to look but FSharpMemberOrFunctionOrValue does not seems to have a Symbol property for access.

However, I found that there is a DeclaringEntity which seems to have the Attributes information, would this be a good place to look for Attributes if we detect that the FSharpMemberOrFunctionOrValue is a setter?

Image

@T-Gro
Copy link
Member

T-Gro commented Jan 3, 2025

.DeclaringEntity.Attributes gets attributes of the type, not of the property - I assume this is not what you want.

.DeclaringEntity.MembersFunctionsAndValues should yield a collection of members, one of which will be a P with PropInfo (i.e. the full property, not just the set_... method. Can you locate the attribute like that?

(locate the one you want via .LogicalName and .IsProperty)

@MangelMaxime
Copy link
Author

.DeclaringEntity.Attributes gets attributes of the type, not of the property - I assume this is not what you want.

This explains why I had a strange result when removing [<Erase>] from the top level entity.

.DeclaringEntity.MembersFunctionsAndValues should yield a collection of members, one of which will be a P with PropInfo (i.e. the full property, not just the set_... method. Can you locate the attribute like that?

(locate the one you want via .LogicalName and .IsProperty)

This seems possible indeed, it just make the code more verbose compared to the other case where Attributes is propagated.

let isErasedPropertySetter (memb : FSharpMemberOrFunctionOrValue) =
    if memb.IsPropertySetterMethod then
        match memb.DeclaringEntity with
        | Some ent ->
            // Remove the "set_" prefix
            let logicalName = memb.LogicalName.Substring(4)

            ent.MembersFunctionsAndValues
            |> Seq.tryFind(fun (m : FSharpMemberOrFunctionOrValue) ->
                m.LogicalName = logicalName && hasAttrib Atts.erase m.Attributes
            )
            |> Option.isSome
        | _ -> false
    else
        false

Depending on if the attributes should be propagated to the setter, this can be a workaround or the final solution.

Thank you for the guidance.

@T-Gro
Copy link
Member

T-Gro commented Jan 3, 2025

I think automatic propagation might be a breaking change, since AttributeTargets.Property and AttributeTargets.Method are two distinct options.

https://learn.microsoft.com/en-us/dotnet/api/system.attributetargets?view=net-8.0

(i.e. it might lead to an error saying that attribute intended for properties only appeared on a method)

@MangelMaxime
Copy link
Author

I think automatic propagation might be a breaking change, since AttributeTargets.Property and AttributeTargets.Method are two distinct options.

Is it because in F# it could be a Method, and then map it to both a Method / Property in the "generated C#" code?

My first thinking when learning about AttributeTargets which was from the first comment on this issue, was that perhaps "automatic propagation" could check if the AttributeTargets is correct or not. If yes, they the attributes is forwarded.

This is probably a naive idea and I don't know if this would be worth the effort or even fix the issue I originally faced.

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

No branches or pull requests

3 participants