Skip to content

Commit a6b72a4

Browse files
jwostyKevinRansom
jwosty
authored and
KevinRansom
committed
Enum match warning (#4522)
* Add compiler warning for enum matches * Refactor * Make incomplete enum match warning only show itself when the pattern match covers all known enum values * small change * Update xlf * Update xlf (take 2) * Fix xlf format mistake * Add green test * Remove leftover printfn and register test * Attempt to make enum warning work in more cases * Fix inverted logic mistake * Update tests * Fix enum match warning not producing locations * New items should use translation state "new" * Update a test * Update another test * Undo changes out of the scope of this PR * Undo more whitespace
1 parent 65d87f0 commit a6b72a4

23 files changed

+175
-28
lines changed

‎src/fsharp/CompileOps.fs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@ let GetRangeOfDiagnostic(err:PhasedDiagnostic) =
154154
| InterfaceNotRevealed(_, _, m)
155155
| WrappedError (_, m)
156156
| PatternMatchCompilation.MatchIncomplete (_, _, m)
157+
| PatternMatchCompilation.EnumMatchIncomplete (_, _, m)
157158
| PatternMatchCompilation.RuleNeverMatched m
158159
| ValNotMutable(_, _, m)
159160
| ValNotLocal(_, _, m)
@@ -355,6 +356,7 @@ let GetDiagnosticNumber(err:PhasedDiagnostic) =
355356
| ExtensionTyping.ProvidedTypeResolutionNoRange _
356357
| ExtensionTyping.ProvidedTypeResolution _ -> 103
357358
#endif
359+
| PatternMatchCompilation.EnumMatchIncomplete _ -> 104
358360
(* DO NOT CHANGE THE NUMBERS *)
359361

360362
// Strip TargetInvocationException wrappers
@@ -560,6 +562,7 @@ let MatchIncomplete2E() = DeclareResourceString("MatchIncomplete2", "%s")
560562
let MatchIncomplete3E() = DeclareResourceString("MatchIncomplete3", "%s")
561563
let MatchIncomplete4E() = DeclareResourceString("MatchIncomplete4", "")
562564
let RuleNeverMatchedE() = DeclareResourceString("RuleNeverMatched", "")
565+
let EnumMatchIncomplete1E() = DeclareResourceString("EnumMatchIncomplete1", "")
563566
let ValNotMutableE() = DeclareResourceString("ValNotMutable", "%s")
564567
let ValNotLocalE() = DeclareResourceString("ValNotLocal", "")
565568
let Obsolete1E() = DeclareResourceString("Obsolete1", "")
@@ -1402,6 +1405,15 @@ let OutputPhasedErrorR (os:StringBuilder) (err:PhasedDiagnostic) =
14021405
if isComp then
14031406
os.Append(MatchIncomplete4E().Format) |> ignore
14041407

1408+
| PatternMatchCompilation.EnumMatchIncomplete (isComp, cexOpt, _) ->
1409+
os.Append(EnumMatchIncomplete1E().Format) |> ignore
1410+
match cexOpt with
1411+
| None -> ()
1412+
| Some (cex, false) -> os.Append(MatchIncomplete2E().Format cex) |> ignore
1413+
| Some (cex, true) -> os.Append(MatchIncomplete3E().Format cex) |> ignore
1414+
if isComp then
1415+
os.Append(MatchIncomplete4E().Format) |> ignore
1416+
14051417
| PatternMatchCompilation.RuleNeverMatched _ -> os.Append(RuleNeverMatchedE().Format) |> ignore
14061418

14071419
| ValNotMutable(_, valRef, _) -> os.Append(ValNotMutableE().Format(valRef.DisplayName)) |> ignore

‎src/fsharp/FSStrings.resx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -969,6 +969,9 @@
969969
<data name="MatchIncomplete4" xml:space="preserve">
970970
<value> Unmatched elements will be ignored.</value>
971971
</data>
972+
<data name="EnumMatchIncomplete1" xml:space="preserve">
973+
<value>Enums may take values outside known cases.</value>
974+
</data>
972975
<data name="RuleNeverMatched" xml:space="preserve">
973976
<value>This rule will never be matched</value>
974977
</data>

‎src/fsharp/PatternMatchCompilation.fs‎

Lines changed: 46 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ open Microsoft.FSharp.Compiler.Lib
1919

2020
exception MatchIncomplete of bool * (string * bool) option * range
2121
exception RuleNeverMatched of range
22+
exception EnumMatchIncomplete of bool * (string * bool) option * range
2223

2324
type ActionOnFailure =
2425
| ThrowIncompleteMatchException
@@ -177,33 +178,37 @@ let RefuteDiscrimSet g m path discrims =
177178
| PathConj (p,_j) ->
178179
go p tm
179180
| PathTuple (p,tys,j) ->
180-
go p (fun _ -> mkRefTupled g m (mkOneKnown tm j tys) tys)
181+
let k, eCoversVals = mkOneKnown tm j tys
182+
go p (fun _ -> mkRefTupled g m k tys, eCoversVals)
181183
| PathRecd (p,tcref,tinst,j) ->
182-
let flds = tcref |> actualTysOfInstanceRecdFields (mkTyconRefInst tcref tinst) |> mkOneKnown tm j
183-
go p (fun _ -> Expr.Op(TOp.Recd(RecdExpr, tcref),tinst, flds,m))
184+
let flds, eCoversVals = tcref |> actualTysOfInstanceRecdFields (mkTyconRefInst tcref tinst) |> mkOneKnown tm j
185+
go p (fun _ -> Expr.Op(TOp.Recd(RecdExpr, tcref),tinst, flds,m), eCoversVals)
184186

185187
| PathUnionConstr (p,ucref,tinst,j) ->
186-
let flds = ucref |> actualTysOfUnionCaseFields (mkTyconRefInst ucref.TyconRef tinst)|> mkOneKnown tm j
187-
go p (fun _ -> Expr.Op(TOp.UnionCase(ucref),tinst, flds,m))
188+
let flds, eCoversVals = ucref |> actualTysOfUnionCaseFields (mkTyconRefInst ucref.TyconRef tinst)|> mkOneKnown tm j
189+
go p (fun _ -> Expr.Op(TOp.UnionCase(ucref),tinst, flds,m), eCoversVals)
188190

189191
| PathArray (p,ty,len,n) ->
190-
go p (fun _ -> Expr.Op(TOp.Array,[ty], mkOneKnown tm n (List.replicate len ty) ,m))
192+
let flds, eCoversVals = mkOneKnown tm n (List.replicate len ty)
193+
go p (fun _ -> Expr.Op(TOp.Array,[ty], flds ,m), eCoversVals)
191194

192195
| PathExnConstr (p,ecref,n) ->
193-
let flds = ecref |> recdFieldTysOfExnDefRef |> mkOneKnown tm n
194-
go p (fun _ -> Expr.Op(TOp.ExnConstr(ecref),[], flds,m))
196+
let flds, eCoversVals = ecref |> recdFieldTysOfExnDefRef |> mkOneKnown tm n
197+
go p (fun _ -> Expr.Op(TOp.ExnConstr(ecref),[], flds,m), eCoversVals)
195198

196199
| PathEmpty(ty) -> tm ty
197200

198-
and mkOneKnown tm n tys = List.mapi (fun i ty -> if i = n then tm ty else mkUnknown ty) tys
199-
and mkUnknowns tys = List.map mkUnknown tys
201+
and mkOneKnown tm n tys =
202+
let flds = List.mapi (fun i ty -> if i = n then tm ty else (mkUnknown ty, false)) tys
203+
List.map fst flds, List.fold (fun acc (_, eCoversVals) -> eCoversVals || acc) false flds
204+
and mkUnknowns tys = List.map (fun x -> mkUnknown x) tys
200205

201206
let tm ty =
202207
match discrims with
203208
| [DecisionTreeTest.IsNull] ->
204-
snd(mkCompGenLocal m notNullText ty)
209+
snd(mkCompGenLocal m notNullText ty), false
205210
| [DecisionTreeTest.IsInst (_,_)] ->
206-
snd(mkCompGenLocal m otherSubtypeText ty)
211+
snd(mkCompGenLocal m otherSubtypeText ty), false
207212
| (DecisionTreeTest.Const c :: rest) ->
208213
let consts = Set.ofList (c :: List.choose (function DecisionTreeTest.Const(c) -> Some c | _ -> None) rest)
209214
let c' =
@@ -227,12 +232,23 @@ let RefuteDiscrimSet g m path discrims =
227232
| Const.Decimal _ -> seq { 1 .. System.Int32.MaxValue } |> Seq.map (fun v -> Const.Decimal(decimal v))
228233
| _ ->
229234
raise CannotRefute)
235+
236+
let coversKnownEnumValues =
237+
match tryDestAppTy g ty with
238+
| Some tcref when tcref.IsEnumTycon ->
239+
let knownValues =
240+
tcref.AllFieldsArray |> Array.choose (fun f ->
241+
match f.rfield_const, f.rfield_static with
242+
| Some value, true -> Some value
243+
| _, _ -> None)
244+
Array.forall (fun ev -> consts.Contains ev) knownValues
245+
| _ -> false
230246

