styleguide/java/java.md
For other languages, please see the Chromium style guides.
Chromium follows the Android Open Source style guide unless an exception is listed below.
You can propose changes to this style guide by sending an email to
[email protected]. Ideally, the list will arrive at some consensus and you can
request review for a change to this file. If there's no consensus,
//styleguide/java/OWNERS
get to decide.
[TOC]
A variable declaration can use the var keyword in place of the type (similar
to the auto keyword in C++). In line with the
guidance for C++,
the var keyword may be used when it aids readability and the type of the value
is already clear (ex. var bundle = new Bundle() is OK, but
var something = returnValueIsNotObvious() may be unclear to readers who are
new to this part of the code).
The var keyword may also be used in try-with-resources when the resource is
not directly accessed (or when it falls under the previous guidance), such as:
try (var ignored = StrictModeContext.allowDiskWrites()) {
// 'var' is permitted so long as the 'ignored' variable is not used directly
// in the code.
}
A quick primer:
Throwable: Base class for all exceptions
Error: Base class for exceptions which are meant to crash the app.Exception: Base class for exceptions that make sense the catch.
RuntimeException: Base class for exceptions that do not need to be
declared as throws ("unchecked exceptions").Use catch statements that do not catch exceptions they are not meant to.
catch (Throwable t), since that includes
the (generally unrecoverable) Error types.Use catch (Exception e) when working with OS APIs that might throw (assuming
the program can recover from them).
IllegalStateException /
IllegalArgumentException / SecurityException being thrown where only
RemoteException was being caught. Unless catch handlers will differ based on
exception type, just catch Exception.Do not use catch (RuntimeException e).
RuntimeException to make unchecked exception types,
but the type does not make much sense in catch clauses, as there are not
times when you'd want to catch all unchecked exceptions, but not also want to
catch all checked exceptions.Avoid adding messages to exceptions that do not aid in debugging. For example:
try {
somethingThatThrowsIOException();
} catch (IOException e) {
// Bad - message does not tell you more than the stack trace does:
throw new RuntimeException("Failed to parse a file.", e);
// Good - conveys that this block failed along with the "caused by" exception.
throw new RuntimeException(e);
// Good - adds useful information.
throw new RuntimeException(String.format("Failed to parse %s", fileName), e);
}
It is common to wrap a checked exception with a RuntimeException for cases where
a checked exception is not recoverable, or not possible. In order to reduce the
number of stack trace "caused by" clauses, and to save on binary size, use
JavaUtils.throwUnchecked() instead.
try {
somethingThatThrowsIOException();
} catch (IOException e) {
// Bad - RuntimeException adds no context and creates longer stack traces.
throw new RuntimeException(e);
// Good - Original exception is preserved.
throw JavaUtils.throwUnchecked(e);
}
*** note Do not use throwUnchecked() when the exception may want to be
caught.
The build system:
// Code for assert expressions & messages is removed when asserts are disabled.
assert someCallWithoutSideEffects(param) : "Call failed with: " + param;
Use your judgement for when to use asserts vs exceptions. Generally speaking, use asserts to check program invariants (e.g. parameter constraints) and exceptions for unrecoverable error conditions (e.g. OS errors). You should tend to use exceptions more in privacy / security-sensitive code.
Do not add checks when the code will crash anyways. E.g.:
// Don't do this.
assert(foo != null);
foo.method(); // This will throw anyways.
For multi-statement asserts, use BuildConfig.ENABLE_ASSERTS to guard your
code (similar to #if DCHECK_IS_ON() in C++). E.g.:
import org.chromium.build.BuildConfig;
...
if (BuildConfig.ENABLE_ASSERTS) {
// Any code here will be stripped in release builds by R8.
...
}
DCHECK and assert are similar, but our guidance for them differs:
This is because as a memory-safe language, logic bugs in Java are much less likely to be exploitable.
Use explicit serialization methods (e.g. toDebugString() or
getDescription()) instead of toString() when dynamic dispatch is not
required.
toString() is unused, so overrides will not be
stripped when unused.// Banned.
record Rectangle(float length, float width) {}
Rationale:
@AutoValue generate equals(), hashCode(), and
toString(), which R8 is unable to remove when unused.record requires build system work (crbug/1493366).Example with equals() and hashCode():
public class ValueClass {
private final SomeClass mObjMember;
private final int mIntMember;
@Override
public boolean equals(Object o) {
return o instanceof ValueClass vc
&& Objects.equals(mObjMember, vc.mObjMember)
&& mIntMember == vc.mIntMember;
}
@Override
public int hashCode() {
return Objects.hash(mObjMember, mIntMember);
}
}
Banned. Use @IntDef / @LongDef / @StringDef instead.
Rationale:
Java enums generate a lot of bytecode and some of their APIs rely on reflection
under-the-hood. They also implement Serializable, which can lead to the
inability to every change them.
Avoid it if possible. Use @Nullable instead.
Optional adds binary size overhead and requires an extra allocation for each
use. It may still be a good choice if you need to distinguish between "null" and
"unset", but prefer null or a sentinel value when possible. The same applies
to OptionalInt, etc.
NullAway warnings will ensure that null checks are not missed.
In line with Google's Java style guide and Android's Java style guide, never
override Object.finalize().
Custom finalizers:
Classes that need destructor logic should provide an explicit destroy()
method. Use
LifetimeAssert
to ensure in debug builds and tests that destroy() is called.
All non-test Java files within the main repository should use @NullMarked.
Tests are encouraged to use them, but there is currently no effort to annotate
existing ones.
See nullaway.md for how to use @Nullable and related annotations.
Android provides the ability to bundle copies of java.* APIs alongside
application code, known as Java Library Desugaring. However, since this
bundling comes with a performance cost, Chrome does not use it. Treat java.*
APIs the same as you would android.* ones and guard them with
Build.VERSION.SDK_INT checks when necessary. The one exception is if the
method is directly backported by D8 (these are okay to use, since they are
lightweight). Android Lint will fail if you try to use an API without a
corresponding Build.VERSION.SDK_INT guard or @RequiresApi annotation.
org.chromium.base.Log instead of android.util.Log.
%s support, and ensures log stripping works correctly.Log.w() and Log.e().
Log.d(TAG, "There are %d cats", countCats()); // countCats() not stripped.
Using Java streams outside of tests is strongly discouraged. If you can write your code as an explicit loop, then do so. The primary reason for this guidance is because the lambdas and method references needed for streams almost always result in larger binary size than their loop equivalents (see crbug.com/344943957 for examples).
The parallel() and parallelStream() APIs are simpler than their loop
equivalents, but are banned due to a lack of a compelling use case in Chrome. If
you find one, please discuss on [email protected].
Use of stream() without a lambda / method reference is allowed. E.g.:
@SuppressWarnings("NoStreams")
private static List<Integer> boxInts(int[] arr) {
return Arrays.stream(arr).boxed().collect(Collectors.toList());
}
@SuppressWarnings("NoStreams")
private static List<String> readLines(BufferedReader bufferedReader) {
return bufferedReader.lines().collect(Collectors.toList());
}
androidx.annotation.Nullable?
Values can be declared outside or inside the @interface. Chromium style is to
declare inside.
@IntDef({ContactsPickerAction.CANCEL, ContactsPickerAction.CONTACTS_SELECTED,
ContactsPickerAction.SELECT_ALL, ContactsPickerAction.UNDO_SELECT_ALL})
@Retention(RetentionPolicy.SOURCE)
public @interface ContactsPickerAction {
int CANCEL = 0;
int CONTACTS_SELECTED = 1;
int SELECT_ALL = 2;
int UNDO_SELECT_ALL = 3;
int NUM_ENTRIES = 4;
}
// ...
void onContactsPickerUserAction(@ContactsPickerAction int action, ...);
Values of Integer type are also supported, which allows using a sentinel
null if needed.
TODO(username): Some sentence here.TODO(crbug.com/40192027): Even better to use a bug for context.Use parameter comments when they aid in the readability of a function call.
E.g.:
someMethod(/* enabled= */ true, /* target= */ null, defaultValue);
Conditional braces should be used, but are optional if the conditional and the statement can be on a single line.
Do:
if (someConditional) return false;
for (int i = 0; i < 10; ++i) callThing(i);
or
if (someConditional) {
return false;
}
Do NOT do:
if (someConditional)
return false;
This is the order of the import groups:
org.chromium.foo.Bar). Only use fully qualified class names inline when
absolutely necessary (e.g. to disambiguate name collisions).import org.chromium.foo.OuterClass.DescriptiveInner; and using
DescriptiveInner over OuterClass.DescriptiveInner.OuterClass.InnerClass) should be reserved for
generically named inner classes, such as Observer, where mentioning the
outer class provides essential context.Googlers, see go/clank-test-strategy.
In summary:
Use real dependencies when feasible and fast. Use Mockito’s @Mock most of
the time, but write fakes for frequently used dependencies.
Do not use Robolectric Shadows for Chromium code.
setFooForTesting() [...] method to make the test contract explicit.
ResettersForTesting.register() from within ForTesting() methods
to ensure that state is reset between tests.Use Robolectric when possible (when tests do not require native). Other times, use on-device tests with one of the following annotations:
@Batch(UNIT_TESTS) for unit tests@Batch(PER_CLASS) for integration tests@DoNotBatch for when each test method requires an app restartFunctions and fields used only for testing should have ForTesting as a suffix
so that:
android-binary-size trybot can ensure they are removed in non-test
optimized builds (by R8).PRESUMBIT.py can ensure no calls are made to such methods outside of
tests, andForTesting methods that are @CalledByNative should use
@CalledByNativeForTesting instead.
Symbols that are made public (or package-private) for the sake of tests should
be annotated with @VisibleForTesting. Android Lint will check that calls
from non-test code respect the "otherwise" visibility.
Symbols with a ForTesting suffix should not be annotated with
@VisibleForTesting. While otherwise=VisibleForTesting.NONE exists, it is
redundant given the "ForTesting" suffix and the associated lint check is
redundant given our trybot check. You should, however, use it for test-only
constructors.
"Top level directories" are defined as directories with a GN file, such as
//base and
//content,
Chromium Java should live in a directory named
<top level directory>/android/java, with a package name
org.chromium.<top level directory>. Each top level directory's Java should
build into a distinct JAR that honors the abstraction specified in a native
checkdeps
(e.g. org.chromium.base does not import org.chromium.content). The full path
of any java file should contain the complete package name.
For example, top level directory //base might contain a file named
base/android/java/org/chromium/base/Class.java. This would get compiled into a
chromium_base.jar (final JAR name TBD).
org.chromium.chrome.browser.foo.Class would live in
chrome/android/java/org/chromium/chrome/browser/foo/Class.java.
New <top level directory>/android directories should have an OWNERS file
much like
//base/android/OWNERS.
google-java-format is used to auto-format Java files. Formatting of its code
should be accepted in code reviews.
You can run git cl format to apply the automatic formatting.
Chromium also makes use of several static analysis tools.