This is an archive of the discontinued LLVM Phabricator instance.

Differential D118361

clang-format: [JS] sort import aliases.
Needs ReviewPublic

Authored by mprobst on Jan 27 2022, 6:35 AM.

Download Raw Diff

This revision needs review, but there are no reviewers specified.

Details

Reviewers: None

Summary

Users can define aliases for long symbols using import aliases:

import X = A.B.C;

Previously, these were unhandled and would terminate import sorting.
With this change, aliases sort as their own group, coming last after all
other imports.

Aliases are not sorted within their group, as they may reference each
other, so order is significant. Aliases sort before ES module exports,
as exports may reference aliases.

import {A} from 'foo';

import X = A.B.C;

export {X};

Diff Detail

Repository: rG LLVM Github Monorepo

Event Timeline

mprobst created this revision.Jan 27 2022, 6:35 AM

Herald added a subscriber: jeroen.dobbelaere. · View Herald TranscriptJan 27 2022, 6:35 AM

mprobst requested review of this revision.Jan 27 2022, 6:35 AM

Herald added a project: Restricted Project. · View Herald TranscriptJan 27 2022, 6:35 AM

mprobst mentioned this in D118363: clang-format: [JS] sort import aliases..Jan 27 2022, 6:38 AM

Superseded by https://reviews.llvm.org/D118363 (sorry for the diff confusion).

make test break if we used alphasort

Harbormaster completed remote builds in B146027: Diff 403646.Jan 27 2022, 7:14 AM

mprobst mentioned this in rGc6d5efb5d980: clang-format: [JS] sort import aliases..Jan 27 2022, 7:17 AM

After https://github.com/llvm/llvm-project/commit/c6d5efb5d98093c4bd7578b2ea52c9032d20dea3
https://lab.llvm.org/buildbot/#/builders/74/builds/9049/steps/15/logs/stdio

Note: Google Test filter = SortImportsTestJS.ImportEqAliases
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from SortImportsTestJS
[ RUN      ] SortImportsTestJS.ImportEqAliases
==6361==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x1341207 in clang::format::JavaScriptImportSorter::parseModuleReference(clang::format::AdditionalKeywords const&, clang::format::JsModuleReference&) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Format/SortJavaScriptImports.cpp:456:9
    #1 0x133ac9d in clang::format::JavaScriptImportSorter::parseModuleReferences(clang::format::AdditionalKeywords const&, llvm::SmallVectorImpl<clang::format::AnnotatedLine*>&) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Format/SortJavaScriptImports.cpp:406:12
    #2 0x1336c97 in clang::format::JavaScriptImportSorter::analyze(clang::format::TokenAnnotator&, llvm::SmallVectorImpl<clang::format::AnnotatedLine*>&, clang::format::FormatTokenLexer&) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Format/SortJavaScriptImports.cpp:149:9
    #3 0x1358d64 in clang::format::TokenAnalyzer::process() /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Format/TokenAnalyzer.cpp:126:9
    #4 0x133635f in clang::format::sortJavaScriptImports(clang::format::FormatStyle const&, llvm::StringRef, llvm::ArrayRef<clang::tooling::Range>, llvm::StringRef) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Format/SortJavaScriptImports.cpp:575:46
    #5 0x11cddd1 in clang::format::sortIncludes(clang::format::FormatStyle const&, llvm::StringRef, llvm::ArrayRef<clang::tooling::Range>, llvm::StringRef, unsigned int*) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Format/Format.cpp:2889:12
    #6 0xd9ead7 in sort /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/unittests/Format/SortImportsTestJS.cpp:28:36
    #7 0xd9ead7 in clang::format::(anonymous namespace)::SortImportsTestJS::_verifySort(char const*, int, llvm::StringRef, llvm::StringRef, unsigned int, unsigned int) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/unittests/Format/SortImportsTestJS.cpp:40:26
    #8 0xda4ea6 in clang::format::(anonymous namespace)::SortImportsTestJS_ImportEqAliases_Test::TestBody() /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/unittests/Format/SortImportsTestJS.cpp:450:3
    #9 0x106fe3d in HandleExceptionsInMethodIfSupported<testing::Test, void> /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc
    #10 0x106fe3d in testing::Test::Run() /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:2508:5
    #11 0x1075215 in testing::TestInfo::Run() /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:2684:11
    #12 0x10776b0 in testing::TestSuite::Run() /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:2816:28
    #13 0x10b8336 in testing::internal::UnitTestImpl::RunAllTests() /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:5338:44
    #14 0x10b5f4d in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc
    #15 0x10b5f4d in testing::UnitTest::Run() /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:4925:10
    #16 0x1047984 in RUN_ALL_TESTS /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest.h:2473:46
    #17 0x1047984 in main /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/llvm/utils/unittest/UnitTestMain/TestMain.cpp:50:10
    #18 0x7f9bc766509a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a) (BuildId: 18b9a9a8c523e5cfe5b5d946d605d09242f09798)
    #19 0x459779 in _start (/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm_build_msan_track_origins/tools/clang/unittests/Format/FormatTests+0x459779)

  Uninitialized value was created by an allocation of 'ref.tmp3' in the stack frame of function '_ZN5clang6format21sortJavaScriptImportsERKNS0_11FormatStyleEN4llvm9StringRefENS4_8ArrayRefINS_7tooling5RangeEEES5_'
    #0 0x1335d20 in clang::format::sortJavaScriptImports(clang::format::FormatStyle const&, llvm::StringRef, llvm::ArrayRef<clang::tooling::Range>, llvm::StringRef) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Format/SortJavaScriptImports.cpp:570