231247
(* REVIEW: we could return a better enumeration literal field here if a field matches one of the enumeration cases *)
232248

233249
match c' with
234250
| None -> raise CannotRefute
235-
| Some c -> Expr.Const(c,m,ty)
251+
| Some c -> Expr.Const(c,m,ty), coversKnownEnumValues
236252

237253
| (DecisionTreeTest.UnionCase (ucref1,tinst) :: rest) ->
238254
let ucrefs = ucref1 :: List.choose (function DecisionTreeTest.UnionCase(ucref,_) -> Some ucref | _ -> None) rest
@@ -246,10 +262,10 @@ let RefuteDiscrimSet g m path discrims =
246262
| [] -> raise CannotRefute
247263
| ucref2 :: _ ->
248264
let flds = ucref2 |> actualTysOfUnionCaseFields (mkTyconRefInst tcref tinst) |> mkUnknowns
249-
Expr.Op(TOp.UnionCase(ucref2),tinst, flds,m)
265+
Expr.Op(TOp.UnionCase(ucref2),tinst, flds,m), false
250266

251267
| [DecisionTreeTest.ArrayLength (n,ty)] ->
252-
Expr.Op(TOp.Array,[ty], mkUnknowns (List.replicate (n+1) ty) ,m)
268+
Expr.Op(TOp.Array,[ty], mkUnknowns (List.replicate (n+1) ty) ,m), false
253269

