clang-format should sort java imports |
||||||||
Issue descriptionAlthough clang-format can sort #includes, and javascript imports, it doesn't yet have support for java imports. Java import order is documented in the style guide here: https://chromium.googlesource.com/chromium/src/+/master/styleguide/java/java.md#import-order
,
Sep 26 2017
https://reviews.llvm.org/D38294 is a hack that makes clang-format's #include sorting work for import lines too. It behaves like the #include sorting, which is fairly different from https://chromium.googlesource.com/chromium/src/+/master/styleguide/java/java.md#import-order : * clang-format only reorders lines within an import block (which is a newline-separated list of imports) * within an import block, it puts `import static` lines at the top * it then sorts alphabetically * users can opt to use the IncludeCategories style option to order entries within a block by some system (e.g. the "android, then com, then dalvik, then junit, then org, then com.google, then org.chromium, then java, then javax" -- but this is almost but not quite alphabetical -- why don't we just sort alphabetically?) While consistency with the c++ behavior is kind of nice, I'm assuming that this behavior is not what we want, and I should implement dedicated java import sorting that sorts across import blocks, inserts newlines as desired, etc. Is that correct?
,
Sep 26 2017
Sorry about the weird sorting. It's mostly based off of https://source.android.com/source/code-style#order-import-statements which is our "parent" style guide for Java. I don't think we want to keep consistency with C++ here. Sorry about that. So yeah, it would be greatly appreciated if you added dedicated Java import sorting like you specified. If you for some reason want to try and see whether clang-format creates the same format as what the IDE of the people writing Java produces, you can follow the guidelines here to get the import scheme from https://chromium.googlesource.com/chromium/src/+/7ea184854a516e5352ef11cd482eec783b0aaa52/tools/android/android_studio/ChromiumStyle.xml#18 : https://chromium.googlesource.com/chromium/src/+/master/docs/android_studio.md#android-studio-tips
,
Oct 26 2017
,
Aug 1
,
Oct 2
I've uploaded a patch here: https://reviews.llvm.org/D52800 I'm seeing "context not available" for the files - not sure what to do there. I believe this will meet all our requirements for Java import formatting. Hopefully someone with clang knowledge (thakis@?) can help me with getting it reviewed/submitted.
,
Oct 10
Looks like that landed upstream in https://reviews.llvm.org/rC343862 , so it's "only" blocked on a clang-format roll.
,
Oct 10
Yep, I've got the clang-format roll on the way. All 3 binaries are built, review should be up today.
,
Oct 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d049675203babf675a40e2cb583ae61211057e68 commit d049675203babf675a40e2cb583ae61211057e68 Author: Sam Maier <smaier@chromium.org> Date: Wed Oct 10 17:35:35 2018 Java: Fixing all imports with clang-format change This is a test run with the clang-format Java import reorderer. TBR=mkearney@chromium.org let me know if the docs ordering is intentional No-Presubmit: true Bug: 768983 Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;master.tryserver.chromium.android:android_cronet_tester Change-Id: Ic2bce530e5e806ca2de6c0ad266f2e5ae38dba82 Reviewed-on: https://chromium-review.googlesource.com/c/1262969 Commit-Queue: Sam Maier <smaier@chromium.org> Reviewed-by: agrieve <agrieve@chromium.org> Reviewed-by: Yaron Friedman <yfriedman@chromium.org> Reviewed-by: Misha Efimov <mef@chromium.org> Reviewed-by: Theresa <twellington@chromium.org> Cr-Commit-Position: refs/heads/master@{#598388} [modify] https://crrev.com/d049675203babf675a40e2cb583ae61211057e68/chrome/android/java/src/org/chromium/chrome/browser/engagement/SiteEngagementService.java [modify] https://crrev.com/d049675203babf675a40e2cb583ae61211057e68/chrome/android/javatests/src/org/chromium/chrome/browser/contextmenu/TabularContextMenuUiTest.java [modify] https://crrev.com/d049675203babf675a40e2cb583ae61211057e68/chrome/android/javatests/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchBackgroundTaskTest.java [modify] https://crrev.com/d049675203babf675a40e2cb583ae61211057e68/chrome/android/junit/src/org/chromium/chrome/browser/DelayedScreenLockIntentHandlerTest.java [modify] https://crrev.com/d049675203babf675a40e2cb583ae61211057e68/chrome/android/junit/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchSelectionControllerTest.java [modify] https://crrev.com/d049675203babf675a40e2cb583ae61211057e68/chrome/android/junit/src/org/chromium/chrome/browser/media/ui/MediaNotificationTestTabHolder.java [modify] https://crrev.com/d049675203babf675a40e2cb583ae61211057e68/chrome/android/junit/src/org/chromium/chrome/browser/omaha/VersionNumberTest.java [modify] https://crrev.com/d049675203babf675a40e2cb583ae61211057e68/chrome/android/junit/src/org/chromium/chrome/browser/snackbar/SnackbarCollectionUnitTest.java [modify] https://crrev.com/d049675203babf675a40e2cb583ae61211057e68/chrome/common/extensions/docs/examples/apps/hello-java/HelloLicenseServlet.java [modify] https://crrev.com/d049675203babf675a40e2cb583ae61211057e68/chrome/common/extensions/docs/examples/extensions/irc/servlet/src/org/chromium/IRCProxyWebSocket.java [modify] https://crrev.com/d049675203babf675a40e2cb583ae61211057e68/components/cronet/android/java/src/org/chromium/net/impl/CronetEngineBase.java [modify] https://crrev.com/d049675203babf675a40e2cb583ae61211057e68/components/cronet/android/java/src/org/chromium/net/impl/UrlRequestBase.java [modify] https://crrev.com/d049675203babf675a40e2cb583ae61211057e68/content/public/android/junit/src/org/chromium/content/browser/SpareChildConnectionTest.java [modify] https://crrev.com/d049675203babf675a40e2cb583ae61211057e68/ui/android/junit/src/org/chromium/ui/base/ClipboardTest.java
,
Nov 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/buildtools/+/24ebce4578745db15274e180da1938ebc1358243 commit 24ebce4578745db15274e180da1938ebc1358243 Author: Sam Maier <smaier@chromium.org> Date: Fri Nov 16 15:36:49 2018 Update clang-format binaries and scripts for all platforms. I followed these instructions: https://chromium.googlesource.com/chromium/src/+/master/docs/updating_clang_format_binaries.md The binaries were built at clang revision 346566 on November 15, 2018. Rolling because 344070 introduces Java import reordering in clang-format, and 346566 fixes a critical bug with Objective C. OLD: https://chromium-review.googlesource.com/c/chromium/src/+/1296816 NEW: https://chromium-review.googlesource.com/c/chromium/src/+/1331609 ls -l output -rwxr-xr-x 1 smaier primarygroup 1967524 Nov 15 16:47 buildtools/mac/clang-format -rwxr-xr-x 1 smaier primarygroup 1862936 Nov 15 14:06 buildtools/linux64/clang-format -rw-r--r-- 1 smaier primarygroup 2637824 Nov 15 14:04 buildtools/win/clang-format.exe Bug: 768586 , 768983 Change-Id: I2761fc91201d2ffa0d999e637f8f8ba53bcccb5e [modify] https://crrev.com/24ebce4578745db15274e180da1938ebc1358243/linux64/clang-format.sha1 [modify] https://crrev.com/24ebce4578745db15274e180da1938ebc1358243/deps_revisions.gni [modify] https://crrev.com/24ebce4578745db15274e180da1938ebc1358243/win/clang-format.exe.sha1 [modify] https://crrev.com/24ebce4578745db15274e180da1938ebc1358243/clang_format/README.chromium [modify] https://crrev.com/24ebce4578745db15274e180da1938ebc1358243/DEPS [modify] https://crrev.com/24ebce4578745db15274e180da1938ebc1358243/mac/clang-format.sha1
,
Nov 21
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bf89178f52ee6fe536140f83338af3494e8355cc commit bf89178f52ee6fe536140f83338af3494e8355cc Author: Nico Weber <thakis@chromium.org> Date: Wed Nov 21 02:07:20 2018 Roll src/buildtools/ 13a00f110..da9b2941c (3 commits) https://chromium.googlesource.com/chromium/buildtools.git/+log/13a00f110ef9..da9b2941cbf6 $ git log 13a00f110..da9b2941c --date=short --no-merges --format='%ad %ae %s' 2018-11-20 thakis Revert "Roll libcxx{abi} 2018-11-16 smaier Update clang-format binaries and scripts for all platforms. 2018-10-29 thomasanderson Roll libcxx{abi} Created with: roll-dep src/buildtools Bug: 768586 , 768983 Change-Id: Ib72e2e4f38b9ed662652a440925ff9260f97abcc Reviewed-on: https://chromium-review.googlesource.com/c/1344293 Reviewed-by: Sam Maier <smaier@chromium.org> Commit-Queue: Nico Weber <thakis@chromium.org> Cr-Commit-Position: refs/heads/master@{#609898} [modify] https://crrev.com/bf89178f52ee6fe536140f83338af3494e8355cc/DEPS
,
Nov 26
A clang-format with the fix got deployed on Nov 21 2018. |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by nyquist@chromium.org
, Sep 26 2017