Skip to content

Commit

Permalink
Add warning when compiler selects among multiple record type candidat…
Browse files Browse the repository at this point in the history
…es, fslang-suggestion 1091 (#15256)
  • Loading branch information
dawedawe authored May 29, 2023
1 parent cd3b581 commit 00fae43
Show file tree
Hide file tree
Showing 20 changed files with 420 additions and 0 deletions.
30 changes: 30 additions & 0 deletions src/Compiler/Checking/NameResolution.fs
Original file line number Diff line number Diff line change
Expand Up @@ -2726,6 +2726,36 @@ let rec ResolveLongIdentInTypePrim (ncenv: NameResolver) nenv lookupKind (resInf

let errorTextF s =
match tryTcrefOfAppTy g ty with
| ValueSome tcref when tcref.IsRecordTycon ->
let alternative = nenv.eFieldLabels |> Map.tryFind nm
match alternative with
| Some fieldLabels ->
let fieldsOfResolvedType = tcref.AllFieldsArray |> Array.map (fun f -> f.LogicalName) |> Set.ofArray
let fieldsOfAlternatives =
fieldLabels
|> Seq.collect (fun l -> l.Tycon.AllFieldsArray |> Array.map (fun f -> f.LogicalName))
|> Set.ofSeq
let intersect = Set.intersect fieldsOfAlternatives fieldsOfResolvedType

if not intersect.IsEmpty then
let resolvedTypeName = NicePrint.fqnOfEntityRef g tcref
let namesOfAlternatives =
fieldLabels
|> List.map (fun l -> $" %s{NicePrint.fqnOfEntityRef g l.TyconRef}")
|> fun names -> $" %s{resolvedTypeName}" :: names
let candidates = System.String.Join("\n", namesOfAlternatives)
let overlappingNames =
intersect
|> Set.toArray
|> Array.sort
|> Array.map (fun s -> $" %s{s}")
|> fun a -> System.String.Join("\n", a)
if g.langVersion.SupportsFeature(LanguageFeature.WarningWhenMultipleRecdTypeChoice) then
warning(Error(FSComp.SR.tcMultipleRecdTypeChoice(candidates, resolvedTypeName, overlappingNames), m))
else
informationalWarning(Error(FSComp.SR.tcMultipleRecdTypeChoice(candidates, resolvedTypeName, overlappingNames), m))
| _ -> ()
FSComp.SR.undefinedNameFieldConstructorOrMemberWhenTypeIsKnown(tcref.DisplayNameWithStaticParametersAndUnderscoreTypars, s)
| ValueSome tcref ->
FSComp.SR.undefinedNameFieldConstructorOrMemberWhenTypeIsKnown(tcref.DisplayNameWithStaticParametersAndUnderscoreTypars, s)
| _ ->
Expand Down
2 changes: 2 additions & 0 deletions src/Compiler/Checking/NicePrint.fs
Original file line number Diff line number Diff line change
Expand Up @@ -2615,6 +2615,8 @@ let stringOfFSAttrib denv x = x |> PrintTypes.layoutAttrib denv |> squareAngleL

let stringOfILAttrib denv x = x |> PrintTypes.layoutILAttrib denv |> squareAngleL |> showL

let fqnOfEntityRef g x = x |> layoutTyconRefImpl false (DisplayEnv.Empty g) |> showL

let layoutImpliedSignatureOfModuleOrNamespace showHeader denv infoReader ad m contents =
InferredSigPrinting.layoutImpliedSignatureOfModuleOrNamespace showHeader denv infoReader ad m contents

Expand Down
2 changes: 2 additions & 0 deletions src/Compiler/Checking/NicePrint.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,8 @@ val stringOfFSAttrib: denv: DisplayEnv -> x: Attrib -> string

val stringOfILAttrib: denv: DisplayEnv -> ILType * ILAttribElem list -> string

val fqnOfEntityRef: g: TcGlobals -> x: EntityRef -> string

val layoutImpliedSignatureOfModuleOrNamespace:
showHeader: bool ->
denv: DisplayEnv ->
Expand Down
2 changes: 2 additions & 0 deletions src/Compiler/FSComp.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1572,6 +1572,7 @@ featureStaticMembersInInterfaces,"Static members in interfaces"
featureNonInlineLiteralsAsPrintfFormat,"String values marked as literals and IL constants as printf format"
featureNestedCopyAndUpdate,"Nested record field copy-and-update"
featureExtendedStringInterpolation,"Extended string interpolation similar to C# raw string literals."
featureWarningWhenMultipleRecdTypeChoice,"Raises warnings when multiple record type matches were found during name resolution because of overlapping field names."
3353,fsiInvalidDirective,"Invalid directive '#%s %s'"
3354,tcNotAFunctionButIndexerNamedIndexingNotYetEnabled,"This value supports indexing, e.g. '%s.[index]'. The syntax '%s[index]' requires /langversion:preview. See https://aka.ms/fsharp-index-notation."
3354,tcNotAFunctionButIndexerIndexingNotYetEnabled,"This expression supports indexing, e.g. 'expr.[index]'. The syntax 'expr[index]' requires /langversion:preview. See https://aka.ms/fsharp-index-notation."
Expand Down Expand Up @@ -1692,3 +1693,4 @@ featureEscapeBracesInFormattableString,"Escapes curly braces before calling Form
3563,lexInvalidIdentifier,"This is not a valid identifier"
3564,parsMissingUnionCaseName,"Missing union case name"
3565,parsExpectingType,"Expecting type"
3566,tcMultipleRecdTypeChoice,"Multiple type matches were found:\n%s\nThe type '%s' was used. Due to the overlapping field names\n%s\nconsider using type annotations or change the order of open statements."
3 changes: 3 additions & 0 deletions src/Compiler/Facilities/LanguageFeatures.fs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ type LanguageFeature =
| NonInlineLiteralsAsPrintfFormat
| NestedCopyAndUpdate
| ExtendedStringInterpolation
| WarningWhenMultipleRecdTypeChoice

/// LanguageVersion management
type LanguageVersion(versionText) =
Expand Down Expand Up @@ -157,6 +158,7 @@ type LanguageVersion(versionText) =
LanguageFeature.NonInlineLiteralsAsPrintfFormat, previewVersion
LanguageFeature.NestedCopyAndUpdate, previewVersion
LanguageFeature.ExtendedStringInterpolation, previewVersion
LanguageFeature.WarningWhenMultipleRecdTypeChoice, previewVersion

]

Expand Down Expand Up @@ -279,6 +281,7 @@ type LanguageVersion(versionText) =
| LanguageFeature.NonInlineLiteralsAsPrintfFormat -> FSComp.SR.featureNonInlineLiteralsAsPrintfFormat ()
| LanguageFeature.NestedCopyAndUpdate -> FSComp.SR.featureNestedCopyAndUpdate ()
| LanguageFeature.ExtendedStringInterpolation -> FSComp.SR.featureExtendedStringInterpolation ()
| LanguageFeature.WarningWhenMultipleRecdTypeChoice -> FSComp.SR.featureWarningWhenMultipleRecdTypeChoice ()

/// Get a version string associated with the given feature.
static member GetFeatureVersionString feature =
Expand Down
1 change: 1 addition & 0 deletions src/Compiler/Facilities/LanguageFeatures.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ type LanguageFeature =
| NonInlineLiteralsAsPrintfFormat
| NestedCopyAndUpdate
| ExtendedStringInterpolation
| WarningWhenMultipleRecdTypeChoice

/// LanguageVersion management
type LanguageVersion =
Expand Down
10 changes: 10 additions & 0 deletions src/Compiler/xlf/FSComp.txt.cs.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,11 @@
<target state="translated">Vyvolá upozornění, když se použije „let inline ... =“ společně s atributem [&lt;MethodImpl(MethodImplOptions.NoInlining)&gt;]. Funkce není vkládána.</target>
<note />
</trans-unit>
<trans-unit id="featureWarningWhenMultipleRecdTypeChoice">
<source>Raises warnings when multiple record type matches were found during name resolution because of overlapping field names.</source>
<target state="new">Raises warnings when multiple record type matches were found during name resolution because of overlapping field names.</target>
<note />
</trans-unit>
<trans-unit id="featureWildCardInForLoop">
<source>wild card in for loop</source>
<target state="translated">zástupný znak ve smyčce for</target>
Expand Down Expand Up @@ -1077,6 +1082,11 @@
<target state="translated">Je třeba inicializovat následující požadované vlastnosti:{0}</target>
<note />
</trans-unit>
<trans-unit id="tcMultipleRecdTypeChoice">
<source>Multiple type matches were found:\n{0}\nThe type '{1}' was used. Due to the overlapping field names\n{2}\nconsider using type annotations or change the order of open statements.</source>
<target state="new">Multiple type matches were found:\n{0}\nThe type '{1}' was used. Due to the overlapping field names\n{2}\nconsider using type annotations or change the order of open statements.</target>
<note />
</trans-unit>
<trans-unit id="tcNoEagerConstraintApplicationAttribute">
<source>Using methods with 'NoEagerConstraintApplicationAttribute' requires /langversion:6.0 or later</source>
<target state="translated">Použití metod s atributem NoEagerConstraintApplicationAttribute vyžaduje /langversion:6.0 nebo novější.</target>
Expand Down
10 changes: 10 additions & 0 deletions src/Compiler/xlf/FSComp.txt.de.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,11 @@
<target state="translated">Löst Warnungen aus, wenn „let inline ... =“ zusammen mit dem Attribut [&lt;MethodImpl(MethodImplOptions.NoInlining)&gt;] verwendet wird. Die Funktion wird nicht inline gesetzt.</target>
<note />
</trans-unit>
<trans-unit id="featureWarningWhenMultipleRecdTypeChoice">
<source>Raises warnings when multiple record type matches were found during name resolution because of overlapping field names.</source>
<target state="new">Raises warnings when multiple record type matches were found during name resolution because of overlapping field names.</target>
<note />
</trans-unit>
<trans-unit id="featureWildCardInForLoop">
<source>wild card in for loop</source>
<target state="translated">Platzhalter in for-Schleife</target>
Expand Down Expand Up @@ -1077,6 +1082,11 @@
<target state="translated">Die folgenden erforderlichen Eigenschaften müssen initialisiert werden:{0}</target>
<note />
</trans-unit>
<trans-unit id="tcMultipleRecdTypeChoice">
<source>Multiple type matches were found:\n{0}\nThe type '{1}' was used. Due to the overlapping field names\n{2}\nconsider using type annotations or change the order of open statements.</source>
<target state="new">Multiple type matches were found:\n{0}\nThe type '{1}' was used. Due to the overlapping field names\n{2}\nconsider using type annotations or change the order of open statements.</target>
<note />
</trans-unit>
<trans-unit id="tcNoEagerConstraintApplicationAttribute">
<source>Using methods with 'NoEagerConstraintApplicationAttribute' requires /langversion:6.0 or later</source>
<target state="translated">Die Verwendung von Methoden mit "NoEagerConstraintApplicationAttribute" erfordert /langversion:6.0 oder höher.</target>
Expand Down
10 changes: 10 additions & 0 deletions src/Compiler/xlf/FSComp.txt.es.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,11 @@
<target state="translated">Genera advertencias cuando se usa "let inline ... =" junto con el atributo [&lt;MethodImpl(MethodImplOptions.NoInlining)&gt;]. La función no se está insertando.</target>
<note />
</trans-unit>
<trans-unit id="featureWarningWhenMultipleRecdTypeChoice">
<source>Raises warnings when multiple record type matches were found during name resolution because of overlapping field names.</source>
<target state="new">Raises warnings when multiple record type matches were found during name resolution because of overlapping field names.</target>
<note />
</trans-unit>
<trans-unit id="featureWildCardInForLoop">
<source>wild card in for loop</source>
<target state="translated">carácter comodín en bucle for</target>
Expand Down Expand Up @@ -1077,6 +1082,11 @@
<target state="translated">Se deben inicializar las siguientes propiedades necesarias:{0}</target>
<note />
</trans-unit>
<trans-unit id="tcMultipleRecdTypeChoice">
<source>Multiple type matches were found:\n{0}\nThe type '{1}' was used. Due to the overlapping field names\n{2}\nconsider using type annotations or change the order of open statements.</source>
<target state="new">Multiple type matches were found:\n{0}\nThe type '{1}' was used. Due to the overlapping field names\n{2}\nconsider using type annotations or change the order of open statements.</target>
<note />
</trans-unit>
<trans-unit id="tcNoEagerConstraintApplicationAttribute">
<source>Using methods with 'NoEagerConstraintApplicationAttribute' requires /langversion:6.0 or later</source>
<target state="translated">El uso de métodos con "NoEagerConstraintApplicationAttribute" requiere /langversion:6.0 o posteriores</target>
Expand Down
10 changes: 10 additions & 0 deletions src/Compiler/xlf/FSComp.txt.fr.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,11 @@
<target state="translated">Génère des avertissements lorsque « let inline ... = » est utilisé avec l’attribut [&lt;MethodImpl(MethodImplOptions.NoInlining)&gt;]. La fonction n’est pas inlined.</target>
<note />
</trans-unit>
<trans-unit id="featureWarningWhenMultipleRecdTypeChoice">
<source>Raises warnings when multiple record type matches were found during name resolution because of overlapping field names.</source>
<target state="new">Raises warnings when multiple record type matches were found during name resolution because of overlapping field names.</target>
<note />
</trans-unit>
<trans-unit id="featureWildCardInForLoop">
<source>wild card in for loop</source>
<target state="translated">caractère générique dans une boucle for</target>
Expand Down Expand Up @@ -1077,6 +1082,11 @@
<target state="translated">Les propriétés requises suivantes doivent être initialisées :{0}</target>
<note />
</trans-unit>
<trans-unit id="tcMultipleRecdTypeChoice">
<source>Multiple type matches were found:\n{0}\nThe type '{1}' was used. Due to the overlapping field names\n{2}\nconsider using type annotations or change the order of open statements.</source>
<target state="new">Multiple type matches were found:\n{0}\nThe type '{1}' was used. Due to the overlapping field names\n{2}\nconsider using type annotations or change the order of open statements.</target>
<note />
</trans-unit>
<trans-unit id="tcNoEagerConstraintApplicationAttribute">
<source>Using methods with 'NoEagerConstraintApplicationAttribute' requires /langversion:6.0 or later</source>
<target state="translated">L’utilisation de méthodes avec « NoEagerConstraintApplicationAttribute » requiert/langversion:6.0 ou ultérieur</target>
Expand Down
10 changes: 10 additions & 0 deletions src/Compiler/xlf/FSComp.txt.it.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,11 @@
<target state="translated">Genera avvisi quando 'let inline ... =' viene usato insieme all'attributo [&lt;MethodImpl(MethodImplOptions.NoInlining)&gt;]. La funzione non viene resa inline.</target>
<note />
</trans-unit>
<trans-unit id="featureWarningWhenMultipleRecdTypeChoice">
<source>Raises warnings when multiple record type matches were found during name resolution because of overlapping field names.</source>
<target state="new">Raises warnings when multiple record type matches were found during name resolution because of overlapping field names.</target>
<note />
</trans-unit>
<trans-unit id="featureWildCardInForLoop">
<source>wild card in for loop</source>
<target state="translated">carattere jolly nel ciclo for</target>
Expand Down Expand Up @@ -1077,6 +1082,11 @@
<target state="translated">È necessario inizializzare le proprietà obbligatorie seguenti:{0}</target>
<note />
</trans-unit>
<trans-unit id="tcMultipleRecdTypeChoice">
<source>Multiple type matches were found:\n{0}\nThe type '{1}' was used. Due to the overlapping field names\n{2}\nconsider using type annotations or change the order of open statements.</source>
<target state="new">Multiple type matches were found:\n{0}\nThe type '{1}' was used. Due to the overlapping field names\n{2}\nconsider using type annotations or change the order of open statements.</target>
<note />
</trans-unit>
<trans-unit id="tcNoEagerConstraintApplicationAttribute">
<source>Using methods with 'NoEagerConstraintApplicationAttribute' requires /langversion:6.0 or later</source>
<target state="translated">L'utilizzo di metodi con 'NoEagerConstraintApplicationAttribute' richiede /langversion: 6.0 o versione successiva</target>
Expand Down
10 changes: 10 additions & 0 deletions src/Compiler/xlf/FSComp.txt.ja.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,11 @@
<target state="translated">'let inline ... =' が [&lt;MethodImpl(MethodImplOptions.NoInlining)&gt;] 属性と一緒に使用されるときに警告を生成します。関数はインライン化されていません。</target>
<note />
</trans-unit>
<trans-unit id="featureWarningWhenMultipleRecdTypeChoice">
<source>Raises warnings when multiple record type matches were found during name resolution because of overlapping field names.</source>
<target state="new">Raises warnings when multiple record type matches were found during name resolution because of overlapping field names.</target>
<note />
</trans-unit>
<trans-unit id="featureWildCardInForLoop">
<source>wild card in for loop</source>
<target state="translated">for ループのワイルド カード</target>
Expand Down Expand Up @@ -1077,6 +1082,11 @@
<target state="translated">次の必須プロパティを初期化する必要があります:{0}</target>
<note />
</trans-unit>
<trans-unit id="tcMultipleRecdTypeChoice">
<source>Multiple type matches were found:\n{0}\nThe type '{1}' was used. Due to the overlapping field names\n{2}\nconsider using type annotations or change the order of open statements.</source>
<target state="new">Multiple type matches were found:\n{0}\nThe type '{1}' was used. Due to the overlapping field names\n{2}\nconsider using type annotations or change the order of open statements.</target>
<note />
</trans-unit>
<trans-unit id="tcNoEagerConstraintApplicationAttribute">
<source>Using methods with 'NoEagerConstraintApplicationAttribute' requires /langversion:6.0 or later</source>
<target state="translated">'NoEagerConstraintApplicationAttribute' を指定してメソッドを使用するには、/langversion:6.0 以降が必要です</target>
Expand Down
10 changes: 10 additions & 0 deletions src/Compiler/xlf/FSComp.txt.ko.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,11 @@
<target state="translated">'let inline ... ='을(를) [&lt;MethodImpl(MethodImplOptions.NoInlining)&gt;] 특성과 함께 사용하는 경우 경고를 발생합니다. 함수가 인라인되지 않습니다.</target>
<note />
</trans-unit>
<trans-unit id="featureWarningWhenMultipleRecdTypeChoice">
<source>Raises warnings when multiple record type matches were found during name resolution because of overlapping field names.</source>
<target state="new">Raises warnings when multiple record type matches were found during name resolution because of overlapping field names.</target>
<note />
</trans-unit>
<trans-unit id="featureWildCardInForLoop">
<source>wild card in for loop</source>
<target state="translated">for 루프의 와일드카드</target>
Expand Down Expand Up @@ -1077,6 +1082,11 @@
<target state="translated">다음 필수 속성을 초기화해야 합니다. {0}</target>
<note />
</trans-unit>
<trans-unit id="tcMultipleRecdTypeChoice">
<source>Multiple type matches were found:\n{0}\nThe type '{1}' was used. Due to the overlapping field names\n{2}\nconsider using type annotations or change the order of open statements.</source>
<target state="new">Multiple type matches were found:\n{0}\nThe type '{1}' was used. Due to the overlapping field names\n{2}\nconsider using type annotations or change the order of open statements.</target>
<note />
</trans-unit>
<trans-unit id="tcNoEagerConstraintApplicationAttribute">
<source>Using methods with 'NoEagerConstraintApplicationAttribute' requires /langversion:6.0 or later</source>
<target state="translated">'NoEagerConstraintApplicationAttribute'와 함께 메서드를 사용하려면 /langversion:6.0 이상이 필요합니다.</target>
Expand Down
Loading

1 comment on commit 00fae43

@WillEhrendreich
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a fantastic idea, I love it. This very concern was a barrier to "fearlessly" relying on type inference. There are people quite reasonably recommending against relying on it at all simply because of the possibility of ambiguity, but if the compiler will warn you about it, then it really goes a long way to ease the mind that things are remaining as correct as we would want them to be. Thanks for this commit, it's a good one.

Please sign in to comment.