254270
| _ ->
255271
raise CannotRefute
@@ -302,15 +318,16 @@ let rec CombineRefutations g r1 r2 =
302318
let ShowCounterExample g denv m refuted =
303319
try
304320
let refutations = refuted |> List.collect (function RefutedWhenClause -> [] | (RefutedInvestigation(path,discrim)) -> [RefuteDiscrimSet g m path discrim])
305-
let counterExample =
321+
let counterExample, enumCoversKnown =
306322
match refutations with
307323
| [] -> raise CannotRefute
308-
| h :: t ->
309-
if verbose then dprintf "h = %s\n" (Layout.showL (exprL h))
310-
List.fold (CombineRefutations g) h t
324+
| (r, eck) :: t ->
325+
if verbose then dprintf "r = %s (enumCoversKnownValue = %b)\n" (Layout.showL (exprL r)) eck
326+
List.fold (fun (rAcc, eckAcc) (r, eck) ->
327+
CombineRefutations g rAcc r, eckAcc || eck) (r, eck) t
311328
let text = Layout.showL (NicePrint.dataExprL denv counterExample)
312329
let failingWhenClause = refuted |> List.exists (function RefutedWhenClause -> true | _ -> false)
313-
Some(text,failingWhenClause)
330+
Some(text,failingWhenClause,enumCoversKnown)
314331

315332
with
316333
| CannotRefute ->
@@ -689,10 +706,15 @@ let CompilePatternBasic
689706
(* Emit the incomplete match warning *)
690707
if warnOnIncomplete then
691708
match actionOnFailure with
692-
| ThrowIncompleteMatchException ->
693-
warning (MatchIncomplete (false,ShowCounterExample g denv matchm refuted, matchm))
694-
| IgnoreWithWarning ->
695-
warning (MatchIncomplete (true,ShowCounterExample g denv matchm refuted, matchm))
709+
| ThrowIncompleteMatchException | IgnoreWithWarning ->
710+
let ignoreWithWarning = (actionOnFailure = IgnoreWithWarning)
711+
match ShowCounterExample g denv matchm refuted with
712+
| Some(text,failingWhenClause,true) ->
713+
warning (EnumMatchIncomplete(ignoreWithWarning, Some(text,failingWhenClause), matchm))
714+
| Some(text,failingWhenClause,false) ->
715+
warning (MatchIncomplete(ignoreWithWarning, Some(text,failingWhenClause), matchm))
716+
| None ->
717+
warning (MatchIncomplete(ignoreWithWarning, None, matchm))
696718
| _ ->
697719
()
698720

