Skip to content

Commit ad3218b

Browse files
authored
Checkstyle: Catch wide snippets (#34163)
We use wrap code in `// tag` and `//end` to include it in our docs. Our current docs style wraps code snippets in a box that is only wide enough for 76 characters and adds a horizontal scroll bar for wider snippets which makes the snippet much harder to read. This adds a checkstyle check that looks for java code that is included in the docs and is wider than that 76 characters so all snippets fit into the box. It solves many of the failures that this catches but suppresses many more. I will clean those up in a follow up change.
1 parent 2923fb5 commit ad3218b

File tree

7 files changed

+175
-92
lines changed

7 files changed

+175
-92
lines changed

buildSrc/src/main/resources/checkstyle.xml

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,25 @@
1212

1313
<!-- Checks Java files and forbids empty Javadoc comments -->
1414
<module name="RegexpMultiline">
15+
<property name="id" value="EmptyJavadoc"/>
1516
<property name="format" value="\/\*[\s\*]*\*\/"/>
1617
<property name="fileExtensions" value="java"/>
1718
<property name="message" value="Empty javadoc comments are forbidden"/>
1819
</module>
1920

21+
<!--
22+
We include snippets that are wrapped in `// tag` and `// end` into the
23+
docs, stripping the leading spaces. If the context is wider than 76
24+
characters then it'll need to scroll. This fails the build if it sees
25+
such snippets.
26+
-->
27+
<module name="RegexpMultiline">
28+
<property name="id" value="SnippetLength"/>
29+
<property name="format" value="^( *)\/\/\s*tag(.+)\s*\n(.*\n)*\1.{77,}\n(.*\n)*\1\/\/\s*end\2\s*$"/>
30+
<property name="fileExtensions" value="java"/>
31+
<property name="message" value="Code snippets longer than 76 characters get cut off when rendered in the docs"/>
32+
</module>
33+
2034
<module name="TreeWalker">
2135
<!-- Its our official line length! See checkstyle_suppressions.xml for the files that don't pass this. For now we
2236
suppress the check there but enforce it everywhere else. This prevents the list from getting longer even if it is

buildSrc/src/main/resources/checkstyle_suppressions.xml

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,34 @@
2121
configuration of classes that aren't in packages. -->
2222
<suppress files="test[/\\]framework[/\\]src[/\\]test[/\\]java[/\\]Dummy.java" checks="PackageDeclaration" />
2323

24+
<!--
25+
Truly temporary suppressions suppression of snippets included in
26+
documentation that are so wide that they scroll.
27+
-->
28+
<suppress files="client[/\\]rest-high-level[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]client[/\\]documentation[/\\]CRUDDocumentationIT.java" id="SnippetLength" />
29+
<suppress files="client[/\\]rest-high-level[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]client[/\\]documentation[/\\]ClusterClientDocumentationIT.java" id="SnippetLength" />
30+
<suppress files="client[/\\]rest-high-level[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]client[/\\]documentation[/\\]GraphDocumentationIT.java" id="SnippetLength" />
31+
<suppress files="client[/\\]rest-high-level[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]client[/\\]documentation[/\\]IndicesClientDocumentationIT.java" id="SnippetLength" />
32+
<suppress files="client[/\\]rest-high-level[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]client[/\\]documentation[/\\]IngestClientDocumentationIT.java" id="SnippetLength" />
33+
<suppress files="client[/\\]rest-high-level[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]client[/\\]documentation[/\\]LicensingDocumentationIT.java" id="SnippetLength" />
34+
<suppress files="client[/\\]rest-high-level[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]client[/\\]documentation[/\\]MigrationDocumentationIT.java" id="SnippetLength" />
35+
<suppress files="client[/\\]rest-high-level[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]client[/\\]documentation[/\\]MigrationClientDocumentationIT.java" id="SnippetLength" />
36+
<suppress files="client[/\\]rest-high-level[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]client[/\\]documentation[/\\]MiscellaneousDocumentationIT.java" id="SnippetLength" />
37+
<suppress files="client[/\\]rest-high-level[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]client[/\\]documentation[/\\]MlClientDocumentationIT.java" id="SnippetLength" />
38+
<suppress files="client[/\\]rest-high-level[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]client[/\\]documentation[/\\]RollupDocumentationIT.java" id="SnippetLength" />
39+
<suppress files="client[/\\]rest-high-level[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]client[/\\]documentation[/\\]SearchDocumentationIT.java" id="SnippetLength" />
40+
<suppress files="client[/\\]rest-high-level[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]client[/\\]documentation[/\\]SecurityDocumentationIT.java" id="SnippetLength" />
41+
<suppress files="client[/\\]rest-high-level[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]client[/\\]documentation[/\\]SnapshotClientDocumentationIT.java" id="SnippetLength" />
42+
<suppress files="client[/\\]rest-high-level[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]client[/\\]documentation[/\\]StoredScriptsDocumentationIT.java" id="SnippetLength" />
43+
<suppress files="client[/\\]rest-high-level[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]client[/\\]documentation[/\\]TasksClientDocumentationIT.java" id="SnippetLength" />
44+
<suppress files="client[/\\]rest-high-level[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]client[/\\]documentation[/\\]WatcherDocumentationIT.java" id="SnippetLength" />
45+
<!--
46+
This one is in plugins/examples/script-expert-scoring but we need to
47+
suppress it like this because we build that project twice, once in for
48+
real and once as a test for our build system. -->
49+
<suppress files="src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]example[/\\]expertscript[/\\]ExpertScriptPlugin.java" id="SnippetLength" />
50+
<suppress files="modules[/\\]reindex[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]client[/\\]documentation[/\\]ReindexDocumentationIT.jav" id="SnippetLength" />
51+
2452
<!-- Hopefully temporary suppression of LineLength on files that don't pass it. We should remove these when we the
2553
files start to pass. -->
2654
<suppress files="client[/\\]rest[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]client[/\\]HeapBufferedAsyncResponseConsumerTests.java" checks="LineLength" />

client/rest/src/test/java/org/elasticsearch/client/documentation/RestClientDocumentation.java

Lines changed: 93 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import org.elasticsearch.client.ResponseListener;
4646
import org.elasticsearch.client.RestClient;
4747
import org.elasticsearch.client.RestClientBuilder;
48+
import org.elasticsearch.client.RestClientBuilder.HttpClientConfigCallback;
4849

4950
import javax.net.ssl.SSLContext;
5051
import java.io.IOException;
@@ -93,8 +94,8 @@ public void testUsage() throws IOException, InterruptedException {
9394

9495
//tag::rest-client-init
9596
RestClient restClient = RestClient.builder(
96-
new HttpHost("localhost", 9200, "http"),
97-
new HttpHost("localhost", 9201, "http")).build();
97+
new HttpHost("localhost", 9200, "http"),
98+
new HttpHost("localhost", 9201, "http")).build();
9899
//end::rest-client-init
99100

100101
//tag::rest-client-close
@@ -103,26 +104,30 @@ public void testUsage() throws IOException, InterruptedException {
103104

104105
{
105106
//tag::rest-client-init-default-headers
106-
RestClientBuilder builder = RestClient.builder(new HttpHost("localhost", 9200, "http"));
107+
RestClientBuilder builder = RestClient.builder(
108+
new HttpHost("localhost", 9200, "http"));
107109
Header[] defaultHeaders = new Header[]{new BasicHeader("header", "value")};
108110
builder.setDefaultHeaders(defaultHeaders); // <1>
109111
//end::rest-client-init-default-headers
110112
}
111113
{
112114
//tag::rest-client-init-max-retry-timeout
113-
RestClientBuilder builder = RestClient.builder(new HttpHost("localhost", 9200, "http"));
115+
RestClientBuilder builder = RestClient.builder(
116+
new HttpHost("localhost", 9200, "http"));
114117
builder.setMaxRetryTimeoutMillis(10000); // <1>
115118
//end::rest-client-init-max-retry-timeout
116119
}
117120
{
118121
//tag::rest-client-init-node-selector
119-
RestClientBuilder builder = RestClient.builder(new HttpHost("localhost", 9200, "http"));
122+
RestClientBuilder builder = RestClient.builder(
123+
new HttpHost("localhost", 9200, "http"));
120124
builder.setNodeSelector(NodeSelector.SKIP_DEDICATED_MASTERS); // <1>
121125
//end::rest-client-init-node-selector
122126
}
123127
{
124128
//tag::rest-client-init-allocation-aware-selector
125-
RestClientBuilder builder = RestClient.builder(new HttpHost("localhost", 9200, "http"));
129+
RestClientBuilder builder = RestClient.builder(
130+
new HttpHost("localhost", 9200, "http"));
126131
builder.setNodeSelector(new NodeSelector() { // <1>
127132
@Override
128133
public void select(Iterable<Node> nodes) {
@@ -155,7 +160,8 @@ public void select(Iterable<Node> nodes) {
155160
}
156161
{
157162
//tag::rest-client-init-failure-listener
158-
RestClientBuilder builder = RestClient.builder(new HttpHost("localhost", 9200, "http"));
163+
RestClientBuilder builder = RestClient.builder(
164+
new HttpHost("localhost", 9200, "http"));
159165
builder.setFailureListener(new RestClient.FailureListener() {
160166
@Override
161167
public void onFailure(Node node) {
@@ -166,24 +172,30 @@ public void onFailure(Node node) {
166172
}
167173
{
168174
//tag::rest-client-init-request-config-callback
169-
RestClientBuilder builder = RestClient.builder(new HttpHost("localhost", 9200, "http"));
170-
builder.setRequestConfigCallback(new RestClientBuilder.RequestConfigCallback() {
171-
@Override
172-
public RequestConfig.Builder customizeRequestConfig(RequestConfig.Builder requestConfigBuilder) {
173-
return requestConfigBuilder.setSocketTimeout(10000); // <1>
174-
}
175-
});
175+
RestClientBuilder builder = RestClient.builder(
176+
new HttpHost("localhost", 9200, "http"));
177+
builder.setRequestConfigCallback(
178+
new RestClientBuilder.RequestConfigCallback() {
179+
@Override
180+
public RequestConfig.Builder customizeRequestConfig(
181+
RequestConfig.Builder requestConfigBuilder) {
182+
return requestConfigBuilder.setSocketTimeout(10000); // <1>
183+
}
184+
});
176185
//end::rest-client-init-request-config-callback
177186
}
178187
{
179188
//tag::rest-client-init-client-config-callback
180-
RestClientBuilder builder = RestClient.builder(new HttpHost("localhost", 9200, "http"));
181-
builder.setHttpClientConfigCallback(new RestClientBuilder.HttpClientConfigCallback() {
182-
@Override
183-
public HttpAsyncClientBuilder customizeHttpClient(HttpAsyncClientBuilder httpClientBuilder) {
184-
return httpClientBuilder.setProxy(new HttpHost("proxy", 9000, "http")); // <1>
185-
}
186-
});
189+
RestClientBuilder builder = RestClient.builder(
190+
new HttpHost("localhost", 9200, "http"));
191+
builder.setHttpClientConfigCallback(new HttpClientConfigCallback() {
192+
@Override
193+
public HttpAsyncClientBuilder customizeHttpClient(
194+
HttpAsyncClientBuilder httpClientBuilder) {
195+
return httpClientBuilder.setProxy(
196+
new HttpHost("proxy", 9000, "http")); // <1>
197+
}
198+
});
187199
//end::rest-client-init-client-config-callback
188200
}
189201

@@ -281,58 +293,74 @@ public void onFailure(Exception exception) {
281293
public void testCommonConfiguration() throws Exception {
282294
{
283295
//tag::rest-client-config-timeouts
284-
RestClientBuilder builder = RestClient.builder(new HttpHost("localhost", 9200))
285-
.setRequestConfigCallback(new RestClientBuilder.RequestConfigCallback() {
296+
RestClientBuilder builder = RestClient.builder(
297+
new HttpHost("localhost", 9200))
298+
.setRequestConfigCallback(
299+
new RestClientBuilder.RequestConfigCallback() {
286300
@Override
287-
public RequestConfig.Builder customizeRequestConfig(RequestConfig.Builder requestConfigBuilder) {
288-
return requestConfigBuilder.setConnectTimeout(5000)
289-
.setSocketTimeout(60000);
301+
public RequestConfig.Builder customizeRequestConfig(
302+
RequestConfig.Builder requestConfigBuilder) {
303+
return requestConfigBuilder
304+
.setConnectTimeout(5000)
305+
.setSocketTimeout(60000);
290306
}
291307
})
292-
.setMaxRetryTimeoutMillis(60000);
308+
.setMaxRetryTimeoutMillis(60000);
293309
//end::rest-client-config-timeouts
294310
}
295311
{
296312
//tag::rest-client-config-threads
297-
RestClientBuilder builder = RestClient.builder(new HttpHost("localhost", 9200))
298-
.setHttpClientConfigCallback(new RestClientBuilder.HttpClientConfigCallback() {
299-
@Override
300-
public HttpAsyncClientBuilder customizeHttpClient(HttpAsyncClientBuilder httpClientBuilder) {
301-
return httpClientBuilder.setDefaultIOReactorConfig(
302-
IOReactorConfig.custom().setIoThreadCount(1).build());
303-
}
304-
});
313+
RestClientBuilder builder = RestClient.builder(
314+
new HttpHost("localhost", 9200))
315+
.setHttpClientConfigCallback(new HttpClientConfigCallback() {
316+
@Override
317+
public HttpAsyncClientBuilder customizeHttpClient(
318+
HttpAsyncClientBuilder httpClientBuilder) {
319+
return httpClientBuilder.setDefaultIOReactorConfig(
320+
IOReactorConfig.custom()
321+
.setIoThreadCount(1)
322+
.build());
323+
}
324+
});
305325
//end::rest-client-config-threads
306326
}
307327
{
308328
//tag::rest-client-config-basic-auth
309-
final CredentialsProvider credentialsProvider = new BasicCredentialsProvider();
329+
final CredentialsProvider credentialsProvider =
330+
new BasicCredentialsProvider();
310331
credentialsProvider.setCredentials(AuthScope.ANY,
311-
new UsernamePasswordCredentials("user", "password"));
332+
new UsernamePasswordCredentials("user", "password"));
312333

313-
RestClientBuilder builder = RestClient.builder(new HttpHost("localhost", 9200))
314-
.setHttpClientConfigCallback(new RestClientBuilder.HttpClientConfigCallback() {
315-
@Override
316-
public HttpAsyncClientBuilder customizeHttpClient(HttpAsyncClientBuilder httpClientBuilder) {
317-
return httpClientBuilder.setDefaultCredentialsProvider(credentialsProvider);
318-
}
319-
});
334+
RestClientBuilder builder = RestClient.builder(
335+
new HttpHost("localhost", 9200))
336+
.setHttpClientConfigCallback(new HttpClientConfigCallback() {
337+
@Override
338+
public HttpAsyncClientBuilder customizeHttpClient(
339+
HttpAsyncClientBuilder httpClientBuilder) {
340+
return httpClientBuilder
341+
.setDefaultCredentialsProvider(credentialsProvider);
342+
}
343+
});
320344
//end::rest-client-config-basic-auth
321345
}
322346
{
323347
//tag::rest-client-config-disable-preemptive-auth
324-
final CredentialsProvider credentialsProvider = new BasicCredentialsProvider();
348+
final CredentialsProvider credentialsProvider =
349+
new BasicCredentialsProvider();
325350
credentialsProvider.setCredentials(AuthScope.ANY,
326-
new UsernamePasswordCredentials("user", "password"));
351+
new UsernamePasswordCredentials("user", "password"));
327352

328-
RestClientBuilder builder = RestClient.builder(new HttpHost("localhost", 9200))
329-
.setHttpClientConfigCallback(new RestClientBuilder.HttpClientConfigCallback() {
330-
@Override
331-
public HttpAsyncClientBuilder customizeHttpClient(HttpAsyncClientBuilder httpClientBuilder) {
332-
httpClientBuilder.disableAuthCaching(); // <1>
333-
return httpClientBuilder.setDefaultCredentialsProvider(credentialsProvider);
334-
}
335-
});
353+
RestClientBuilder builder = RestClient.builder(
354+
new HttpHost("localhost", 9200))
355+
.setHttpClientConfigCallback(new HttpClientConfigCallback() {
356+
@Override
357+
public HttpAsyncClientBuilder customizeHttpClient(
358+
HttpAsyncClientBuilder httpClientBuilder) {
359+
httpClientBuilder.disableAuthCaching(); // <1>
360+
return httpClientBuilder
361+
.setDefaultCredentialsProvider(credentialsProvider);
362+
}
363+
});
336364
//end::rest-client-config-disable-preemptive-auth
337365
}
338366
{
@@ -343,15 +371,18 @@ public HttpAsyncClientBuilder customizeHttpClient(HttpAsyncClientBuilder httpCli
343371
try (InputStream is = Files.newInputStream(keyStorePath)) {
344372
truststore.load(is, keyStorePass.toCharArray());
345373
}
346-
SSLContextBuilder sslBuilder = SSLContexts.custom().loadTrustMaterial(truststore, null);
374+
SSLContextBuilder sslBuilder = SSLContexts.custom()
375+
.loadTrustMaterial(truststore, null);
347376
final SSLContext sslContext = sslBuilder.build();
348-
RestClientBuilder builder = RestClient.builder(new HttpHost("localhost", 9200, "https"))
349-
.setHttpClientConfigCallback(new RestClientBuilder.HttpClientConfigCallback() {
350-
@Override
351-
public HttpAsyncClientBuilder customizeHttpClient(HttpAsyncClientBuilder httpClientBuilder) {
352-
return httpClientBuilder.setSSLContext(sslContext);
353-
}
354-
});
377+
RestClientBuilder builder = RestClient.builder(
378+
new HttpHost("localhost", 9200, "https"))
379+
.setHttpClientConfigCallback(new HttpClientConfigCallback() {
380+
@Override
381+
public HttpAsyncClientBuilder customizeHttpClient(
382+
HttpAsyncClientBuilder httpClientBuilder) {
383+
return httpClientBuilder.setSSLContext(sslContext);
384+
}
385+
});
355386
//end::rest-client-config-encrypted-communication
356387
}
357388
}

0 commit comments

Comments
 (0)