Skip to content

Commit a5d2251

Browse files
authored
Order imports when reformatting (#74059)
Change the formatter config to sort / order imports, and reformat the codebase. We already had a config file for Eclipse users, so Spotless now uses that. The "Eclipse Code Formatter" plugin ought to be able to use this file as well for import ordering, but in my experiments the results were poor. Instead, use IntelliJ's `.editorconfig` support to configure import ordering. I've also added a config file for the formatter plugin. Other changes: * I've quietly enabled the `toggleOnOff` option for Spotless. It was already possible to disable formatting for sections using the markers for docs snippets, so enabling this option just accepts this reality and makes it possible via `formatter:off` and `formatter:on` without the restrictions around line length. It should still only be used as a very last resort and with good reason. * I've removed mention of the `paddedCell` option from the contributing guide, since I haven't had to use that option for a very long time. I moved the docs to the spotless config.
1 parent 64d3f5f commit a5d2251

File tree

256 files changed

+592
-500
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

256 files changed

+592
-500
lines changed

.editorconfig

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,20 @@ trim_trailing_whitespace = true
88
insert_final_newline = true
99
indent_style = space
1010

11+
# IntelliJ-specific options
12+
ij_java_class_count_to_use_import_on_demand = 999
13+
ij_java_names_count_to_use_import_on_demand = 999
14+
ij_java_imports_layout = *,|,com.**,|,org.**,|,java.**,javax.**,|,$*
15+
1116
[*.gradle]
1217
indent_size = 2
1318

1419
[*.groovy]
1520
indent_size = 4
1621
max_line_length = 140
22+
ij_groovy_class_count_to_use_import_on_demand = 999
23+
ij_groovy_names_count_to_use_import_on_demand = 999
24+
ij_groovy_imports_layout = *,|,com.**,|,org.**,|,java.**,javax.**,|,$*
1725

1826
[*.java]
1927
indent_size = 4

.idea/eclipseCodeFormatter.xml

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

CONTRIBUTING.md

Lines changed: 14 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -181,8 +181,12 @@ action is required.
181181
#### Formatting
182182

183183
We are in the process of migrating towards automatic formatting Java file
184-
using [spotless], backed by the Eclipse formatter. If you have the [Eclipse
185-
Code Formatter] installed, you can apply formatting directly in IntelliJ.
184+
using [spotless], backed by the Eclipse formatter. **We strongly recommend
185+
installing using the [Eclipse Code Formatter] plugin** so that you can
186+
apply the correct formatting directly in IntelliJ. The configuration for
187+
the plugin is held in `.idea/eclipseCodeFormatter.xml` and should be
188+
automatically applied, but manual instructions are below in case you you
189+
need them.
186190

187191
1. Open **Preferences > Other Settings > Eclipse Code Formatter**
188192
2. Click "Use the Eclipse Code Formatter"
@@ -194,7 +198,8 @@ Code Formatter] installed, you can apply formatting directly in IntelliJ.
194198

195199
Note that only some sub-projects in the Elasticsearch project are currently
196200
fully-formatted. You can see a list of project that **are not**
197-
automatically formatted in [gradle/formatting.gradle](gradle/formatting.gradle).
201+
automatically formatted in
202+
[build-tools-internal/src/main/groovy/elasticsearch.formatting.gradle](build-tools-internal/src/main/groovy/elasticsearch.formatting.gradle).
198203

199204
### Importing the project into Eclipse
200205

@@ -270,12 +275,10 @@ form.
270275

271276
### Java Language Formatting Guidelines
272277

