Formatting escape hatch (#81806)

Thanks to https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437,
we've run into a situation where Spotless is incorrectly formatting
a particular piece of syntax (due the underlying Eclipse bug). We
were able to turn off formatting of this syntax using `// @formatter:off`
and `// @formatter:on`, but there was a further problem. We configure
IntelliJ to use the Eclipse formatter plugin, but this doesn't
respect the `@formatter` tags since these are set at the Spotless
level, not the Eclipse formatter level. Note that these tags aren't
set in the Eclipse formatter config, because there we use `// tag::`
and `// end::` in order to avoid reformatting docs snippets, which
have a much narrower line width.

What a mess.

So, to get around all this, drop the `@formatter` tags and tweak
our custom `SnippetLengthCheck` Checkstyle rule so that
`// tag:noformat` regions are not subject to the narrower line length
check, but are still exempt from formatting.

.editorconfig

@@ -10,10 +10,6 @@ trim_trailing_whitespace = true
 insert_final_newline = true
 indent_style = space
 
-ij_formatter_off_tag = @formatter:off
-ij_formatter_on_tag = @formatter:on
-ij_formatter_tags_enabled = false
-
 [*.gradle]
 ij_continuation_indent_size = 2
 indent_size = 2

CONTRIBUTING.md

@@ -299,7 +299,7 @@ Please follow these formatting guidelines:
 * Wildcard imports (`import foo.bar.baz.*`) are forbidden and will cause
   the build to fail.
 * If *absolutely* necessary, you can disable formatting for regions of code
-  with the `// @formatter:off` and `// @formatter:on` directives, but
+  with the `// tag::noformat` and `// end::noformat` directives, but
   only do this where the benefit clearly outweighs the decrease in formatting
   consistency.
 * Note that Javadoc and block comments i.e. `/* ... */` are not formatted,

build-conventions/src/main/java/org/elasticsearch/gradle/internal/checkstyle/SnippetLengthCheck.java

@@ -22,9 +22,12 @@ import java.util.regex.Pattern;
 /**
  * Checks the snippets included in the docs aren't too wide to fit on
  * the page.
+ * <p>
+ * Regions contained in the special <code>noformat</code> tag are exempt from the length
+ * check. This region is also exempt from automatic formatting.
  */
 public class SnippetLengthCheck extends AbstractFileSetCheck {
-    private static final Pattern START = Pattern.compile("^( *)//\\s*tag::(.+?)\\s*$", Pattern.MULTILINE);
+    private static final Pattern START = Pattern.compile("^( *)//\\s*tag::(?!noformat)(.+?)\\s*$", Pattern.MULTILINE);
     private int max;
 
     /**

build-conventions/src/main/java/org/elasticsearch/gradle/internal/conventions/precommit/FormattingPrecommitPlugin.java

@@ -60,9 +60,6 @@ public class FormattingPrecommitPlugin implements Plugin<Project> {
 
                 java.target("src/**/*.java");
 
-                // Use `@formatter:off` and `@formatter:on` to toggle formatting - ONLY IF STRICTLY NECESSARY
-                java.toggleOffOn("@formatter:off", "@formatter:on");
-
                 java.removeUnusedImports();
 
                 // We enforce a standard order for imports

client/rest-high-level/src/main/java/org/elasticsearch/client/graph/Vertex.java

@@ -154,7 +154,7 @@ public class Vertex implements ToXContentFragment {
         this.weight = weight;
     }
 
-    // @formatter:off
+    // tag::noformat
     /**
      * If the {@link GraphExploreRequest#useSignificance(boolean)} is true (the default)
      * this statistic is available.
@@ -163,12 +163,12 @@ public class Vertex implements ToXContentFragment {
      * href="https://www.elastic.co/guide/en/elasticsearch/reference/current/search-aggregations-bucket-significantterms-aggregation.html"
      * >the significant_terms aggregation</a>)
      */
-    // @formatter:on
+    // end::noformat
     public long getBg() {
         return bg;
     }
 
-    // @formatter:off
+    // tag::noformat
     /**
      * If the {@link GraphExploreRequest#useSignificance(boolean)} is true (the default)
      * this statistic is available.
@@ -178,7 +178,7 @@ public class Vertex implements ToXContentFragment {
      * href="https://www.elastic.co/guide/en/elasticsearch/reference/current/search-aggregations-bucket-significantterms-aggregation.html"
      * >the significant_terms aggregation</a>)
      */
-    // @formatter:on
+    // end::noformat
     public long getFg() {
         return fg;
     }

client/rest/src/main/java/org/elasticsearch/client/Response.java

@@ -120,7 +120,7 @@ public class Response {
      * Length of RFC 1123 format (with quotes and leading space), used in
      * matchWarningHeaderPatternByPrefix(String).
      */
-    // @formatter:off
+    // tag::noformat
     private static final int WARNING_HEADER_DATE_LENGTH = 0
             + 1
             + 1
@@ -131,7 +131,7 @@ public class Response {
             + 2 + 1 + 2 + 1 + 2 + 1
             + 3
             + 1;
-    // @formatter:on
+    // end::noformat
 
     /**
      * Tests if a string matches the RFC 7234 specification for warning headers.

server/src/internalClusterTest/java/org/elasticsearch/search/aggregations/pipeline/BucketMetricsPipeLineAggregationTestCase.java

@@ -440,7 +440,7 @@ abstract class BucketMetricsPipeLineAggregationTestCase<T extends NumericMetrics
         // you need to add an additional index with no fields in order to trigger this (or potentially a shard)
         // so that there is an UnmappedTerms in the list to reduce.
         createIndex("foo_1");
-        // @formatter:off
+        // tag::noformat
         XContentBuilder builder = jsonBuilder().startObject()
             .startObject("properties")
               .startObject("@timestamp")
@@ -464,9 +464,9 @@ abstract class BucketMetricsPipeLineAggregationTestCase<T extends NumericMetrics
               .endObject()
             .endObject()
           .endObject();
-        // @formatter:on
+        // end::noformat
         assertAcked(client().admin().indices().prepareCreate("foo_2").setMapping(builder).get());
-        // @formatter:off
+        // tag::noformat
         XContentBuilder docBuilder = jsonBuilder().startObject()
             .startObject("license")
               .field("partnumber", "foobar")
@@ -474,7 +474,7 @@ abstract class BucketMetricsPipeLineAggregationTestCase<T extends NumericMetrics
             .endObject()
             .field("@timestamp", "2018-07-08T08:07:00.599Z")
           .endObject();
-        // @formatter:on
+        // end::noformat
         client().prepareIndex("foo_2").setSource(docBuilder).setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE).get();
 
         client().admin().indices().prepareRefresh();

server/src/main/java/org/elasticsearch/search/aggregations/metrics/AbstractHyperLogLog.java

@@ -21,7 +21,7 @@ public abstract class AbstractHyperLogLog extends AbstractCardinalityAlgorithm {
     private static final int BIAS_K = 6;
 
     // these static tables come from the appendix of the paper
-    // @formatter:off
+    // tag::noformat
     private static final double[][] RAW_ESTIMATE_DATA = {
             // precision 4
             { 11, 11.717, 12.207, 12.7896, 13.2882, 13.8204, 14.3772, 14.9342, 15.5202, 16.161, 16.7722, 17.4636, 18.0396, 18.6766, 19.3566,
@@ -689,7 +689,7 @@ public abstract class AbstractHyperLogLog extends AbstractCardinalityAlgorithm {
                     -404.317000000039, -528.898999999976, -506.621000000043, -513.205000000075, -479.351000000024, -596.139999999898,
                     -527.016999999993, -664.681000000099, -680.306000000099, -704.050000000047, -850.486000000034, -757.43200000003,
                     -713.308999999892, } };
-    // @formatter:on
+    // end::noformat
 
     private static final long[] THRESHOLDS = new long[] {
         10,

x-pack/plugin/core/src/main/java/org/elasticsearch/protocol/xpack/graph/Vertex.java

@@ -168,7 +168,7 @@ public class Vertex implements ToXContentFragment {
         this.weight = weight;
     }
 
-    // @formatter:off
+    // tag::noformat
     /**
      * If the {@link GraphExploreRequest#useSignificance(boolean)} is true (the default)
      * this statistic is available.
@@ -177,12 +177,12 @@ public class Vertex implements ToXContentFragment {
      * href="https://www.elastic.co/guide/en/elasticsearch/reference/current/search-aggregations-bucket-significantterms-aggregation.html"
      * >the significant_terms aggregation</a>)
      */
-    // @formatter:on
+    // end::noformat
     public long getBg() {
         return bg;
     }
 
-    // @formatter:off
+    // tag::noformat
     /**
      * If the {@link GraphExploreRequest#useSignificance(boolean)} is true (the default)
      * this statistic is available.
@@ -192,7 +192,7 @@ public class Vertex implements ToXContentFragment {
      * href="https://www.elastic.co/guide/en/elasticsearch/reference/current/search-aggregations-bucket-significantterms-aggregation.html"
      * >the significant_terms aggregation</a>)
      */
-    // @formatter:on
+    // end::noformat
     public long getFg() {
         return fg;
     }

x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/plugin/TransportEqlSearchAction.java

@@ -306,9 +306,9 @@ public class TransportEqlSearchAction extends HandledTransportAction<EqlSearchRe
 
     private static Exception qualifyException(Exception e, String[] indices, String clusterAlias) {
         Exception finalException = e;
-        // @formatter:off - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
+        // tag::noformat - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
         if (e instanceof RemoteTransportException && e.getCause() instanceof IndexNotFoundException infe) {
-            // @formatter:on
+            // end::noformat
             if (infe.getIndex() != null) {
                 String qualifiedIndex;
                 String exceptionIndexName = infe.getIndex().getName();

x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/DataTypes.java

@@ -20,7 +20,7 @@ import static java.util.stream.Collectors.toUnmodifiableMap;
 
 public final class DataTypes {
 
-    // @formatter:off
+    // tag::noformat
     public static final DataType UNSUPPORTED      = new DataType("UNSUPPORTED", null, 0,                 false, false, false);
 
     public static final DataType NULL             = new DataType("null",              0,                 false, false, false);
@@ -48,7 +48,7 @@ public final class DataTypes {
     // complex types
     public static final DataType OBJECT           = new DataType("object",            0,                 false, false, false);
     public static final DataType NESTED           = new DataType("nested",            0,                 false, false, false);
-    //@formatter:on
+    //end::noformat
 
     private static final Collection<DataType> TYPES = Arrays.asList(
         UNSUPPORTED,

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Analyzer.java

@@ -846,9 +846,9 @@ public class Analyzer extends RuleExecutor<LogicalPlan> {
         @Override
         protected LogicalPlan rule(LogicalPlan plan) {
             if (plan instanceof Project p) {
-                // @formatter:off - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
+                // tag::noformat - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
                 if (p.child() instanceof Filter f) {
-                    // @formatter:on
+                    // end::noformat
                     Expression condition = f.condition();
                     if (condition.resolved() == false && f.childrenResolved()) {
                         Expression newCondition = replaceAliases(condition, p.projections());
@@ -860,9 +860,9 @@ public class Analyzer extends RuleExecutor<LogicalPlan> {
             }
 
             if (plan instanceof Aggregate a) {
-                // @formatter:off - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
+                // tag::noformat - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
                 if (a.child() instanceof Filter f) {
-                    // @formatter:on
+                    // end::noformat
                     Expression condition = f.condition();
                     if (condition.resolved() == false && f.childrenResolved()) {
                         Expression newCondition = replaceAliases(condition, a.aggregates());
@@ -1029,9 +1029,9 @@ public class Analyzer extends RuleExecutor<LogicalPlan> {
 
         @Override
         protected LogicalPlan rule(Filter f) {
-            // @formatter:off - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
+            // tag::noformat - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
             if (f.child() instanceof Project p) {
-                // @formatter:on
+                // end::noformat
                 for (Expression n : p.projections()) {
                     if (n instanceof Alias) {
                         n = ((Alias) n).child();
@@ -1072,9 +1072,9 @@ public class Analyzer extends RuleExecutor<LogicalPlan> {
         @Override
         protected LogicalPlan rule(Filter f) {
             // HAVING = Filter followed by an Agg
-            // @formatter:off - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
+            // tag::noformat - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
             if (f.child() instanceof Aggregate agg && agg.resolved()) {
-                // @formatter:on
+                // end::noformat
                 Set<NamedExpression> missing = null;
                 Expression condition = f.condition();
 
@@ -1248,9 +1248,9 @@ public class Analyzer extends RuleExecutor<LogicalPlan> {
 
         @Override
         protected LogicalPlan rule(UnaryPlan plan) {
-            // @formatter:off - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
+            // tag::noformat - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
             if (plan.child() instanceof SubQueryAlias a) {
-                // @formatter:on
+                // end::noformat
                 return plan.transformExpressionsDown(FieldAttribute.class, f -> {
                     if (f.qualifier() != null && f.qualifier().equals(a.alias())) {
                         // Find the underlying concrete relation (EsIndex) and its name as the new qualifier

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Verifier.java

@@ -396,9 +396,9 @@ public final class Verifier {
         AttributeMap<Expression> attributeRefs
     ) {
         if (p instanceof Having h) {
-            // @formatter:off - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
+            // tag::noformat - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
             if (h.child() instanceof Aggregate a) {
-                // @formatter:on
+                // end::noformat
                 Set<Expression> missing = new LinkedHashSet<>();
                 Set<Expression> unsupported = new LinkedHashSet<>();
                 Expression condition = h.condition();

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/optimizer/Optimizer.java

@@ -324,9 +324,9 @@ public class Optimizer extends RuleExecutor<LogicalPlan> {
         @Override
         protected LogicalPlan rule(Project project) {
             // check whether OrderBy relies on nested fields which are not used higher up
-            // @formatter:off - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
+            // tag::noformat - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
             if (project.child() instanceof OrderBy ob) {
-                // @formatter:on
+                // end::noformat
                 // resolve function references (that maybe hiding the target)
                 AttributeMap.Builder<Function> collectRefs = AttributeMap.builder();
 
@@ -1036,23 +1036,23 @@ public class Optimizer extends RuleExecutor<LogicalPlan> {
 
             // count the extended stats
             p.forEachExpressionUp(InnerAggregate.class, ia -> {
-                // @formatter:off - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
+                // tag::noformat - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
                 if (ia.outer() instanceof ExtendedStats extStats) {
                     seen.putIfAbsent(extStats.field(), extStats);
                 }
-                // @formatter:on
+                // end::noformat
             });
 
             // then if there's a match, replace the stat inside the InnerAgg
             return p.transformExpressionsUp(InnerAggregate.class, ia -> {
-                // @formatter:off - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
+                // tag::noformat - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
                 if (ia.outer() instanceof Stats stats) {
                     ExtendedStats ext = seen.get(stats.field());
                     if (ext != null && stats.field().equals(ext.field())) {
                         return new InnerAggregate(ia.inner(), ext);
                     }
                 }
-                // @formatter:on
+                // end::noformat
                 return ia;
             });
         }
@@ -1187,7 +1187,7 @@ public class Optimizer extends RuleExecutor<LogicalPlan> {
 
         @Override
         protected LogicalPlan rule(UnaryPlan plan) {
-            // @formatter:off - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
+            // tag::noformat - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
             if ((plan instanceof Project || plan instanceof Aggregate) && plan.child() instanceof LocalRelation relation) {
                 List<Object> foldedValues = null;
 
@@ -1198,7 +1198,7 @@ public class Optimizer extends RuleExecutor<LogicalPlan> {
                         foldedValues = extractLiterals(((Project) plan).projections());
                     }
                 } else if (relation.executable() instanceof EmptyExecutable) {
-                    // @formatter:on
+                    // end::noformat
                     if (plan instanceof Aggregate agg) {
                         if (agg.groupings().isEmpty()) {
                             // Implicit groupings on empty relations must produce a singleton result set with the aggregation results

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryFolder.java

@@ -136,9 +136,9 @@ class QueryFolder extends RuleExecutor<PhysicalPlan> {
 
         @Override
         protected PhysicalPlan rule(ProjectExec project) {
-            // @formatter:off - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
+            // tag::noformat - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
             if (project.child() instanceof EsQueryExec exec) {
-                // @formatter:on
+                // end::noformat
                 QueryContainer queryC = exec.queryContainer();
 
                 AttributeMap.Builder<Expression> aliases = AttributeMap.<Expression>builder().putAll(queryC.aliases());
@@ -203,9 +203,9 @@ class QueryFolder extends RuleExecutor<PhysicalPlan> {
     private static class FoldFilter extends FoldingRule<FilterExec> {
         @Override
         protected PhysicalPlan rule(FilterExec plan) {
-            // @formatter:off - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
+            // tag::noformat - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
             if (plan.child() instanceof EsQueryExec exec) {
-                // @formatter:on
+                // end::noformat
                 QueryContainer qContainer = exec.queryContainer();
 
                 QueryTranslation qt = toQuery(plan.condition(), plan.isHaving());
@@ -434,11 +434,11 @@ class QueryFolder extends RuleExecutor<PhysicalPlan> {
 
         @Override
         protected PhysicalPlan rule(AggregateExec a) {
-            // @formatter:off - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
+            // tag::noformat - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
             if (a.child() instanceof EsQueryExec exec) {
                 return fold(a, exec);
             }
-            // @formatter:on
+            // end::noformat
             return a;
         }
 
@@ -751,9 +751,9 @@ class QueryFolder extends RuleExecutor<PhysicalPlan> {
     private static class FoldOrderBy extends FoldingRule<OrderExec> {
         @Override
         protected PhysicalPlan rule(OrderExec plan) {
-            // @formatter:off - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
+            // tag::noformat - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
             if (plan.child() instanceof EsQueryExec exec) {
-                // @formatter:on
+                // end::noformat
                 QueryContainer qContainer = exec.queryContainer();
 
                 // Reverse traversal together with the upwards fold direction ensures that sort clauses are added in reverse order of
@@ -824,14 +824,14 @@ class QueryFolder extends RuleExecutor<PhysicalPlan> {
 
         @Override
         protected PhysicalPlan rule(LimitExec plan) {
-            // @formatter:off - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
+            // tag::noformat - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
             if (plan.child() instanceof EsQueryExec exec) {
                 int limit = (Integer) SqlDataTypeConverter.convert(Foldables.valueOf(plan.limit()), DataTypes.INTEGER);
                 int currentSize = exec.queryContainer().limit();
                 int newSize = currentSize < 0 ? limit : Math.min(currentSize, limit);
                 return exec.with(exec.queryContainer().withLimit(newSize));
             }
-            // @formatter:on
+            // end::noformat
             return plan;
         }
     }
@@ -859,9 +859,9 @@ class QueryFolder extends RuleExecutor<PhysicalPlan> {
 
         @Override
         protected PhysicalPlan rule(PivotExec plan) {
-            // @formatter:off - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
+            // tag::noformat - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574437
             if (plan.child() instanceof EsQueryExec exec) {
-                // @formatter:on
+                // end::noformat
                 Pivot p = plan.pivot();
                 EsQueryExec fold = FoldAggregate.fold(
                     new AggregateExec(plan.source(), exec, new ArrayList<>(p.groupingSet()), combine(p.groupingSet(), p.aggregates())),

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/SqlDataTypes.java

@@ -52,7 +52,7 @@ import static org.elasticsearch.xpack.ql.util.CollectionUtils.mapSize;
 
 public class SqlDataTypes {
 
-    // @formatter:off
+    // tag::noformat
     // date-only, time-only
     public static final DataType DATE = new DataType("DATE", null, Long.BYTES, false, false, true);
     public static final DataType TIME = new DataType("TIME", null, Long.BYTES, false, false, true);
@@ -87,7 +87,7 @@ public class SqlDataTypes {
     public static final DataType GEO_SHAPE = new DataType("geo_shape", Integer.MAX_VALUE, false, false, false);
     public static final DataType GEO_POINT = new DataType("geo_point", Double.BYTES * 2,  false, false, false);
     public static final DataType SHAPE =     new DataType("shape",     Integer.MAX_VALUE, false, false, false);
-    // @formatter:on
+    // end::noformat
 
     private static final Map<String, DataType> ODBC_TO_ES = new HashMap<>(mapSize(38));