Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CultureInfo support for errors #1040

Merged
merged 2 commits into from
Jan 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 21 additions & 4 deletions src/libraries/Microsoft.PowerFx.Core/Public/CheckResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ public CheckResult SetText(string expression, ParserOptions parserOptions = null

_expression = expression;
_parserOptions = parserOptions ?? Engine.GetDefaultParserOptionsCopy();
this.ParserCultureInfo = _parserOptions.Culture;

return this;
}
Expand Down Expand Up @@ -239,12 +240,27 @@ internal TexlBinding Binding
/// </summary>
public IEnumerable<ExpressionError> Errors
{
get => _errors;
get => GetErrorsInLocale(null);

[Obsolete("use constructor to set errors")]
set => _errors.AddRange(value);
}

/// <summary>
/// Get errors localized with the given culture.
/// </summary>
/// <param name="culture"></param>
/// <returns></returns>
public IEnumerable<ExpressionError> GetErrorsInLocale(CultureInfo culture)
{
culture ??= this.ParserCultureInfo;

foreach (var error in this._errors)
{
yield return error.GetInLocale(culture);
}
}

/// <summary>
/// True if no errors for stages run so far.
/// </summary>
Expand Down Expand Up @@ -303,9 +319,10 @@ internal ReadOnlySymbolTable Parameters
}

/// <summary>
/// Culture info passed to this binding. May be null.
/// Culture info used for parsing.
/// By default, this is also used for error messages.
/// </summary>
internal CultureInfo CultureInfo => this.Engine.Config.CultureInfo;
internal CultureInfo ParserCultureInfo { get; private set; }

