Skip to content

Commit

Permalink
Add support for '(' flag in Flogger for negative numbers.
Browse files Browse the repository at this point in the history 
RELNOTES=Add support for '(' flag in Flogger for negative numbers

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=275018832
  • Loading branch information
@darthninja1 @kluever
darthninja1 authored and kluever committed Oct 17, 2019
1 parent d7f22f2 commit df6dfe5
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 14 deletions.
16 changes: 10 additions & 6 deletions api/src/main/java/com/google/common/flogger/backend/FormatChar.java
View file
Expand Up @@ -56,42 +56,46 @@ public enum FormatChar {
* <p>
* This is a numeric format.
*/
DECIMAL('d', FormatType.INTEGRAL, "-0+ ,", false /* lower-case only */),
DECIMAL('d', FormatType.INTEGRAL, "-0+ ,(", false /* lower-case only */),

/**
* Formats the argument as an unsigned octal integer.
* <p>
* This is a numeric format.
* <p>
* '(' is only supported for {@link java.math.BigInteger} or {@link java.math.BigDecimal}
*/
OCTAL('o', FormatType.INTEGRAL, "-#0", false /* lower-case only */),
OCTAL('o', FormatType.INTEGRAL, "-#0(", false /* lower-case only */),

/**
* Formats the argument as an unsigned hexadecimal integer.
* <p>
* This is a numeric format with an upper-case variant.
* <p>
* '(' is only supported for {@link java.math.BigInteger} or {@link java.math.BigDecimal}
*/
HEX('x', FormatType.INTEGRAL, "-#0", true /* upper-case variant */),
HEX('x', FormatType.INTEGRAL, "-#0(", true /* upper-case variant */),

/**
* Formats the argument as a signed decimal floating value.
* <p>
* This is a numeric format.
*/
FLOAT('f', FormatType.FLOAT, "-#0+ ,", false /* lower-case only */),
FLOAT('f', FormatType.FLOAT, "-#0+ ,(", false /* lower-case only */),

/**
* Formats the argument using computerized scientific notation.
* <p>
* This is a numeric format with an upper-case variant.
*/
EXPONENT('e', FormatType.FLOAT, "-#0+ ", true /* upper-case variant */),
EXPONENT('e', FormatType.FLOAT, "-#0+ (", true /* upper-case variant */),

/**
* Formats the argument using general scientific notation.
* <p>
* This is a numeric format with an upper-case variant.
*/
GENERAL('g', FormatType.FLOAT, "-0+ ,", true /* upper-case variant */),
GENERAL('g', FormatType.FLOAT, "-0+ ,(", true /* upper-case variant */),

/**
* Formats the argument using hexadecimal exponential form. This formatting option is primarily
Expand Down
2 changes: 1 addition & 1 deletion api/src/main/java/com/google/common/flogger/backend/SimpleMessageFormatter.java
View file
Expand Up @@ -344,7 +344,7 @@ private static void appendFormatted(
default:
// Fall through.
}
// Default handle for rare cases that need non-tivial formatting.
// Default handle for rare cases that need non-trivial formatting.
String formatString = format.getDefaultFormatString();
if (!options.isDefault()) {
char chr = format.getChar();
Expand Down
28 changes: 21 additions & 7 deletions api/src/test/java/com/google/common/flogger/backend/FormatCharTest.java
View file
Expand Up @@ -55,36 +55,50 @@ public void testTypes() {
assertFormatType(FormatType.FLOAT, FLOAT, GENERAL, EXPONENT, EXPONENT_HEX);
}

// Testing the exact set of allowed flags for each format type.
@Test
public void testCase() {
public void testAllowedFlags() {
// Grouped by similar allowed flags.
assertFlags(FormatChar.STRING, "-#", true);

assertFlags(FormatChar.BOOLEAN, "-", true);
assertFlags(FormatChar.CHAR, "-", true);

assertFlags(FormatChar.DECIMAL, "-0+ ,", false);
assertFlags(FormatChar.GENERAL, "-0+ ,", true);
assertFlags(FormatChar.DECIMAL, "(-0+ ,", false);
assertFlags(FormatChar.GENERAL, "-0(+ ,", true);

assertFlags(FormatChar.HEX, "-#0", true);
assertFlags(FormatChar.OCTAL, "-#0", false);
assertFlags(FormatChar.HEX, "-#(0", true);
assertFlags(FormatChar.OCTAL, "-(#0", false);

assertFlags(FormatChar.FLOAT, "-#0+ ,", false);
assertFlags(FormatChar.FLOAT, "-#0+ ,(", false);

assertFlags(FormatChar.EXPONENT, "-#0+ ", true);
assertFlags(FormatChar.EXPONENT, "-#0+ (", true);
assertFlags(FormatChar.EXPONENT_HEX, "-#0+ ", true);
}

// Testing conditional rules and special cases for flags/width/precision etc.
// See also the "Details" section in:
// https://docs.oracle.com/javase/9/docs/api/java/util/Formatter.html
// These are not exhaustive tests for all illegal formatting options.
@Test
public void testAppendPrintfBadOptions() {
// String formatting cannot have zero padding.
assertThat(parseOptions("#016").areValidFor(STRING)).isFalse();
// Integer formatting cannot have precision.
assertThat(parseOptions("10.5").areValidFor(DECIMAL)).isFalse();
// Exponential formatting cannot use grouping (even though other numeric formats do).
assertThat(parseOptions(",").areValidFor(EXPONENT)).isFalse();
// Gereral scientific notation cannot specify a radix.
assertThat(parseOptions("#").areValidFor(GENERAL)).isFalse();
// Octal numbers are never negative, so ' ' is not meaningful.
assertThat(parseOptions(" ").areValidFor(OCTAL)).isFalse();
// Left alignment or zero padding must have a width.
assertThat(parseOptions("-").areValidFor(DECIMAL)).isFalse();
assertThat(parseOptions("0").areValidFor(HEX)).isFalse();

// Assert that '(' is not valid for other formats
assertThat(parseOptions("(").areValidFor(EXPONENT_HEX)).isFalse();
assertThat(parseOptions("(").areValidFor(STRING)).isFalse();
}

private static FormatOptions parseOptions(String s) {
Expand Down
91 changes: 91 additions & 0 deletions api/src/test/java/com/google/common/flogger/backend/SimpleMessageFormatterTest.java
View file
Expand Up @@ -19,14 +19,19 @@
import static com.google.common.flogger.backend.FormatOptions.FLAG_UPPER_CASE;
import static com.google.common.flogger.backend.FormatOptions.UNSET;
import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.fail;

import com.google.common.flogger.LogContext.Key;
import com.google.common.flogger.LogSite;
import com.google.common.flogger.MetadataKey;
import com.google.common.flogger.backend.SimpleMessageFormatter.Option;
import com.google.common.flogger.backend.SimpleMessageFormatter.SimpleLogHandler;
import com.google.common.flogger.parser.ParseException;
import com.google.common.flogger.testing.FakeLogData;
import java.io.IOException;
import java.math.BigDecimal;
import java.math.BigInteger;
import java.util.FormatFlagsConversionMismatchException;
import java.util.Formattable;
import java.util.Formatter;
import java.util.logging.Level;
Expand Down Expand Up @@ -108,6 +113,76 @@ public void formatTo(Formatter formatter, int flags, int width, int precision) {
assertThat(log("%-#32.16S", arg)).isEqualTo("[f=7,w=32,p=16]");
}

@Test
public void testNumberFormatting() {
// TODO: add more tests with other flags '#', ',', ' ', '-', '+'
assertThat(log("%d", -123)).isEqualTo("-123");
assertThat(log("%d", -123L)).isEqualTo("-123");
assertThat(log("%G", -123f)).isEqualTo("-123.000");
assertThat(log("%e", -123f)).isEqualTo("-1.230000e+02");
assertThat(log("%f", -123f)).isEqualTo("-123.000000");
assertThat(log("%g", -123.456789)).isEqualTo("-123.457");
assertThat(log("%.6G", -123.456789)).isEqualTo("-123.457"); // Precision is ignored
assertThat(log("%.8E", -123.456789)).isEqualTo("-1.23456789E+02");
assertThat(log("%f", -123.456789)).isEqualTo("-123.456789");

assertThat(log("%(d", 123)).isEqualTo("123");
assertThat(log("%(d", -123)).isEqualTo("(123)");
assertThat(log("%(d", -123L)).isEqualTo("(123)");
assertThat(log("%(g", -123f)).isEqualTo("(123.000)");
assertThat(log("%(E", -123f)).isEqualTo("(1.230000E+02)");
assertThat(log("%(f", -123f)).isEqualTo("(123.000000)");
assertThat(log("%(4.10f", -123f)).isEqualTo("(123.0000000000)");
assertThat(log("%(1.2f", -123f)).isEqualTo("(123.00)");
assertThat(log("%(.2f", -123f)).isEqualTo("(123.00)");
assertThat(log("%(f", -123.0)).isEqualTo("(123.000000)");

// Hex int and BigInteger
assertThat(log("%x", 123)).isEqualTo("7b");
assertThat(log("%X", -123)).isEqualTo("FFFFFF85");
assertThat(log("%x", BigInteger.valueOf(123))).isEqualTo("7b");
assertThat(log("%X", BigInteger.valueOf(-123))).isEqualTo("-7B");
assertThat(log("%(x", BigInteger.valueOf(-123))).isEqualTo("(7b)");
assertThat(log("%(x", BigInteger.valueOf(123))).isEqualTo("7b");

// Octal ints and BigInteger
assertThat(log("%o", 123)).isEqualTo("173");
assertThat(log("%o", -123)).isEqualTo("37777777605");
assertThat(log("%o", BigInteger.valueOf(123))).isEqualTo("173");
assertThat(log("%o", BigInteger.valueOf(-123))).isEqualTo("-173");
assertThat(log("%(o", BigInteger.valueOf(-123))).isEqualTo("(173)");
assertThat(log("%(o", BigInteger.valueOf(123))).isEqualTo("173");

// BigDecimal
assertThat(log("%f", BigDecimal.ONE)).isEqualTo("1.000000");
assertThat(log("%f", BigDecimal.valueOf(-1234.56789))).isEqualTo("-1234.567890");
assertThat(log("%g", BigDecimal.ONE)).isEqualTo("1.00000");
assertThat(log("%g", BigDecimal.valueOf(-123456789))).isEqualTo("-1.23457e+08");
assertThat(log("%G", BigDecimal.valueOf(-1234.56789))).isEqualTo("-1234.57");
assertThat(log("%G", BigDecimal.valueOf(-123456789))).isEqualTo("-1.23457E+08");
assertThat(log("%e", BigDecimal.valueOf(1234.56789))).isEqualTo("1.234568e+03");
assertThat(log("%E", BigDecimal.valueOf(-1234.56789))).isEqualTo("-1.234568E+03");
assertThat(log("%(f", BigDecimal.valueOf(-1234.56789))).isEqualTo("(1234.567890)");
assertThat(log("%(g", BigDecimal.valueOf(-1234.56789))).isEqualTo("(1234.57)");
assertThat(log("%(e", BigDecimal.valueOf(-1234.56789))).isEqualTo("(1.234568e+03)");
}

@Test
public void testInvalidFlags() {
assertFormatFailure("%(s", 123);
assertFormatFailure("%(b", 123);
assertFormatFailure("%(s", -123);
assertFormatFailure("%(b", -123);
}

@Test
public void testFormatFlagsConversionMismatchException() {
assertFormatFlagsConversionMismatchException("%(x", 123);
assertFormatFlagsConversionMismatchException("%(o", 123);
assertFormatFlagsConversionMismatchException("%(x", -123);
assertFormatFlagsConversionMismatchException("%(o", -123);
}

@Test
public void testFormatWithOption() {
assertThat(logWithOption(Option.DEFAULT, "Hello World")).isEqualTo("Hello World");
Expand Down Expand Up @@ -147,6 +222,22 @@ public void formatTo(Formatter formatter, int flags, int width, int precision) {
assertThat(log("%s", arg)).doesNotContain("UNEXPECTED");
}

private static void assertFormatFailure(String format, Object arg) {
try {
log(format, arg);
fail("expected ParseException");
} catch (ParseException expected) {
}
}

private static void assertFormatFlagsConversionMismatchException(String format, Object arg) {
try {
log(format, arg);
fail("expected FormatFlagsConversionMismatchException");
} catch (FormatFlagsConversionMismatchException expected) {
}
}

private static String formatHex(Number n, FormatOptions options) {
StringBuilder out = new StringBuilder();
SimpleMessageFormatter.appendHex(out, n, options);
Expand Down

0 comments on commit df6dfe5

Please sign in to comment.