RFC FS 1126 Allow lower-case DU cases when RequireQualifiedAccess is specified #13432
Conversation
edgarfgp
changed the title
| let hasRequireQualifiedAccessAttribute = HasFSharpAttribute cenv.g cenv.g.attrib_RequireQualifiedAccessAttribute tycon.Attribs | ||
|
|
||
| if not hasRequireQualifiedAccessAttribute then | ||
| CheckUnionCaseName cenv id |
There was a problem hiding this comment.
We still need these parts of CheckUnionCaseName because the compiled form must generate a type and enum case for these - better to pass in hasRequireQualifiedAccessAttribute to that routine?
if name = "Tags" then
errorR(Error(FSComp.SR.tcUnionCaseNameConflictsWithGeneratedType(name, "Tags"), id.idRange))
CheckNamespaceModuleOrTypeName g idThere was a problem hiding this comment.
Updated based on your suggestion.
|
|
||
| [<RequireQualifiedAccess>] | ||
| type DU1 = | ||
| | a |
There was a problem hiding this comment.
Please add a test case with a dodgy lower case name that is not a valid type name for .NET, e.g.
type DU =
| ``not.allowed``There was a problem hiding this comment.
Added more testing. Thanks for pointing this out .
There was a problem hiding this comment.
@vzarytovskii I was wondering if should put this behind a preview flag ?
There was a problem hiding this comment.
Yeah, let's put it under preview version.
edgarfgp
force-pushed
the
allow-lower-case-du-cases-when-require-qualified-access-is-specified
branch
from
811eac2
to
a9bf35c
Compare
|
@dsyme this is ready for another review . Added more testing and put the feature under the preview. |
| // #Conformance #TypesAndModules #Unions | ||
| // This testcase verifies that lower-case discriminated union are not allowed | ||
| //<Expects status="error"></Expects> |
There was a problem hiding this comment.
Just a note: those are actually not required for newly written tests, they are there for the tests which were auto-migrated from the old framework.
There was a problem hiding this comment.
Cool I will remove them :)
|
@edgarfgp thank you a lot for this addition! Sorry it took long to merge it. |
|
@vzarytovskii No worries . I already looking for the next contribution to help the compiler 😀 |
* ValRepInfoForDisplay added for improved quick info for functions defined in expressions * Update * Update QuickInfoTests.fs * Update QuickInfoTests.fs * Update * add identifier analysis script (#13486) * add identifier analysis script * add identifier analysis script * Update fantomas alpha 11 (#13481) * Allow lower-case DU cases when RequireQualifiedAccess is specified (#13432) * Allow lower-case DU cases when RequireQualifiedAccess is specified * Fix PR suggestions and Add more testing * Protect feature under preview version * Add a NotUpperCaseConstructorWithoutRQA warning to be raised in lang version preview * Fix formatting * regularize some names (#13489) * normalize some names * format code * fix build * Subtraction of two chars, new conversions, and fixes for dynamic operator invocations and QuotationToExpression (#11681) Co-authored-by: Vlad Zarytovskii <vzaritovsky@hotmail.com> Co-authored-by: Don Syme <dsyme@users.noreply.github.com> * Mark backing fields as CompilerGenerated (fixes serialization of fields in FSI multiemit) (#13494) Co-authored-by: Don Syme <donsyme@fastmail.fm> Co-authored-by: Peter Semkin <petersemkin@duck.com> Co-authored-by: Don Syme <dsyme@users.noreply.github.com> Co-authored-by: Florian Verdonck <florian.verdonck@outlook.com> Co-authored-by: Petr Semkin <psfinaki@users.noreply.github.com> Co-authored-by: Edgar Gonzalez <edgar.gonzalez@fundourselves.com> Co-authored-by: Hadrian Tang <hadrianwttang@outlook.com> Co-authored-by: Vlad Zarytovskii <vzaritovsky@hotmail.com> Co-authored-by: Kevin Ransom (msft) <codecutter@hotmail.com>
edgarfgp
changed the title
Implementation for Allow lower-case DU cases when RequireQualifiedAccess is specified fsharp/fslang-suggestions#131
RFC: https://github.com/fsharp/fslang-design/blob/main/FSharp-7.0/FS-1126-allow-lower-case-du-cases%20when-require-qualified-access-is%20specified.md