‎src/fsharp/PatternMatchCompilation.fsi‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,3 +67,4 @@ val internal CompilePattern :
6767

6868
exception internal MatchIncomplete of bool * (string * bool) option * range
6969
exception internal RuleNeverMatched of range
70+
exception internal EnumMatchIncomplete of bool * (string * bool) option * range

‎src/fsharp/xlf/FSStrings.cs.xlf‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1422,6 +1422,11 @@
14221422
<target state="translated"> Nespárované prvky se budou ignorovat.</target>
14231423
<note />
14241424
</trans-unit>
1425+
<trans-unit id="EnumMatchIncomplete1">
1426+
<source>Enums may take values outside known cases.</source>
1427+
<target state="new">Enums may take values outside known cases.</target>
1428+
<note />
1429+
</trans-unit>
14251430
<trans-unit id="RuleNeverMatched">
14261431
<source>This rule will never be matched</source>
14271432
<target state="translated">Pro toto pravidlo nebude nikdy existovat shoda.</target>

‎src/fsharp/xlf/FSStrings.de.xlf‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1422,6 +1422,11 @@
14221422
<target state="translated"> Nicht zugeordnete Elemente werden ignoriert.</target>
14231423
<note />
14241424
</trans-unit>
1425+
<trans-unit id="EnumMatchIncomplete1">
1426+
<source>Enums may take values outside known cases.</source>
1427+
<target state="new">Enums may take values outside known cases.</target>
1428+
<note />
1429+
</trans-unit>
14251430
<trans-unit id="RuleNeverMatched">
14261431
<source>This rule will never be matched</source>
14271432
<target state="translated">Für diese Regel wird niemals eine Übereinstimmung gefunden.</target>

‎src/fsharp/xlf/FSStrings.en.xlf‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1422,6 +1422,11 @@
14221422
<target state="new"> Unmatched elements will be ignored.</target>
14231423
<note />
14241424
</trans-unit>
1425+
<trans-unit id="EnumMatchIncomplete1">
1426+
<source>Enums may take values outside known cases.</source>
1427+
<target state="new">Enums may take values outside known cases.</target>
1428+
<note />
1429+
</trans-unit>
14251430
<trans-unit id="RuleNeverMatched">
14261431
<source>This rule will never be matched</source>
14271432
<target state="new">This rule will never be matched</target>

‎src/fsharp/xlf/FSStrings.es.xlf‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1422,6 +1422,11 @@
14221422
<target state="translated"> Los elementos que no coinciden se omitirán.</target>
14231423
<note />
14241424
</trans-unit>
1425+
<trans-unit id="EnumMatchIncomplete1">
1426+
<source>Enums may take values outside known cases.</source>
1427+
<target state="new">Enums may take values outside known cases.</target>
1428+
<note />
1429+
</trans-unit>
14251430
<trans-unit id="RuleNeverMatched">
14261431
<source>This rule will never be matched</source>
14271432
<target state="translated">Nunca se buscarán coincidencias con esta regla.</target>

‎src/fsharp/xlf/FSStrings.fr.xlf‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1422,6 +1422,11 @@
14221422
<target state="translated"> Les éléments non appariés seront ignorés.</target>
14231423
<note />
14241424
</trans-unit>
1425+
<trans-unit id="EnumMatchIncomplete1">
1426+
<source>Enums may take values outside known cases.</source>
1427+
<target state="new">Enums may take values outside known cases.</target>
1428+
<note />
1429+
</trans-unit>
14251430
<trans-unit id="RuleNeverMatched">
14261431
<source>This rule will never be matched</source>
14271432
<target state="translated">Cette règle n'aura aucune correspondance</target>

‎src/fsharp/xlf/FSStrings.it.xlf‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1422,6 +1422,11 @@
14221422
<target state="translated"> Gli elementi senza corrispondenza verranno ignorati.</target>
14231423
<note />
14241424
</trans-unit>
1425+
<trans-unit id="EnumMatchIncomplete1">
1426+
<source>Enums may take values outside known cases.</source>
1427+
<target state="new">Enums may take values outside known cases.</target>
1428+
<note />
1429+
</trans-unit>
14251430
<trans-unit id="RuleNeverMatched">
14261431
<source>This rule will never be matched</source>
14271432
<target state="translated">Questa regola non avrà mai alcuna corrispondenza</target>

0 commit comments

Comments
 (0)