Skip to content

Commit

Permalink
Give an example in the non-exhaustive diagnostic (#44702)
Browse files Browse the repository at this point in the history
* When reporting a switch is not exhaustive, give an example why.
Fixes #43943
* Skip a flaky test
Related to #45234
  • Loading branch information
Neal Gafter authored Jun 23, 2020
1 parent 4129ecb commit 212fc8f
Show file tree
Hide file tree
Showing 56 changed files with 1,652 additions and 166 deletions.
505 changes: 505 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/PatternExplainer.cs

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,15 @@ private bool CheckSwitchExpressionExhaustive(
// nulls is the job of the nullable walker.
if (!hasErrors)
{
foreach (var n in TopologicalSort.IterativeSort<BoundDecisionDagNode>(new[] { decisionDag.RootNode }, nonNullSuccessors))
var nodes = TopologicalSort.IterativeSort<BoundDecisionDagNode>(new[] { decisionDag.RootNode }, nonNullSuccessors);
foreach (var n in nodes)
{
if (n is BoundLeafDecisionDagNode leaf && leaf.Label == defaultLabel)
{
diagnostics.Add(ErrorCode.WRN_SwitchExpressionNotExhaustive, node.SwitchKeyword.GetLocation());
diagnostics.Add(
ErrorCode.WRN_SwitchExpressionNotExhaustive,
node.SwitchKeyword.GetLocation(),
PatternExplainer.SamplePatternForPathToDagNode(BoundDagTemp.ForOriginalInput(boundInputExpression), nodes, n, nullPaths: false));
return true;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -5750,7 +5750,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<value>The syntax 'var' for a pattern is not permitted to refer to a type, but '{0}' is in scope here.</value>
</data>
<data name="WRN_SwitchExpressionNotExhaustive" xml:space="preserve">
<value>The switch expression does not handle all possible values of its input type (it is not exhaustive).</value>
<value>The switch expression does not handle all possible values of its input type (it is not exhaustive). For example, the pattern '{0}' is not covered.</value>
</data>
<data name="WRN_SwitchExpressionNotExhaustive_Title" xml:space="preserve">
<value>The switch expression does not handle all possible values of its input type (it is not exhaustive).</value>
Expand Down Expand Up @@ -5981,7 +5981,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<value>Type '{0}' cannot be embedded because it has a non-abstract member. Consider setting the 'Embed Interop Types' property to false.</value>
</data>
<data name="WRN_SwitchExpressionNotExhaustiveForNull" xml:space="preserve">
<value>The switch expression does not handle some null inputs (it is not exhaustive).</value>
<value>The switch expression does not handle some null inputs (it is not exhaustive). For example, the pattern '{0}' is not covered.</value>
</data>
<data name="WRN_SwitchExpressionNotExhaustiveForNull_Title" xml:space="preserve">
<value>The switch expression does not handle some null inputs.</value>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.PooledObjects;
Expand Down Expand Up @@ -590,7 +591,13 @@ private void VisitSwitchExpressionCore(BoundSwitchExpression node, bool inferTyp
labelStateMap.TryGetValue(node.DefaultLabel, out var defaultLabelState) && defaultLabelState.believedReachable)
{
SetState(defaultLabelState.state);
ReportDiagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull, ((SwitchExpressionSyntax)node.Syntax).SwitchKeyword.GetLocation());
var nodes = node.DecisionDag.TopologicallySortedNodes;
var leaf = nodes.Where(n => n is BoundLeafDecisionDagNode leaf && leaf.Label == node.DefaultLabel).First();
ReportDiagnostic(
ErrorCode.WRN_SwitchExpressionNotExhaustiveForNull,
((SwitchExpressionSyntax)node.Syntax).SwitchKeyword.GetLocation(),
PatternExplainer.SamplePatternForPathToDagNode(BoundDagTemp.ForOriginalInput(node.Expression), nodes, leaf, nullPaths: true)
);
}

// collect expressions, conversions and result types
Expand Down
8 changes: 8 additions & 0 deletions src/Compilers/CSharp/Portable/Utilities/IValueSet.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

#nullable enable

using System;

namespace Microsoft.CodeAnalysis.CSharp
{
/// <summary>
Expand Down Expand Up @@ -51,6 +53,12 @@ internal interface IValueSet
/// Does this value set contain no values?
/// </summary>
bool IsEmpty { get; }

/// <summary>
/// Produce a sample value contained in the set. Throws <see cref="ArgumentException"/> if the set is empty. If the set
/// contains values but we cannot produce a particular value (e.g. for the set `nint > int.MaxValue`), returns null.
/// </summary>
ConstantValue? Sample { get; }
}

/// <summary>
Expand Down
10 changes: 10 additions & 0 deletions src/Compilers/CSharp/Portable/Utilities/IValueSetFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,16 @@ internal interface IValueSetFactory
/// Produce a random value for testing.
/// </summary>
ConstantValue RandomValue(Random random);

/// <summary>
/// The set containing all values of the type.
/// </summary>
IValueSet AllValues { get; }

/// <summary>
/// The empty set of values.
/// </summary>
IValueSet NoValues { get; }
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ public static BoolValueSet Create(bool hasFalse, bool hasTrue)

bool IValueSet.IsEmpty => !_hasFalse && !_hasTrue;

ConstantValue IValueSet.Sample => ConstantValue.Create(_hasTrue ? true : _hasFalse ? false : throw new ArgumentException());


public bool Any(BinaryOperatorKind relation, bool value)
{
switch (relation, value)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ private sealed class BoolValueSetFactory : IValueSetFactory<bool>

private BoolValueSetFactory() { }

IValueSet IValueSetFactory.AllValues => BoolValueSet.AllValues;

IValueSet IValueSetFactory.NoValues => BoolValueSet.None;

public IValueSet<bool> Related(BinaryOperatorKind relation, bool value)
{
switch (relation, value)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ private struct ByteTC : INumericTC<byte>

byte INumericTC<byte>.MaxValue => byte.MaxValue;

byte INumericTC<byte>.Zero => 0;

bool INumericTC<byte>.Related(BinaryOperatorKind relation, byte left, byte right)
{
switch (relation)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ private struct CharTC : INumericTC<char>

char INumericTC<char>.MaxValue => char.MaxValue;

char INumericTC<char>.Zero => (char)0;

bool INumericTC<char>.Related(BinaryOperatorKind relation, char left, char right)
{
switch (relation)
Expand Down Expand Up @@ -49,11 +51,7 @@ char INumericTC<char>.Next(char value)

string INumericTC<char>.ToString(char c)
{
var isPrintable = char.IsWhiteSpace(c) ||
// exclude the Unicode character categories containing non-rendering,
// unknown, or incomplete characters.
char.GetUnicodeCategory(c) switch { UnicodeCategory.Control => false, UnicodeCategory.OtherNotAssigned => false, UnicodeCategory.Surrogate => false, _ => true };
return isPrintable ? $"'{c}'" : $"\\u{(int)c:X4}";
return ObjectDisplay.FormatPrimitive(c, ObjectDisplayOptions.EscapeNonPrintableCharacters | ObjectDisplayOptions.UseQuotes);
}

char INumericTC<char>.Prev(char value)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ private struct DecimalTC : INumericTC<decimal>

decimal INumericTC<decimal>.MaxValue => decimal.MaxValue;

decimal INumericTC<decimal>.Zero => 0M;

public decimal FromConstantValue(ConstantValue constantValue) => constantValue.IsBad ? 0m : constantValue.DecimalValue;

public ConstantValue ToConstantValue(decimal value) => ConstantValue.Create(value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ private sealed class DecimalValueSetFactory : IValueSetFactory<decimal>, IValueS

private readonly IValueSetFactory<decimal> _underlying = NumericValueSetFactory<decimal, DecimalTC>.Instance;

IValueSet IValueSetFactory.AllValues => NumericValueSet<decimal, DecimalTC>.AllValues;

IValueSet IValueSetFactory.NoValues => NumericValueSet<decimal, DecimalTC>.NoValues;

public IValueSet<decimal> Related(BinaryOperatorKind relation, decimal value) => _underlying.Related(relation, DecimalTC.Normalize(value));

IValueSet IValueSetFactory.Random(int expectedSize, Random random) => _underlying.Random(expectedSize, random);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ private struct DoubleTC : FloatingTC<double>, INumericTC<double>

double FloatingTC<double>.NaN => double.NaN;

double INumericTC<double>.Zero => 0.0;

/// <summary>
/// The implementation of Next depends critically on the internal representation of an IEEE floating-point
/// number. Every bit sequence between the representation of 0 and MaxValue represents a distinct
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#nullable enable

using System;
using System.Collections.Immutable;
using System.Linq;
using Roslyn.Utilities;
Expand All @@ -24,18 +25,50 @@ private sealed class EnumeratedValueSet<T, TTC> : IValueSet<T> where TTC : struc
/// In <see cref="_included"/>, then members are listed by inclusion. Otherwise all members
/// are assumed to be contained in the set unless excluded.
/// </summary>
private bool _included;
private readonly bool _included;

private ImmutableHashSet<T> _membersIncludedOrExcluded;
private readonly ImmutableHashSet<T> _membersIncludedOrExcluded;

private EnumeratedValueSet(bool included, ImmutableHashSet<T> membersIncludedOrExcluded) =>
(this._included, this._membersIncludedOrExcluded) = (included, membersIncludedOrExcluded);

public static EnumeratedValueSet<T, TTC> AllValues = new EnumeratedValueSet<T, TTC>(included: false, ImmutableHashSet<T>.Empty);
public static readonly EnumeratedValueSet<T, TTC> AllValues = new EnumeratedValueSet<T, TTC>(included: false, ImmutableHashSet<T>.Empty);

public static readonly EnumeratedValueSet<T, TTC> NoValues = new EnumeratedValueSet<T, TTC>(included: true, ImmutableHashSet<T>.Empty);

internal static EnumeratedValueSet<T, TTC> Including(T value) => new EnumeratedValueSet<T, TTC>(included: true, ImmutableHashSet<T>.Empty.Add(value));

bool IValueSet.IsEmpty => _included && _membersIncludedOrExcluded.IsEmpty;
public bool IsEmpty => _included && _membersIncludedOrExcluded.IsEmpty;

ConstantValue IValueSet.Sample
{
get
{
if (IsEmpty) throw new ArgumentException();
var tc = default(TTC);
if (_included)
return tc.ToConstantValue(_membersIncludedOrExcluded.OrderBy(k => k).First());
if (typeof(T) == typeof(string))
{
// try some simple strings.
if (this.Any(BinaryOperatorKind.Equal, (T)(object)""))
return tc.ToConstantValue((T)(object)"");
for (char c = 'A'; c <= 'z'; c++)
if (this.Any(BinaryOperatorKind.Equal, (T)(object)c.ToString()))
return tc.ToConstantValue((T)(object)c.ToString());
}
// If that doesn't work, choose from a sufficiently large random selection of values.
// Since this is an excluded set, they cannot all be excluded
var candidates = tc.RandomValues(_membersIncludedOrExcluded.Count + 1, new Random(0), _membersIncludedOrExcluded.Count + 1);
foreach (var value in candidates)
{
if (this.Any(BinaryOperatorKind.Equal, value))
return tc.ToConstantValue(value);
}

throw ExceptionUtilities.Unreachable;
}
}

public bool Any(BinaryOperatorKind relation, T value)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@ internal static partial class ValueSetFactory
/// </summary>
private sealed class EnumeratedValueSetFactory<T, TTC> : IValueSetFactory<T> where TTC : struct, IEquatableValueTC<T> where T : notnull
{
public static EnumeratedValueSetFactory<T, TTC> Instance = new EnumeratedValueSetFactory<T, TTC>();
public static readonly EnumeratedValueSetFactory<T, TTC> Instance = new EnumeratedValueSetFactory<T, TTC>();

IValueSet IValueSetFactory.AllValues => EnumeratedValueSet<T, TTC>.AllValues;

IValueSet IValueSetFactory.NoValues => EnumeratedValueSet<T, TTC>.NoValues;

private EnumeratedValueSetFactory() { }

Expand All @@ -44,11 +48,11 @@ bool IValueSetFactory.Related(BinaryOperatorKind relation, ConstantValue left, C
return tc.FromConstantValue(left).Equals(tc.FromConstantValue(right));
}

IValueSet IValueSetFactory.Random(int expectedSize, Random random)
public IValueSet Random(int expectedSize, Random random)
{
TTC tc = default;
T[] values = tc.RandomValues(expectedSize, random, expectedSize * 2);
IValueSet<T> result = EnumeratedValueSet<T, TTC>.AllValues.Complement();
IValueSet<T> result = EnumeratedValueSet<T, TTC>.NoValues;
Debug.Assert(result.IsEmpty);
foreach (T value in values)
result = result.Union(Related(Equal, value));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#nullable enable

using System;
using System.Diagnostics;
using System.Text;
using Roslyn.Utilities;

Expand Down Expand Up @@ -47,7 +48,27 @@ internal static IValueSet<TFloating> Random(int expectedSize, Random random)
numbers: (IValueSet<TFloating>)NumericValueSetFactory<TFloating, TFloatingTC>.Instance.Random(expectedSize, random), hasNaN: hasNan);
}

bool IValueSet.IsEmpty => !_hasNaN && _numbers.IsEmpty;
public bool IsEmpty => !_hasNaN && _numbers.IsEmpty;

ConstantValue IValueSet.Sample
{
get
{
if (IsEmpty)
throw new ArgumentException();

if (!_numbers.IsEmpty)
{
var sample = _numbers.Sample;
Debug.Assert(sample is { });
return sample;
}

Debug.Assert(_hasNaN);
var tc = default(TFloatingTC);
return tc.ToConstantValue(tc.NaN);
}
}

public static IValueSet<TFloating> Related(BinaryOperatorKind relation, TFloating value)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ private sealed class FloatingValueSetFactory<TFloating, TFloatingTC> : IValueSet

private FloatingValueSetFactory() { }

IValueSet IValueSetFactory.AllValues => FloatingValueSet<TFloating, TFloatingTC>.AllValues;

IValueSet IValueSetFactory.NoValues => FloatingValueSet<TFloating, TFloatingTC>.NoValues;

public IValueSet<TFloating> Related(BinaryOperatorKind relation, TFloating value) =>
FloatingValueSet<TFloating, TFloatingTC>.Related(relation, value);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ private interface INumericTC<T>
/// </summary>
T Random(Random random);

/// <summary>
/// Produce the zero value for the type.
/// </summary>
T Zero { get; }

/// <summary>
/// A formatter for values of type <typeparamref name="T"/>. This is needed for testing because
/// the default ToString output for float and double changed between desktop and .net core,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ private struct IntTC : INumericTC<int>

int INumericTC<int>.MaxValue => int.MaxValue;

int INumericTC<int>.Zero => 0;

public bool Related(BinaryOperatorKind relation, int left, int right)
{
switch (relation)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ private struct LongTC : INumericTC<long>

long INumericTC<long>.MaxValue => long.MaxValue;

long INumericTC<long>.Zero => 0;

bool INumericTC<long>.Related(BinaryOperatorKind relation, long left, long right)
{
switch (relation)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#nullable enable

using System;
using Microsoft.CodeAnalysis.PooledObjects;
using Roslyn.Utilities;

Expand All @@ -17,6 +18,8 @@ private sealed class NintValueSet : IValueSet<int>, IValueSet
{
public readonly static NintValueSet AllValues = new NintValueSet(hasSmall: true, values: NumericValueSet<int, IntTC>.AllValues, hasLarge: true);

public readonly static NintValueSet NoValues = new NintValueSet(hasSmall: false, values: NumericValueSet<int, IntTC>.NoValues, hasLarge: false);

private readonly IValueSet<int> _values;

/// <summary>
Expand All @@ -42,7 +45,24 @@ internal NintValueSet(bool hasSmall, IValueSet<int> values, bool hasLarge)
_hasLarge = hasLarge;
}

bool IValueSet.IsEmpty => !_hasSmall && !_hasLarge && _values.IsEmpty;
public bool IsEmpty => !_hasSmall && !_hasLarge && _values.IsEmpty;

ConstantValue? IValueSet.Sample
{
get
{
if (IsEmpty)
throw new ArgumentException();

if (!_values.IsEmpty)
return _values.Sample;

// We do not support sampling from a nint value set without a specific value. The caller
// must arrange another way to get a sample, since we can return no specific value. This
// occurs when the value set was constructed from a pattern like `> (nint)int.MaxValue`.
return null;
}
}

public bool All(BinaryOperatorKind relation, int value)
{
Expand Down
Loading

0 comments on commit 212fc8f

Please sign in to comment.