SQL: Replace String.format() with LoggerMessageFormat.format() (#37216)

matriv · web-flow · commit 5f2fbedd8c75 · 2019-01-08T23:56:00.000+02:00
Fixes: #36532
diff --git a/x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/JdbcPreparedStatementTests.java b/x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/JdbcPreparedStatementTests.java
@@ -5,10 +5,8 @@
  */
 package org.elasticsearch.xpack.sql.jdbc;
 
+import org.elasticsearch.common.logging.LoggerMessageFormat;
 import org.elasticsearch.test.ESTestCase;
-import org.elasticsearch.xpack.sql.jdbc.EsType;
-import org.elasticsearch.xpack.sql.jdbc.JdbcConfiguration;
-import org.elasticsearch.xpack.sql.jdbc.JdbcPreparedStatement;
 
 import java.net.URL;
 import java.nio.charset.StandardCharsets;
@@ -287,12 +285,12 @@ public void testThrownExceptionsWhenSettingFloatValues() throws SQLException {
 
         Float floatNotInt =  5_155_000_000f;
         sqle = expectThrows(SQLException.class, () -> jps.setObject(1, floatNotInt, Types.INTEGER));
-        assertEquals(String.format(Locale.ROOT, "Numeric %s out of range",
-                Long.toString(Math.round(floatNotInt.doubleValue()))), sqle.getMessage());
+        assertEquals(LoggerMessageFormat.format("Numeric {} out of range",
+            Math.round(floatNotInt.doubleValue())), sqle.getMessage());
 
         sqle = expectThrows(SQLException.class, () -> jps.setObject(1, floatNotInt, Types.SMALLINT));
-        assertEquals(String.format(Locale.ROOT, "Numeric %s out of range",
-                Long.toString(Math.round(floatNotInt.doubleValue()))), sqle.getMessage());
+        assertEquals(LoggerMessageFormat.format("Numeric {} out of range",
+                Math.round(floatNotInt.doubleValue())), sqle.getMessage());
     }
 
     public void testSettingDoubleValues() throws SQLException {
@@ -328,8 +326,8 @@ public void testThrownExceptionsWhenSettingDoubleValues() throws SQLException {
 
         Double doubleNotInt = 5_155_000_000d;
         sqle = expectThrows(SQLException.class, () -> jps.setObject(1, doubleNotInt, Types.INTEGER));
-        assertEquals(String.format(Locale.ROOT, "Numeric %s out of range",
-                Long.toString(((Number) doubleNotInt).longValue())), sqle.getMessage());
+        assertEquals(LoggerMessageFormat.format("Numeric {} out of range",
+                ((Number) doubleNotInt).longValue()), sqle.getMessage());
     }
 
     public void testUnsupportedClasses() throws SQLException {
diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/AnalysisException.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/AnalysisException.java
@@ -10,7 +10,7 @@
 import org.elasticsearch.xpack.sql.tree.Location;
 import org.elasticsearch.xpack.sql.tree.Node;
 
-import java.util.Locale;
+import static org.elasticsearch.common.logging.LoggerMessageFormat.format;
 
 public class AnalysisException extends ClientSqlException {
 
@@ -54,6 +54,6 @@ public RestStatus status() {
 
     @Override
     public String getMessage() {
-        return String.format(Locale.ROOT, "line %s:%s: %s", getLineNumber(), getColumnNumber(), super.getMessage());
+        return format("line {}:{}: {}", getLineNumber(), getColumnNumber(), super.getMessage());
     }
 }
diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/Cast.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/Cast.java
@@ -9,15 +9,15 @@
 import org.elasticsearch.xpack.sql.expression.Nullability;
 import org.elasticsearch.xpack.sql.expression.gen.processor.Processor;
 import org.elasticsearch.xpack.sql.expression.gen.script.ScriptTemplate;
-import org.elasticsearch.xpack.sql.tree.Source;
 import org.elasticsearch.xpack.sql.tree.NodeInfo;
+import org.elasticsearch.xpack.sql.tree.Source;
 import org.elasticsearch.xpack.sql.type.DataType;
 import org.elasticsearch.xpack.sql.type.DataTypeConversion;
 import org.elasticsearch.xpack.sql.type.DataTypes;
 
-import java.util.Locale;
 import java.util.Objects;
 
+import static org.elasticsearch.common.logging.LoggerMessageFormat.format;
 import static org.elasticsearch.xpack.sql.expression.gen.script.ParamsBuilder.paramsBuilder;
 
 public class Cast extends UnaryScalarFunction {
@@ -86,7 +86,7 @@ protected Processor makeProcessor() {
     public ScriptTemplate asScript() {
         ScriptTemplate fieldAsScript = asScript(field());
         return new ScriptTemplate(
-                formatTemplate(String.format(Locale.ROOT, "{sql}.cast(%s,{})", fieldAsScript.template())),
+                formatTemplate(format("{sql}.", "cast({},{})", fieldAsScript.template())),
                 paramsBuilder()
                     .script(fieldAsScript.params())
                     .variable(dataType.name())
diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/gen/script/Param.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/gen/script/Param.java
@@ -5,7 +5,7 @@
  */
 package org.elasticsearch.xpack.sql.expression.gen.script;
 
-import java.util.Locale;
+import static org.elasticsearch.common.logging.LoggerMessageFormat.format;
 
 abstract class Param<T> {
     private final T value;
@@ -22,6 +22,6 @@ T value() {
 
     @Override
     public String toString() {
-        return String.format(Locale.ROOT, "{%s=%s}", prefix(), value);
+        return format(null, "{{}={}}", prefix(), value);
     }
 }
diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/comparison/In.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/comparison/In.java
@@ -12,19 +12,19 @@
 import org.elasticsearch.xpack.sql.expression.function.scalar.ScalarFunction;
 import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe;
 import org.elasticsearch.xpack.sql.expression.gen.script.ScriptTemplate;
-import org.elasticsearch.xpack.sql.tree.Source;
 import org.elasticsearch.xpack.sql.tree.NodeInfo;
+import org.elasticsearch.xpack.sql.tree.Source;
 import org.elasticsearch.xpack.sql.type.DataType;
 import org.elasticsearch.xpack.sql.util.CollectionUtils;
 
 import java.util.ArrayList;
 import java.util.LinkedHashSet;
 import java.util.List;
-import java.util.Locale;
 import java.util.Objects;
 import java.util.StringJoiner;
 import java.util.stream.Collectors;
 
+import static org.elasticsearch.common.logging.LoggerMessageFormat.format;
 import static org.elasticsearch.xpack.sql.expression.gen.script.ParamsBuilder.paramsBuilder;
 
 public class In extends ScalarFunction {
@@ -100,7 +100,7 @@ public ScriptTemplate asScript() {
         List<Object> values = new ArrayList<>(new LinkedHashSet<>(Foldables.valuesOf(list, value.dataType())));
 
         return new ScriptTemplate(
-            formatTemplate(String.format(Locale.ROOT, "{sql}.in(%s, {})", leftScript.template())),
+            formatTemplate(format("{sql}.","in({}, {})", leftScript.template())),
             paramsBuilder()
                 .script(leftScript.params())
                 .variable(values)
diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/ParsingException.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/ParsingException.java
@@ -9,7 +9,7 @@
 import org.elasticsearch.xpack.sql.ClientSqlException;
 import org.elasticsearch.xpack.sql.tree.Source;
 
-import java.util.Locale;
+import static org.elasticsearch.common.logging.LoggerMessageFormat.format;
 
 public class ParsingException extends ClientSqlException {
     private final int line;
@@ -56,6 +56,6 @@ public RestStatus status() {
 
     @Override
     public String getMessage() {
-        return String.format(Locale.ROOT, "line %s:%s: %s", getLineNumber(), getColumnNumber(), getErrorMessage());
+        return format("line {}:{}: {}", getLineNumber(), getColumnNumber(), getErrorMessage());
     }
 }
diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/FoldingException.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/FoldingException.java
@@ -10,7 +10,7 @@
 import org.elasticsearch.xpack.sql.tree.Location;
 import org.elasticsearch.xpack.sql.tree.Node;
 
-import java.util.Locale;
+import static org.elasticsearch.common.logging.LoggerMessageFormat.format;
 
 public class FoldingException extends ClientSqlException {
 
@@ -54,6 +54,6 @@ public RestStatus status() {
 
     @Override
     public String getMessage() {
-        return String.format(Locale.ROOT, "line %s:%s: %s", getLineNumber(), getColumnNumber(), super.getMessage());
+        return format("line {}:{}: {}", getLineNumber(), getColumnNumber(), super.getMessage());
     }
 }
diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/index/IndexResolverTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/index/IndexResolverTests.java
@@ -15,10 +15,11 @@
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
-import java.util.Locale;
 import java.util.Map;
 import java.util.Map.Entry;
 
+import static org.elasticsearch.common.logging.LoggerMessageFormat.format;
+
 public class IndexResolverTests extends ESTestCase {
 
     public void testMergeSameMapping() throws Exception {
@@ -218,14 +219,14 @@ public String[] nonAggregatableIndices() {
 
         @Override
         public String toString() {
-            return String.format(Locale.ROOT, "%s,%s->%s", getName(), getType(), indices);
+            return format("{},{}->{}", getName(), getType(), indices);
         }
     }
 
     private static <K, V> void assertEqualsMaps(Map<K, V> left, Map<K, V> right) {
         for (Entry<K, V> entry : left.entrySet()) {
             V rv = right.get(entry.getKey());
-            assertEquals(String.format(Locale.ROOT, "Key [%s] has different values", entry.getKey()), entry.getValue(), rv);
+            assertEquals(format("Key [{}] has different values", entry.getKey()), entry.getValue(), rv);
         }
     }
     
@@ -235,4 +236,4 @@ private void addFieldCaps(Map<String, Map<String, FieldCapabilities>> fieldCaps,
         cap.put(name, new FieldCapabilities(name, type, isSearchable, isAggregatable));
         fieldCaps.put(name, cap);
     }
-}
+}
diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/QuotingTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/QuotingTests.java
@@ -14,14 +14,14 @@
 
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Locale;
 
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.nullValue;
 
+import static org.elasticsearch.common.logging.LoggerMessageFormat.format;
 
 public class QuotingTests extends ESTestCase {
 
@@ -48,7 +48,7 @@ public void testSingleQuoteLiteral() {
     public void testMultiSingleQuotedLiteral() {
         String first = "bucket";
         String second = "head";
-        Expression exp = new SqlParser().createExpression(String.format(Locale.ROOT, "'%s' '%s'", first, second));
+        Expression exp = new SqlParser().createExpression(format(null, "'{}' '{}'", first, second));
         assertThat(exp, instanceOf(Literal.class));
         Literal l = (Literal) exp;
         assertThat(l.value(), equalTo(first + second));
diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/LikeEscapingParsingTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/LikeEscapingParsingTests.java
@@ -12,19 +12,19 @@
 import org.elasticsearch.xpack.sql.proto.SqlTypedParamValue;
 import org.elasticsearch.xpack.sql.type.DataType;
 
-import java.util.Locale;
-
 import static java.util.Collections.singletonList;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
 
+import static org.elasticsearch.common.logging.LoggerMessageFormat.format;
+
 public class LikeEscapingParsingTests extends ESTestCase {
 
     private final SqlParser parser = new SqlParser();
 
     private String error(String pattern) {
         ParsingException ex = expectThrows(ParsingException.class,
-                () -> parser.createExpression(String.format(Locale.ROOT, "exp LIKE %s", pattern)));
+                () -> parser.createExpression(format(null, "exp LIKE {}", pattern)));
 
         return ex.getMessage();
     }
@@ -35,7 +35,7 @@ private LikePattern like(String pattern) {
         if (parameterized) {
             exp = parser.createExpression("exp LIKE ?", singletonList(new SqlTypedParamValue(DataType.KEYWORD.esType, pattern)));
         } else {
-            exp = parser.createExpression(String.format(Locale.ROOT, "exp LIKE '%s'", pattern));
+            exp = parser.createExpression(format(null, "exp LIKE '{}'", pattern));
         }
         assertThat(exp, instanceOf(Like.class));
         Like l = (Like) exp;

Original file line number	Diff line number	Diff line change
`@@ -10,7 +10,7 @@`
`10`	`10`	`import org.elasticsearch.xpack.sql.tree.Location;`
`11`	`11`	`import org.elasticsearch.xpack.sql.tree.Node;`
`12`	`12`
`13`		`-import java.util.Locale;`
	`13`	`+import static org.elasticsearch.common.logging.LoggerMessageFormat.format;`
`14`	`14`
`15`	`15`	`public class AnalysisException extends ClientSqlException {`
`16`	`16`
`@@ -54,6 +54,6 @@ public RestStatus status() {`
`54`	`54`
`55`	`55`	`@Override`
`56`	`56`	`public String getMessage() {`
`57`		`- return String.format(Locale.ROOT, "line %s:%s: %s", getLineNumber(), getColumnNumber(), super.getMessage());`
	`57`	`+ return format("line {}:{}: {}", getLineNumber(), getColumnNumber(), super.getMessage());`
`58`	`58`	`}`
`59`	`59`	`}`
Original file line number	Diff line number	Diff line change
`@@ -5,7 +5,7 @@`
`5`	`5`	`*/`
`6`	`6`	`package org.elasticsearch.xpack.sql.expression.gen.script;`
`7`	`7`
`8`		`-import java.util.Locale;`
	`8`	`+import static org.elasticsearch.common.logging.LoggerMessageFormat.format;`
`9`	`9`
`10`	`10`	`abstract class Param<T> {`
`11`	`11`	`private final T value;`
`@@ -22,6 +22,6 @@ T value() {`
`22`	`22`
`23`	`23`	`@Override`
`24`	`24`	`public String toString() {`
`25`		`- return String.format(Locale.ROOT, "{%s=%s}", prefix(), value);`
	`25`	`+ return format(null, "{{}={}}", prefix(), value);`
`26`	`26`	`}`
`27`	`27`	`}`
Original file line number	Diff line number	Diff line change
`@@ -9,7 +9,7 @@`
`9`	`9`	`import org.elasticsearch.xpack.sql.ClientSqlException;`
`10`	`10`	`import org.elasticsearch.xpack.sql.tree.Source;`
`11`	`11`
`12`		`-import java.util.Locale;`
	`12`	`+import static org.elasticsearch.common.logging.LoggerMessageFormat.format;`
`13`	`13`
`14`	`14`	`public class ParsingException extends ClientSqlException {`
`15`	`15`	`private final int line;`
`@@ -56,6 +56,6 @@ public RestStatus status() {`
`56`	`56`
`57`	`57`	`@Override`
`58`	`58`	`public String getMessage() {`
`59`		`- return String.format(Locale.ROOT, "line %s:%s: %s", getLineNumber(), getColumnNumber(), getErrorMessage());`
	`59`	`+ return format("line {}:{}: {}", getLineNumber(), getColumnNumber(), getErrorMessage());`
`60`	`60`	`}`
`61`	`61`	`}`
Original file line number	Diff line number	Diff line change
`@@ -15,10 +15,11 @@`
`15`	`15`	`import java.util.ArrayList;`
`16`	`16`	`import java.util.HashMap;`
`17`	`17`	`import java.util.List;`
`18`		`-import java.util.Locale;`
`19`	`18`	`import java.util.Map;`
`20`	`19`	`import java.util.Map.Entry;`
`21`	`20`
	`21`	`+import static org.elasticsearch.common.logging.LoggerMessageFormat.format;`
	`22`	`+`
`22`	`23`	`public class IndexResolverTests extends ESTestCase {`
`23`	`24`
`24`	`25`	`public void testMergeSameMapping() throws Exception {`
`@@ -218,14 +219,14 @@ public String[] nonAggregatableIndices() {`
`218`	`219`
`219`	`220`	`@Override`
`220`	`221`	`public String toString() {`
`221`		`- return String.format(Locale.ROOT, "%s,%s->%s", getName(), getType(), indices);`
	`222`	`+ return format("{},{}->{}", getName(), getType(), indices);`
`222`	`223`	`}`
`223`	`224`	`}`
`224`	`225`
`225`	`226`	`private static <K, V> void assertEqualsMaps(Map<K, V> left, Map<K, V> right) {`
`226`	`227`	`for (Entry<K, V> entry : left.entrySet()) {`
`227`	`228`	`V rv = right.get(entry.getKey());`
`228`		`- assertEquals(String.format(Locale.ROOT, "Key [%s] has different values", entry.getKey()), entry.getValue(), rv);`
	`229`	`+ assertEquals(format("Key [{}] has different values", entry.getKey()), entry.getValue(), rv);`
`229`	`230`	`}`
`230`	`231`	`}`
`231`	`232`
`@@ -235,4 +236,4 @@ private void addFieldCaps(Map<String, Map<String, FieldCapabilities>> fieldCaps,`
`235`	`236`	`cap.put(name, new FieldCapabilities(name, type, isSearchable, isAggregatable));`
`236`	`237`	`fieldCaps.put(name, cap);`
`237`	`238`	`}`
`238`		`-}`
	`239`	`+}`