Skip to content

Commit

Permalink
Consolidate and filter error messages related to invalid config prope…
Browse files Browse the repository at this point in the history
…rties.
  • Loading branch information
fgimian committed Jan 28, 2025
1 parent 8afe244 commit 972f520
Show file tree
Hide file tree
Showing 9 changed files with 204 additions and 36 deletions.
111 changes: 111 additions & 0 deletions src/TotalMixVC.Tests/ConfigTests.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System.Net;
using System.Windows.Media;
using Tomlyn.Syntax;
using TotalMixVC.Configuration;
using Xunit;

Expand Down Expand Up @@ -522,4 +523,114 @@ out var diagnostics
Assert.Empty(diagnostics);
Assert.Equal(expectedConfig, config);
}

[Fact]
public void CleanDiagnostics_InvalidProperties_ProvidesCleanMessages()
{
var diagnostics = new DiagnosticsBag(
[
new DiagnosticMessage(
DiagnosticMessageKind.Error,
new SourceSpan(
fileName: string.Empty,
start: new TextPosition(offset: 0, line: 0, column: 0),
end: new TextPosition(offset: 27, line: 0, column: 27)
),
"Exception while trying to convert System.String to type "
+ "System.Windows.Media.Color. Reason: Token is not valid."
),
new DiagnosticMessage(
DiagnosticMessageKind.Error,
new SourceSpan(
fileName: string.Empty,
start: new TextPosition(offset: 0, line: 0, column: 0),
end: new TextPosition(offset: 27, line: 0, column: 27)
),
"The property value of type System.String couldn't be converted to "
+ "System.Windows.Media.Color for the property Config/background_color"
),
new DiagnosticMessage(
DiagnosticMessageKind.Warning,
new SourceSpan(
fileName: string.Empty,
start: new TextPosition(offset: 59, line: 2, column: 0),
end: new TextPosition(offset: 87, line: 2, column: 28)
),
"The property `EndPoint` was not found, but `end_point` was. By default "
+ "property names are lowered and split by _ by PascalCase letters. This "
+ "behavior can be changed by passing a TomlModelOptions and specifying "
+ "the TomlModelOptions.ConvertPropertyName delegate."
),
new DiagnosticMessage(
DiagnosticMessageKind.Error,
new SourceSpan(
fileName: string.Empty,
start: new TextPosition(offset: 59, line: 2, column: 0),
end: new TextPosition(offset: 87, line: 2, column: 28)
),
"The property EndPoint was not found on object type Config"
),
new DiagnosticMessage(
DiagnosticMessageKind.Error,
new SourceSpan(
fileName: string.Empty,
start: new TextPosition(offset: 100, line: 5, column: 0),
end: new TextPosition(offset: 112, line: 5, column: 12)
),
"Unsupported type to convert System.String to type System.Single."
),
new DiagnosticMessage(
DiagnosticMessageKind.Error,
new SourceSpan(
fileName: string.Empty,
start: new TextPosition(offset: 100, line: 5, column: 0),
end: new TextPosition(offset: 112, line: 5, column: 12)
),
"The property value of type System.String couldn't be converted to "
+ "System.Single for the property Volume/max"
),
]
);

var diagnosticMessages = Config.CleanDiagnostics(diagnostics);
Assert.Equal(
[
"(1,1) : error : The property value of type System.String couldn't be converted "
+ "to System.Windows.Media.Color for the property Config/background_color. "
+ "Token is not valid.",
"(3,1) : warning : The property `EndPoint` was not found, but `end_point` was. "
+ "By default property names are lowered and split by _ by PascalCase "
+ "letters. This behavior can be changed by passing a TomlModelOptions and "
+ "specifying the TomlModelOptions.ConvertPropertyName delegate.",
"(3,1) : error : The property EndPoint was not found on object type Config.",
"(6,1) : error : The property value of type System.String couldn't be converted "
+ "to System.Single for the property Volume/max.",
],
diagnosticMessages
);
}

[Fact]
public void CleanDiagnostics_InvalidSyntax_DoesNotConsolidateMessages()
{
var diagnostics = new DiagnosticsBag(
[
new DiagnosticMessage(
DiagnosticMessageKind.Error,
new SourceSpan(
fileName: string.Empty,
start: new TextPosition(offset: 16, line: 0, column: 16),
end: new TextPosition(offset: 16, line: 0, column: 16)
),
"Expecting `=` after a key instead of !"
),
]
);

var diagnosticMessages = Config.CleanDiagnostics(diagnostics);
Assert.Equal(
["(1,17) : error : Expecting `=` after a key instead of !."],
diagnosticMessages
);
}
}
4 changes: 2 additions & 2 deletions src/TotalMixVC/App.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,9 @@ public bool LoadConfig(bool running = false)
);
}

