Chromium Code Reviews| OLD | NEW |
|---|---|
| 1 # Chromium Java style guide | 1 # Chromium Java style guide |
| 2 | 2 |
| 3 _For other languages, please see the [Chromium style | 3 _For other languages, please see the [Chromium style |
| 4 guides](https://chromium.googlesource.com/chromium/src/+/master/styleguide/style guide.md)._ | 4 guides](https://chromium.googlesource.com/chromium/src/+/master/styleguide/style guide.md)._ |
| 5 | 5 |
| 6 Chromium follows the [Android Open Source style | 6 Chromium follows the [Android Open Source style |
| 7 guide](http://source.android.com/source/code-style.html) unless an exception | 7 guide](http://source.android.com/source/code-style.html) unless an exception |
| 8 is listed below. | 8 is listed below. |
| 9 | 9 |
| 10 ## Style | 10 A checkout should give you clang-format to automatically format Java code. |
| 11 It is suggested that Clang's formatting of code should be accepted in code | |
| 12 reviews. | |
| 11 | 13 |
| 12 * Copyright header should use | 14 You can propose changes to this style guide by sending an email to |
| 13 [Chromium](https://chromium.googlesource.com/chromium/src/+/master/styleguide/ styleguide.md) | 15 `java@chromium.org`. Ideally, the list will arrive at some consensus and you can |
| 14 style. | 16 request review for a change to this file. If there's no consensus, |
| 15 * TODO should follow chromium convention i.e. `TODO(username)`. | 17 [`//styleguide/java/OWNERS`](https://chromium.googlesource.com/chromium/src/+/ma ster/styleguide/java/OWNERS) |
| 16 * Use of ```assert``` statements are encouraged. | 18 get to decide. |
| 19 | |
| 20 ## Tools | |
| 21 | |
| 22 ### Automatically formatting edited files | |
| 23 | |
| 24 You can run `git cl format` to apply the automatic formatting. | |
| 25 | |
| 26 ### IDE setup | |
| 27 | |
| 28 For automatically using the correct style, follow the guide to set up your | |
| 29 favorite IDE: | |
| 30 | |
| 31 * [Android Studio](https://chromium.googlesource.com/chromium/src/+/master/docs/ android_studio.md) | |
| 32 * [Eclipse](https://chromium.googlesource.com/chromium/src/+/master/docs/eclipse .md) | |
| 33 | |
| 34 ### Checkstyle | |
| 35 | |
| 36 Checkstyle is automatically run by the build bots, and to ensure you do not have | |
| 37 any surprises, you can also set up checkstyle locally using [this | |
| 38 guide](https://sites.google.com/a/chromium.org/dev/developers/checkstyle). | |
| 39 | |
| 40 ### Lint | |
| 41 | |
| 42 Lint is run as part of the build. For more information, see | |
| 43 [here](https://chromium.googlesource.com/chromium/src/+/master/build/android/doc s/lint.md). | |
| 44 | |
| 45 ## File Headers | |
| 46 | |
| 47 Use the same format as in the [C++ style guide](https://chromium.googlesource.co m/chromium/src/+/master/styleguide/c++/c++.md#File-headers). | |
| 48 | |
| 49 ## TODOs | |
| 50 | |
| 51 TODO should follow chromium convention i.e. `TODO(username)`. | |
| 52 | |
| 53 ## Code formatting | |
| 54 | |
| 17 * Fields should not be explicitly initialized to default values (see | 55 * Fields should not be explicitly initialized to default values (see |
| 18 [here](https://groups.google.com/a/chromium.org/d/topic/chromium-dev/ylbLOvLs0 bs/discussion)). | 56 [here](https://groups.google.com/a/chromium.org/d/topic/chromium-dev/ylbLOvLs0 bs/discussion)). |
| 19 * For automated style checking install | |
| 20 [checkstyle](https://sites.google.com/a/chromium.org/dev/developers/checkstyle ). | |
| 21 | 57 |
| 22 ## Location | 58 ### Curly braces |
| 23 | 59 |
| 24 "Top level directories" are defined as directories with a GN file, such as | 60 Conditional braces should be used, but are optional if the conditional and the |
| 25 [//base](https://chromium.googlesource.com/chromium/src/+/master/base/) | 61 statement can be on a single line. |
| 26 and | |
| 27 [//content](https://chromium.googlesource.com/chromium/src/+/master/content/), | |
| 28 Chromium Java should live in a directory named | |
| 29 `<top level directory>/android/java`, with a package name | |
| 30 `org.chromium.<top level directory>`. Each top level directory's Java should | |
| 31 build into a distinct JAR that honors the abstraction specified in a native | |
| 32 [checkdeps](https://chromium.googlesource.com/chromium/buildtools/+/master/check deps/checkdeps.py) | |
| 33 (e.g. `org.chromium.base` does not import `org.chromium.content`). The full | |
| 34 path of any java file should contain the complete package name. | |
| 35 | 62 |
| 36 For example, top level directory `//base` might contain a file named | 63 Do: |
| 37 `base/android/java/org/chromium/base/Class.java`. This would get compiled into a | |
| 38 `chromium_base.jar` (final JAR name TBD). | |
| 39 | 64 |
| 40 `org.chromium.chrome.browser.foo.Class` would live in | 65 ```java |
| 41 `chrome/android/java/org/chromium/chrome/browser/foo/Class.java`. | 66 if (someConditional) return false; |
| 67 for (int i = 0; i < 10; ++i) callThing(i); | |
| 68 ``` | |
| 42 | 69 |
| 43 New `<top level directory>/android` directories should have an `OWNERS` file | 70 or |
| 44 much like | |
| 45 [//base/android/OWNERS](https://chromium.googlesource.com/chromium/src/+/master/ base/android/OWNERS). | |
| 46 | 71 |
| 47 ## Asserts | 72 ```java |
| 73 if (someConditional) { | |
| 74 return false; | |
| 75 } | |
| 76 ``` | |
| 77 | |
| 78 Do NOT do: | |
| 79 | |
| 80 ```java | |
| 81 if (someConditional) | |
| 82 return false; | |
| 83 ``` | |
| 84 | |
| 85 ### Exceptions | |
| 86 | |
| 87 Similarly to the Android style guide, we discourage the use of | |
| 88 `catch Exception`. It is also allowed to catch multiple exceptions in one line. | |
| 89 | |
| 90 It is OK to do: | |
| 91 | |
| 92 ```java | |
| 93 try { | |
| 94 somethingThatThrowsIOException(); | |
| 95 somethingThatThrowsParseException(); | |
| 96 } catch (IOException | ParseException e) { | |
| 97 Log.e(TAG, "Failed to do something with exception: ", e) | |
| 98 } | |
| 99 ``` | |
| 100 | |
| 101 ### Asserts | |
| 48 | 102 |
| 49 The Chromium build system strips asserts in release builds (via ProGuard) and | 103 The Chromium build system strips asserts in release builds (via ProGuard) and |
| 50 enables them in debug builds (or when `dcheck_always_on=true`) (via a [build | 104 enables them in debug builds (or when `dcheck_always_on=true`) (via a [build |
| 51 step](https://codereview.chromium.org/2517203002)). You should use asserts in | 105 step](https://codereview.chromium.org/2517203002)). You should use asserts in |
| 52 the [same | 106 the [same |
| 53 scenarios](https://chromium.googlesource.com/chromium/src/+/master/styleguide/c+ +/c++.md#CHECK_DCHECK_and-NOTREACHED) | 107 scenarios](https://chromium.googlesource.com/chromium/src/+/master/styleguide/c+ +/c++.md#CHECK_DCHECK_and-NOTREACHED) |
| 54 where C++ DCHECK()s make sense. For multi-statement asserts, use | 108 where C++ DCHECK()s make sense. For multi-statement asserts, use |
| 55 `org.chromium.base.BuildConfig.DCHECK_IS_ON` to guard your code. | 109 `org.chromium.base.BuildConfig.DCHECK_IS_ON` to guard your code. |
| 56 | 110 |
| 57 Example assert: | 111 Example assert: |
| 58 | 112 |
| 59 ```java | 113 ```java |
| 60 assert someCallWithSideEffects() : "assert description"; | 114 assert someCallWithSideEffects() : "assert description"; |
| 61 ``` | 115 ``` |
| 62 | 116 |
| 63 Example use of `DCHECK_IS_ON`: | 117 Example use of `DCHECK_IS_ON`: |
| 64 | 118 |
| 65 ```java | 119 ```java |
| 66 if (org.chromium.base.BuildConfig.DCHECK_IS_ON) { | 120 if (org.chromium.base.BuildConfig.DCHECK_IS_ON) { |
| 67 if (!someCallWithSideEffects()) { | 121 if (!someCallWithSideEffects()) { |
| 68 throw new AssertionError("assert description"); | 122 throw new AssertionError("assert description"); |
| 69 } | 123 } |
| 70 } | 124 } |
| 71 ``` | 125 ``` |
| 126 | |
| 127 ## Location | |
|
nyquist
2017/02/08 06:02:31
Location-section is unedited, but moved lower down
| |
| 128 | |
| 129 "Top level directories" are defined as directories with a GN file, such as | |
| 130 [//base](https://chromium.googlesource.com/chromium/src/+/master/base/) | |
| 131 and | |
| 132 [//content](https://chromium.googlesource.com/chromium/src/+/master/content/), | |
| 133 Chromium Java should live in a directory named | |
| 134 `<top level directory>/android/java`, with a package name | |
| 135 `org.chromium.<top level directory>`. Each top level directory's Java should | |
| 136 build into a distinct JAR that honors the abstraction specified in a native | |
| 137 [checkdeps](https://chromium.googlesource.com/chromium/buildtools/+/master/check deps/checkdeps.py) | |
| 138 (e.g. `org.chromium.base` does not import `org.chromium.content`). The full | |
| 139 path of any java file should contain the complete package name. | |
| 140 | |
| 141 For example, top level directory `//base` might contain a file named | |
| 142 `base/android/java/org/chromium/base/Class.java`. This would get compiled into a | |
| 143 `chromium_base.jar` (final JAR name TBD). | |
| 144 | |
| 145 `org.chromium.chrome.browser.foo.Class` would live in | |
| 146 `chrome/android/java/org/chromium/chrome/browser/foo/Class.java`. | |
| 147 | |
| 148 New `<top level directory>/android` directories should have an `OWNERS` file | |
| 149 much like | |
| 150 [//base/android/OWNERS](https://chromium.googlesource.com/chromium/src/+/master/ base/android/OWNERS). | |
| 151 | |
| 152 ## Miscellany | |
| 153 | |
| 154 * Use UTF-8 file encodings and LF line endings. | |
| OLD | NEW |