internal void ThrowIfSymbolsChanged()
{
Expand Down Expand Up @@ -378,7 +395,7 @@ internal TexlBinding ApplyBindingInternal()
this._allSymbols = combinedSymbols;

// Add the errors
IEnumerable<ExpressionError> bindingErrors = ExpressionError.New(binding.ErrorContainer.GetErrors(), CultureInfo);
IEnumerable<ExpressionError> bindingErrors = ExpressionError.New(binding.ErrorContainer.GetErrors(), ParserCultureInfo);
_errors.AddRange(bindingErrors);

if (this.IsSuccess)
Expand Down
71 changes: 65 additions & 6 deletions src/libraries/Microsoft.PowerFx.Core/Public/ExpressionError.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,32 @@ namespace Microsoft.PowerFx
/// </summary>
public class ExpressionError
{
public ExpressionError()
{
}

/// <summary>
/// A description of the error message.
/// </summary>
public string Message { get; set; }
public string Message
{
get
{
if (_message == null && this.MessageKey != null)
{
(var shortMessage, var _) = ErrorUtils.GetLocalizedErrorContent(new ErrorResourceKey(this.MessageKey), _messageLocale, out _);

var msg = ErrorUtils.FormatMessage(shortMessage, _messageLocale, _messageArgs);

_message = msg;
}

return _message;
}

// If this is set directly, it will skip localization.
set => _message = value;
}

/// <summary>
/// Source location for this error.
Expand All @@ -42,6 +64,43 @@ public class ExpressionError
/// </summary>
public bool IsWarning => Severity < ErrorSeverity.Severe;

// localize message lazily
private string _message;
private object[] _messageArgs;
private CultureInfo _messageLocale;

internal CultureInfo MessageLocale => _messageLocale;

/// <summary>
/// Get a copy of this error message for the given locale.
/// <see cref="Message"/> will get lazily localized using <see cref="MessageKey"/>.
/// </summary>
/// <param name="culture"></param>
/// <returns></returns>
public ExpressionError GetInLocale(CultureInfo culture)
{
// In order to localize, we need a message key
if (this.MessageKey != null)
{
var error = new ExpressionError
{
Span = this.Span,
Kind = this.Kind,
Severity = this.Severity,
MessageKey = this.MessageKey
};

// New message can be localized
error._message = null; // will be lazily computed in new locale
error._messageArgs = this._messageArgs;
error._messageLocale = culture;

return error;
}

return this;
}

public override string ToString()
{
var prefix = IsWarning ? "Warning" : "Error";
Expand All @@ -51,7 +110,7 @@ public override string ToString()
}
else
{
return $"{prefix} {Message}";
return $"{prefix}: {Message}";
}
}

Expand All @@ -60,7 +119,8 @@ internal static ExpressionError New(IDocumentError error)
{
return new ExpressionError
{
Message = error.ShortMessage,
_message = error.ShortMessage,
_messageArgs = error.MessageArgs,
Span = error.TextSpan,
Severity = (ErrorSeverity)error.Severity,
MessageKey = error.MessageKey
Expand All @@ -69,11 +129,10 @@ internal static ExpressionError New(IDocumentError error)

internal static ExpressionError New(IDocumentError error, CultureInfo locale)
{
(var shortMessage, var _) = ErrorUtils.GetLocalizedErrorContent(new ErrorResourceKey(error.MessageKey), locale, out _);

return new ExpressionError
{
Message = ErrorUtils.FormatMessage(shortMessage, locale, error.MessageArgs),
_messageLocale = locale,
_messageArgs = error.MessageArgs,
Span = error.TextSpan,
Severity = (ErrorSeverity)error.Severity,
MessageKey = error.MessageKey
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

using System;
using System.Globalization;
using System.Linq;
using Microsoft.PowerFx.Core.Utils;
Expand Down Expand Up @@ -46,7 +47,7 @@ public static bool TryParse(DTypeSpecLexer lexer, out DType type)
return true;
}

var typeIdx = _typeEncodings.IndexOf(token);
var typeIdx = _typeEncodings.IndexOf(token, StringComparison.Ordinal);
if (typeIdx < 0)
{
type = DType.Invalid;
Expand Down Expand Up @@ -114,7 +115,7 @@ private static bool TryParseTypeMap(DTypeSpecLexer lexer, out TypeTree map)
while (lexer.TryNextToken(out token) && token != "]")
{
var name = token;
if (name.Length >= 2 && name.StartsWith("'") && name.EndsWith("'"))
if (name.Length >= 2 && name.StartsWith("'", StringComparison.Ordinal) && name.EndsWith("'", StringComparison.Ordinal))
{
name = TexlLexer.UnescapeName(name);
}
Expand Down Expand Up @@ -168,7 +169,7 @@ private static bool TryParseValueMap(DTypeSpecLexer lexer, out ValueTree map)
while (lexer.TryNextToken(out token) && token != "]")
{
var name = token;
if (name.Length >= 2 && name.StartsWith("'") && name.EndsWith("'"))
if (name.Length >= 2 && name.StartsWith("'", StringComparison.Ordinal) && name.EndsWith("'", StringComparison.Ordinal))
{
name = name.TrimStart('\'').TrimEnd('\'');
}
Expand Down Expand Up @@ -244,7 +245,7 @@ private static bool TryParseEquatableObject(DTypeSpecLexer lexer, out EquatableO
}

// Number (double) support
if (double.TryParse(token, out var numValue))
if (double.TryParse(token, NumberStyles.Float, CultureInfo.InvariantCulture, out var numValue))
{
value = new EquatableObject(numValue);
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ internal static ReadOnlySymbolValues New(
{
// There were existing SymbolValues that don't match.
var kv = existingMap.First();
var msg = $"SymbolValues '{kv.Value.DebugName}' matches to symbol table '{kv.Key.DebugName}', which is not part of symbol table '{symbolTable.DebugName}'.";
var msg = $"SymbolValues '{kv.Value.DebugName}' matches to symbol table '{kv.Key.DebugName}', which is not part of symbol table '{symbolTable?.DebugName}'.";
throw new InvalidOperationException(msg);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ namespace Microsoft.PowerFx.Functions
internal static partial class Library
{
// ColorTable is ARGB
private static readonly Regex RegexColorTable = new (@"^#(?<a>[0-9a-fA-F]{2})(?<r>[0-9a-fA-F]{2})(?<g>[0-9a-fA-F]{2})(?<b>[0-9a-fA-F]{2})?$", RegexOptions.Compiled);
private static readonly Regex RegexColorTable = new Regex(@"^#(?<a>[0-9a-fA-F]{2})(?<r>[0-9a-fA-F]{2})(?<g>[0-9a-fA-F]{2})(?<b>[0-9a-fA-F]{2})?$", LibraryFlags.RegExFlags);

// CSS format is RGBA
private static readonly Regex RegexCSS = new (@"^#(?<r>[0-9a-fA-F]{2})(?<g>[0-9a-fA-F]{2})(?<b>[0-9a-fA-F]{2})(?<a>[0-9a-fA-F]{2})?$", RegexOptions.Compiled);
private static readonly Regex RegexCSS = new Regex(@"^#(?<r>[0-9a-fA-F]{2})(?<g>[0-9a-fA-F]{2})(?<b>[0-9a-fA-F]{2})(?<a>[0-9a-fA-F]{2})?$", LibraryFlags.RegExFlags);

public static FormulaValue ColorValue(IRContext irContext, StringValue[] args)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,30 @@

namespace Microsoft.PowerFx.Functions
{
// Due to .Net static ctor initialization, must place in a separate class from Library.
internal static class LibraryFlags
{
public static readonly RegexOptions RegExFlags = RegexOptions.Compiled | RegexOptions.CultureInvariant;
}

internal static partial class Library
{
private static readonly Regex _ampmReplaceRegex = new Regex("[aA][mM]\\/[pP][mM]", RegexOptions.Compiled);
private static readonly Regex _apReplaceRegex = new Regex("[aA]\\/[pP]", RegexOptions.Compiled);
private static readonly Regex _minutesBeforeSecondsRegex = new Regex("[mM][^dDyYhH]+[sS]", RegexOptions.Compiled);
private static readonly Regex _minutesAfterHoursRegex = new Regex("[hH][^dDyYmM]+[mM]", RegexOptions.Compiled);
private static readonly Regex _minutesRegex = new Regex("[mM]", RegexOptions.Compiled);
private static readonly Regex _internalStringRegex = new Regex("([\"][^\"]*[\"])", RegexOptions.Compiled);
private static readonly Regex _daysDetokenizeRegex = new Regex("[\u0004][\u0004][\u0004][\u0004]+", RegexOptions.Compiled);
private static readonly Regex _monthsDetokenizeRegex = new Regex("[\u0003][\u0003][\u0003][\u0003]+", RegexOptions.Compiled);
private static readonly Regex _yearsDetokenizeRegex = new Regex("[\u0005][\u0005][\u0005]+", RegexOptions.Compiled);
private static readonly Regex _years2DetokenizeRegex = new Regex("[\u0005]+", RegexOptions.Compiled);
private static readonly Regex _hoursDetokenizeRegex = new Regex("[\u0006][\u0006]+", RegexOptions.Compiled);
private static readonly Regex _minutesDetokenizeRegex = new Regex("[\u000A][\u000A]+", RegexOptions.Compiled);
private static readonly Regex _secondsDetokenizeRegex = new Regex("[\u0008][\u0008]+", RegexOptions.Compiled);
private static readonly Regex _milisecondsDetokenizeRegex = new Regex("[\u000e]+", RegexOptions.Compiled);
private static readonly RegexOptions RegExFlags = LibraryFlags.RegExFlags;

private static readonly Regex _ampmReplaceRegex = new Regex("[aA][mM]\\/[pP][mM]", RegExFlags);
private static readonly Regex _apReplaceRegex = new Regex("[aA]\\/[pP]", RegExFlags);
private static readonly Regex _minutesBeforeSecondsRegex = new Regex("[mM][^dDyYhH]+[sS]", RegExFlags);
private static readonly Regex _minutesAfterHoursRegex = new Regex("[hH][^dDyYmM]+[mM]", RegExFlags);
private static readonly Regex _minutesRegex = new Regex("[mM]", RegExFlags);
private static readonly Regex _internalStringRegex = new Regex("([\"][^\"]*[\"])", RegExFlags);
private static readonly Regex _daysDetokenizeRegex = new Regex("[\u0004][\u0004][\u0004][\u0004]+", RegExFlags);
private static readonly Regex _monthsDetokenizeRegex = new Regex("[\u0003][\u0003][\u0003][\u0003]+", RegExFlags);
private static readonly Regex _yearsDetokenizeRegex = new Regex("[\u0005][\u0005][\u0005]+", RegExFlags);
private static readonly Regex _years2DetokenizeRegex = new Regex("[\u0005]+", RegExFlags);
private static readonly Regex _hoursDetokenizeRegex = new Regex("[\u0006][\u0006]+", RegExFlags);
private static readonly Regex _minutesDetokenizeRegex = new Regex("[\u000A][\u000A]+", RegExFlags);
private static readonly Regex _secondsDetokenizeRegex = new Regex("[\u0008][\u0008]+", RegExFlags);
private static readonly Regex _milisecondsDetokenizeRegex = new Regex("[\u000e]+", RegExFlags);

// Char is used for PA string escaping
public static FormulaValue Char(IRContext irContext, NumberValue[] args)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ internal static IExpressionEvaluator GetEvaluator(this CheckResult result, Stack
var irResult = result.ApplyIR();
result.ThrowOnErrors();

var expr = new ParsedExpression(irResult.TopNode, irResult.RuleScopeSymbol, stackMarker, result.CultureInfo)
var expr = new ParsedExpression(irResult.TopNode, irResult.RuleScopeSymbol, stackMarker, result.ParserCultureInfo)
{
_globals = globals,
_allSymbols = result.Symbols,
Expand Down
Loading