From 3c77c50f5f631276c35810746d6ed45719935cf5 Mon Sep 17 00:00:00 2001 From: Rory Hunter Date: Tue, 29 Oct 2019 10:39:34 +0000 Subject: [PATCH] Improve resiliency to auto-formatting in libs, modules (#48619) Backport of #48448. Make a number of changes so that code in the libs and modules directories are more resilient to automatic formatting. This covers: * Remove string concatenation where JSON fits on a single line * Move some comments around to they aren't auto-formatted to a strange place --- .../ConstructingObjectParserTests.java | 179 ++++++------------ .../common/xcontent/ObjectParserTests.java | 55 ++---- .../analysis/common/CommonAnalysisPlugin.java | 4 +- 3 files changed, 77 insertions(+), 161 deletions(-) diff --git a/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ConstructingObjectParserTests.java b/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ConstructingObjectParserTests.java index 5fc3e612c18f7..46b78d695ad67 100644 --- a/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ConstructingObjectParserTests.java +++ b/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ConstructingObjectParserTests.java @@ -93,10 +93,7 @@ public void testRandomOrder() throws Exception { } public void testMissingAllConstructorArgs() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, - "{\n" - + " \"mineral\": 1\n" - + "}"); + XContentParser parser = createParser(JsonXContent.jsonXContent, "{ \"mineral\": 1 }"); ConstructingObjectParser objectParser = randomBoolean() ? HasCtorArguments.PARSER : HasCtorArguments.PARSER_VEGETABLE_OPTIONAL; IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> objectParser.apply(parser, null)); @@ -108,31 +105,20 @@ public void testMissingAllConstructorArgs() throws IOException { } public void testMissingAllConstructorArgsButNotRequired() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, - "{\n" - + " \"mineral\": 1\n" - + "}"); + XContentParser parser = createParser(JsonXContent.jsonXContent, "{ \"mineral\": 1 }"); HasCtorArguments parsed = HasCtorArguments.PARSER_ALL_OPTIONAL.apply(parser, null); assertEquals(1, parsed.mineral); } public void testMissingSecondConstructorArg() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, - "{\n" - + " \"mineral\": 1,\n" - + " \"animal\": \"cat\"\n" - + "}"); + XContentParser parser = createParser(JsonXContent.jsonXContent, "{ \"mineral\": 1, \"animal\": \"cat\" }"); IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> HasCtorArguments.PARSER.apply(parser, null)); assertEquals("Required [vegetable]", e.getMessage()); } public void testMissingSecondConstructorArgButNotRequired() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, - "{\n" - + " \"mineral\": 1,\n" - + " \"animal\": \"cat\"\n" - + "}"); + XContentParser parser = createParser(JsonXContent.jsonXContent, "{ \"mineral\": 1, \"animal\": \"cat\" }"); @SuppressWarnings("unchecked") HasCtorArguments parsed = randomFrom(HasCtorArguments.PARSER_VEGETABLE_OPTIONAL, HasCtorArguments.PARSER_ALL_OPTIONAL).apply(parser, null); @@ -141,11 +127,7 @@ public void testMissingSecondConstructorArgButNotRequired() throws IOException { } public void testMissingFirstConstructorArg() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, - "{\n" - + " \"mineral\": 1,\n" - + " \"vegetable\": 2\n" - + "}"); + XContentParser parser = createParser(JsonXContent.jsonXContent, "{ \"mineral\": 1, \"vegetable\": 2 }"); @SuppressWarnings("unchecked") IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> randomFrom(HasCtorArguments.PARSER, HasCtorArguments.PARSER_VEGETABLE_OPTIONAL).apply(parser, null)); @@ -153,23 +135,19 @@ public void testMissingFirstConstructorArg() throws IOException { } public void testMissingFirstConstructorArgButNotRequired() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, - "{\n" - + " \"mineral\": 1,\n" - + " \"vegetable\": 2\n" - + "}"); + XContentParser parser = createParser(JsonXContent.jsonXContent, "{ \"mineral\": 1, \"vegetable\": 2 }"); HasCtorArguments parsed = HasCtorArguments.PARSER_ALL_OPTIONAL.apply(parser, null); assertEquals(1, parsed.mineral); assertEquals((Integer) 2, parsed.vegetable); } public void testBadParam() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, - "{\n" - + " \"animal\": \"cat\",\n" - + " \"vegetable\": 2,\n" - + " \"a\": \"supercalifragilisticexpialidocious\"\n" - + "}"); + XContentParser parser = createParser( + JsonXContent.jsonXContent, + // The following JSON needs to include newlines, in order to affect the line numbers + // included in the exception + "{\n" + " \"animal\": \"cat\",\n" + " \"vegetable\": 2,\n" + " \"a\": \"supercalifragilisticexpialidocious\"\n" + "}" + ); XContentParseException e = expectThrows(XContentParseException.class, () -> randomFrom(HasCtorArguments.ALL_PARSERS).apply(parser, null)); assertThat(e.getMessage(), containsString("[has_required_arguments] failed to parse field [a]")); @@ -179,12 +157,12 @@ public void testBadParam() throws IOException { } public void testBadParamBeforeObjectBuilt() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, - "{\n" - + " \"a\": \"supercalifragilisticexpialidocious\",\n" - + " \"animal\": \"cat\"\n," - + " \"vegetable\": 2\n" - + "}"); + XContentParser parser = createParser( + JsonXContent.jsonXContent, + // The following JSON needs to include newlines, in order to affect the line numbers + // included in the exception + "{\n" + " \"a\": \"supercalifragilisticexpialidocious\",\n" + " \"animal\": \"cat\"\n," + " \"vegetable\": 2\n" + "}" + ); XContentParseException e = expectThrows(XContentParseException.class, () -> randomFrom(HasCtorArguments.ALL_PARSERS).apply(parser, null)); assertThat(e.getMessage(), containsString("[has_required_arguments] failed to parse field [vegetable]")); @@ -240,40 +218,25 @@ void setFoo(String foo) { parser.declareString(ctorArgOptional ? optionalConstructorArg() : constructorArg(), new ParseField("yeah")); // ctor arg first so we can test for the bug we found one time - XContentParser xcontent = createParser(JsonXContent.jsonXContent, - "{\n" - + " \"yeah\": \"!\",\n" - + " \"foo\": \"foo\"\n" - + "}"); + XContentParser xcontent = createParser(JsonXContent.jsonXContent, "{ \"yeah\": \"!\", \"foo\": \"foo\" }"); CalledOneTime result = parser.apply(xcontent, null); assertTrue(result.fooSet); // and ctor arg second just in case - xcontent = createParser(JsonXContent.jsonXContent, - "{\n" - + " \"foo\": \"foo\",\n" - + " \"yeah\": \"!\"\n" - + "}"); + xcontent = createParser(JsonXContent.jsonXContent, "{ \"foo\": \"foo\", \"yeah\": \"!\" }"); result = parser.apply(xcontent, null); assertTrue(result.fooSet); if (ctorArgOptional) { // and without the constructor arg if we've made it optional - xcontent = createParser(JsonXContent.jsonXContent, - "{\n" - + " \"foo\": \"foo\"\n" - + "}"); + xcontent = createParser(JsonXContent.jsonXContent, "{ \"foo\": \"foo\" }"); result = parser.apply(xcontent, null); } assertTrue(result.fooSet); } public void testIgnoreUnknownFields() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, - "{\n" - + " \"test\" : \"foo\",\n" - + " \"junk\" : 2\n" - + "}"); + XContentParser parser = createParser(JsonXContent.jsonXContent, "{ \"test\" : \"foo\", \"junk\" : 2 }"); class TestStruct { public final String test; TestStruct(String test) { @@ -288,11 +251,7 @@ class TestStruct { } public void testConstructObjectUsingContext() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, - "{\n" - + " \"animal\": \"dropbear\",\n" - + " \"mineral\": -8\n" - + "}"); + XContentParser parser = createParser(JsonXContent.jsonXContent, "{ \"animal\": \"dropbear\", \"mineral\": -8 }"); HasCtorArguments parsed = HasCtorArguments.PARSER_INT_CONTEXT.apply(parser, 42); assertEquals(Integer.valueOf(42), parsed.vegetable); assertEquals("dropbear", parsed.animal); @@ -412,12 +371,10 @@ private static void declareSetters(ConstructingObjectParser } public void testParseNamedObject() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, - "{\"named\": {\n" - + " \"a\": {}" - + "},\"named_in_constructor\": {\n" - + " \"b\": {}" - + "}}"); + XContentParser parser = createParser( + JsonXContent.jsonXContent, + "{\"named\": { \"a\": {} }, \"named_in_constructor\": { \"b\": {} } }" + ); NamedObjectHolder h = NamedObjectHolder.PARSER.apply(parser, null); assertThat(h.named, hasSize(1)); assertEquals("a", h.named.get(0).name); @@ -427,12 +384,10 @@ public void testParseNamedObject() throws IOException { } public void testParseNamedObjectInOrder() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, - "{\"named\": [\n" - + " {\"a\": {}}" - + "],\"named_in_constructor\": [\n" - + " {\"b\": {}}" - + "]}"); + XContentParser parser = createParser( + JsonXContent.jsonXContent, + "{\"named\": [ {\"a\": {}} ], \"named_in_constructor\": [ {\"b\": {}} ]}" + ); NamedObjectHolder h = NamedObjectHolder.PARSER.apply(parser, null); assertThat(h.named, hasSize(1)); assertEquals("a", h.named.get(0).name); @@ -442,12 +397,10 @@ public void testParseNamedObjectInOrder() throws IOException { } public void testParseNamedObjectTwoFieldsInArray() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, - "{\"named\": [\n" - + " {\"a\": {}, \"b\": {}}" - + "],\"named_in_constructor\": [\n" - + " {\"c\": {}}" - + "]}"); + XContentParser parser = createParser( + JsonXContent.jsonXContent, + "{\"named\": [ {\"a\": {}, \"b\": {}}], \"named_in_constructor\": [ {\"c\": {}} ]}" + ); XContentParseException e = expectThrows(XContentParseException.class, () -> NamedObjectHolder.PARSER.apply(parser, null)); assertThat(e.getMessage(), containsString("[named_object_holder] failed to parse field [named]")); assertThat(e.getCause().getMessage(), @@ -456,12 +409,10 @@ public void testParseNamedObjectTwoFieldsInArray() throws IOException { } public void testParseNamedObjectTwoFieldsInArrayConstructorArg() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, - "{\"named\": [\n" - + " {\"a\": {}}" - + "],\"named_in_constructor\": [\n" - + " {\"c\": {}, \"d\": {}}" - + "]}"); + XContentParser parser = createParser( + JsonXContent.jsonXContent, + "{\"named\": [ {\"a\": {}}], \"named_in_constructor\": [ {\"c\": {}, \"d\": {}} ]}" + ); XContentParseException e = expectThrows(XContentParseException.class, () -> NamedObjectHolder.PARSER.apply(parser, null)); assertThat(e.getMessage(), containsString("[named_object_holder] failed to parse field [named_in_constructor]")); assertThat(e.getCause().getMessage(), @@ -470,12 +421,7 @@ public void testParseNamedObjectTwoFieldsInArrayConstructorArg() throws IOExcept } public void testParseNamedObjectNoFieldsInArray() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, - "{\"named\": [\n" - + " {}" - + "],\"named_in_constructor\": [\n" - + " {\"a\": {}}" - + "]}"); + XContentParser parser = createParser(JsonXContent.jsonXContent, "{\"named\": [ {} ], \"named_in_constructor\": [ {\"a\": {}} ]}"); XContentParseException e = expectThrows(XContentParseException.class, () -> NamedObjectHolder.PARSER.apply(parser, null)); assertThat(e.getMessage(), containsString("[named_object_holder] failed to parse field [named]")); assertThat(e.getCause().getMessage(), @@ -484,12 +430,7 @@ public void testParseNamedObjectNoFieldsInArray() throws IOException { } public void testParseNamedObjectNoFieldsInArrayConstructorArg() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, - "{\"named\": [\n" - + " {\"a\": {}}" - + "],\"named_in_constructor\": [\n" - + " {}" - + "]}"); + XContentParser parser = createParser(JsonXContent.jsonXContent, "{\"named\": [ {\"a\": {}} ], \"named_in_constructor\": [ {} ]}"); XContentParseException e = expectThrows(XContentParseException.class, () -> NamedObjectHolder.PARSER.apply(parser, null)); assertThat(e.getMessage(), containsString("[named_object_holder] failed to parse field [named_in_constructor]")); assertThat(e.getCause().getMessage(), @@ -498,12 +439,10 @@ public void testParseNamedObjectNoFieldsInArrayConstructorArg() throws IOExcepti } public void testParseNamedObjectJunkInArray() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, - "{\"named\": [\n" - + " \"junk\"" - + "],\"named_in_constructor\": [\n" - + " {\"a\": {}}" - + "]}"); + XContentParser parser = createParser( + JsonXContent.jsonXContent, + "{\"named\": [ \"junk\" ], \"named_in_constructor\": [ {\"a\": {}} ]}" + ); XContentParseException e = expectThrows(XContentParseException.class, () -> NamedObjectHolder.PARSER.apply(parser, null)); assertThat(e.getMessage(), containsString("[named_object_holder] failed to parse field [named]")); assertThat(e.getCause().getMessage(), @@ -512,12 +451,10 @@ public void testParseNamedObjectJunkInArray() throws IOException { } public void testParseNamedObjectJunkInArrayConstructorArg() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, - "{\"named\": [\n" - + " {\"a\": {}}" - + "],\"named_in_constructor\": [\n" - + " \"junk\"" - + "]}"); + XContentParser parser = createParser( + JsonXContent.jsonXContent, + "{\"named\": [ {\"a\": {}} ], \"named_in_constructor\": [ \"junk\" ]}" + ); XContentParseException e = expectThrows(XContentParseException.class, () -> NamedObjectHolder.PARSER.apply(parser, null)); assertThat(e.getMessage(), containsString("[named_object_holder] failed to parse field [named_in_constructor]")); assertThat(e.getCause().getMessage(), @@ -526,11 +463,10 @@ public void testParseNamedObjectJunkInArrayConstructorArg() throws IOException { } public void testParseNamedObjectInOrderNotSupported() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, - "{\"named\": [\n" - + " {\"a\": {}}" - + "],\"named_in_constructor\": {\"b\": {}}" - + "}"); + XContentParser parser = createParser( + JsonXContent.jsonXContent, + "{\"named\": [\n" + " {\"a\": {}}" + "],\"named_in_constructor\": {\"b\": {}}" + "}" + ); // Create our own parser for this test so we can disable support for the "ordered" mode specified by the array above @SuppressWarnings("unchecked") @@ -547,11 +483,10 @@ public void testParseNamedObjectInOrderNotSupported() throws IOException { } public void testParseNamedObjectInOrderNotSupportedConstructorArg() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, - "{\"named\": {\"a\": {}}" - + ",\"named_in_constructor\": [\n" - + " {\"b\": {}}" - + "]}"); + XContentParser parser = createParser( + JsonXContent.jsonXContent, + "{\"named\": {\"a\": {}}, \"named_in_constructor\": [ {\"b\": {}} ]}" + ); // Create our own parser for this test so we can disable support for the "ordered" mode specified by the array above @SuppressWarnings("unchecked") diff --git a/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java b/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java index a303fb46ec71a..ab9a860aef4d0 100644 --- a/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java +++ b/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java @@ -45,12 +45,10 @@ public class ObjectParserTests extends ESTestCase { public void testBasics() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, - "{\n" - + " \"test\" : \"foo\",\n" - + " \"test_number\" : 2,\n" - + " \"test_array\": [1,2,3,4]\n" - + "}"); + XContentParser parser = createParser( + JsonXContent.jsonXContent, + "{ \"test\" : \"foo\", \"test_number\" : 2, \"test_array\": [1,2,3,4] }" + ); class TestStruct { public String test; int testNumber; @@ -177,8 +175,10 @@ public URI parseURI(XContentParser parser) { } } } - XContentParser parser = createParser(JsonXContent.jsonXContent, - "{\"url\" : { \"host\": \"http://foobar\", \"port\" : 80}, \"name\" : \"foobarbaz\"}"); + XContentParser parser = createParser( + JsonXContent.jsonXContent, + "{\"url\" : { \"host\": \"http://foobar\", \"port\" : 80}, \"name\" : \"foobarbaz\"}" + ); ObjectParser objectParser = new ObjectParser<>("foo"); objectParser.declareString(Foo::setName, new ParseField("name")); objectParser.declareObjectOrDefault(Foo::setURI, (p, s) -> s.parseURI(p), () -> null, new ParseField("url")); @@ -450,10 +450,7 @@ public void setString_or_null(String string_or_null) { } public void testParseNamedObject() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, - "{\"named\": {\n" - + " \"a\": {}" - + "}}"); + XContentParser parser = createParser(JsonXContent.jsonXContent, "{\"named\": { \"a\": {} }}"); NamedObjectHolder h = NamedObjectHolder.PARSER.apply(parser, null); assertThat(h.named, hasSize(1)); assertEquals("a", h.named.get(0).name); @@ -461,10 +458,7 @@ public void testParseNamedObject() throws IOException { } public void testParseNamedObjectInOrder() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, - "{\"named\": [\n" - + " {\"a\": {}}" - + "]}"); + XContentParser parser = createParser(JsonXContent.jsonXContent, "{\"named\": [ {\"a\": {}} ] }"); NamedObjectHolder h = NamedObjectHolder.PARSER.apply(parser, null); assertThat(h.named, hasSize(1)); assertEquals("a", h.named.get(0).name); @@ -472,10 +466,7 @@ public void testParseNamedObjectInOrder() throws IOException { } public void testParseNamedObjectTwoFieldsInArray() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, - "{\"named\": [\n" - + " {\"a\": {}, \"b\": {}}" - + "]}"); + XContentParser parser = createParser(JsonXContent.jsonXContent, "{\"named\": [ {\"a\": {}, \"b\": {}}]}"); XContentParseException e = expectThrows(XContentParseException.class, () -> NamedObjectHolder.PARSER.apply(parser, null)); assertThat(e.getMessage(), containsString("[named_object_holder] failed to parse field [named]")); assertThat(e.getCause().getMessage(), @@ -484,10 +475,7 @@ public void testParseNamedObjectTwoFieldsInArray() throws IOException { } public void testParseNamedObjectNoFieldsInArray() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, - "{\"named\": [\n" - + " {}" - + "]}"); + XContentParser parser = createParser(JsonXContent.jsonXContent, "{\"named\": [ {} ]}"); XContentParseException e = expectThrows(XContentParseException.class, () -> NamedObjectHolder.PARSER.apply(parser, null)); assertThat(e.getMessage(), containsString("[named_object_holder] failed to parse field [named]")); assertThat(e.getCause().getMessage(), @@ -496,10 +484,7 @@ public void testParseNamedObjectNoFieldsInArray() throws IOException { } public void testParseNamedObjectJunkInArray() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, - "{\"named\": [\n" - + " \"junk\"" - + "]}"); + XContentParser parser = createParser(JsonXContent.jsonXContent, "{\"named\": [ \"junk\" ] }"); XContentParseException e = expectThrows(XContentParseException.class, () -> NamedObjectHolder.PARSER.apply(parser, null)); assertThat(e.getMessage(), containsString("[named_object_holder] failed to parse field [named]")); assertThat(e.getCause().getMessage(), @@ -508,10 +493,7 @@ public void testParseNamedObjectJunkInArray() throws IOException { } public void testParseNamedObjectInOrderNotSupported() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, - "{\"named\": [\n" - + " {\"a\": {}}" - + "]}"); + XContentParser parser = createParser(JsonXContent.jsonXContent, "{\"named\": [ {\"a\": {}} ] }"); // Create our own parser for this test so we can disable support for the "ordered" mode specified by the array above ObjectParser objectParser = new ObjectParser<>("named_object_holder", @@ -602,11 +584,10 @@ class TestStruct { } public void testArraysOfGenericValues() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, - "{\n" - + " \"test_array\": [ 1, null, \"3\", 4.2],\n" - + " \"int_array\": [ 1, 2, 3]\n" - + "}"); + XContentParser parser = createParser( + JsonXContent.jsonXContent, + "{ \"test_array\": [ 1, null, \"3\", 4.2], \"int_array\": [ 1, 2, 3] }" + ); class TestStruct { List testArray = new ArrayList<>(); diff --git a/modules/analysis-common/src/main/java/org/elasticsearch/analysis/common/CommonAnalysisPlugin.java b/modules/analysis-common/src/main/java/org/elasticsearch/analysis/common/CommonAnalysisPlugin.java index 94f5de8278f5a..8e29c8d083031 100644 --- a/modules/analysis-common/src/main/java/org/elasticsearch/analysis/common/CommonAnalysisPlugin.java +++ b/modules/analysis-common/src/main/java/org/elasticsearch/analysis/common/CommonAnalysisPlugin.java @@ -445,8 +445,8 @@ public List getPreConfiguredTokenFilters() { filters.add(PreConfiguredTokenFilter.singleton("indic_normalization", true, IndicNormalizationFilter::new)); filters.add(PreConfiguredTokenFilter.singleton("keyword_repeat", false, false, KeywordRepeatFilter::new)); filters.add(PreConfiguredTokenFilter.singleton("kstem", false, KStemFilter::new)); - filters.add(PreConfiguredTokenFilter.singleton("length", false, input -> - new LengthFilter(input, 0, Integer.MAX_VALUE))); // TODO this one seems useless + // TODO this one seems useless + filters.add(PreConfiguredTokenFilter.singleton("length", false, input -> new LengthFilter(input, 0, Integer.MAX_VALUE))); filters.add(PreConfiguredTokenFilter.singleton("limit", false, input -> new LimitTokenCountFilter(input, LimitTokenCountFilterFactory.DEFAULT_MAX_TOKEN_COUNT,