Show warning when DU is accessed without type but RequiredQualifiedAccess was set #103
Conversation
| @@ -1526,13 +1526,16 @@ type RecdFieldInfo = | |||
| /// Describes an F# use of a union case | |||
| [<NoComparison; NoEquality>] | |||
| type UnionCaseInfo = | |||
| | UnionCaseInfo of TypeInst * Tast.UnionCaseRef | |||
| | UnionCaseInfo of TypeInst * Tast.UnionCaseRef * bool | |||
There was a problem hiding this comment.
Could you change the PR to add the flag as Item.UnionCase(ucref,flag) rather than to UnionCaseInfo please? This is because the flag is really a transient thing related to name resolution, whereas the UnionCaseInfo is like a reflection-handle to the union case, representing the logical item.
|
Cool work, you're basically there. |
|
done |
forki
force-pushed
the
rqa
branch
2 times, most recently
from
129d9c6
to
207462e
Compare
|
Fun fact: even the compiler itself had such an invalid case. see 416ea92 |
|
@forki Ok my head just imploded. |
|
|
| | Item.UnionCase ucinfo -> | ||
| | Item.UnionCase(ucinfo,showDeprecated) -> | ||
| if showDeprecated then | ||
| let message = sprintf "The union type for union case '%s' was defined with the RequireQualifiedAccessAttribute. Include the name of the union type ('%s') in the name you are using.'" ucinfo.Name ucinfo.Tycon.DisplayName |
There was a problem hiding this comment.
This needs to go in FSComp.txt (so it can be localized by Microsoft into Japanese etc.) Look around tc.fs and elsewhere for examples.
|
This is good. Just two remaining things
It's easiest to add negative tests at the end of https://github.com/Microsoft/visualfsharp/blob/fsharp4/tests/fsharp/typecheck/sigs/neg90.fs (or one of the other exiting neg* files). Then update the baseline by running the typecheck/sigs tests, letting them fail, checking the error messages look right, and then copying the contents of neg90.err to neg90.bsl (we generally don't mind if each neg* test covers 3-5 different aspects of name resolution of type checking). |
|
done. |
|
Fabulous. This is ready to go as far as I can see. |
|
I will submit my solution to RecordField bug in a separate PR. |
|
Looks good to me, nice work Steffen. Side conversation -- what is the recommended formatting for DUs across the compiler/runtime: |
|
I know it's inconsistent, but I tried to match the style around the modified lines. At least I tried ;-) |
|
vsintegration side of the repo does not build with this, there are a few spots that are now being flagged 😆 |
| @@ -4,3 +4,5 @@ neg90.fs(4,9,4,12): typecheck error FS0001: A generic construct requires that th | |||
| neg90.fs(7,22,7,25): typecheck error FS0039: The type 'foo' is not defined | |||
|
|
|||
| neg90.fs(7,22,7,25): typecheck error FS0039: The type 'foo' is not defined | |||
|
|
|||
| neg90.fs(16,9,16,21): typecheck error FS0035: This construct is deprecated: The union type for union case 'Member' was definded with the RequireQualifiedAccessAttribute. Include the name of the union type ('DU') in the name you are using.' | |||
|
@latkin - Definitely use a space after a comma. At some point we should normalize the entire F# codebase to always have a space after a comma. |
This fixes #95 by showing a deprecated warning.