foreach (var diagnostic in diagnostics)
foreach (var diagnosticMessage in Config.CleanDiagnostics(diagnostics))
{
message.Append(CultureInfo.InvariantCulture, $"- {diagnostic}\n");
message.Append(CultureInfo.InvariantCulture, $"- {diagnosticMessage}\n");
}

message.Append(
Expand Down
67 changes: 66 additions & 1 deletion src/TotalMixVC/Configuration/Config.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
using System.Net;
using System.Text;
using System.Text.RegularExpressions;
using System.Windows.Media;
using Tomlyn;
using Tomlyn.Syntax;
Expand All @@ -9,7 +11,7 @@ namespace TotalMixVC.Configuration;
/// <summary>
/// Provides all configurable settings for the application along with suitable defaults.
/// </summary>
public record Config
public partial record Config
{
/// <summary>Gets configuration related to OSC communication with the device.</summary>
public Osc Osc { get; init; } = new Osc();
Expand Down Expand Up @@ -75,4 +77,67 @@ public static bool TryFromToml(string text, out Config? config, out DiagnosticsB

return isValid;
}

/// <summary>
/// Consolidates diagnostics returned by Tomlyn in an attempt to have one clean message
/// for each property issue discovered.
/// </summary>
/// <param name="diagnostics">
/// The diagnostics bag returned by Tomlyn upon parsing a TOML file with errors or warnings.
/// </param>
/// <returns>A consolidated iterable of formatted diagnostic messages.</returns>
public static IEnumerable<string> CleanDiagnostics(DiagnosticsBag diagnostics)
{
var diagnosticsExceptionRegex = DiagnosticsExceptionRegex();

foreach (var group in diagnostics.GroupBy(diagnostic => diagnostic.Span))
{
var reason = (string?)null;
var span = group.Key;

foreach (var diagnostic in group)
{
if (
diagnostic.Message.StartsWith(
"Unsupported type to convert ",
StringComparison.Ordinal
)
)
{
continue;
}

var match = diagnosticsExceptionRegex.Match(diagnostic.Message);
if (match.Success)
{
reason = match.Groups[1].Value;
continue;
}

var message = new StringBuilder();

message
.Append(span.ToStringSimple())
.Append(" : ")
.Append(diagnostic.Kind == DiagnosticMessageKind.Warning ? "warning" : "error")
.Append(" : ")
.Append(diagnostic.Message);

if (!diagnostic.Message.EndsWith('.'))
{
message.Append('.');
}

if (reason is not null)
{
message.Append(' ').Append(reason);
}

yield return message.ToString();
}
}
}

[GeneratedRegex(@"Exception while trying to convert \S+ to type \S+. Reason: (.*)")]
private static partial Regex DiagnosticsExceptionRegex();
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
namespace TotalMixVC.Configuration.Models;
namespace TotalMixVC.Configuration.Models;

/// <summary>
/// Provides the increment to use when finely increasing or decreasing the volume in decibels.
Expand All @@ -17,7 +17,7 @@ public VolumeFineIncrementDecibels(float value)
}

/// <summary>Gets or sets the volume decibel value.</summary>
/// <exception cref="ArgumentOutOfRangeException">
/// <exception cref="InvalidOperationException">
/// The fine increment specified is not in the supported range.
/// </exception>
public float Value
Expand All @@ -27,10 +27,9 @@ public float Value
{
if (value <= 0.0 || value > 3.0 || value % 0.25f != 0.0f)
{
throw new ArgumentOutOfRangeException(
nameof(value),
"Must be a multiple of 0.25 while being greater than 0 and less than or equal "
+ "to 3.0."
throw new InvalidOperationException(
"The value must be a multiple of 0.25 while being greater than 0 and less "
+ "than or equal to 3.0."
);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
namespace TotalMixVC.Configuration.Models;
namespace TotalMixVC.Configuration.Models;

/// <summary>
/// Provides the increment to use when finely increasing or decreasing the volume in percent.
Expand All @@ -17,7 +17,7 @@ public VolumeFineIncrementPercent(float value)
}

/// <summary>Gets or sets the volume percentage value.</summary>
/// <exception cref="ArgumentOutOfRangeException">
/// <exception cref="InvalidOperationException">
/// The fine increment specified is not in the supported range.
/// </exception>
public float Value
Expand All @@ -27,9 +27,8 @@ public float Value
{
if (value is <= 0.0f or > 0.05f)
{
throw new ArgumentOutOfRangeException(
nameof(value),
"Must be greater than 0 and less than or equal to 0.05."
throw new InvalidOperationException(
"The value must be greater than 0 and less than or equal to 0.05."
);
}

Expand Down
11 changes: 5 additions & 6 deletions src/TotalMixVC/Configuration/Models/VolumeIncrementDecibels.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
namespace TotalMixVC.Configuration.Models;
namespace TotalMixVC.Configuration.Models;

/// <summary>
/// Provides the increment to use when increasing or decreasing the volume in decibels.
Expand All @@ -17,7 +17,7 @@ public VolumeIncrementDecibels(float value)
}

/// <summary>Gets or sets the volume decibel value.</summary>
/// <exception cref="ArgumentOutOfRangeException">
/// <exception cref="InvalidOperationException">
/// The increment specified is not in the supported range.
/// </exception>
public float Value
Expand All @@ -27,10 +27,9 @@ public float Value
{
if (value <= 0.0 || value > 6.0 || value % 0.5f != 0.0f)
{
throw new ArgumentOutOfRangeException(
nameof(value),
"Must be a multiple of 0.5 while being greater than 0 and less than or equal "
+ "to 6.0."
throw new InvalidOperationException(
"The value must be a multiple of 0.5 while being greater than 0 and less "
+ "than or equal to 6.0."
);
}

Expand Down
9 changes: 4 additions & 5 deletions src/TotalMixVC/Configuration/Models/VolumeIncrementPercent.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
namespace TotalMixVC.Configuration.Models;
namespace TotalMixVC.Configuration.Models;

/// <summary>
/// Provides the increment to use when increasing or decreasing the volume in percent.
Expand All @@ -17,7 +17,7 @@ public VolumeIncrementPercent(float value)
}

/// <summary>Gets or sets the volume percentage value.</summary>
/// <exception cref="ArgumentOutOfRangeException">
/// <exception cref="InvalidOperationException">
/// The increment specified is not in the supported range.
/// </exception>
public float Value
Expand All @@ -27,9 +27,8 @@ public float Value
{
if (value is <= 0.0f or > 0.10f)
{
throw new ArgumentOutOfRangeException(
nameof(value),
"Must be greater than 0 and less than or equal to 0.1."
throw new InvalidOperationException(
"The value must be greater than 0 and less than or equal to 0.1."
);
}

Expand Down
9 changes: 3 additions & 6 deletions src/TotalMixVC/Configuration/Models/VolumeMaxDecibels.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
namespace TotalMixVC.Configuration.Models;
namespace TotalMixVC.Configuration.Models;

/// <summary>
/// Provides the maximum volume that should be allowed when increasing the volume in decibels.
Expand All @@ -17,7 +17,7 @@ public VolumeMaxDecibels(float value)
}

/// <summary>Gets or sets the volume decibel value.</summary>
/// <exception cref="ArgumentOutOfRangeException">
/// <exception cref="InvalidOperationException">
/// The max volume specified is not in the supported range.
/// </exception>
public float Value
Expand All @@ -27,10 +27,7 @@ public float Value
{
if (value is > 6.0f)
{
throw new ArgumentOutOfRangeException(
nameof(value),
"Must be less than or equal to 6.0."
);
throw new InvalidOperationException("The value must be less than or equal to 6.0.");
}

_value = value;
Expand Down
9 changes: 4 additions & 5 deletions src/TotalMixVC/Configuration/Models/VolumeMaxPercent.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
namespace TotalMixVC.Configuration.Models;
namespace TotalMixVC.Configuration.Models;

/// <summary>
/// Provides the maximum volume that should be allowed when increasing the volume in percent.
Expand All @@ -17,7 +17,7 @@ public VolumeMaxPercent(float value)
}

/// <summary>Gets or sets the volume percentage value.</summary>
/// <exception cref="ArgumentOutOfRangeException">
/// <exception cref="InvalidOperationException">
/// The max volume specified is not in the supported range.
/// </exception>
public float Value
Expand All @@ -27,9 +27,8 @@ public float Value
{
if (value is <= 0.0f or > 1.0f)
{
throw new ArgumentOutOfRangeException(
nameof(value),
"Must be greater than 0 and less than or equal to 1.0."
throw new InvalidOperationException(
"The value must be greater than 0 and less than or equal to 1.0."
);
}

Expand Down

0 comments on commit 972f520

Please sign in to comment.