SUMMARY: MemorySanitizer: use-of-uninitialized-value /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Format/SortJavaScriptImports.cpp:456:9 in clang::format::JavaScriptImportSorter::parseModuleReference(clang::format::AdditionalKeywords const&, clang::format::JsModuleReference&)
Exiting
********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..

reverted with f750c3d95a0c8bf1d21380ae753fce12010a7561

curdeius added a project: Restricted Project.Jan 28 2022, 12:33 AM

krasimir removed a reviewer: krasimir.Mar 2 2022, 8:39 AM

Herald added a project: Restricted Project. · View Herald TranscriptMar 2 2022, 8:39 AM

Revision Contents

Path

Size

clang/

unittests/

Format/

SortImportsTestJS.cpp

12 lines

Diff 403646

clang/unittests/Format/SortImportsTestJS.cpp

Show First 20 Lines • Show All 444 Lines • ▼ Show 20 Lines	verifySort("// clang-format off\n"
"// clang-format on\n"		"// clang-format on\n"
"const x = 1;");		"const x = 1;");
}		}

TEST_F(SortImportsTestJS, ImportEqAliases) {		TEST_F(SortImportsTestJS, ImportEqAliases) {
verifySort("import {B} from 'bar';\n"		verifySort("import {B} from 'bar';\n"
"import {A} from 'foo';\n"		"import {A} from 'foo';\n"
"\n"		"\n"
"import C = A.C;\n"		"import Z = A.C;\n"
"import Z = B.C.Z;\n"		"import Y = B.C.Z;\n"
"\n"		"\n"
"export {C};\n"		"export {Z};\n"
"\n"		"\n"
"console.log(Z);\n",		"console.log(Z);\n",
"import {A} from 'foo';\n"		"import {A} from 'foo';\n"
"import C = A.C;\n"		"import Z = A.C;\n"
"export {C};\n"		"export {Z};\n"
"import {B} from 'bar';\n"		"import {B} from 'bar';\n"
"import Z = B.C.Z;\n"		"import Y = B.C.Z;\n"
"\n"		"\n"
"console.log(Z);\n");		"console.log(Z);\n");
}		}

} // end namespace		} // end namespace
} // end namespace format		} // end namespace format
} // end namespace clang		} // end namespace clang