273-
Java files in the Elasticsearch codebase are formatted with the Eclipse JDT
274-
formatter, using the [Spotless
275-
Gradle](https://github.com/diffplug/spotless/tree/master/plugin-gradle)
276-
plugin. This plugin is configured on a project-by-project basis, via
277-
`build.gradle` in the root of the repository. The formatting check can be
278-
run explicitly with:
278+
Java files in the Elasticsearch codebase are automatically formatted using
279+
the [Spotless Gradle] plugin. All new projects are automatically formatted,
280+
while existing projects are gradually being opted-in. The formatting check
281+
can be run explicitly with:
279282

280283
./gradlew spotlessJavaCheck
281284

@@ -315,28 +318,12 @@ Please follow these formatting guidelines:
315318

316319
IntelliJ IDEs can
317320
[import](https://blog.jetbrains.com/idea/2014/01/intellij-idea-13-importing-code-formatter-settings-from-eclipse/)
318-
the same settings file, and / or use the [Eclipse Code
319-
Formatter](https://plugins.jetbrains.com/plugin/6546-eclipse-code-formatter)
320-
plugin.
321+
the same settings file, and / or use the [Eclipse Code Formatter] plugin.
321322

322323
You can also tell Spotless to [format a specific
323324
file](https://github.com/diffplug/spotless/tree/master/plugin-gradle#can-i-apply-spotless-to-specific-files)
324325
from the command line.
325326

326-
#### Formatting failures
327-
328-
Sometimes Spotless will report a "misbehaving rule which can't make up its
329-
mind" and will recommend enabling the `paddedCell()` setting. If you
330-
enabled this setting and run the format check again,
331-
Spotless will write files to
332-
`$PROJECT/build/spotless-diagnose-java/` to aid diagnosis. It writes
333-
different copies of the formatted files, so that you can see how they
334-
differ and infer what is the problem.
335-
336-
The `paddedCell()` option is disabled for normal operation so that any
337-
misbehaviour is detected, and not just suppressed. You can enabled the
338-
option from the command line by running Gradle with `-Dspotless.paddedcell`.
339-
340327
### Javadoc
341328

342329
Good Javadoc can help with navigating and understanding code. Elasticsearch
@@ -730,3 +717,4 @@ repeating in this section because it has come up in this context.
730717
[Checkstyle]: https://plugins.jetbrains.com/plugin/1065-checkstyle-idea
731718
[spotless]: https://github.com/diffplug/spotless
732719
[Eclipse Code Formatter]: https://plugins.jetbrains.com/plugin/6546-eclipse-code-formatter
720+
[Spotless Gradle]: https://github.com/diffplug/spotless/tree/master/plugin-gradle

benchmarks/src/main/java/org/elasticsearch/benchmark/search/aggregations/AggConstructionContentionBenchmark.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,12 @@
1414
import org.apache.lucene.search.Query;
1515
import org.elasticsearch.common.breaker.CircuitBreaker;
1616
import org.elasticsearch.common.breaker.PreallocatedCircuitBreakerService;
17-
import org.elasticsearch.core.Releasable;
18-
import org.elasticsearch.core.Releasables;
1917
import org.elasticsearch.common.settings.ClusterSettings;
2018
import org.elasticsearch.common.settings.Settings;
2119
import org.elasticsearch.common.util.BigArrays;
2220
import org.elasticsearch.common.util.PageCacheRecycler;
21+
import org.elasticsearch.core.Releasable;
22+
import org.elasticsearch.core.Releasables;
2323
import org.elasticsearch.index.Index;
2424
import org.elasticsearch.index.IndexSettings;
2525
import org.elasticsearch.index.analysis.NamedAnalyzer;

build-tools-internal/src/main/groovy/elasticsearch.formatting.gradle

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,11 +218,23 @@ subprojects {
218218
target 'src/**/*.java'
219219
}
220220

221+
toggleOffOn('formatter:off', 'formatter:on') // use `formatter:off` and `formatter:on` to toggle formatting - ONLY IF STRICTLY NECESSARY
221222
removeUnusedImports()
223+
importOrderFile rootProject.file('build-tools-internal/elastic.importorder')
222224
eclipse().configFile rootProject.file('build-tools-internal/formatterConfig.xml')
223225
trimTrailingWhitespace()
224226

225-
// See CONTRIBUTING.md for details of when to enabled this.
227+
// Sometimes Spotless will report a "misbehaving rule which can't make up its
228+
// mind" and will recommend enabling the `paddedCell()` setting. If you
229+
// enabled this setting and run the format check again,
230+
// Spotless will write files to
231+
// `$PROJECT/build/spotless-diagnose-java/` to aid diagnosis. It writes
232+
// different copies of the formatted files, so that you can see how they
233+
// differ and infer what is the problem.
234+
235+
// The `paddedCell()` option is disabled for normal operation so that any
236+
// misbehaviour is detected, and not just suppressed. You can enabled the
237+
// option from the command line by running Gradle with `-Dspotless.paddedcell`.
226238
if (System.getProperty('spotless.paddedcell') != null) {
227239
paddedCell()
228240
}

build-tools-internal/src/main/groovy/elasticsearch.ide.gradle

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -104,11 +104,6 @@ if (System.getProperty('idea.active') == 'true') {
104104
taskTriggers {
105105
afterSync tasks.named('configureIdeCheckstyle'), tasks.named('configureIdeaGradleJvm'), tasks.named('buildDependencyArtifacts')
106106
}
107-
codeStyle {
108-
java {
109-
classCountToUseImportOnDemand = 999
110-
}
111-
}
112107
encodings {
113108
encoding = 'UTF-8'
114109
}

distribution/tools/geoip-cli/src/main/java/org/elasticsearch/geoip/GeoIpCli.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,14 @@
1010

1111
import joptsimple.OptionSet;
1212
import joptsimple.OptionSpec;
13+
1314
import org.elasticsearch.cli.Command;
1415
import org.elasticsearch.cli.Terminal;
15-
import org.elasticsearch.core.SuppressForbidden;
1616
import org.elasticsearch.common.hash.MessageDigests;
17-
import org.elasticsearch.core.PathUtils;
1817
import org.elasticsearch.common.xcontent.XContentGenerator;
1918
import org.elasticsearch.common.xcontent.XContentType;
19+
import org.elasticsearch.core.PathUtils;
20+
import org.elasticsearch.core.SuppressForbidden;
2021

2122
import java.io.BufferedInputStream;
2223
import java.io.BufferedOutputStream;

distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,12 @@
1010

1111
import joptsimple.OptionSet;
1212
import joptsimple.OptionSpec;
13+
1314
import org.elasticsearch.cli.ExitCodes;
1415
import org.elasticsearch.cli.Terminal;
1516
import org.elasticsearch.cli.UserException;
16-
import org.elasticsearch.core.SuppressForbidden;
1717
import org.elasticsearch.core.PathUtils;
18+
import org.elasticsearch.core.SuppressForbidden;
1819
import org.elasticsearch.env.Environment;
1920

2021
import java.nio.file.Files;

distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
import joptsimple.OptionSet;
1212
import joptsimple.OptionSpec;
13+
1314
import org.elasticsearch.cli.ExitCodes;
1415
import org.elasticsearch.cli.Terminal;
1516
import org.elasticsearch.cli.UserException;

distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/BaseKeyStoreCommand.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
import joptsimple.OptionSet;
1212
import joptsimple.OptionSpec;
13+
1314
import org.elasticsearch.cli.ExitCodes;
1415
import org.elasticsearch.cli.KeyStoreAwareCommand;
1516
import org.elasticsearch.cli.Terminal;

0 commit comments

Comments
 (0)