mprobst created this revision. mprobst added a reviewer: jankuehle. Herald added a subscriber: jeroen.dobbelaere. mprobst requested review of this revision. Herald added a project: clang.
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. This reverts commit f750c3d95a0c8bf1d21380ae753fce12010a7561 <https://reviews.llvm.org/rGf750c3d95a0c8bf1d21380ae753fce12010a7561>. It fixes the msan issue by not parsing past the end of the line when handling import aliases. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D118446 Files: clang/lib/Format/SortJavaScriptImports.cpp clang/unittests/Format/SortImportsTestJS.cpp Index: clang/unittests/Format/SortImportsTestJS.cpp =================================================================== --- clang/unittests/Format/SortImportsTestJS.cpp +++ clang/unittests/Format/SortImportsTestJS.cpp @@ -446,6 +446,25 @@ "const x = 1;"); } +TEST_F(SortImportsTestJS, ImportEqAliases) { + verifySort("import {B} from 'bar';\n" + "import {A} from 'foo';\n" + "\n" + "import Z = A.C;\n" + "import Y = B.C.Z;\n" + "\n" + "export {Z};\n" + "\n" + "console.log(Z);\n", + "import {A} from 'foo';\n" + "import Z = A.C;\n" + "export {Z};\n" + "import {B} from 'bar';\n" + "import Y = B.C.Z;\n" + "\n" + "console.log(Z);\n"); +} + } // end namespace } // end namespace format } // end namespace clang Index: clang/lib/Format/SortJavaScriptImports.cpp =================================================================== --- clang/lib/Format/SortJavaScriptImports.cpp +++ clang/lib/Format/SortJavaScriptImports.cpp @@ -78,6 +78,7 @@ ABSOLUTE, // from 'something' RELATIVE_PARENT, // from '../*' RELATIVE, // from './*' + ALIAS, // import X = A.B; }; ReferenceCategory Category = ReferenceCategory::SIDE_EFFECT; // The URL imported, e.g. `import .. from 'url';`. Empty for `export {a, b};`. @@ -105,10 +106,12 @@ return LHS.IsExport < RHS.IsExport; if (LHS.Category != RHS.Category) return LHS.Category < RHS.Category; - if (LHS.Category == JsModuleReference::ReferenceCategory::SIDE_EFFECT) - // Side effect imports might be ordering sensitive. Consider them equal so - // that they maintain their relative order in the stable sort below. - // This retains transitivity because LHS.Category == RHS.Category here. + if (LHS.Category == JsModuleReference::ReferenceCategory::SIDE_EFFECT || + LHS.Category == JsModuleReference::ReferenceCategory::ALIAS) + // Side effect imports and aliases might be ordering sensitive. Consider + // them equal so that they maintain their relative order in the stable sort + // below. This retains transitivity because LHS.Category == RHS.Category + // here. return false; // Empty URLs sort *last* (for export {...};). if (LHS.URL.empty() != RHS.URL.empty()) @@ -398,6 +401,8 @@ JsModuleReference Reference; Reference.FormattingOff = FormattingOff; Reference.Range.setBegin(Start); + // References w/o a URL, e.g. export {A}, groups with RELATIVE. + Reference.Category = JsModuleReference::ReferenceCategory::RELATIVE; if (!parseModuleReference(Keywords, Reference)) { if (!FirstNonImportLine) FirstNonImportLine = Line; // if no comment before. @@ -463,9 +468,6 @@ Reference.Category = JsModuleReference::ReferenceCategory::RELATIVE; else Reference.Category = JsModuleReference::ReferenceCategory::ABSOLUTE; - } else { - // w/o URL groups with "empty". - Reference.Category = JsModuleReference::ReferenceCategory::RELATIVE; } return true; } @@ -501,6 +503,20 @@ nextToken(); if (Current->is(Keywords.kw_from)) return true; + // import X = A.B.C; + if (Current->is(tok::equal)) { + Reference.Category = JsModuleReference::ReferenceCategory::ALIAS; + nextToken(); + while (Current->is(tok::identifier)) { + nextToken(); + if (Current->is(tok::semi)) { + return true; + } + if (!Current->is(tok::period)) + return false; + nextToken(); + } + } if (Current->isNot(tok::comma)) return false; nextToken(); // eat comma.
Index: clang/unittests/Format/SortImportsTestJS.cpp =================================================================== --- clang/unittests/Format/SortImportsTestJS.cpp +++ clang/unittests/Format/SortImportsTestJS.cpp @@ -446,6 +446,25 @@ "const x = 1;"); } +TEST_F(SortImportsTestJS, ImportEqAliases) { + verifySort("import {B} from 'bar';\n" + "import {A} from 'foo';\n" + "\n" + "import Z = A.C;\n" + "import Y = B.C.Z;\n" + "\n" + "export {Z};\n" + "\n" + "console.log(Z);\n", + "import {A} from 'foo';\n" + "import Z = A.C;\n" + "export {Z};\n" + "import {B} from 'bar';\n" + "import Y = B.C.Z;\n" + "\n" + "console.log(Z);\n"); +} + } // end namespace } // end namespace format } // end namespace clang Index: clang/lib/Format/SortJavaScriptImports.cpp =================================================================== --- clang/lib/Format/SortJavaScriptImports.cpp +++ clang/lib/Format/SortJavaScriptImports.cpp @@ -78,6 +78,7 @@ ABSOLUTE, // from 'something' RELATIVE_PARENT, // from '../*' RELATIVE, // from './*' + ALIAS, // import X = A.B; }; ReferenceCategory Category = ReferenceCategory::SIDE_EFFECT; // The URL imported, e.g. `import .. from 'url';`. Empty for `export {a, b};`. @@ -105,10 +106,12 @@ return LHS.IsExport < RHS.IsExport; if (LHS.Category != RHS.Category) return LHS.Category < RHS.Category; - if (LHS.Category == JsModuleReference::ReferenceCategory::SIDE_EFFECT) - // Side effect imports might be ordering sensitive. Consider them equal so - // that they maintain their relative order in the stable sort below. - // This retains transitivity because LHS.Category == RHS.Category here. + if (LHS.Category == JsModuleReference::ReferenceCategory::SIDE_EFFECT || + LHS.Category == JsModuleReference::ReferenceCategory::ALIAS) + // Side effect imports and aliases might be ordering sensitive. Consider + // them equal so that they maintain their relative order in the stable sort + // below. This retains transitivity because LHS.Category == RHS.Category + // here. return false; // Empty URLs sort *last* (for export {...};). if (LHS.URL.empty() != RHS.URL.empty()) @@ -398,6 +401,8 @@ JsModuleReference Reference; Reference.FormattingOff = FormattingOff; Reference.Range.setBegin(Start); + // References w/o a URL, e.g. export {A}, groups with RELATIVE. + Reference.Category = JsModuleReference::ReferenceCategory::RELATIVE; if (!parseModuleReference(Keywords, Reference)) { if (!FirstNonImportLine) FirstNonImportLine = Line; // if no comment before. @@ -463,9 +468,6 @@ Reference.Category = JsModuleReference::ReferenceCategory::RELATIVE; else Reference.Category = JsModuleReference::ReferenceCategory::ABSOLUTE; - } else { - // w/o URL groups with "empty". - Reference.Category = JsModuleReference::ReferenceCategory::RELATIVE; } return true; } @@ -501,6 +503,20 @@ nextToken(); if (Current->is(Keywords.kw_from)) return true; + // import X = A.B.C; + if (Current->is(tok::equal)) { + Reference.Category = JsModuleReference::ReferenceCategory::ALIAS; + nextToken(); + while (Current->is(tok::identifier)) { + nextToken(); + if (Current->is(tok::semi)) { + return true; + } + if (!Current->is(tok::period)) + return false; + nextToken(); + } + } if (Current->isNot(tok::comma)) return false; nextToken(); // eat comma.
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits