nyquist | 9d61f98 | 2017-02-10 00:29:08 | [diff] [blame] | 1 | # Chromium Java style guide |
| 2 | |
| 3 | _For other languages, please see the [Chromium style |
| 4 | guides](https://chromium.googlesource.com/chromium/src/+/master/styleguide/styleguide.md)._ |
| 5 | |
| 6 | Chromium follows the [Android Open Source style |
| 7 | guide](http://source.android.com/source/code-style.html) unless an exception |
| 8 | is listed below. |
| 9 | |
nyquist | aae4c7c | 2017-02-15 20:41:42 | [diff] [blame] | 10 | You can propose changes to this style guide by sending an email to |
| 11 | `[email protected]`. Ideally, the list will arrive at some consensus and you can |
| 12 | request review for a change to this file. If there's no consensus, |
| 13 | [`//styleguide/java/OWNERS`](https://chromium.googlesource.com/chromium/src/+/master/styleguide/java/OWNERS) |
| 14 | get to decide. |
| 15 | |
agrieve | 0e6bdf2 | 2018-08-03 14:25:24 | [diff] [blame] | 16 | [TOC] |
| 17 | |
| 18 | ## Java 8 Language Features |
| 19 | [Desugar](https://github.com/bazelbuild/bazel/blob/master/src/tools/android/java/com/google/devtools/build/android/desugar/Desugar.java) |
| 20 | is used to rewrite some Java 7 & 8 language constructs in a way that is |
| 21 | compatible with Java 6 (and thus all Android versions). Use of |
| 22 | [these features](https://developer.android.com/studio/write/java8-support) |
| 23 | is encouraged, but there are some gotchas: |
| 24 | |
| 25 | ### Default Interface Methods |
| 26 | * Desugar makes default interface methods work by copy & pasting the default |
| 27 | implementations into all implementing classes. |
| 28 | * This technique is fine for infrequently-used interfaces, but should be |
| 29 | avoided (e.g. via a base class) if it noticeably increases method count. |
| 30 | |
| 31 | ### Lambdas and Method References |
| 32 | * These are syntactic sugar for creating anonymous inner classes. |
| 33 | * Use them only where the cost of an extra class & method definition is |
| 34 | justified. |
| 35 | |
| 36 | ### try-with-resources |
| 37 | * Some library classes do not implement Closeable on older platform APIs. |
| 38 | Runtime exceptions are thrown if you use them with a try-with-resources. |
| 39 | Do not use the following classes in a try-with-resources: |
| 40 | * java.util.zip.ZipFile (implemented in API 19) |
| 41 | * java.net.Socket (implemented in API 19) |
| 42 | |
agrieve | 398286b | 2018-08-15 01:44:45 | [diff] [blame] | 43 | ## Other Language Features & APIs |
| 44 | |
| 45 | ### Exceptions |
| 46 | * As with the Android style guide, we discourage overly broad catches via |
| 47 | `Exception` / `Throwable` / `RuntimeException`. |
| 48 | * If you need to have a broad catch expression, use a comment to explain why. |
| 49 | * Catching multiple exceptions in one line is fine. |
| 50 | |
| 51 | It is OK to do: |
| 52 | ```java |
| 53 | try { |
| 54 | somethingThatThrowsIOException(filePath); |
| 55 | somethingThatThrowsParseException(filePath); |
| 56 | } catch (IOException | ParseException e) { |
| 57 | Log.w(TAG, "Failed to read: %s", filePath, e); |
| 58 | } |
| 59 | ``` |
| 60 | |
| 61 | ### Logging |
| 62 | * Use `org.chromium.base.Log` instead of `android.util.Log`. |
| 63 | * It provides `%s` support, and ensures log stripping works correctly. |
| 64 | * Minimize the use of `Log.w()` and `Log.e()`. |
| 65 | * Debug and Info log levels are stripped by ProGuard in release builds, and |
| 66 | so have no performance impact for shipping builds. However, Warning and |
| 67 | Error log levels are not stripped. |
| 68 | * Function calls in log parameters are *not* stripped by ProGuard. |
| 69 | |
| 70 | ```java |
| 71 | Log.d(TAG, "There are %d cats", countCats()); // countCats() not stripped. |
| 72 | ``` |
| 73 | |
| 74 | ### Asserts |
| 75 | The Chromium build system strips asserts in release builds (via ProGuard) and |
| 76 | enables them in debug builds (or when `dcheck_always_on=true`) (via a [build |
| 77 | step](https://codereview.chromium.org/2517203002)). You should use asserts in |
| 78 | the [same |
| 79 | scenarios](https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md#CHECK_DCHECK_and-NOTREACHED) |
| 80 | where C++ DCHECK()s make sense. For multi-statement asserts, use |
| 81 | `org.chromium.base.BuildConfig.DCHECK_IS_ON` to guard your code. |
| 82 | |
| 83 | Example assert: |
| 84 | |
| 85 | ```java |
| 86 | assert someCallWithoutSideEffects() : "assert description"; |
| 87 | ``` |
| 88 | |
| 89 | Example use of `DCHECK_IS_ON`: |
| 90 | |
| 91 | ```java |
| 92 | if (org.chromium.base.BuildConfig.DCHECK_IS_ON) { |
| 93 | // Any code here will be stripped in Release by ProGuard. |
| 94 | ... |
| 95 | } |
| 96 | ``` |
| 97 | |
| 98 | ### Other Android Support Library Annotations |
| 99 | * Use them! They are [documented here](https://developer.android.com/studio/write/annotations). |
| 100 | * They generally improve readability. |
| 101 | * Some make lint more useful. |
| 102 | * `javax.annotation.Nullable` vs `android.support.annotation.Nullable` |
| 103 | * Always prefer `android.support.annotation.Nullable`. |
| 104 | * It uses `@Retention(SOURCE)` rather than `@Retention(RUNTIME)`. |
| 105 | |
nyquist | aae4c7c | 2017-02-15 20:41:42 | [diff] [blame] | 106 | ## Tools |
| 107 | |
| 108 | ### Automatically formatting edited files |
agrieve | 0e6bdf2 | 2018-08-03 14:25:24 | [diff] [blame] | 109 | A checkout should give you clang-format to automatically format Java code. |
| 110 | It is suggested that Clang's formatting of code should be accepted in code |
| 111 | reviews. |
| 112 | |
nyquist | aae4c7c | 2017-02-15 20:41:42 | [diff] [blame] | 113 | You can run `git cl format` to apply the automatic formatting. |
| 114 | |
agrieve | 398286b | 2018-08-15 01:44:45 | [diff] [blame] | 115 | ### IDE Setup |
nyquist | aae4c7c | 2017-02-15 20:41:42 | [diff] [blame] | 116 | For automatically using the correct style, follow the guide to set up your |
| 117 | favorite IDE: |
| 118 | |
| 119 | * [Android Studio](https://chromium.googlesource.com/chromium/src/+/master/docs/android_studio.md) |
| 120 | * [Eclipse](https://chromium.googlesource.com/chromium/src/+/master/docs/eclipse.md) |
| 121 | |
| 122 | ### Checkstyle |
nyquist | aae4c7c | 2017-02-15 20:41:42 | [diff] [blame] | 123 | Checkstyle is automatically run by the build bots, and to ensure you do not have |
| 124 | any surprises, you can also set up checkstyle locally using [this |
| 125 | guide](https://sites.google.com/a/chromium.org/dev/developers/checkstyle). |
| 126 | |
| 127 | ### Lint |
nyquist | aae4c7c | 2017-02-15 20:41:42 | [diff] [blame] | 128 | Lint is run as part of the build. For more information, see |
| 129 | [here](https://chromium.googlesource.com/chromium/src/+/master/build/android/docs/lint.md). |
| 130 | |
agrieve | 398286b | 2018-08-15 01:44:45 | [diff] [blame] | 131 | ## Style / Formatting |
nyquist | aae4c7c | 2017-02-15 20:41:42 | [diff] [blame] | 132 | |
agrieve | 398286b | 2018-08-15 01:44:45 | [diff] [blame] | 133 | ### File Headers |
| 134 | * Use the same format as in the [C++ style guide](https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md#File-headers). |
nyquist | aae4c7c | 2017-02-15 20:41:42 | [diff] [blame] | 135 | |
agrieve | 398286b | 2018-08-15 01:44:45 | [diff] [blame] | 136 | ### TODOs |
| 137 | * TODO should follow chromium convention. Examples: |
| 138 | * `TODO(username): Some sentence here.` |
| 139 | * `TODO(crbug.com/123456): Even better to use a bug for context.` |
nyquist | aae4c7c | 2017-02-15 20:41:42 | [diff] [blame] | 140 | |
agrieve | 398286b | 2018-08-15 01:44:45 | [diff] [blame] | 141 | ### Code formatting |
nyquist | 9d61f98 | 2017-02-10 00:29:08 | [diff] [blame] | 142 | * Fields should not be explicitly initialized to default values (see |
| 143 | [here](https://groups.google.com/a/chromium.org/d/topic/chromium-dev/ylbLOvLs0bs/discussion)). |
nyquist | aae4c7c | 2017-02-15 20:41:42 | [diff] [blame] | 144 | |
| 145 | ### Curly braces |
nyquist | aae4c7c | 2017-02-15 20:41:42 | [diff] [blame] | 146 | Conditional braces should be used, but are optional if the conditional and the |
| 147 | statement can be on a single line. |
| 148 | |
| 149 | Do: |
| 150 | |
| 151 | ```java |
| 152 | if (someConditional) return false; |
| 153 | for (int i = 0; i < 10; ++i) callThing(i); |
| 154 | ``` |
| 155 | |
| 156 | or |
| 157 | |
| 158 | ```java |
| 159 | if (someConditional) { |
| 160 | return false; |
| 161 | } |
| 162 | ``` |
| 163 | |
| 164 | Do NOT do: |
| 165 | |
| 166 | ```java |
| 167 | if (someConditional) |
| 168 | return false; |
| 169 | ``` |
| 170 | |
nyquist | 2d192c4c | 2017-03-06 21:36:51 | [diff] [blame] | 171 | ### Import Order |
nyquist | 2d192c4c | 2017-03-06 21:36:51 | [diff] [blame] | 172 | * Static imports go before other imports. |
| 173 | * Each import group must be separated by an empty line. |
| 174 | |
| 175 | This is the order of the import groups: |
| 176 | |
| 177 | 1. android |
| 178 | 1. com (except com.google.android.apps.chrome) |
| 179 | 1. dalvik |
| 180 | 1. junit |
| 181 | 1. org |
| 182 | 1. com.google.android.apps.chrome |
| 183 | 1. org.chromium |
| 184 | 1. java |
| 185 | 1. javax |
| 186 | |
| 187 | This is enforced by the |
| 188 | [Chromium Checkstyle configuration](../../tools/android/checkstyle/chromium-style-5.0.xml) |
| 189 | under the ImportOrder module. |
| 190 | |
nyquist | 9d61f98 | 2017-02-10 00:29:08 | [diff] [blame] | 191 | ## Location |
nyquist | 9d61f98 | 2017-02-10 00:29:08 | [diff] [blame] | 192 | "Top level directories" are defined as directories with a GN file, such as |
| 193 | [//base](https://chromium.googlesource.com/chromium/src/+/master/base/) |
| 194 | and |
| 195 | [//content](https://chromium.googlesource.com/chromium/src/+/master/content/), |
| 196 | Chromium Java should live in a directory named |
| 197 | `<top level directory>/android/java`, with a package name |
| 198 | `org.chromium.<top level directory>`. Each top level directory's Java should |
| 199 | build into a distinct JAR that honors the abstraction specified in a native |
| 200 | [checkdeps](https://chromium.googlesource.com/chromium/buildtools/+/master/checkdeps/checkdeps.py) |
| 201 | (e.g. `org.chromium.base` does not import `org.chromium.content`). The full |
| 202 | path of any java file should contain the complete package name. |
| 203 | |
| 204 | For example, top level directory `//base` might contain a file named |
| 205 | `base/android/java/org/chromium/base/Class.java`. This would get compiled into a |
| 206 | `chromium_base.jar` (final JAR name TBD). |
| 207 | |
| 208 | `org.chromium.chrome.browser.foo.Class` would live in |
| 209 | `chrome/android/java/org/chromium/chrome/browser/foo/Class.java`. |
| 210 | |
| 211 | New `<top level directory>/android` directories should have an `OWNERS` file |
| 212 | much like |
| 213 | [//base/android/OWNERS](https://chromium.googlesource.com/chromium/src/+/master/base/android/OWNERS). |
| 214 | |
nyquist | aae4c7c | 2017-02-15 20:41:42 | [diff] [blame] | 215 | ## Miscellany |
nyquist | aae4c7c | 2017-02-15 20:41:42 | [diff] [blame] | 216 | * Use UTF-8 file encodings and LF line endings. |