Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Format Java source files automatically #46745

Merged
merged 22 commits into from
Sep 26, 2019
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
f6176bf
Format Java using google-java-format
pugnascotia Sep 16, 2019
e833d42
Add notes on handling file that do not format
pugnascotia Sep 16, 2019
72274ca
Apply format plugin in the right place
pugnascotia Sep 16, 2019
a5eec34
Some big examples of reformatted code
pugnascotia Sep 16, 2019
1194956
Switch to Spotless for code formatting
pugnascotia Sep 17, 2019
b5d2b34
Changes to make Spotless format consistently
pugnascotia Sep 18, 2019
d85a1d4
Only format projects on an accept list.
pugnascotia Sep 18, 2019
5ef0dc4
Revert formatted outside of buildSrc
pugnascotia Sep 18, 2019
06ab706
Format buildSrc directory
pugnascotia Sep 18, 2019
5c1ee6a
Merge remote-tracking branch 'upstream/master' into auto-format-java-…
pugnascotia Sep 18, 2019
6a32487
Reformat after merge
pugnascotia Sep 18, 2019
4423991
Update checkstyle suppressions
pugnascotia Sep 18, 2019
0c432a5
Add an option to enable paddedCell() from the gradle CLI
pugnascotia Sep 21, 2019
e1d27cd
Format java with with Eclipse JDT instead
pugnascotia Sep 21, 2019
b0d0d6d
Reformat buildSrc/
pugnascotia Sep 21, 2019
7d69c76
More formatter settings tweaks
pugnascotia Sep 23, 2019
8418c15
Revert formatted ahead of merge with master
pugnascotia Sep 25, 2019
82c44b7
Merge remote-tracking branch 'upstream/master' into auto-format-java-…
pugnascotia Sep 25, 2019
5b76280
Leave the formatting project list empty, so we can merge the Gradle c…
pugnascotia Sep 25, 2019
2a2856b
Update CONTRIBUTING.md
pugnascotia Sep 25, 2019
896cbe3
Update CONTRIBUTING.md
pugnascotia Sep 25, 2019
7314c3a
Remove spotless command that the formatter already handles
pugnascotia Sep 26, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 39 additions & 11 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -155,19 +155,47 @@ For Eclipse, go to `Preferences->Java->Installed JREs` and add `-ea` to

### Java Language Formatting Guidelines

Java files in the Elasticsearch codebase are formatted with
[google-java-format](https://github.com/google/google-java-format), which
enforces the [Google Java
Style](https://google.github.io/styleguide/javaguide.html). There are
plugins available for Eclipse, JetBrains IDEs such as IntelliJ IDEA, and a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we link directly to the plugins ?
Alternatively @mark-vieira do you know a way to generate an IDEA configuration that would at least suggests users to install the plugin ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not aware of any. I suspect something like installing a plugin is not something most folks want to happen when importing a project. There are likely security concerns there.

CLI interface. Formatting checks are also integrated into the build - check
the code with:

./gradlew verifyGoogleJavaFormat

Format the code with:

./gradlew googleJavaFormat

Code is formatted with the `--aosp` option (Android Open Source Project),
which indents with 4 spaces instead of 2. There are no other formatting
options available.

These tasks can also be run for specific subprojects.

Please follow these formatting guidelines:

* Java indent is 4 spaces
* Line width is 140 characters
* Lines of code surrounded by `// tag` and `// end` comments are included in the
documentation and should only be 76 characters wide not counting
leading indentation
* The rest is left to Java coding standards
* Disable “auto-format on save” to prevent unnecessary format changes. This makes reviews much harder as it generates unnecessary formatting changes. If your IDE supports formatting only modified chunks that is fine to do.
* Wildcard imports (`import foo.bar.baz.*`) are forbidden and will cause the build to fail. This can be done automatically by your IDE:
* Eclipse: `Preferences->Java->Code Style->Organize Imports`. There are two boxes labeled "`Number of (static )? imports needed for .*`". Set their values to 99999 or some other absurdly high value.
* IntelliJ: `Preferences/Settings->Editor->Code Style->Java->Imports`. There are two configuration options: `Class count to use import with '*'` and `Names count to use static import with '*'`. Set their values to 99999 or some other absurdly high value.
* Don't worry too much about import order. Try not to change it but don't worry about fighting your IDE to stop it from doing so.
* Lines of code surrounded by `// tag` and `// end` comments are included
in the documentation and should only be 76 characters wide not counting
leading indentation
* Wildcard imports (`import foo.bar.baz.*`) are forbidden and will cause
the build to fail. This can be done automatically by your IDE:
* Eclipse: `Preferences->Java->Code Style->Organize Imports`. There are
two boxes labeled "`Number of (static )? imports needed for .*`". Set
their values to 99999 or some other absurdly high value.
* IntelliJ: `Preferences/Settings->Editor->Code Style->Java->Imports`.
There are two configuration options: `Class count to use import with
'*'` and `Names count to use static import with '*'`. Set their values
to 99999 or some other absurdly high value.

If a file compiles successfully but fails to format, the Gradle plugin does
pugnascotia marked this conversation as resolved.
Show resolved Hide resolved
not give much help. Instead, download the `google-java-format` JAR
directly, ensuring to fetch the one with all dependencies included, and
check the files like so:

java -jar path/to/google-java-format-1.7-all-deps.jar --aosp path/to/File.java

### License Headers

Expand Down
14 changes: 14 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ plugins {
id 'com.gradle.build-scan' version '2.4'
id 'lifecycle-base'
id 'elasticsearch.global-build-info'
id 'com.github.sherter.google-java-format' version '0.8'
}

apply plugin: 'nebula.info-scm'
Expand Down Expand Up @@ -98,6 +99,15 @@ subprojects {
plugins.withType(BuildPlugin).whenPluginAdded {
project.licenseFile = project.rootProject.file('licenses/APACHE-LICENSE-2.0.txt')
project.noticeFile = project.rootProject.file('NOTICE.txt')

// Although the plugin is applied at the top-level, that doesn't make
// it available on subprojects.
project.apply plugin: 'com.github.sherter.google-java-format'
check.dependsOn 'verifyGoogleJavaFormat'

googleJavaFormat {
options style: 'AOSP'
}
}
}

Expand Down Expand Up @@ -530,3 +540,7 @@ allprojects {
}
}
}

googleJavaFormat {
options style: 'AOSP'
}

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@

import org.apache.lucene.document.LatLonDocValuesField;
import org.apache.lucene.document.LatLonPoint;
//import org.apache.lucene.geo.Rectangle;
import org.apache.lucene.search.IndexOrDocValuesQuery;
import org.apache.lucene.search.MatchNoDocsQuery;
import org.apache.lucene.search.Query;
Expand Down
22 changes: 11 additions & 11 deletions server/src/main/java/org/elasticsearch/script/ScriptException.java
Original file line number Diff line number Diff line change
@@ -1,14 +1,3 @@
package org.elasticsearch.script;

import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.rest.RestStatus;

/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
Expand All @@ -28,6 +17,17 @@
* under the License.
*/

package org.elasticsearch.script;

import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.rest.RestStatus;

import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
Expand Down
Loading