[WIP] fix for #960

[smoothdeveloper]

suggested message clarifying usage of augmentation attributes for issue #960

Seeing what the test coverage says, will add specific tests.

[<CustomEquality>]
type T() =
    override __.Equals(_) = true

gives:

error FS0388: The attributes 'NoEquality', 'ReferenceEquality', 'StructuralEquality', 'NoComparison' and 'StructuralComparison' aren't meant to be used on adhoc classes, interfaces, structures, enums, delegates but only on discriminated unions or record types.

cc: @snuup, @nojaf, @kbattocchi please let me know if there are things we'd like to nudge / adjust.

[vzarytovskii]

Maybe something like

The following attributes aren't meant to be used with ...: ...

?

[smoothdeveloper]

what do we want to do for:

[<NoComparison>]
type T3() = class end

It doesn't cause an error, so the suggested error message would not be accurate by listing NoComparison.

Looking at the implementation, and with a bit of efforts, we could make the error message only list the attributes that are used (I think it makes the error message more friendly), and we could have a "more info" page in the docs that list all the attributes, give some example of usages and incorrect usages.

[charlesroddie]

[<CustomEquality>]
type T() =
    override __.Equals(_) = true

error FS0388: The attributes 'NoEquality', 'ReferenceEquality', 'StructuralEquality', 'NoComparison' and 'StructuralComparison' aren't meant to be used on adhoc classes, interfaces, structures, enums, delegates but only on discriminated unions or record types.

The user has used an attribute x and the error message says that l attributes should not be used, where l does not contain x. So the message seems irrelevant.

[smoothdeveloper]

@charlesroddie sure, what do you suggest to make it relevant?

given @vzarytovskii feedback, I was tweaking it to

The following attributes 'NoEquality', 'ReferenceEquality', 'StructuralEquality', 'NoComparison' and 'StructuralComparison' are not meant to be used with classes, interfaces, structures, enums, delegates but with discriminated unions or record types.

And given how it is implemented, I'd recommend to only list the attribute x (not I), and link to an aka.ms page that can be kept up to date at all(any, is the same) times.

[charlesroddie]

And given how it is implemented, I'd recommend to only list the attribute x (not I), and link to an aka.ms page that can be kept up to date at all(any, is the same) times.

Yes either adding x to the list or using only x would make sense to me. The removal of the word "adhoc" was also right, but you could simplify that part further to a negative: the attribute ... may only be used with discriminated unions and record types.

[github-actions]

❗ Release notes required

@smoothdeveloper,

Caution

No release notes found for the changed paths (see table below).

Please make sure to add an entry with an informative description of the change as well as link to this pull request, issue and language suggestion if applicable. Release notes for this repository are based on Keep A Changelog format.

The following format is recommended for this repository:

* <Informative description>. ([PR #XXXXX](https://github.com/dotnet/fsharp/pull/XXXXX))

See examples in the files, listed in the table below or in th full documentation at https://fsharp.github.io/fsharp-compiler-docs/release-notes/About.html.

If you believe that release notes are not necessary for this PR, please add NO_RELEASE_NOTES label to the pull request.

You can open this PR in browser to add release notes: open in github.dev

Change path	Release notes path	Description
src/Compiler	docs/release-notes/.FSharp.Compiler.Service/8.0.300.md	No release notes found or release notes format is not correct

[smoothdeveloper]

This is the current state in this branch:

The message only clarifies that the offending attribute is meant for DU & records, and it will only list the attributes employed, that triggered the error message.

There are two variants, for singular offending attribute, and multiple ones.

Modules still fallback to the existing/original error message.

@kbattocchi, @nojaf, @snuup, @vzarytovskii any feeling if this is going the right direction and if there are other things I should attempt / adjust?

Thanks.